Skip to content

Commit bb53722

Browse files
Address PR suggestions
1 parent 9abcdde commit bb53722

5 files changed

Lines changed: 64 additions & 77 deletions

File tree

lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,25 +31,25 @@ import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/visitors
3131
///
3232
class AvoidUnnecessaryReturnVariableRule extends AnalysisRule {
3333
/// The name of the lint rule.
34-
static const lintName = 'avoid_unnecessary_return_variable';
34+
static const _lintName = 'avoid_unnecessary_return_variable';
3535

3636
/// The message shown when the lint is triggered.
37-
static const String lintMessage = """
37+
static const String _lintMessage = """
3838
Avoid creating unnecessary variable only for return.
3939
Rewrite the variable evaluation into return statement instead.""";
4040

4141
/// Lint code.
4242
static const LintCode _code = LintCode(
43-
lintName,
44-
lintMessage,
43+
_lintName,
44+
_lintMessage,
4545
);
4646

4747
/// Creates a new instance of [AvoidUnnecessaryReturnVariableRule]
4848
AvoidUnnecessaryReturnVariableRule()
49-
: super(
50-
name: lintName,
51-
description: lintMessage,
52-
);
49+
: super(
50+
name: _lintName,
51+
description: _lintMessage,
52+
);
5353

5454
@override
5555
LintCode get diagnosticCode => _code;

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

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,23 @@ class AvoidUnnecessaryReturnVariableVisitor extends SimpleAstVisitor<void> {
1616
@override
1717
void visitReturnStatement(ReturnStatement node) {
1818
final expr = node.expression;
19-
if (expr is! SimpleIdentifier) {
20-
return;
21-
}
19+
if (expr is! SimpleIdentifier) return;
2220

2321
final element = expr.element;
24-
if (element is! LocalVariableElement) {
25-
return;
26-
}
22+
if (element is! LocalVariableElement) return;
2723

28-
if (!element.isFinal && !element.isConst) {
29-
return;
30-
}
24+
if (!element.isFinal && !element.isConst) return;
3125

3226
final block = node.parent;
3327
if (block == null) return;
3428

35-
final returnVariableUsageVisitor =
36-
ReturnVariableUsageVisitor(node, element);
29+
final returnVariableUsageVisitor = ReturnVariableUsageVisitor(
30+
node,
31+
element,
32+
);
3733

3834
block.visitChildren(returnVariableUsageVisitor);
39-
if (!returnVariableUsageVisitor.hasBadStatementCount()) return;
35+
if (!returnVariableUsageVisitor.hasBadStatementCount) return;
4036

4137
//it is 100% bad if return statement follows declaration
4238
if (!returnVariableUsageVisitor.foundTokensBetweenDeclarationAndReturn) {
@@ -63,17 +59,12 @@ class AvoidUnnecessaryReturnVariableVisitor extends SimpleAstVisitor<void> {
6359
}
6460

6561
bool _isSimpleIdentifierImmutable(SimpleIdentifier identifier) {
66-
switch (identifier.element) {
67-
case final VariableElement variable:
68-
return variable.isFinal || variable.isConst;
69-
70-
case ClassElement _:
71-
return true;
72-
73-
case GetterElement(:final PropertyInducingElement variable):
74-
return variable.isFinal || variable.isConst;
75-
}
76-
77-
return false;
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: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_un
88
/// return variable is used somewhere except return statement and
99
/// whether it is immutable.
1010
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+
1117
final ReturnStatement _returnStatement;
1218
final LocalVariableElement _returnVariableElement;
1319

@@ -17,16 +23,10 @@ class ReturnVariableUsageVisitor extends RecursiveAstVisitor<void> {
1723
/// After visiting holds info about whether there are any tokens
1824
bool foundTokensBetweenDeclarationAndReturn = false;
1925

20-
/// The problem expects that exactly 1 mention of return variable.
21-
/// VariableDeclarationStatement doesn't count when visiting SimpleIdentifier.
22-
/// Any other amount of variable mentions implies that it is used somewhere
23-
/// except return, so its existence is justified.
24-
static const _badStatementCount = 1;
25-
2626
int _variableStatementCounter = 0;
2727

2828
/// Defines whether the variables is used in return statement only.
29-
bool hasBadStatementCount() =>
29+
bool get hasBadStatementCount =>
3030
_variableStatementCounter == _badStatementCount;
3131

3232
/// Creates a new instance of [ReturnVariableUsageVisitor].
@@ -66,8 +66,9 @@ class ReturnVariableUsageVisitor extends RecursiveAstVisitor<void> {
6666
VariableDeclarationStatement variableDeclaration,
6767
ReturnStatement returnStatement,
6868
) {
69-
final tokenBeforeReturn =
70-
_returnStatement.findPrevious(_returnStatement.beginToken);
69+
final tokenBeforeReturn = _returnStatement.findPrevious(
70+
_returnStatement.beginToken,
71+
);
7172

7273
if (tokenBeforeReturn != variableDeclaration.endToken) {
7374
foundTokensBetweenDeclarationAndReturn = true;

pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ documentation: https://solid-software.github.io/solid_lints/docs/intro
88
topics: [lints, linter, lint, analysis, analyzer]
99

1010
environment:
11-
sdk: ">=3.5.0 <4.0.0"
11+
sdk: ">=3.9.0 <4.0.0"
1212

1313
dependencies:
1414
analyzer: ^10.0.1

test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart

Lines changed: 30 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -17,44 +17,37 @@ class AvoidUnnecessaryReturnVariableTest extends AnalysisRuleTest {
1717
}
1818

1919
@override
20-
String get analysisRule => AvoidUnnecessaryReturnVariableRule.lintName;
20+
String get analysisRule => rule.name;
2121

2222
void test_does_not_report_if_return_good_trivial() async {
23-
await assertNoDiagnostics(
24-
r'''
23+
await assertNoDiagnostics(r'''
2524
int returnVarTestTrivial() {
2625
return 1;
2726
}
28-
''',
29-
);
27+
''');
3028
}
3129

3230
void test_does_not_report_if_return_is_mutable() async {
33-
await assertNoDiagnostics(
34-
r'''
31+
await assertNoDiagnostics(r'''
3532
int returnVarTestMutable() {
3633
var a = 1;
3734
a++;
3835
3936
return a;
4037
}
41-
''',
42-
);
38+
''');
4339
}
4440

4541
void test_does_not_report_if_returns_parameter() async {
46-
await assertNoDiagnostics(
47-
r'''
42+
await assertNoDiagnostics(r'''
4843
int returnVarTestReturnParameter(int param) {
4944
return param;
5045
}
51-
''',
52-
);
46+
''');
5347
}
5448

5549
void test_does_not_report_if_return_is_cached_mutable() async {
56-
await assertNoDiagnostics(
57-
r'''
50+
await assertNoDiagnostics(r'''
5851
int returnVarTestCachedMutable() {
5952
var a = 1;
6053
final result = a;
@@ -64,12 +57,12 @@ int returnVarTestCachedMutable() {
6457
}
6558
6659
void _doNothing() {}
67-
''',
68-
);
60+
''');
6961
}
7062

7163
void test_reports_if_return_follows_declaration() async {
72-
await assertDiagnostics(r'''
64+
await assertDiagnostics(
65+
r'''
7366
int returnVarTestReturnFollowsDeclaration() {
7467
var a = 1;
7568
final result = a;
@@ -78,12 +71,13 @@ int returnVarTestReturnFollowsDeclaration() {
7871
7972
return result;
8073
}
81-
''', [lint(105, 14)]);
74+
''',
75+
[lint(105, 14)],
76+
);
8277
}
8378

8479
void test_does_not_report_if_return_is_cached_another_method_result() async {
85-
await assertNoDiagnostics(
86-
r'''
80+
await assertNoDiagnostics(r'''
8781
int returnVarTestCachedAnotherMethodResult() {
8882
var a = 1;
8983
final result = _testValueEval();
@@ -97,13 +91,11 @@ int _testValueEval() {
9791
}
9892
9993
void _doNothing() {}
100-
''',
101-
);
94+
''');
10295
}
10396

10497
void test_does_not_report_if_return_is_cached_object_field() async {
105-
await assertNoDiagnostics(
106-
r'''
98+
await assertNoDiagnostics(r'''
10799
int returnVarTestCachedObjectField() {
108100
final obj = _TestClass();
109101
final result = obj.varField;
@@ -121,36 +113,37 @@ class _TestClass {
121113
}
122114
123115
void _doNothing() {}
124-
''',
125-
);
116+
''');
126117
}
127118

128119
void test_does_not_report_if_return_used_variable() async {
129-
await assertNoDiagnostics(
130-
r'''
120+
await assertNoDiagnostics(r'''
131121
int returnVarTestUsedVariable() {
132122
var a = 1;
133123
final result = 2;
134124
a += result;
135125
136126
return result;
137127
}
138-
''',
139-
);
128+
''');
140129
}
141130

142131
void test_reports_if_return_is_bad_trivial() async {
143-
await assertDiagnostics(r'''
132+
await assertDiagnostics(
133+
r'''
144134
int returnVarTestBadTrivial() {
145135
final result = 1;
146136
147137
return result;
148138
}
149-
''', [lint(55, 14)]);
139+
''',
140+
[lint(55, 14)],
141+
);
150142
}
151143

152144
void test_reports_if_return_is_bad_immutable_expression() async {
153-
await assertDiagnostics(r'''
145+
await assertDiagnostics(
146+
r'''
154147
int returnVarTestBadImmutableExpression() {
155148
const constLocal = 1;
156149
final finalLocal = 1;
@@ -175,6 +168,8 @@ class _TestClass {
175168
}
176169
177170
void _doNothing() {}
178-
''', [lint(304, 14)]);
171+
''',
172+
[lint(304, 14)],
173+
);
179174
}
180175
}

0 commit comments

Comments
 (0)