[meta] rationalize/align/document different lint rule sets

[pq]

Rationalization

The Dart ecosystem has a number of "default" rulesets.

• the pana package uses one that should mirror what is used by stagehand but could consider another (for example, pedantic?)
• package:pedantic describes google standard defaults, which are close but not quite to a subset of those in Effective Dart
• and what's the rationale for what's defined by stagehand anyway; wouldn't it be better if it just used one of the existing rulesets (consider updating template to use package:pedantic lints dart-archive/stagehand#593)?
• the flutter repo uses still another and the flutter analyze tool still one more
• (finally?!?) the dartanalyzer tool uses yet another set of rules (What are the default lints enforced? #57872)

Needless to say, this is VERY confusing. Ideally we could converge on one set of defaults with variants well documented.

Alignment

Concretely, I'd propose we converge on package:pedantic where possible. Specifically, I'd suggest defaults should be:

• stagehand : pedantic (consider updating template to use package:pedantic lints dart-archive/stagehand#593)
• pana : pedantic (Consider using pedantic as a baseline for linting? pana#459)
• dartanalyzer : none (NPE given invalid import #1376, Remove lint rule registry default list #35708)
• flutter
  • repo unchanged
  • user: superset of pedantic? (leave informal for now)

Documentation

Having aligned as best we can, we'll want to document. Some thoughts for where:

• analyzer package README (shows in pub) PR
• linter
  • README
  • generated rule docs
• dart-lang "Customize Static Analysis" doc

/cc @bwilkerson @davidmorgan @kevmoo @mit-mit @stereotype441 @devoncarew @kwalrath

[bwilkerson]

It makes sense to me to change stagehand to build a project that imports from pedantic.

It also makes sense to me that pana would be based on pedantic, but it might be the case that it makes sense for it to be a superset. I'm not familiar with the differences between the two rule sets.

My personal preference would be for dartanalyzer to not enable any lints by default. Lints were always designed to be opt-in, but enabling some by default violates that contract.

I agree that we're not going to change Flutter's lists of rules.

[mit-mit]

It seems inconsistent to me to have stagehand and flutter produce new Dart packages that are not equally pedantic.

[pq]

For context, here's a breakdown of what rules are currently defined where.

name	flutter user	flutter repo	pedantic	stagehand
always_declare_return_types		✅
always_put_control_body_on_new_line		✅
always_put_required_named_parameters_first
always_require_non_null_named_parameters		✅
always_specify_types		✅
annotate_overrides		✅
avoid_annotating_with_dynamic
avoid_as		✅
avoid_bool_literals_in_conditional_expressions
avoid_catches_without_on_clauses
avoid_catching_errors
avoid_classes_with_only_static_members		✅
avoid_double_and_int_checks
avoid_empty_else	✅	✅	✅
avoid_field_initializers_in_const_classes		✅
avoid_function_literals_in_foreach_calls		✅
avoid_implementing_value_types
avoid_init_to_null	✅	✅	✅
avoid_js_rounded_ints
avoid_null_checks_in_equality_operators		✅
avoid_positional_boolean_parameters
avoid_private_typedef_functions
avoid_relative_lib_imports		✅	✅
avoid_renaming_method_parameters		✅
avoid_return_types_on_setters	✅	✅	✅
avoid_returning_null
avoid_returning_null_for_future
avoid_returning_null_for_void		✅
avoid_returning_this
avoid_setters_without_getters
avoid_shadowing_type_parameters
avoid_single_cascade_in_expression_statements
avoid_slow_async_io		✅
avoid_types_as_parameter_names		✅	✅
avoid_types_on_closure_parameters
avoid_unused_constructor_parameters		✅
avoid_void_async		✅
await_only_futures	✅	✅
camel_case_types	✅	✅
cancel_subscriptions	✅	✅		✅
cascade_invocations
close_sinks	✅
comment_references
constant_identifier_names
control_flow_in_finally	✅	✅
curly_braces_in_flow_control_structures
directives_ordering		✅
empty_catches		✅
empty_constructor_bodies	✅	✅
empty_statements	✅	✅
file_names
flutter_style_todos		✅
hash_and_equals	✅	✅		✅
implementation_imports	✅	✅
invariant_booleans
iterable_contains_unrelated_type		✅		✅
join_return_with_assignment
library_names	✅	✅
library_prefixes		✅
lines_longer_than_80_chars
list_remove_unrelated_type		✅		✅
literal_only_boolean_expressions
no_adjacent_strings_in_list		✅
no_duplicate_case_values		✅	✅
non_constant_identifier_names	✅	✅
null_closures			✅
omit_local_variable_types
one_member_abstracts
only_throw_errors
overridden_fields		✅
package_api_docs	✅	✅
package_names	✅	✅
package_prefixed_library_names	✅	✅
parameter_assignments
prefer_adjacent_string_concatenation		✅
prefer_asserts_in_initializer_lists		✅
prefer_bool_in_asserts
prefer_collection_literals		✅
prefer_conditional_assignment		✅
prefer_const_constructors		✅
prefer_const_constructors_in_immutables		✅
prefer_const_declarations		✅
prefer_const_literals_to_create_immutables		✅
prefer_constructors_over_static_methods
prefer_contains		✅	✅
prefer_equal_for_default_values		✅	✅
prefer_expression_function_bodies
prefer_final_fields		✅
prefer_final_in_for_each
prefer_final_locals		✅
prefer_foreach		✅
prefer_function_declarations_over_variables
prefer_generic_function_type_aliases		✅
prefer_initializing_formals		✅
prefer_int_literals
prefer_interpolation_to_compose_strings
prefer_is_empty		✅	✅
prefer_is_not_empty	✅	✅	✅
prefer_iterable_whereType		✅
prefer_mixin
prefer_single_quotes		✅
prefer_typing_uninitialized_variables		✅
prefer_void_to_null		✅
public_member_api_docs
recursive_getters		✅	✅
slash_for_doc_comments	✅	✅
sort_constructors_first		✅
sort_pub_dependencies		✅
sort_unnamed_constructors_first		✅
super_goes_last	✅	✅
test_types_in_equals	✅	✅		✅
throw_in_finally	✅	✅
type_annotate_public_apis
type_init_formals	✅	✅
unawaited_futures			✅
unnecessary_await_in_return
unnecessary_brace_in_string_interps	✅	✅
unnecessary_const		✅
unnecessary_getters_setters	✅	✅
unnecessary_lambdas
unnecessary_new		✅
unnecessary_null_aware_assignments		✅
unnecessary_null_in_if_null_operators		✅
unnecessary_overrides		✅
unnecessary_parenthesis		✅
unnecessary_statements	✅	✅
unnecessary_this		✅
unrelated_type_equality_checks	✅	✅	✅	✅
use_function_type_syntax_for_parameters
use_rethrow_when_possible		✅	✅
use_setters_to_change_properties
use_string_buffers
use_to_and_as_if_applicable
valid_regexps	✅	✅	✅	✅
void_checks

[bwilkerson]

Very nice way of comparing, thanks. I kind of wish it included 'pana', though.

So, 'pedantic' defines 16 rules and 'flutter user' defines 28, but only 10 of those are in the intersection. Similarly, stagehand defines 7 rules and only 2 are in the intersection with 'pedantic'.

The rule sets are farther apart than I had guessed.

[mit-mit]

pana uses stagehand for Dart code, and analysis_options_user.yaml for Flutter code.

[kwalrath]

I'm fine with using pedantic for stagehand templates.

[pq]

I'm fine with using pedantic for stagehand templates.

💎

Assuming we can find consensus, dart-archive/stagehand#594 updates stagehand.

If that sticks, we can update pana to follow suit.

[kwalrath]

I don't see package_names under https://github.com/dart-lang/linter/tree/master/lib/src/rules.

[pq]

package_names lives in https://github.com/dart-lang/linter/tree/master/lib/src/rules/pub. (Or were you asking something else?)

While we're at it, you helped me notice a bug in my table generator... Notice the sorting?

public_member_api_docs
package_names

The package_names lint is in a file called PubPackageNames and it looks like I sorted on filename and not rule name.

Thanks for the catch!

(EDIT: order fixed 👍)

[mit-mit]

I don't think we should update Stagehand before we resolve the larger issue: Customers increasingly use Dart across multiple platforms and frameworks, and will no doubt be very confused if they try to move a few lines of code from one context to another and suddenly see different static errors.

I suggest we collaborate with the Flutter team on getting to a single set of default rules for Dart libraries and apps, regardless of framework or platform.