Skip to content

Commit 477ae52

Browse files
tozpeaktozpeak
committed
Issue #92: implemented advanced rule with visitor, checking where variable is defined as well as how often is used
1 parent 1c12bbb commit 477ae52

File tree

2 files changed

+155
-25
lines changed

2 files changed

+155
-25
lines changed

‎lib/src/lints/dont_create_a_return_var/dont_create_a_return_var_rule.dart‎

Lines changed: 73 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import 'package:analyzer/dart/ast/ast.dart';
2-
import 'package:analyzer/dart/ast/syntactic_entity.dart';
2+
import 'package:analyzer/dart/element/element2.dart';
33
import 'package:analyzer/error/listener.dart';
4-
import 'package:collection/collection.dart';
54
import 'package:custom_lint_builder/custom_lint_builder.dart';
5+
import 'package:solid_lints/src/lints/dont_create_a_return_var/visitors/dont_create_a_return_var_visitor.dart';
66
import 'package:solid_lints/src/models/rule_config.dart';
77
import 'package:solid_lints/src/models/solid_lint_rule.dart';
88

@@ -77,34 +77,82 @@ Rewrite the variable evaluation into return statement instead.""",
7777
}) {
7878
final expr = statement.expression;
7979
if (expr is! SimpleIdentifier) return;
80-
final returnVariableToken = expr.token;
8180

82-
//Looking for statement previous to return
83-
final parent = statement.parent;
84-
if (parent == null) return;
81+
final element = expr.element;
82+
if(element is! LocalVariableElement2) return;
83+
final returnVariableElement = element;
8584

86-
SyntacticEntity? previous;
87-
for (final child in parent.childEntities) {
88-
if (child == statement) break;
89-
previous = child;
85+
if(!returnVariableElement.isFinal && !returnVariableElement.isConst) return;
86+
87+
final blockBody = statement.parent;
88+
if (blockBody == null) return;
89+
90+
final visitor = DontCreateAReturnVarVisitor(
91+
returnVariableElement,
92+
statement,
93+
);
94+
blockBody.visitChildren(visitor);
95+
96+
if (!visitor.hasBadStatementCount()) return;
97+
98+
//it is 100% bad if return statement follows declaration
99+
if (!visitor.foundTokensBetweenDeclarationAndReturn) {
100+
reporter.atNode(statement, code);
101+
return;
90102
}
91-
if (previous == null) return;
92103

93-
if (previous is! VariableDeclarationStatement) return;
94-
final declarationStatement = previous;
104+
final declaration = visitor.variableDeclaration;
95105

96-
//Checking if return variable was declared in previous statement
97-
final VariableDeclaration? variableDeclaration =
98-
declarationStatement.variables.variables
99-
.firstWhereOrNull(
100-
(v) => v.name.toString() == returnVariableToken.toString(),
101-
);
102-
if (variableDeclaration == null) return;
103-
104-
//Skipping mutable variables
105-
if (!variableDeclaration.isFinal && !variableDeclaration.isConst) return;
106-
107-
reporter.atNode(expr, code);
106+
//check if immutable
107+
final initializer = declaration?.initializer;
108+
if (initializer == null) return;
109+
110+
if (!_isExpressionImmutable(initializer)) return;
111+
112+
reporter.atNode(statement, code);
108113
}
109114

115+
bool _isExpressionImmutable(Expression expr) {
116+
if (expr is Literal) return true;
117+
118+
if (expr case final PrefixedIdentifier prefixed) {
119+
return
120+
_isExpressionImmutable(prefixed.prefix)
121+
&& _isExpressionImmutable(prefixed.identifier);
122+
}
123+
124+
if (expr case final BinaryExpression binExpr) {
125+
return
126+
_isExpressionImmutable(binExpr.leftOperand)
127+
&& _isExpressionImmutable(binExpr.rightOperand);
128+
}
129+
130+
if (expr case final SimpleIdentifier identifier) {
131+
return _isSimpleIdentifierImmutable(identifier);
132+
}
133+
134+
return false;
135+
}
136+
137+
bool _isSimpleIdentifierImmutable(SimpleIdentifier identifier) {
138+
if (identifier.element
139+
case final VariableElement2 variable
140+
when variable.isFinal || variable.isConst
141+
) {
142+
return true;
143+
}
144+
145+
if (identifier.element is ClassElement2) return true;
146+
147+
if (identifier.element case final GetterElement getter) {
148+
if (getter.variable3
149+
case final PropertyInducingElement2 property
150+
when property.isFinal || property.isConst
151+
) {
152+
return true;
153+
}
154+
}
155+
156+
return false;
157+
}
110158
}

‎lib/src/lints/dont_create_a_return_var/visitors/dont_create_a_return_var_visitor.dart‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import 'package:analyzer/dart/ast/ast.dart';
2+
import 'package:analyzer/dart/ast/visitor.dart';
3+
import 'package:analyzer/dart/element/element2.dart';
4+
import 'package:collection/collection.dart';
5+
6+
/// This visitor is searches all uses of a single local variable,
7+
/// including variable declaration.
8+
class DontCreateAReturnVarVisitor 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 existance is justified.
13+
static const _badStatementCount = 1;
14+
15+
final LocalVariableElement2 _returnVariableElement;
16+
17+
bool _foundTokensBetweenDeclarationAndReturn = false;
18+
VariableDeclaration? _variableDeclaration;
19+
int _variableStatementCounter = 0;
20+
21+
final ReturnStatement _returnStatement;
22+
23+
/// After visiting holds info about whether there are any tokens
24+
/// between variable declaration and return statement
25+
bool get foundTokensBetweenDeclarationAndReturn =>
26+
_foundTokensBetweenDeclarationAndReturn;
27+
/// Returns statement of local variable declaration
28+
VariableDeclaration? get variableDeclaration
29+
=> _variableDeclaration;
30+
31+
/// Creates a new instance of [DontCreateAReturnVarVisitor].
32+
DontCreateAReturnVarVisitor(
33+
this._returnVariableElement,
34+
this._returnStatement,
35+
);
36+
37+
/// Defines whether the variables is used in return statement only.
38+
bool hasBadStatementCount() =>
39+
_variableStatementCounter == _badStatementCount;
40+
41+
@override
42+
void visitVariableDeclarationStatement(VariableDeclarationStatement node) {
43+
if (_collectVariableDeclaration(node)) {
44+
_checkTokensInBetween(node, _returnStatement);
45+
}
46+
super.visitVariableDeclarationStatement(node);
47+
}
48+
49+
@override
50+
void visitSimpleIdentifier(SimpleIdentifier node) {
51+
if(node.element?.id == _returnVariableElement.id) {
52+
_variableStatementCounter++;
53+
}
54+
55+
super.visitSimpleIdentifier(node);
56+
}
57+
58+
bool _collectVariableDeclaration(VariableDeclarationStatement node) {
59+
final targetVariable =
60+
node.variables.variables
61+
.firstWhereOrNull(
62+
(v)
63+
=> v.declaredElement2?.id == _returnVariableElement.id,
64+
);
65+
if (targetVariable == null) return false;
66+
67+
_variableDeclaration = targetVariable;
68+
return true;
69+
}
70+
71+
void _checkTokensInBetween(
72+
VariableDeclarationStatement variableDeclaration,
73+
ReturnStatement returnStatement,
74+
) {
75+
final tokenBeforeReturn =
76+
_returnStatement.findPrevious(_returnStatement.beginToken);
77+
78+
if(tokenBeforeReturn != variableDeclaration.endToken) {
79+
_foundTokensBetweenDeclarationAndReturn = true;
80+
}
81+
}
82+
}

0 commit comments

Comments
 (0)