Skip to content

What to do with style lints that enforce rules that don't exist any more in Effective Dart #57875

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

Closed
a14n opened this issue Jan 16, 2019 · 5 comments
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@a14n
Copy link
Contributor

a14n commented Jan 16, 2019

For instance prefer_constructors_over_static_methods was created to enforce a lint that has disappeared:

Prefer defining constructors instead of static methods to create instances.

@pq
Copy link
Member

pq commented Jan 17, 2019

I'd be inclined to deprecate.

It's noteworthy that prefer_constructors_over_static_methods is not included in any of the rulesets we're trying to align (https://github.com/dart-lang/linter/issues/1365#issuecomment-454416014) so this one in particular would I think effect no-one.

@a14n, @kwalrath: are you aware of any others?

@bwilkerson
Copy link
Member

I disagree. The purpose of the linter is not to enforce the style guide. The purpose of the linter is to enable users to customize the set of diagnostics that are produced by the analyzer.

If we have evidence that there are no users that are enabling this lint, then I think it's reasonable to remove it. But we should not remove it just because it's no longer mentioned in the style guide. It's OK for users to choose to follow additional guidelines that are not in the style guide. At the risk of being heretical, it's even OK for users to choose to not follow guidelines that are in the style guide.

@kwalrath
Copy link
Contributor

I disagree. The purpose of the linter is not to enforce the style guide. The purpose of the linter is to enable users to customize the set of diagnostics that are produced by the analyzer.

If we have evidence that there are no users that are enabling this lint, then I think it's reasonable to remove it. But we should not remove it just because it's no longer mentioned in the style guide. It's OK for users to choose to follow additional guidelines that are not in the style guide. At the risk of being heretical, it's even OK for users to choose to not follow guidelines that are in the style guide.

Agreed. But we should certainly remove lints that are irrelevant or problematic in Dart 2. Some used to be in the style guide but were removed (see dart-lang/site-www#863, dart-lang/site-www#889, and other changes to Effective Dart). Here are some possibilities:

  • always_declare_return_types
  • avoid_types_on_closure_parameters
  • type_annotate_public_apis
  • super_goes_last: Irrelevant because Dart 2 enforces this. (I can't find this in the SDK changelog, but it's mentioned in More revisions to "Effective Dart" based on feedback. site-www#863.)
  • prefer_constructors_over_static_methods: Irrelevant from the POV of reading code (though not from the implementer's or doc reader's POV).

Also, it's hard to tell the difference between these two lints, but maybe that's just a matter for the help text:

  • package_api_docs
  • public_member_api_docs

@pq
Copy link
Member

pq commented Jan 17, 2019

I disagree. The purpose of the linter is not to enforce the style guide. The purpose of the linter is to enable users to customize the set of diagnostics that are produced by the analyzer.

💎

I completely agree.

My thinking was that in this case in particular the advice may have gone stale in the presence of optional new and const. Which is just to say @kwalrath is totally on to something:

But we should certainly remove lints that are irrelevant or problematic in Dart 2.

Emphatic +1.

I'm surprised this isn't being tracked already since we've chatted about it a bunch. Anyway, I opened #57876 to track. @kwalrath: thanks so much for seeding the list!

it's hard to tell the difference between these two lints ...

Opened #57877. Thanks!

@pq pq added type-enhancement A request for a change that isn't a bug area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). labels Dec 23, 2020
@srawlins srawlins added the P2 A bug or feature request we're likely to work on label Sep 22, 2022
@srawlins
Copy link
Member

srawlins commented Mar 7, 2023

I think this has been resolved then. We can keep rules even if their backlink to Effective Dart disappears.

@srawlins srawlins closed this as completed Mar 7, 2023
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants