canonical / ubuntu-flutter-plugins

A collection of Flutter plugins and packages for Ubuntu applications.
48 stars 12 forks source link

feat: Introduce the `ubuntu_lints` package #336

Closed spydon closed 10 months ago

spydon commented 10 months ago

Since there are multiple different sets of linting rules across the different repositories I thought it would be a good idea to come up with a a rule-set that can be consistently used across all repositories containing Dart or Flutter code.

I have taken all the rules that have been previously applied across the repositories in ubuntu-flutter-plugins, ubuntu-desktop-provision and ubuntu-desktop-installer.

It also includes a script I made to check whether there are any included rules that are already included in the externally imported rule-set (like flutter_lints and lints), since there was a lot of overlap with these currently.

The full list of rules can be seen here, if there are any additional rules that people would like to include: https://dart-lang.github.io/linter/lints/options/options.html

Since it can be a bit hard to see which rules that are actually included without going to the imported analysis files, this is a full list of the rules that would be in effect when using this package:

always_declare_return_types
annotate_overrides
avoid_catches_without_on_clauses
avoid_empty_else
avoid_equals_and_hash_code_on_mutable_classes
avoid_function_literals_in_foreach_calls
avoid_init_to_null
avoid_null_checks_in_equality_operators
avoid_print
avoid_redundant_argument_values
avoid_relative_lib_imports
avoid_renaming_method_parameters
avoid_return_types_on_setters
avoid_returning_null_for_void
avoid_shadowing_type_parameters
avoid_single_cascade_in_expression_statements
avoid_types_as_parameter_names
avoid_types_on_closure_parameters
avoid_unnecessary_containers
avoid_web_libraries_in_flutter
await_only_futures
camel_case_extensions
camel_case_types
cancel_subscriptions
collection_methods_unrelated_type
constant_identifier_names
control_flow_in_finally
curly_braces_in_flow_control_structures
dangling_library_doc_comments
depend_on_referenced_packages
directives_ordering
empty_catches
empty_constructor_bodies
empty_statements
eol_at_end_of_file
exhaustive_cases
file_names
hash_and_equals
implementation_imports
implicit_call_tearoffs
library_names
library_prefixes
library_private_types_in_public_api
no_duplicate_case_values
no_leading_underscores_for_library_prefixes
no_leading_underscores_for_local_identifiers
no_logic_in_create_state
no_wildcard_variable_uses
non_constant_identifier_names
null_check_on_nullable_type_parameter
null_closures
omit_local_variable_types
overridden_fields
package_names
package_prefixed_library_names
prefer_adjacent_string_concatenation
prefer_asserts_in_initializer_lists
prefer_collection_literals
prefer_conditional_assignment
prefer_const_constructors
prefer_const_constructors_in_immutables
prefer_const_declarations
prefer_const_literals_to_create_immutables
prefer_contains
prefer_final_fields
prefer_final_in_for_each
prefer_final_locals
prefer_for_elements_to_map_fromIterable
prefer_function_declarations_over_variables
prefer_generic_function_type_aliases
prefer_if_null_operators
prefer_initializing_formals
prefer_inlined_adds
prefer_interpolation_to_compose_strings
prefer_is_empty
prefer_is_not_empty
prefer_is_not_operator
prefer_iterable_whereType
prefer_null_aware_method_calls
prefer_null_aware_operators
prefer_relative_imports
prefer_single_quotes
prefer_spread_collections
prefer_typing_uninitialized_variables
provide_deprecation_message
recursive_getters
secure_pubspec_urls
sized_box_for_whitespace
sized_box_shrink_expand
slash_for_doc_comments
sort_child_properties_last
sort_constructors_first
sort_pub_dependencies
sort_unnamed_constructors_first
type_annotate_public_apis
type_init_formals
type_literal_in_constant_pattern
unawaited_futures
unnecessary_brace_in_string_interps
unnecessary_const
unnecessary_constructor_name
unnecessary_getters_setters
unnecessary_lambdas
unnecessary_late
unnecessary_new
unnecessary_null_aware_assignments
unnecessary_null_in_if_null_operators
unnecessary_nullable_for_final_variable_declarations
unnecessary_overrides
unnecessary_parenthesis
unnecessary_string_escapes
unnecessary_string_interpolations
unnecessary_this
unnecessary_to_list_in_spreads
unrelated_type_equality_checks
use_build_context_synchronously
use_decorated_box
use_full_hex_values_for_flutter_colors
use_function_type_syntax_for_parameters
use_key_in_widget_constructors
use_named_constants
use_rethrow_when_possible
use_string_in_part_of_directives
use_super_parameters
valid_regexps
spydon commented 10 months ago

I added a few more uncontroversial rules that was previously not used in Canonical repos - https://github.com/canonical/ubuntu-flutter-plugins/pull/336/commits/e8f53f2411975ad6eec0427f9f940ec831462017. You can read about each of them here:

https://dart.dev/tools/linter-rules/always_put_required_named_parameters_first https://dart.dev/tools/linter-rules/avoid_multiple_declarations_per_line https://dart.dev/tools/linter-rules/avoid_returning_this https://dart.dev/tools/linter-rules/avoid_type_to_string https://dart.dev/tools/linter-rules/avoid_void_async https://dart.dev/tools/linter-rules/close_sinks https://dart.dev/tools/linter-rules/comment_references https://dart.dev/tools/linter-rules/deprecated_consistency https://dart.dev/tools/linter-rules/literal_only_boolean_expressions https://dart.dev/tools/linter-rules/no_adjacent_strings_in_list https://dart.dev/tools/linter-rules/no_runtimeType_toString https://dart.dev/tools/linter-rules/noop_primitive_operations https://dart.dev/tools/linter-rules/parameter_assignments https://dart.dev/tools/linter-rules/prefer_constructors_over_static_methods https://dart.dev/tools/linter-rules/prefer_void_to_null https://dart.dev/tools/linter-rules/test_types_in_equals https://dart.dev/tools/linter-rules/throw_in_finally https://dart.dev/tools/linter-rules/unnecessary_await_in_return https://dart.dev/tools/linter-rules/unnecessary_null_checks https://dart.dev/tools/linter-rules/unnecessary_raw_strings https://dart.dev/tools/linter-rules/unnecessary_statements https://dart.dev/tools/linter-rules/use_enums https://dart.dev/tools/linter-rules/use_if_null_to_convert_nulls_to_bools https://dart.dev/tools/linter-rules/use_is_even_rather_than_modulo https://dart.dev/tools/linter-rules/use_late_for_private_fields_and_variables https://dart.dev/tools/linter-rules/void_checks

There is also a more "controversial" rule, require_trailing_commas, that I think we should add. But we can discuss that one later and possibly do it in a follow-up.

I also saw that lines_longer_than_80_chars had been explicitly deactivated before, but the rule seems to be follow almost everywhere, maybe we could check if we would want to add that again too?

d-loose commented 10 months ago

Didn't yet have the time to look at all those rules, but I'm wondering how we should go about enforcing them in the existing projects. Agree on a set of rules here, publish the package, dart/flutter fix .. all the things in a single, large PR for each project? Or would it be better to break it down into smaller steps and enforce one rule after the other until we converge?

spydon commented 10 months ago

Didn't yet have the time to look at all those rules, but I'm wondering how we should go about enforcing them in the existing projects. Agree on a set of rules here, publish the package, dart/flutter fix .. all the things in a single, large PR for each project? Or would it be better to break it down into smaller steps and enforce one rule after the other until we converge?

I think it depends on how many fixes that the project needs, I think most projects in this repository won't need much changing at all, but some other projects probably need a bit more changes. I can explore it a bit once it is merged.

d-loose commented 10 months ago

Didn't yet have the time to look at all those rules, but I'm wondering how we should go about enforcing them in the existing projects. Agree on a set of rules here, publish the package, dart/flutter fix .. all the things in a single, large PR for each project? Or would it be better to break it down into smaller steps and enforce one rule after the other until we converge?

I think it depends on how many fixes that the project needs, I think most projects in this repository won't need much changing at all, but some other projects probably need a bit more changes. I can explore it a bit once it is merged.

Just had a closer look and tried applying those rules in ubuntu-desktop-provision and app-center - there'd be only a few trivial changes in the app-center :)

So should we merge this as-is and publish it, then think about adding require_trailing_commas and lines_longer_than_80_chars later?

spydon commented 10 months ago

Just had a closer look and tried applying those rules in ubuntu-desktop-provision and app-center - there'd be only a few trivial changes in the app-center :)

So should we merge this as-is and publish it, then think about adding require_trailing_commas and lines_longer_than_80_chars later?

Sounds like a good plan!