Skip to content

proposal: deprecate (and remove?) lints for 3.0 #58947

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
6 of 7 tasks
pq opened this issue Dec 6, 2022 · 10 comments
Closed
6 of 7 tasks

proposal: deprecate (and remove?) lints for 3.0 #58947

pq opened this issue Dec 6, 2022 · 10 comments
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-lint-proposal linter-set-recommended P3 A lower priority bug or feature request
Milestone

Comments

@pq
Copy link
Member

pq commented Dec 6, 2022

3.0 would be a good time to deprecate a host of lints that don't make sense in a null-safe world, are backed by stale advice or a have poor user experience (e.g., too many false positives).

  • ✅ deprecate: always_require_non_null_named_parameters [recommended]
  • ✅ deprecate: avoid_returning_null_for_future
  • ✅ deprecate: avoid_returning_null
  • ✅ deprecate prefer_equal_for_default_values [recommended] (analyzer now produces a diagnostic)
  • Deprecate iterable_contains_unrelated_type and list_remove_unrelated_type #58958 [core] (DELAYED)
  • 🤔 deprecate avoid_annotating_with_dynamic
  • deprecate: null_closures [recommended] (DECISION: still valuable)

We might also take this opportunity to deprecate lints that we no longer want to support.

@pq pq added P3 A lower priority bug or feature request linter-lint-proposal labels Dec 6, 2022
@pq pq added this to the Dart 3 beta 2 milestone Dec 6, 2022
@pq pq changed the title proposal: deprecate non-NNBD lints proposal: deprecate lints for 3.0 Dec 6, 2022
@pq pq changed the title proposal: deprecate lints for 3.0 [proposal] deprecate lints for 3.0 Dec 6, 2022
@pq pq changed the title [proposal] deprecate lints for 3.0 proposal: deprecate lints for 3.0 Dec 6, 2022
@pq pq modified the milestones: Dart 3 beta 2, Dart 3 alpha 1 Dec 8, 2022
@srawlins
Copy link
Member

null_closures is still necessary in Dart 3.0. For example, Iterable.firstWhere's orElse parameter is nullable.

@pq
Copy link
Member Author

pq commented Dec 14, 2022

Good catch @srawlins! We should also update the docs (and implementation) to reflect SDK API changes?

null_closures docs:

This rule only catches null literals being passed where closures are expected in the following locations:

Constructors
From dart:async
Future at the 0th positional parameter
Future.microtask at the 0th positional parameter
Future.sync at the 0th positional parameter
Timer at the 0th positional parameter
Timer.periodic at the 1st positional parameter
From dart:core
List.generate at the 1st positional parameter
Static functions
From dart:async
scheduleMicrotask at the 0th positional parameter
Future.doWhile at the 0th positional parameter
Future.forEach at the 0th positional parameter
Future.wait at the named parameter cleanup
Timer.run at the 0th positional parameter
Instance methods
From dart:async
Future.then at the 0th positional parameter
Future.complete at the 0th positional parameter
From dart:collection
Queue.removeWhere at the 0th positional parameter
`Queue.retain
Iterable.firstWhere at the 0th positional parameter, and the named parameter orElse
Iterable.forEach at the 0th positional parameter
Iterable.fold at the 1st positional parameter
Iterable.lastWhere at the 0th positional parameter, and the named parameter orElse
Iterable.map at the 0th positional parameter
Iterable.reduce at the 0th positional parameter
Iterable.singleWhere at the 0th positional parameter, and the named parameter orElse
Iterable.skipWhile at the 0th positional parameter
Iterable.takeWhile at the 0th positional parameter
Iterable.where at the 0th positional parameter
List.removeWhere at the 0th positional parameter
List.retainWhere at the 0th positional parameter
String.replaceAllMapped at the 1st positional parameter
String.replaceFirstMapped at the 1st positional parameter
String.splitMapJoin at the named parameters onMatch and onNonMatch

@srawlins
Copy link
Member

Thanks for the idea, @pq ! I audited the rule and found it could be dramatically simplified for null safety. dart-archive/linter#3918

@pq pq mentioned this issue Dec 16, 2022
2 tasks
@srawlins
Copy link
Member

Can we just delete these for 3.0?

@bwilkerson
Copy link
Member

My understanding is that users would only see deprecation notices if they use one of the 3.0-dev releases. I don't know how common that will be, but that's the only reason I can think of to go through a deprecation period.

@srawlins
Copy link
Member

I don't know why it would be meaningful to provide a deprecation period. The action-to-take is the same for a deprecated linter rule as a deleted one (remove the reference). And the consequence of using a deprecated linter rule is the same as the consequence of using a deleted one (a Hint in your analysis_options.yaml file).

I think they can just be deleted.

@pq
Copy link
Member Author

pq commented Dec 16, 2022

And the consequence of using a deprecated linter rule is the same as the consequence of using a deleted one (a Hint in your analysis_options.yaml file).

This is true for lints that don't have behavior (for example ones that are short-circuited in the presence of null-safety or language version like prefer_equal_for_default_values) but not otherwise. A deprecate lint can still fire ...

@srawlins
Copy link
Member

That's true.

@pq
Copy link
Member Author

pq commented Dec 22, 2022

#58988 suggests we might consider deprecating avoid_annotating_with_dynamic too.

/fyi @munificent

@pq pq changed the title proposal: deprecate lints for 3.0 proposal: deprecate (and remove?) lints for 3.0 Jan 4, 2023
@pq pq modified the milestones: Dart 3 alpha 1, Dart 3 beta 1 Jan 31, 2023
@pq pq added this to the Dart 3 beta 2 milestone Feb 2, 2023
@pq pq modified the milestones: Dart 3 beta 2, Dart 3 beta 3 Mar 13, 2023
@pq
Copy link
Member Author

pq commented Mar 20, 2023

Remaining work is picked up in dart-lang/core#789. Closing out.

@pq pq closed this as completed Mar 20, 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 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-lint-proposal linter-set-recommended P3 A lower priority bug or feature request
Projects
None yet
Development

No branches or pull requests

4 participants