Skip to content

Commit

Permalink
149-new-lint-avoid_final_with_getter (#161)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
4akloon authored Apr 19, 2024
1 parent 8db68e6 commit 8dc2cd5
Show file tree
Hide file tree
Showing 8 changed files with 186 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
1 change: 1 addition & 0 deletions lib/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/solid_lints.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -61,6 +62,7 @@ class _SolidLints extends PluginBase {
PreferMatchFileNameRule.createRule(configs),
ProperSuperCallsRule.createRule(configs),
AvoidDebugPrint.createRule(configs),
AvoidFinalWithGetterRule.createRule(configs),
];

// Return only enabled rules
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
});
}
}
Original file line number Diff line number Diff line change
@@ -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<void> {
final _getters = <MethodDeclaration>[];

/// List of getters
Iterable<MethodDeclaration> 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);
}
}
Original file line number Diff line number Diff line change
@@ -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<void> {
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,
};
}
1 change: 1 addition & 0 deletions lint_test/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,4 @@ custom_lint:
- prefer_last
- prefer_match_file_name
- proper_super_calls
- avoid_final_with_getter
35 changes: 35 additions & 0 deletions lint_test/avoid_final_with_getter_test.dart
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit 8dc2cd5

Please sign in to comment.