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

add server quickfixes/assists for all the lints #57878

Closed
pq opened this issue Jan 18, 2019 · 15 comments
Closed

add server quickfixes/assists for all the lints #57878

pq opened this issue Jan 18, 2019 · 15 comments
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). linter-set-core linter-set-flutter linter-set-recommended 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

@pq
Copy link
Member

pq commented Jan 18, 2019

Ideally we should have fixes (or assists) associated w/ all the lint rules (that can naturally be fixed automatically).

For context, this scorecard tells us where we currently stand.

name linter dart sdk fix pedantic stagehand flutter user flutter repo status bug refs
always_declare_return_types 0.1.4 2.0.0
always_put_control_body_on_new_line 0.1.31 2.0.0
always_put_required_named_parameters_first 0.1.33 2.0.0
always_require_non_null_named_parameters 0.1.31 2.0.0 💡
always_specify_types 0.1.4 2.0.0
annotate_overrides 0.1.11 2.0.0 💡
avoid_annotating_with_dynamic 0.1.31 2.0.0 💡 #57789
avoid_as 0.1.5 2.0.0
avoid_bool_literals_in_conditional_expressions 0.1.46 2.0.0
avoid_catches_without_on_clauses 0.1.31 2.0.0
avoid_catching_errors 0.1.31 2.0.0
avoid_classes_with_only_static_members 0.1.31 2.0.0
avoid_double_and_int_checks 0.1.47 2.0.0
avoid_empty_else 0.1.8 2.0.0 💡
avoid_field_initializers_in_const_classes 0.1.48 2.0.0
avoid_function_literals_in_foreach_calls 0.1.30 2.0.0
avoid_implementing_value_types 0.1.62 2.1.0-dev.5.0
avoid_init_to_null 0.1.11 2.0.0 💡
avoid_js_rounded_ints 0.1.48 2.0.0
avoid_null_checks_in_equality_operators 0.1.31 2.0.0
avoid_positional_boolean_parameters 0.1.31 2.0.0
avoid_private_typedef_functions 0.1.46 2.0.0
avoid_relative_lib_imports 0.1.44 2.0.0
avoid_renaming_method_parameters 0.1.45 2.0.0
avoid_return_types_on_setters 0.1.11 2.0.0 💡
avoid_returning_null 0.1.31 2.0.0
avoid_returning_null_for_future 0.1.72 2.1.1-dev.0.0
avoid_returning_null_for_void 0.1.69 2.1.0-dev.8.0
avoid_returning_this 0.1.31 2.0.0
avoid_setters_without_getters 0.1.31 2.0.0
avoid_shadowing_type_parameters 0.1.72 2.1.1-dev.0.0
avoid_single_cascade_in_expression_statements 0.1.46 2.0.0
avoid_slow_async_io 0.1.30 2.0.0
avoid_types_as_parameter_names 0.1.45 2.0.0
avoid_types_on_closure_parameters 0.1.31 2.0.0 💡 #57756
avoid_unused_constructor_parameters 0.1.36 2.0.0
avoid_void_async 0.1.60 2.1.0-dev.3.0
await_only_futures 0.1.16 2.0.0 💡
camel_case_types 0.1.1 2.0.0
cancel_subscriptions 0.1.20 2.0.0
cascade_invocations 0.1.29 2.0.0 #57721, #57640, #57626
close_sinks 0.1.19 2.0.0
comment_references 0.1.17 2.0.0
constant_identifier_names 0.1.1 2.0.0
control_flow_in_finally 0.1.16 2.0.0
curly_braces_in_flow_control_structures 0.1.57 2.0.0
directives_ordering 0.1.30 2.0.0
empty_catches 0.1.22 2.0.0 💡
empty_constructor_bodies 0.1.1 2.0.0 💡
empty_statements 0.1.21 2.0.0 💡
file_names 0.1.54 2.0.0
flutter_style_todos 0.1.61 2.1.0-dev.5.0
hash_and_equals 0.1.11 2.0.0
implementation_imports 0.1.4 2.0.0 #57792
invariant_booleans 0.1.25 2.0.0 deprecated #57853, #57684, #57643, #57601, #57575
iterable_contains_unrelated_type 0.1.17 2.0.0 #57708
join_return_with_assignment 0.1.31 2.0.0
library_names 0.1.1 2.0.0
library_prefixes 0.1.1 2.0.0
lines_longer_than_80_chars 0.1.56 2.0.0
list_remove_unrelated_type 0.1.22 2.0.0
literal_only_boolean_expressions 0.1.25 2.0.0
no_adjacent_strings_in_list 0.1.30 2.0.0
no_duplicate_case_values 0.1.30 2.0.0
non_constant_identifier_names 0.1.1 2.0.0 💡
null_closures 0.1.56 2.0.0
omit_local_variable_types 0.1.30 2.0.0 #57710
one_member_abstracts 0.1.1 2.0.0 #57701
only_throw_errors 0.1.21 2.0.0
overridden_fields 0.1.18 2.0.0
package_api_docs 0.1.1 2.0.0 #57310
package_names 0.1.31 2.0.0
package_prefixed_library_names 0.1.1 2.0.0
parameter_assignments 0.1.27 2.0.0
prefer_adjacent_string_concatenation 0.1.30 2.0.0 #57796
prefer_asserts_in_initializer_lists 0.1.33 2.0.0
prefer_bool_in_asserts 0.1.36 2.0.0 deprecated
prefer_collection_literals 0.1.30 2.0.0 💡
prefer_conditional_assignment 0.1.31 2.0.0 💡
prefer_const_constructors 0.1.30 2.0.0
prefer_const_constructors_in_immutables 0.1.33 2.0.0
prefer_const_declarations 0.1.43 2.0.0 💡
prefer_const_literals_to_create_immutables 0.1.43 2.0.0
prefer_constructors_over_static_methods 0.1.31 2.0.0
prefer_contains 0.1.30 2.0.0
prefer_equal_for_default_values 0.1.46 2.0.0
prefer_expression_function_bodies 0.1.30 2.0.0
prefer_final_fields 0.1.27 2.0.0 💡
prefer_final_in_for_each 0.1.78 null
prefer_final_locals 0.1.27 2.0.0 💡
prefer_foreach 0.1.31 2.0.0
prefer_function_declarations_over_variables 0.1.30 2.0.0
prefer_generic_function_type_aliases 0.1.47 2.0.0
prefer_initializing_formals 0.1.30 2.0.0
prefer_int_literals 0.1.71 2.1.0
prefer_interpolation_to_compose_strings 0.1.30 2.0.0
prefer_is_empty 0.1.30 2.0.0
prefer_is_not_empty 0.1.5 2.0.0 💡 #57817
prefer_iterable_whereType 0.1.47 2.0.0
prefer_mixin 0.1.62 2.1.0-dev.5.0
prefer_single_quotes 0.1.33 2.0.0
prefer_typing_uninitialized_variables 0.1.36 2.0.0
prefer_void_to_null 0.1.59 2.1.0-dev.1.0
public_member_api_docs 0.1.11 2.0.0
recursive_getters 0.1.30 2.0.0
slash_for_doc_comments 0.1.1 2.0.0
sort_constructors_first 0.1.11 2.0.0
sort_pub_dependencies 0.1.63 2.1.0-dev.6.0
sort_unnamed_constructors_first 0.1.11 2.0.0
super_goes_last 0.1.1 2.0.0
test_types_in_equals 0.1.16 2.0.0 #57448
throw_in_finally 0.1.16 2.0.0
type_annotate_public_apis 0.1.5 2.0.0 #57767
type_init_formals 0.1.1 2.0.0 💡
unawaited_futures 0.1.19 2.0.0 #57711, #57653, #57500, #57437
unnecessary_await_in_return 0.1.73 2.1.1-dev.0.0
unnecessary_brace_in_string_interps 0.1.30 2.0.0
unnecessary_const 0.1.54 2.0.0
unnecessary_getters_setters 0.1.1 2.0.0 #57354
unnecessary_lambdas 0.1.30 2.0.0 💡
unnecessary_new 0.1.54 2.0.0
unnecessary_null_aware_assignments 0.1.30 2.0.0
unnecessary_null_in_if_null_operators 0.1.30 2.0.0
unnecessary_overrides 0.1.31 2.0.0
unnecessary_parenthesis 0.1.44 2.0.0
unnecessary_statements 0.1.36 2.0.0
unnecessary_this 0.1.30 2.0.0 💡
unrelated_type_equality_checks 0.1.16 2.0.0
use_function_type_syntax_for_parameters 0.1.72 2.1.1-dev.0.0
use_rethrow_when_possible 0.1.31 2.0.0
use_setters_to_change_properties 0.1.31 2.0.0
use_string_buffers 0.1.31 2.0.0 #57619
use_to_and_as_if_applicable 0.1.31 2.0.0
valid_regexps 0.1.22 2.0.0
void_checks 0.1.49 2.0.0
@pq pq added the area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). label Jan 18, 2019
@devoncarew
Copy link
Member

This is great! Perhaps we should have a shorter list of quick fixes we intend to address sooner? We could start with the lints from package:pedantic, remove lints where quick fixes don't make sense, and possibly add one or two high value lints.

@kevmoo
Copy link
Member

kevmoo commented Jan 18, 2019

Please tell me you have a script to update that table. 😄

@kevmoo
Copy link
Member

kevmoo commented Jan 18, 2019

Please please please prefer_single_quotes

@devoncarew
Copy link
Member

slash_for_doc_comments? prefer_generic_function_type_aliases?

@kevmoo
Copy link
Member

kevmoo commented Jan 18, 2019 via email

@srawlins
Copy link
Member

Sorry, I don't quite get the light bulb image. Does that mean a quickfix exists for that lint?

@bwilkerson
Copy link
Member

I'm totally in favor of adding fixes where possible, but there are cases where a fix doesn't make sense, such as avoid_types_as_parameter_names. It will be interesting to see how many have reasonable fixes.

Also, we often choose to implement such "fixes" as quick assists, because then they're available to users even if they haven't enabled the lint. The scorecard doesn't take that into account (and I don't know how to compute that; there's no indication anywhere in the code that the assist is appropriate for the lint).

Please please please prefer_single_quotes

slash_for_doc_comments? prefer_generic_function_type_aliases?

Great examples! We have assists for all of those.

@pq
Copy link
Member Author

pq commented Jan 18, 2019

Please tell me you have a script to update that table.

👍 https://github.com/dart-lang/linter/blob/master/tool/scorecard.dart

I don't quite get the light bulb image. Does that mean a quickfix exists for that lint?

Sorry, yeah. May just as well have been a checkmark but assists are often displayed with lightbulbs in IDEs so I was trying to be cute.

I'm totally in favor of adding fixes where possible, but there are cases where a fix doesn't make sense

Right. That's what I was trying to say with the "that can naturally be fixed automatically" qualifier above. Can you think of a better way to say that?

However we describe it, I'd like to capture the information somewhere so we can exclude lints that aren't fixable from consideration. Ideas for where that should live most welcome!

@pq
Copy link
Member Author

pq commented Jan 18, 2019

Also, we often choose to implement such "fixes" as quick assists, because then they're available to users even if they haven't enabled the lint. The scorecard doesn't take that into account (and I don't know how to compute that; there's no indication anywhere in the code that the assist is appropriate for the lint).

Yes, right (as in prefer_single_quotes). We should capture this somewhere and use it to inform the scorecard. (EDIT: tracking here: #57879.)

@pq pq changed the title add server quickfixes for all the lints add server quickfixes/assists for all the lints Jan 18, 2019
@a14n
Copy link
Contributor

a14n commented Jan 21, 2019

I'd prefer to see #57600 (and probably #57754) be addressed first.

@bwilkerson
Copy link
Member

Addressing #57754 would address #57600, but I don't believe we have the resources to address either one this quarter, and I wouldn't want this to wait until we did. (And it won't be any harder to move 20 fixes to a plugin than to move 10.)

That said, the right way (imo) to address this issue is to

  1. Identify which lints have neither fixes nor assists but could have one.
  2. Prioritize them.
  3. Decide for each whether it should be a fix or an assist (I'm guessing many of them should be assists).
  4. Implement one or the other.

dart-bot referenced this issue Mar 6, 2019
See: https://github.com/dart-lang/linter/issues/1374.

Change-Id: Ieeddf25718ddde77a9e52cb93cbcf34ca2c1f81f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/95542
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-bot referenced this issue Mar 6, 2019
See: https://github.com/dart-lang/linter/issues/1374.

Change-Id: I945d393b0b46cf87b5400be1bfdb501c31646988
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/95669
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-bot referenced this issue Mar 7, 2019
See: https://github.com/dart-lang/linter/issues/1374.

Change-Id: I7e79ad37681afdbd94d7968451a768e74217578e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/95708
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-bot referenced this issue Mar 7, 2019
See: https://github.com/dart-lang/linter/issues/1374.

Change-Id: Ia09f2530c9a943b8b80fda63fb313b752748264a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/95709
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
@pq pq added the type-enhancement A request for a change that isn't a bug label Apr 29, 2019
@ThinkDigitalSoftware
Copy link

showing support for prefer_final_in_for_each

dart-bot referenced this issue Sep 3, 2019
See: https://github.com/dart-lang/linter/issues/1374

Change-Id: I151170ee2374a726dc6be037b25e1c7a54729bec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/115266
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
@DetachHead
Copy link

sort_pub_dependencies is one that i can see being quite frustrating to manually fix if you have a lot of dependencies

@srawlins
Copy link
Member

srawlins commented Apr 3, 2022

@DetachHead this issue is not really actionable (naturally every lint should have a quick fix or assist if applicable, but this is more of just a policy now than a concrete task); you will probably get more traction opening a request specifically for sort_pub_dependencies.

@srawlins srawlins added the P2 A bug or feature request we're likely to work on label Sep 25, 2022
@pq
Copy link
Member Author

pq commented Nov 15, 2022

This list is stale. The current source of truth is: https://github.com/dart-lang/sdk/blob/main/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml and also tracked in the individual rule docs:

image

@pq pq closed this as completed Nov 15, 2022
@devoncarew devoncarew added analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). linter-set-core linter-set-flutter linter-set-recommended 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

8 participants