Skip to content

Commit ba7d7fb

Browse files
daria-trusca-soliddaria-trusca-solid
authored
Merge branch 'analysis_server_migration' into migrate_newline_before_return_visitor
2 parents 8134563 + be820c7 commit ba7d7fb

18 files changed

Lines changed: 969 additions & 873 deletions

‎lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart‎

Lines changed: 29 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
1-
import 'package:analyzer/dart/ast/ast.dart';
2-
import 'package:analyzer/dart/element/element.dart';
3-
import 'package:analyzer/error/listener.dart';
4-
import 'package:custom_lint_builder/custom_lint_builder.dart';
5-
import 'package:solid_lints/src/common/visitors/select_expression_identifiers_visitor.dart';
1+
import 'package:analyzer/analysis_rule/analysis_rule.dart';
2+
import 'package:analyzer/analysis_rule/rule_context.dart';
3+
import 'package:analyzer/analysis_rule/rule_visitor_registry.dart';
4+
import 'package:analyzer/error/error.dart';
65
import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart';
7-
import 'package:solid_lints/src/models/rule_config.dart';
8-
import 'package:solid_lints/src/models/solid_lint_rule.dart';
96

107
/// An `avoid_unnecessary_return_variable` rule which forbids returning
118
/// an immutable variable if it can be rewritten in return statement itself.
@@ -32,109 +29,37 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart';
3229
/// }
3330
/// ```
3431
///
35-
class AvoidUnnecessaryReturnVariableRule extends SolidLintRule {
36-
/// This lint rule represents the error
37-
/// when unnecessary return variable statement is found
38-
static const lintName = 'avoid_unnecessary_return_variable';
32+
class AvoidUnnecessaryReturnVariableRule extends AnalysisRule {
33+
/// The name of the lint rule.
34+
static const _lintName = 'avoid_unnecessary_return_variable';
3935

40-
AvoidUnnecessaryReturnVariableRule._(super.config);
36+
/// The message shown when the lint is triggered.
37+
static const String _lintMessage = """
38+
Avoid creating unnecessary variable only for return.
39+
Rewrite the variable evaluation into return statement instead.""";
40+
41+
/// Lint code.
42+
static const LintCode _code = LintCode(
43+
_lintName,
44+
_lintMessage,
45+
);
4146

4247
/// Creates a new instance of [AvoidUnnecessaryReturnVariableRule]
43-
/// based on the lint configuration.
44-
factory AvoidUnnecessaryReturnVariableRule.createRule(
45-
CustomLintConfigs configs,
46-
) {
47-
final rule = RuleConfig(
48-
configs: configs,
49-
name: lintName,
50-
problemMessage: (_) => """
51-
Avoid creating unnecessary variable only for return.
52-
Rewrite the variable evaluation into return statement instead.""",
53-
);
48+
AvoidUnnecessaryReturnVariableRule()
49+
: super(
50+
name: _lintName,
51+
description: _lintMessage,
52+
);
5453

55-
return AvoidUnnecessaryReturnVariableRule._(rule);
56-
}
54+
@override
55+
LintCode get diagnosticCode => _code;
5756

5857
@override
59-
void run(
60-
CustomLintResolver resolver,
61-
DiagnosticReporter reporter,
62-
CustomLintContext context,
58+
void registerNodeProcessors(
59+
RuleVisitorRegistry registry,
60+
RuleContext context,
6361
) {
64-
context.registry.addReturnStatement(
65-
(statement) {
66-
_checkReturnStatement(
67-
statement: statement,
68-
reporter: reporter,
69-
context: context,
70-
);
71-
},
72-
);
73-
}
74-
75-
void _checkReturnStatement({
76-
required ReturnStatement statement,
77-
required DiagnosticReporter reporter,
78-
required CustomLintContext context,
79-
}) {
80-
final expr = statement.expression;
81-
if (expr is! SimpleIdentifier) return;
82-
83-
final element = expr.element;
84-
if (element is! LocalVariableElement) return;
85-
final returnVariableElement = element;
86-
87-
if (!returnVariableElement.isFinal && !returnVariableElement.isConst) {
88-
return;
89-
}
90-
91-
final blockBody = statement.parent;
92-
if (blockBody == null) return;
93-
94-
final visitor = AvoidUnnecessaryReturnVariableVisitor(
95-
returnVariableElement,
96-
statement,
97-
);
98-
blockBody.visitChildren(visitor);
99-
100-
if (!visitor.hasBadStatementCount()) return;
101-
102-
//it is 100% bad if return statement follows declaration
103-
if (!visitor.foundTokensBetweenDeclarationAndReturn) {
104-
reporter.atNode(statement, code);
105-
return;
106-
}
107-
108-
final declaration = visitor.variableDeclaration;
109-
110-
//check if immutable
111-
final initializer = declaration?.initializer;
112-
if (initializer == null) return;
113-
114-
if (!_isExpressionImmutable(initializer)) return;
115-
116-
reporter.atNode(statement, code);
117-
}
118-
119-
bool _isExpressionImmutable(Expression expr) {
120-
final visitor = SelectExpressionIdentifiersVisitor();
121-
expr.accept(visitor);
122-
123-
return visitor.identifiers.every(_isSimpleIdentifierImmutable);
124-
}
125-
126-
bool _isSimpleIdentifierImmutable(SimpleIdentifier identifier) {
127-
switch (identifier.element) {
128-
case final VariableElement variable:
129-
return variable.isFinal || variable.isConst;
130-
131-
case ClassElement _:
132-
return true;
133-
134-
case GetterElement(:final PropertyInducingElement variable):
135-
return variable.isFinal || variable.isConst;
136-
}
137-
138-
return false;
62+
final visitor = AvoidUnnecessaryReturnVariableVisitor(this);
63+
registry.addReturnStatement(this, visitor);
13964
}
14065
}

‎lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart‎

Lines changed: 48 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,79 +1,70 @@
11
import 'package:analyzer/dart/ast/ast.dart';
22
import 'package:analyzer/dart/ast/visitor.dart';
33
import 'package:analyzer/dart/element/element.dart';
4-
import 'package:collection/collection.dart';
4+
import 'package:solid_lints/src/common/visitors/select_expression_identifiers_visitor.dart';
5+
import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart';
6+
import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart';
57

6-
/// This visitor is searches all uses of a single local variable,
7-
/// including variable declaration.
8-
class AvoidUnnecessaryReturnVariableVisitor extends RecursiveAstVisitor<void> {
9-
/// The problem expects that exactly 1 mention of return variable.
10-
/// VariableDeclarationStatement doesn't count when visiting SimpleIdentifier.
11-
/// Any other amount of variable mentions implies that it is used somewhere
12-
/// except return, so its existence is justified.
13-
static const _badStatementCount = 1;
8+
/// Visitor for [AvoidUnnecessaryReturnVariableRule].
9+
class AvoidUnnecessaryReturnVariableVisitor extends SimpleAstVisitor<void> {
10+
/// The rule associated with this visitor.
11+
final AvoidUnnecessaryReturnVariableRule _rule;
1412

15-
final LocalVariableElement _returnVariableElement;
13+
/// Creates a new instance of [AvoidUnnecessaryReturnVariableVisitor].
14+
AvoidUnnecessaryReturnVariableVisitor(this._rule);
1615

17-
bool _foundTokensBetweenDeclarationAndReturn = false;
18-
VariableDeclaration? _variableDeclaration;
19-
int _variableStatementCounter = 0;
16+
@override
17+
void visitReturnStatement(ReturnStatement node) {
18+
final expr = node.expression;
19+
if (expr is! SimpleIdentifier) return;
2020

21-
final ReturnStatement _returnStatement;
21+
final element = expr.element;
22+
if (element is! LocalVariableElement) return;
2223

23-
/// After visiting holds info about whether there are any tokens
24-
/// between variable declaration and return statement
25-
bool get foundTokensBetweenDeclarationAndReturn =>
26-
_foundTokensBetweenDeclarationAndReturn;
24+
if (!element.isFinal && !element.isConst) return;
2725

28-
/// Returns statement of local variable declaration
29-
VariableDeclaration? get variableDeclaration => _variableDeclaration;
26+
final block = node.parent;
27+
if (block == null) return;
3028

31-
/// Creates a new instance of [AvoidUnnecessaryReturnVariableVisitor].
32-
AvoidUnnecessaryReturnVariableVisitor(
33-
this._returnVariableElement,
34-
this._returnStatement,
35-
);
29+
final returnVariableUsageVisitor = ReturnVariableUsageVisitor(
30+
node,
31+
element,
32+
);
3633

37-
/// Defines whether the variables is used in return statement only.
38-
bool hasBadStatementCount() =>
39-
_variableStatementCounter == _badStatementCount;
34+
block.visitChildren(returnVariableUsageVisitor);
35+
if (!returnVariableUsageVisitor.hasBadStatementCount) return;
4036

41-
@override
42-
void visitVariableDeclarationStatement(VariableDeclarationStatement node) {
43-
if (_collectVariableDeclaration(node)) {
44-
_checkTokensInBetween(node, _returnStatement);
37+
//it is 100% bad if return statement follows declaration
38+
if (!returnVariableUsageVisitor.foundTokensBetweenDeclarationAndReturn) {
39+
_rule.reportAtNode(node);
40+
return;
4541
}
46-
super.visitVariableDeclarationStatement(node);
47-
}
4842

49-
@override
50-
void visitSimpleIdentifier(SimpleIdentifier node) {
51-
if (node.element?.id == _returnVariableElement.id) {
52-
_variableStatementCounter++;
53-
}
43+
final declaration = returnVariableUsageVisitor.variableDeclaration;
5444

55-
super.visitSimpleIdentifier(node);
56-
}
45+
//check if immutable
46+
final initializer = declaration?.initializer;
47+
if (initializer == null) return;
5748

58-
bool _collectVariableDeclaration(VariableDeclarationStatement node) {
59-
final targetVariable = node.variables.variables.firstWhereOrNull(
60-
(v) => v.declaredFragment?.element.id == _returnVariableElement.id,
61-
);
62-
if (targetVariable == null) return false;
49+
if (!_isExpressionImmutable(initializer)) return;
6350

64-
_variableDeclaration = targetVariable;
65-
return true;
51+
_rule.reportAtNode(node);
6652
}
6753

68-
void _checkTokensInBetween(
69-
VariableDeclarationStatement variableDeclaration,
70-
ReturnStatement returnStatement,
71-
) {
72-
final tokenBeforeReturn =
73-
_returnStatement.findPrevious(_returnStatement.beginToken);
54+
bool _isExpressionImmutable(Expression expr) {
55+
final visitor = SelectExpressionIdentifiersVisitor();
56+
expr.accept(visitor);
7457

75-
if (tokenBeforeReturn != variableDeclaration.endToken) {
76-
_foundTokensBetweenDeclarationAndReturn = true;
77-
}
58+
return visitor.identifiers.every(_isSimpleIdentifierImmutable);
59+
}
60+
61+
bool _isSimpleIdentifierImmutable(SimpleIdentifier identifier) {
62+
return switch (identifier.element) {
63+
ClassElement _ => true,
64+
final VariableElement variable => variable.isFinal || variable.isConst,
65+
GetterElement(:final PropertyInducingElement variable) =>
66+
variable.isFinal || variable.isConst,
67+
_ => false,
68+
};
7869
}
7970
}

‎lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import 'package:analyzer/dart/ast/ast.dart';
2+
import 'package:analyzer/dart/ast/visitor.dart';
3+
import 'package:analyzer/dart/element/element.dart';
4+
import 'package:collection/collection.dart';
5+
import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart';
6+
7+
/// Visitor for [AvoidUnnecessaryReturnVariableRule] which checks whether the
8+
/// return variable is used somewhere except return statement and
9+
/// whether it is immutable.
10+
class ReturnVariableUsageVisitor extends RecursiveAstVisitor<void> {
11+
/// The problem expects that exactly 1 mention of return variable.
12+
/// VariableDeclarationStatement doesn't count when visiting SimpleIdentifier.
13+
/// Any other amount of variable mentions implies that it is used somewhere
14+
/// except return, so its existence is justified.
15+
static const _badStatementCount = 1;
16+
17+
final ReturnStatement _returnStatement;
18+
final LocalVariableElement _returnVariableElement;
19+
20+
/// After visiting holds the declaration of return variable
21+
VariableDeclaration? variableDeclaration;
22+
23+
/// After visiting holds info about whether there are any tokens
24+
bool foundTokensBetweenDeclarationAndReturn = false;
25+
26+
int _variableStatementCounter = 0;
27+
28+
/// Defines whether the variables is used in return statement only.
29+
bool get hasBadStatementCount =>
30+
_variableStatementCounter == _badStatementCount;
31+
32+
/// Creates a new instance of [ReturnVariableUsageVisitor].
33+
ReturnVariableUsageVisitor(
34+
this._returnStatement,
35+
this._returnVariableElement,
36+
);
37+
38+
@override
39+
void visitVariableDeclarationStatement(VariableDeclarationStatement node) {
40+
if (_collectVariableDeclaration(node)) {
41+
_checkTokensInBetween(node, _returnStatement);
42+
}
43+
super.visitVariableDeclarationStatement(node);
44+
}
45+
46+
@override
47+
void visitSimpleIdentifier(SimpleIdentifier node) {
48+
if (node.element?.id == _returnVariableElement.id) {
49+
_variableStatementCounter++;
50+
}
51+
52+
super.visitSimpleIdentifier(node);
53+
}
54+
55+
bool _collectVariableDeclaration(VariableDeclarationStatement node) {
56+
final targetVariable = node.variables.variables.firstWhereOrNull(
57+
(v) => v.declaredFragment?.element.id == _returnVariableElement.id,
58+
);
59+
if (targetVariable == null) return false;
60+
variableDeclaration = targetVariable;
61+
62+
return true;
63+
}
64+
65+
void _checkTokensInBetween(
66+
VariableDeclarationStatement variableDeclaration,
67+
ReturnStatement returnStatement,
68+
) {
69+
final tokenBeforeReturn = _returnStatement.findPrevious(
70+
_returnStatement.beginToken,
71+
);
72+
73+
if (tokenBeforeReturn != variableDeclaration.endToken) {
74+
foundTokensBetweenDeclarationAndReturn = true;
75+
}
76+
}
77+
}

0 commit comments

Comments
 (0)