Skip to content

[augmentations] consider updating avoid_renaming_method_parameters to check augmented functions #59409

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

Open
Tracked by #59399
pq opened this issue Feb 20, 2024 · 3 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-new-language-feature linter-set-recommended P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@pq
Copy link
Member

pq commented Feb 20, 2024

avoid_renaming_method_parameters is specific to overridden methods.

Should it be extended to augmented methods/functions too?

EDIT: methods are now supported. Whether we want to extend this lint (or add another) for functions is an open question.

/cc @goderbauer @munificent @lrhn @natebosch @jakemac53

@pq pq added P3 A lower priority bug or feature request linter-new-language-feature labels Feb 20, 2024
@pq pq self-assigned this Feb 20, 2024
@jakemac53
Copy link
Contributor

Yes, definitely I think it should. All the same justifications apply.

@pq pq changed the title [macros] evaluate support for avoid_renaming_method_parameters [macros] update avoid_renaming_method_parameters to check augmented methods/functions Feb 21, 2024
@lrhn
Copy link
Member

lrhn commented Feb 22, 2024

Completely generally, lints should apply to the end result of augmentation, since that's what the program really is. Something isn't missing if it's added by an augmentation.

Some lints can also apply to intermediate steps, like this one. There is no reason to have a declaration with a different parameter name than the superclass, and then an augmentation on top which changes back to the superclass name.

Other lints only care about the end result. Something like "always type public API" should only trigger if there are no types after augmentation.

@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Mar 29, 2024
copybara-service bot referenced this issue Apr 10, 2024
@pq
Copy link
Member Author

pq commented Apr 10, 2024

Augmented methods are supported w/ 33f94fe. Will update the title to track future (possible) support for functions.

@pq pq changed the title [macros] update avoid_renaming_method_parameters to check augmented methods/functions [macros] consider updating avoid_renaming_method_parameters to check augmented functions Apr 10, 2024
@pq pq removed their assignment Apr 10, 2024
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 20, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 20, 2024
@pq pq changed the title [macros] consider updating avoid_renaming_method_parameters to check augmented functions [augmentations] consider updating avoid_renaming_method_parameters to check augmented functions Feb 5, 2025
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-new-language-feature linter-set-recommended P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants