dart-lang/linter

avoid_renaming_method_parameters - Document that it's ignored in the test/ directory

lrhn opened this issue ยท 4 comments

lrhn commented

Page URL

https://dart.dev/tools/linter-rules/avoid_renaming_method_parameters.html

Page source

https://github.com/dart-lang/site-www/tree/main/src/content/tools/linter-rules/individual-rules.md

Describe the problem

Tried to trigger the lint. It failed until I moved the file I used out of test/ and into lib/.

Do document such exceptions in the lint documentation.
Are other directories exempt?

Expected fix

Document precisely how the lint works, including all exceptions.

(Doesn't seem to apply to setters, maybe mention that too.)

Additional context

Just created a new pub package, added avoid_renaming_method_parameters to analysis_options.yaml as described, and tried to write code that trigger the lint.

I would like to fix this problem.

  • I will try and fix this problem on dart.dev.

@bwilkerson, you're my go-to for anything related to lints when Marya is OOO. ๐Ÿ˜„

Can you provide any detail here on the engineering side of @lrhn's question? I'm happy to update the docs if you can advise me on the right change to make.

It appears to be the case that the lint is disabled everywhere except in the lib directory:

if (!context.isInLibDir) return;

and I confirmed that the documentation doesn't include that caveat (and I agree that it should). I believe that the reasoning is that dart doc only produces documentation for code in lib, and because the rule is designed to prevent problems in generated documentation there's no value in running the rule over other code.

The page source is generated from the source code. For this particular lint, the source of the documentation is in the string literal at https://github.com/dart-lang/sdk/blob/main/pkg/linter/lib/src/rules/avoid_renaming_method_parameters.dart#L19.

You're welcome to make the change yourself, or you can transfer this issue to the linter repository and someone from our team will be happy to update the docs for you.

@parlough For visibility.

Thanks! It's been a long time since I contributed anything to the SDK itself, and I no longer have the codebase on my machine, so I'd love to take your offer and pass it off to dart-lang/linter.

I don't actually have the access rights to do transfers to that repo, though. Would you mind doing the honors, @bwilkerson?

lrhn commented

I believe that the reasoning is that dart doc only produces documentation for code in lib, and because the rule is designed to prevent problems in generated documentation there's no value in running the rule over other code.

It's probably not for generated documentation (you shouldn't edit generated code, not its generated docs either, so you shoudn't fix it). A mistake like not quoting a type like List<String> happens easily in hand-written code too.

Docs are not only relevant for lib or for DartDoc.
If I have a function in test/src/helper.dart with a comment of /// My helper on List<String> is great!,
that text is shown in the IDE when hovering over the name of the function ... minus the <String>, which is ignored.

I think the lint should apply to all code that has doc-comments. If there are doc comments, they're likely intended to be shown to someone, and they should show what they intend. This lint triggers when they likely don't show that.