dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
633 stars 170 forks source link

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

Closed a14n closed 1 year ago

a14n commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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:

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

pq commented 5 years ago

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 https://github.com/dart-lang/linter/issues/1372 to track. @kwalrath: thanks so much for seeding the list!

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

Opened #1373. Thanks!

srawlins commented 1 year ago

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