Skip to content

When two conflicting lint rules are enabled, consider not reporting lint from either one #58087

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

Open
dvorapa opened this issue Dec 16, 2019 · 7 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package P3 A lower priority bug or feature request type-documentation A request to add or improve documentation type-enhancement A request for a change that isn't a bug

Comments

@dvorapa
Copy link

dvorapa commented Dec 16, 2019

Describe the issue
After upgrade to new Flutter stable (1.12.13), I got multiple unnecessary_final warnings. But when I remove these, I get similar amount of prefer_final_locals. What should I do? This definitely looks like a bug.

Expected behavior
There should not be any conflicting warnings.

Additional context
Dart 2.7.0, Flutter 1.12.13

@pq
Copy link
Member

pq commented Dec 16, 2019

Hi @dvorapa. Could you share your analysis options (.analysis_options.yaml)? Are you seeing these issues in the IDE or when you run flutter analyze?

@dvorapa
Copy link
Author

dvorapa commented Dec 16, 2019

flutter analyze

linter:
  rules:
    - 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_equals_and_hash_code_on_mutable_classes
    - 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_print
    - 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_unnecessary_containers
    - avoid_unused_constructor_parameters
    - avoid_void_async
    - avoid_web_libraries_in_flutter
    - await_only_futures
    - camel_case_extensions
    - camel_case_types
    - cancel_subscriptions
    - cascade_invocations
    - close_sinks
    - comment_references
    - constant_identifier_names
    - control_flow_in_finally
    - curly_braces_in_flow_control_structures
    - diagnostic_describe_all_properties
    - 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
    - no_logic_in_create_state
    - 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_asserts_with_message
    - 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_double_quotes
    - prefer_equal_for_default_values
    - prefer_expression_function_bodies
    - prefer_final_fields
    - prefer_final_in_for_each
    - prefer_final_locals
    - prefer_for_elements_to_map_fromIterable
    - prefer_foreach
    - prefer_function_declarations_over_variables
    - prefer_generic_function_type_aliases
    - prefer_if_elements_to_conditional_expressions
    - prefer_if_null_operators
    - prefer_initializing_formals
    - prefer_inlined_adds
   # - prefer_int_literals
    - prefer_interpolation_to_compose_strings
    - prefer_is_empty
    - prefer_is_not_empty
    - prefer_is_not_operator
    - prefer_iterable_whereType
    - prefer_mixin
    - prefer_null_aware_operators
    - prefer_relative_imports
    - prefer_single_quotes
    - prefer_spread_collections
    - prefer_typing_uninitialized_variables
    - prefer_void_to_null
    - provide_deprecation_message
    - public_member_api_docs
    - recursive_getters
    - slash_for_doc_comments
    - sort_child_properties_last
    - sort_constructors_first
    - sort_pub_dependencies
    - sort_unnamed_constructors_first
    - 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_final
    - 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
    - unsafe_html
    - use_full_hex_values_for_flutter_colors
    - 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
Copy link
Member

I'm sorry for the frustration this is causing you.

It's true that unnecessary_final conflicts with prefer_final_locals. They're actually opposites of each other, similar to prefer_single_quotes and prefer_double_quotes. But that's working as intended.

The role of the linter is not to enforce a single universal coding style. It's to allow users to automate enforcement of some aspects of the coding style they have chosen. If some users want to always use final for local variables that aren't required to be reassigned and other users think that it's better to always use var for local variables, then the linter can (as it does in this case) provide conflicting rules and thereby support both of those users.

That means that you can't enable all of the rules, but instead need to decide for yourself which rules match the style you want to enforce, just as you did for the two quote related lints.

After upgrade to new Flutter stable ...

What concerns me, though, is why this only happened after upgrading flutter. Did you explicitly change the analysis options file as part of that process? Did flutter upgrade change it for you? If the file wasn't changed, then it's possible that we fixed a bug in one rule or the other that caused them to start behaving this way. (I haven't checked to see whether that's the case.)

@dvorapa
Copy link
Author

dvorapa commented Dec 16, 2019

I haven't changed the file, it is the same as it was in Dart 2.5.0/Flutter 1.9.6.

If these two (4 if you count single/double quotes) are one-or-another (depends on the programmer choice), this should be explicitly written in the linter docs. Also I would consider add sections into the full list here, like this (example):

linter:
  rules:
    - 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

# Disable these if aiming for strict type checking:
    - avoid_catches_without_on_clauses
    - avoid_catching_errors
    - avoid_classes_with_only_static_members
    - avoid_double_and_int_checks
    - avoid_empty_else
    - avoid_equals_and_hash_code_on_mutable_classes
    - avoid_field_initializers_in_const_classes

# Choose one option from the following pairs:
    - 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

@srawlins srawlins added the type-documentation A request to add or improve documentation label Apr 24, 2020
@lrhn
Copy link
Member

lrhn commented Oct 11, 2021

Would it make sense for the analyzer to warn if you have two mutually exclusive/incompatible lints enabled?

The analyzer/linter would have to have some sort of list of counter-indications for each lint, but I guess that could be a fairly sparse map to maintain. Then, if you have both "prefer_single_quotes" and "prefer_double_quotes" enabled, you would not get warnings about string quotes (effectively disabling both lints), but you would get a general warning about incompatible lints being enabled, so you can fix that.

@a14n
Copy link
Contributor

a14n commented Oct 11, 2021

Actually there are already some informations available on each lint rule about conflicting rules - for instance see https://github.com/dart-lang/linter/blob/186b6c5099accd0e179594445d9d67409249b7ab/lib/src/rules/unnecessary_final.dart#L47-L48

@pq
Copy link
Member

pq commented Oct 11, 2021

As @a14n point out, there is some support for reporting incompatibilities. For example, this is what you'd see in IDEA

image

(With a similar warning produced by the CLI.)

Then, if you have both "prefer_single_quotes" and "prefer_double_quotes" enabled, you would not get warnings about string quotes (effectively disabling both lints), but you would get a general warning about incompatible lints being enabled, so you can fix that.

This though we don't do and it's an interesting idea.

/fyi @bwilkerson

@srawlins srawlins added the P3 A lower priority bug or feature request label Sep 22, 2022
@srawlins srawlins changed the title unnecessary_final conflicts with prefer_final_locals When two conflicting lint rules are enabled, consider not reporting lint from either one Apr 6, 2023
@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Mar 29, 2024
@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
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package P3 A lower priority bug or feature request type-documentation A request to add or improve documentation type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants