Skip to content

Commit 01fb176

Browse files
oprypinoprypin
authored
Lint against assigning a variable to itself (dart-archive/linter#4067)
* Lint against assigning a variable to itself This lint against any assignment like `x = x` or `foo.bar = foo.bar`, only for simple identifiers. For setters with custom update logic, there *are* valid use cases for this, but they should be refactored anyway. There is an example about that included. * Address review comments * Rename lint * Fix punctuation
1 parent 8a401d1 commit 01fb176

File tree

5 files changed

+304
-0
lines changed

5 files changed

+304
-0
lines changed

‎example/all.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ linter:
9898
- no_literal_bool_comparisons
9999
- no_logic_in_create_state
100100
- no_runtimeType_toString
101+
- no_self_assignments
101102
- non_constant_identifier_names
102103
- noop_primitive_operations
103104
- null_check_on_nullable_type_parameter

‎lib/src/rules.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ import 'rules/no_leading_underscores_for_local_identifiers.dart';
107107
import 'rules/no_literal_bool_comparisons.dart';
108108
import 'rules/no_logic_in_create_state.dart';
109109
import 'rules/no_runtimeType_toString.dart';
110+
import 'rules/no_self_assignments.dart';
110111
import 'rules/non_constant_identifier_names.dart';
111112
import 'rules/noop_primitive_operations.dart';
112113
import 'rules/null_check_on_nullable_type_parameter.dart';
@@ -339,6 +340,7 @@ void registerLintRules({bool inTestMode = false}) {
339340
..register(NoLogicInCreateState())
340341
..register(NoopPrimitiveOperations())
341342
..register(NoRuntimeTypeToString())
343+
..register(NoSelfAssignments())
342344
..register(NullCheckOnNullableTypeParameter())
343345
..register(NullClosures())
344346
..register(OmitLocalVariableTypes())

‎lib/src/rules/no_self_assignments.dart

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analyzer/dart/ast/ast.dart';
6+
import 'package:analyzer/dart/ast/token.dart';
7+
import 'package:analyzer/dart/ast/visitor.dart';
8+
import '../analyzer.dart';
9+
10+
const _desc = r"Don't assign a variable to itself.";
11+
12+
const _details = r'''
13+
**DON'T** assign a variable to itself. Usually this is a mistake.
14+
15+
**BAD:**
16+
```dart
17+
class C {
18+
int x;
19+
20+
C(int x) {
21+
x = x;
22+
}
23+
}
24+
```
25+
26+
**GOOD:**
27+
```dart
28+
class C {
29+
int x;
30+
31+
C(int x) : x = x;
32+
}
33+
```
34+
35+
**GOOD:**
36+
```dart
37+
class C {
38+
int x;
39+
40+
C(int x) {
41+
this.x = x;
42+
}
43+
}
44+
```
45+
46+
**BAD:**
47+
```dart
48+
class C {
49+
int _x = 5;
50+
51+
int get x => _x;
52+
53+
set x(int x) {
54+
_x = x;
55+
_customUpdateLogic();
56+
}
57+
58+
void _customUpdateLogic() {
59+
print('updated');
60+
}
61+
62+
void example() {
63+
x = x;
64+
}
65+
}
66+
```
67+
68+
**GOOD:**
69+
```dart
70+
class C {
71+
int _x = 5;
72+
73+
int get x => _x;
74+
75+
set x(int x) {
76+
_x = x;
77+
_customUpdateLogic();
78+
}
79+
80+
void _customUpdateLogic() {
81+
print('updated');
82+
}
83+
84+
void example() {
85+
_customUpdateLogic();
86+
}
87+
}
88+
```
89+
90+
**BAD:**
91+
```dart
92+
class C {
93+
int x = 5;
94+
95+
void update(C other) {
96+
this.x = this.x;
97+
}
98+
}
99+
```
100+
101+
**GOOD:**
102+
```dart
103+
class C {
104+
int x = 5;
105+
106+
void update(C other) {
107+
this.x = other.x;
108+
}
109+
}
110+
```
111+
112+
''';
113+
114+
class NoSelfAssignments extends LintRule {
115+
static const LintCode code = LintCode('no_self_assignments',
116+
'The variable or property is being assigned to itself.',
117+
correctionMessage:
118+
'Try removing the assignment that has no direct effect.');
119+
120+
NoSelfAssignments()
121+
: super(
122+
name: 'no_self_assignments',
123+
description: _desc,
124+
details: _details,
125+
group: Group.errors);
126+
127+
@override
128+
LintCode get lintCode => code;
129+
130+
@override
131+
void registerNodeProcessors(
132+
NodeLintRegistry registry, LinterContext context) {
133+
var visitor = _Visitor(this);
134+
registry.addAssignmentExpression(this, visitor);
135+
}
136+
}
137+
138+
class _Visitor extends SimpleAstVisitor<void> {
139+
final LintRule rule;
140+
141+
_Visitor(this.rule);
142+
143+
@override
144+
void visitAssignmentExpression(AssignmentExpression node) {
145+
if (node.operator.type != TokenType.EQ) return;
146+
var lhs = node.leftHandSide;
147+
var rhs = node.rightHandSide;
148+
if (lhs is Identifier && rhs is Identifier) {
149+
if (lhs.name == rhs.name) {
150+
rule.reportLint(node);
151+
}
152+
}
153+
}
154+
}

‎test/rules/all.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ import 'missing_whitespace_between_adjacent_strings_test.dart'
7070
import 'no_duplicate_case_values_test.dart' as no_duplicate_case_values;
7171
import 'no_leading_underscores_for_local_identifiers_test.dart'
7272
as no_leading_underscores_for_local_identifiers;
73+
import 'no_self_assignments_test.dart' as no_self_assignments;
7374
import 'non_adjacent_strings_in_list_test.dart' as no_adjacent_strings_in_list;
7475
import 'non_constant_identifier_names_test.dart'
7576
as non_constant_identifier_names;
@@ -199,6 +200,7 @@ void main() {
199200
no_adjacent_strings_in_list.main();
200201
no_duplicate_case_values.main();
201202
no_leading_underscores_for_local_identifiers.main();
203+
no_self_assignments.main();
202204
non_constant_identifier_names.main();
203205
null_check_on_nullable_type_parameter.main();
204206
null_closures.main();

‎test/rules/no_self_assignments_test.dart

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:test_reflective_loader/test_reflective_loader.dart';
6+
7+
import '../rule_test_support.dart';
8+
9+
main() {
10+
defineReflectiveSuite(() {
11+
defineReflectiveTests(NoSelfAssignmentsTest);
12+
});
13+
}
14+
15+
@reflectiveTest
16+
class NoSelfAssignmentsTest extends LintRuleTest {
17+
@override
18+
String get lintRule => 'no_self_assignments';
19+
20+
test_fieldAssignment() async {
21+
await assertDiagnostics(r'''
22+
class C {
23+
int x = 5;
24+
25+
C(int x) {
26+
x = x;
27+
}
28+
}
29+
''', [lint(41, 5)]);
30+
}
31+
32+
test_fieldAssignmentDifferentVar() async {
33+
await assertNoDiagnostics(r'''
34+
class C {
35+
int x = 5;
36+
37+
C(int y) {
38+
x = y;
39+
}
40+
}
41+
''');
42+
}
43+
44+
test_fieldAssignmentExplicit() async {
45+
await assertNoDiagnostics(r'''
46+
class C {
47+
int x = 5;
48+
49+
C(int x) {
50+
this.x = x;
51+
}
52+
}
53+
''');
54+
}
55+
56+
test_fieldAssignmentExplicitSameVar() async {
57+
await assertDiagnostics(r'''
58+
class C {
59+
int x = 5;
60+
61+
void update(C other) {
62+
other.x = other.x;
63+
}
64+
}
65+
''', [lint(53, 17)]);
66+
}
67+
68+
test_fieldAssignmentThisAndDifferentTarget() async {
69+
await assertNoDiagnostics(r'''
70+
class C {
71+
int x = 5;
72+
73+
void update(C other) {
74+
this.x = other.x;
75+
}
76+
}
77+
''');
78+
}
79+
80+
test_fieldAssignmentDifferentTargets() async {
81+
await assertNoDiagnostics(r'''
82+
class C {
83+
String hello = 'ok';
84+
}
85+
86+
void test(C one, C two) {
87+
one.hello = two.hello;
88+
}
89+
''');
90+
}
91+
92+
test_fieldInitialization() async {
93+
await assertNoDiagnostics(r'''
94+
class C {
95+
int x;
96+
97+
C(int x) : x = x;
98+
}
99+
''');
100+
}
101+
102+
test_propertyAssignment() async {
103+
await assertDiagnostics(r'''
104+
class C {
105+
int _x = 5;
106+
107+
int get x => _x;
108+
109+
set x(int x) {
110+
_x = x;
111+
}
112+
113+
void example() {
114+
x = x;
115+
}
116+
}
117+
''', [lint(102, 5)]);
118+
}
119+
120+
test_classMemberAssignment() async {
121+
await assertDiagnostics(r'''
122+
class C {
123+
static String foo = "foo";
124+
}
125+
126+
void main() {
127+
C.foo = C.foo;
128+
}
129+
''', [lint(58, 13)]);
130+
}
131+
132+
test_classMemberAssignmentUnrelated() async {
133+
await assertNoDiagnostics(r'''
134+
class C {
135+
static String foo = "foo";
136+
}
137+
138+
void main() {
139+
String foo;
140+
foo = C.foo; // OK
141+
print(foo);
142+
}
143+
''');
144+
}
145+
}

0 commit comments

Comments
 (0)