Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task
lrhn opened this issue Aug 9, 2024 · 4 comments
Open
1 task
Labels
P3 A lower priority bug or feature request set-recommended Affects a rule in the recommended Dart rule set type-documentation A request to add or improve documentation

Comments

@lrhn
Copy link
Member

lrhn commented Aug 9, 2024

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.
@RedBrogdon
Copy link

@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.

@bwilkerson
Copy link
Member

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.

@RedBrogdon
Copy link

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?

@bwilkerson bwilkerson transferred this issue from dart-lang/site-www Aug 12, 2024
@github-actions github-actions bot added the set-recommended Affects a rule in the recommended Dart rule set label Aug 12, 2024
@pq pq added the type-documentation A request to add or improve documentation label Aug 12, 2024
@lrhn
Copy link
Member Author

lrhn commented Aug 14, 2024

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.

@keertip keertip added the P3 A lower priority bug or feature request label Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 A lower priority bug or feature request set-recommended Affects a rule in the recommended Dart rule set type-documentation A request to add or improve documentation
Projects
None yet
Development

No branches or pull requests

5 participants