Skip to content
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

149-new-lint-avoid_final_with_getter #161

Merged
merged 9 commits into from
Apr 19, 2024
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.
solid-vovabeloded marked this conversation as resolved.
Show resolved Hide resolved
///
/// 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 @@ -59,3 +59,4 @@ custom_lint:
- prefer_last
- prefer_match_file_name
- proper_super_calls
- avoid_final_with_getter
28 changes: 28 additions & 0 deletions lint_test/avoid_final_with_getter_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// 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;
solid-vovabeloded marked this conversation as resolved.
Show resolved Hide resolved
}

class Fail2 {
final int _myField = 0;

// expect_lint: avoid_final_with_getter
int get myFieldInt => _myField;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to add a test for the static field and getter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it does


class Skipped {
final int _myField = 0;

int get myField => _myField + 1; // it is not a getter for the field
}

class Good {
final int myField = 0;
}
Loading