From 8dc2cd559206dd3098198e5cdfc24525350f34ab Mon Sep 17 00:00:00 2001 From: Yevhenii Aleksiuk <38440891+4akloon@users.noreply.github.com> Date: Fri, 19 Apr 2024 10:38:09 +0300 Subject: [PATCH] 149-new-lint-avoid_final_with_getter (#161) * add avoid_final_with_getter rule * Update analysis_options.yaml to exclude final fields with getters * Refactor avoid_final_with_getter_rule.dart and avoid_final_with_getter_visitor.dart * Fix incorrect test comments in avoid_final_with_getter_test.dart * change avoid_final_with_getter, now it lints getter, no getter name equality * Refactor avoid_final_with_getter_visitor.dart to remove isStatic property from declaredElement * add more tests to avoid_final_with_getter_test.dart * Add avoid_final_with_getter rule to CHANGELOG.md --- CHANGELOG.md | 1 + lib/analysis_options.yaml | 1 + lib/solid_lints.dart | 2 + .../avoid_final_with_getter_rule.dart | 67 +++++++++++++++++++ .../avoid_final_with_getter_visitor.dart | 33 +++++++++ .../visitors/getter_variable_visitor.dart | 46 +++++++++++++ lint_test/analysis_options.yaml | 1 + lint_test/avoid_final_with_getter_test.dart | 35 ++++++++++ 8 files changed, 186 insertions(+) create mode 100644 lib/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart create mode 100644 lib/src/lints/avoid_final_with_getter/visitors/avoid_final_with_getter_visitor.dart create mode 100644 lib/src/lints/avoid_final_with_getter/visitors/getter_variable_visitor.dart create mode 100644 lint_test/avoid_final_with_getter_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index 81d0d0b..554e7eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## 0.2.0 +- Added `avoid_final_with_getter` rule - Improve `avoid_late_keyword` - `ignored_types` to support ignoring subtype of the node type (https://github.com/solid-software/solid_lints/issues/157) - Abstract methods should be omitted by `proper_super_calls` (https://github.com/solid-software/solid_lints/issues/159) diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index 6d1ce56..4960753 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -51,6 +51,7 @@ custom_lint: - avoid_unrelated_type_assertions - avoid_unused_parameters - avoid_debug_print + - avoid_final_with_getter - cyclomatic_complexity: max_complexity: 10 diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index f6524d0..ad6d9a1 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -2,6 +2,7 @@ library solid_metrics; import 'package:custom_lint_builder/custom_lint_builder.dart'; import 'package:solid_lints/src/lints/avoid_debug_print/avoid_debug_print_rule.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart'; import 'package:solid_lints/src/lints/avoid_global_state/avoid_global_state_rule.dart'; import 'package:solid_lints/src/lints/avoid_late_keyword/avoid_late_keyword_rule.dart'; import 'package:solid_lints/src/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart'; @@ -61,6 +62,7 @@ class _SolidLints extends PluginBase { PreferMatchFileNameRule.createRule(configs), ProperSuperCallsRule.createRule(configs), AvoidDebugPrint.createRule(configs), + AvoidFinalWithGetterRule.createRule(configs), ]; // Return only enabled rules diff --git a/lib/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart b/lib/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart new file mode 100644 index 0000000..1745170 --- /dev/null +++ b/lib/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart @@ -0,0 +1,67 @@ +import 'package:analyzer/error/listener.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/visitors/avoid_final_with_getter_visitor.dart'; +import 'package:solid_lints/src/models/rule_config.dart'; +import 'package:solid_lints/src/models/solid_lint_rule.dart'; + +/// Avoid using final private fields with getters. +/// +/// Final private variables used in a pair with a getter +/// must be changed to a final public type without a getter +/// because it is the same as a public field. +/// +/// ### Example +/// +/// #### BAD: +/// +/// ```dart +/// class MyClass { +/// final int _myField = 0; +/// +/// int get myField => _myField; +/// } +/// ``` +/// +/// #### GOOD: +/// +/// ```dart +/// class MyClass { +/// final int myField = 0; +/// } +/// ``` +/// +class AvoidFinalWithGetterRule extends SolidLintRule { + /// The [LintCode] of this lint rule that represents + /// the error whether we use final private fields with getters. + static const lintName = 'avoid_final_with_getter'; + + AvoidFinalWithGetterRule._(super.config); + + /// Creates a new instance of [AvoidFinalWithGetterRule] + /// based on the lint configuration. + factory AvoidFinalWithGetterRule.createRule(CustomLintConfigs configs) { + final rule = RuleConfig( + configs: configs, + name: lintName, + problemMessage: (_) => 'Avoid final private fields with getters.', + ); + + return AvoidFinalWithGetterRule._(rule); + } + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addCompilationUnit((node) { + final visitor = AvoidFinalWithGetterVisitor(); + node.accept(visitor); + + for (final getter in visitor.getters) { + reporter.reportErrorForNode(code, getter); + } + }); + } +} diff --git a/lib/src/lints/avoid_final_with_getter/visitors/avoid_final_with_getter_visitor.dart b/lib/src/lints/avoid_final_with_getter/visitors/avoid_final_with_getter_visitor.dart new file mode 100644 index 0000000..3672c7c --- /dev/null +++ b/lib/src/lints/avoid_final_with_getter/visitors/avoid_final_with_getter_visitor.dart @@ -0,0 +1,33 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/visitors/getter_variable_visitor.dart'; + +/// A visitor that checks for final private fields with getters. +/// If a final private field has a getter, it is considered as a public field. +class AvoidFinalWithGetterVisitor extends RecursiveAstVisitor { + final _getters = []; + + /// List of getters + Iterable get getters => _getters; + + @override + void visitMethodDeclaration(MethodDeclaration node) { + if (node + case MethodDeclaration( + isGetter: true, + declaredElement: ExecutableElement( + isAbstract: false, + isPublic: true, + ) + )) { + final visitor = GetterVariableVisitor(node); + node.parent?.accept(visitor); + + if (visitor.hasVariable) { + _getters.add(node); + } + } + super.visitMethodDeclaration(node); + } +} diff --git a/lib/src/lints/avoid_final_with_getter/visitors/getter_variable_visitor.dart b/lib/src/lints/avoid_final_with_getter/visitors/getter_variable_visitor.dart new file mode 100644 index 0000000..2c7f77c --- /dev/null +++ b/lib/src/lints/avoid_final_with_getter/visitors/getter_variable_visitor.dart @@ -0,0 +1,46 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; + +/// A visitor that checks the association of the getter with +/// the final private variable +class GetterVariableVisitor extends RecursiveAstVisitor { + final int? _getterId; + VariableDeclaration? _variable; + + /// Creates a new instance of [GetterVariableVisitor] + GetterVariableVisitor(MethodDeclaration getter) + : _getterId = getter.getterReferenceId; + + /// Is there a variable associated with the getter + bool get hasVariable => _variable != null; + + @override + void visitVariableDeclaration(VariableDeclaration node) { + if (node + case VariableDeclaration( + isFinal: true, + declaredElement: VariableElement(id: final id, isPrivate: true) + ) when id == _getterId) { + _variable = node; + } + + super.visitVariableDeclaration(node); + } +} + +extension on MethodDeclaration { + int? get getterReferenceId => switch (body) { + ExpressionFunctionBody( + expression: SimpleIdentifier( + staticElement: Element( + declaration: PropertyAccessorElement( + variable: PropertyInducingElement(:final id) + ) + ) + ) + ) => + id, + _ => null, + }; +} diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index cea6387..abb6249 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -56,3 +56,4 @@ custom_lint: - prefer_last - prefer_match_file_name - proper_super_calls + - avoid_final_with_getter diff --git a/lint_test/avoid_final_with_getter_test.dart b/lint_test/avoid_final_with_getter_test.dart new file mode 100644 index 0000000..4f0fa32 --- /dev/null +++ b/lint_test/avoid_final_with_getter_test.dart @@ -0,0 +1,35 @@ +// ignore_for_file: type_annotate_public_apis, prefer_match_file_name, unused_local_variable + +/// Check final private field with getter fail +/// `avoid_final_with_getter` + +class Fail { + final int _myField = 0; + + // expect_lint: avoid_final_with_getter + int get myField => _myField; +} + +class FailOtherName { + final int _myField = 0; + + // expect_lint: avoid_final_with_getter + int get myFieldInt => _myField; +} + +class FailStatic { + static final int _myField = 0; + + // expect_lint: avoid_final_with_getter + static int get myField => _myField; +} + +class Skip { + final int _myField = 0; + + int get myField => _myField + 1; // it is not a getter for the field +} + +class Good { + final int myField = 0; +}