Skip to content
This repository was archived by the owner on Jul 16, 2023. It is now read-only.

Commit ba7c199

Browse files
incendialincendial
authored
feat: add Avoid unnecessary setState (#346)
* chore: fix imports * fear: add Avoid unnecessary setstate rule
1 parent 316f32c commit ba7c199

File tree

9 files changed

+589
-1
lines changed

9 files changed

+589
-1
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## Unreleased
44

55
* Remove deprecated `Lines of Executable Code` metric, use `Source lines of Code` instead.
6+
* Add static code diagnostic `avoid-unnecessary-setstate`.
67
* Changed the supported `analyzer` version to `^1.7.0`.
78

89
## 3.2.2
@@ -11,7 +12,7 @@
1112

1213
## 3.2.1
1314

14-
* Remove unnecessary scan by `Lines of Executable Code`
15+
* Remove unnecessary scan by `Lines of Executable Code`.
1516

1617
## 3.2.0
1718

‎README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ Rules configuration is [described here](#configuring-a-rules-entry).
340340
### Flutter specific
341341
342342
- [avoid-returning-widgets](https://github.com/dart-code-checker/dart-code-metrics/blob/master/doc/rules/avoid-returning-widgets.md)   [![Configurable](https://img.shields.io/badge/-configurable-informational)](https://github.com/dart-code-checker/dart-code-metrics/blob/master/doc/rules/avoid-returning-widgets.md#config-example)
343+
- [avoid-unnecessary-setstate](https://github.com/dart-code-checker/dart-code-metrics/blob/master/doc/rules/avoid-unnecessary-setstate.md)
343344
344345
### Intl specific
345346

‎doc/rules/avoid-unnecessary-setstate.md

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
# Avoid unnecessary setState
2+
3+
## Rule id
4+
5+
avoid-unnecessary-setstate
6+
7+
## Description
8+
9+
Warns when `setState` is called inside `initState`, `didUpdateWidget` or `build` methods and when it's called from a `sync` method that is called inside those methods.
10+
11+
Calling setState in those cases will lead to an additional widget rerender which is bad for performance.
12+
13+
Consider changing state directly without calling `setState`.
14+
15+
Additional resources:
16+
17+
* <https://stackoverflow.com/questions/53363774/importance-of-calling-setstate-inside-initstate/53373017#53373017>
18+
19+
### Example
20+
21+
Bad:
22+
23+
```dart
24+
class MyWidget extends StatefulWidget {
25+
@override
26+
_MyWidgetState createState() => _MyWidgetState();
27+
}
28+
29+
class _MyWidgetState extends State<MyWidget> {
30+
String myString = '';
31+
32+
@override
33+
void initState() {
34+
super.initState();
35+
36+
// LINT
37+
setState(() {
38+
myString = "Hello";
39+
});
40+
41+
if (condition) {
42+
// LINT
43+
setState(() {
44+
myString = "Hello";
45+
});
46+
}
47+
48+
myStateUpdateMethod(); // LINT
49+
}
50+
51+
@override
52+
void didUpdateWidget(MyWidget oldWidget) {
53+
// LINT
54+
setState(() {
55+
myString = "Hello";
56+
});
57+
}
58+
59+
void myStateUpdateMethod() {
60+
setState(() {
61+
myString = "Hello";
62+
});
63+
}
64+
65+
@override
66+
Widget build(BuildContext context) {
67+
// LINT
68+
setState(() {
69+
myString = "Hello";
70+
});
71+
72+
if (condition) {
73+
// LINT
74+
setState(() {
75+
myString = "Hello";
76+
});
77+
}
78+
79+
myStateUpdateMethod(); // LINT
80+
81+
return ElevatedButton(
82+
onPressed: () => myStateUpdateMethod(),
83+
onLongPress: () {
84+
setState(() {
85+
myString = data;
86+
});
87+
},
88+
child: Text('PRESS'),
89+
);
90+
}
91+
}
92+
```
93+
94+
Good:
95+
96+
```dart
97+
class MyWidget extends StatefulWidget {
98+
@override
99+
_MyWidgetState createState() => _MyWidgetState();
100+
}
101+
102+
class _MyWidgetState extends State<MyWidget> {
103+
String myString = '';
104+
105+
final classWithMethod = SomeClassWithMethod();
106+
107+
@override
108+
void initState() {
109+
super.initState();
110+
111+
myString = "Hello";
112+
113+
classWithMethod.myMethod();
114+
myAsyncMethod();
115+
}
116+
117+
@override
118+
void didUpdateWidget(MyWidget oldWidget) {
119+
myString = "Hello";
120+
}
121+
122+
void myStateUpdateMethod() {
123+
setState(() {
124+
myString = "Hello";
125+
});
126+
}
127+
128+
Future<void> myAsyncMethod() async {
129+
final data = await service.fetchData();
130+
131+
setState(() {
132+
myString = data;
133+
});
134+
}
135+
136+
@override
137+
Widget build(BuildContext context) {
138+
myAsyncMethod();
139+
140+
return ElevatedButton(
141+
onPressed: () => myStateUpdateMethod(),
142+
onLongPress: () {
143+
setState(() {
144+
myString = data;
145+
});
146+
},
147+
child: Text('PRESS'),
148+
);
149+
}
150+
}
151+
```

‎lib/src/analyzers/lint_analyzer/rules/rules_factory.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import 'rules_list/avoid_late_keyword/avoid_late_keyword.dart';
33
import 'rules_list/avoid_non_null_assertion/avoid_non_null_assertion.dart';
44
import 'rules_list/avoid_preserve_whitespace_false/avoid_preserve_whitespace_false.dart';
55
import 'rules_list/avoid_returning_widgets/avoid_returning_widgets.dart';
6+
import 'rules_list/avoid_unnecessary_setstate/avoid_unnecessary_setstate.dart';
67
import 'rules_list/avoid_unused_parameters/avoid_unused_parameters.dart';
78
import 'rules_list/binary_expression_operand_order/binary_expression_operand_order.dart';
89
import 'rules_list/component_annotation_arguments_ordering/component_annotation_arguments_ordering.dart';
@@ -30,6 +31,8 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
3031
AvoidPreserveWhitespaceFalseRule(config),
3132
AvoidReturningWidgetsRule.ruleId: (config) =>
3233
AvoidReturningWidgetsRule(config),
34+
AvoidUnnecessarySetStateRule.ruleId: (config) =>
35+
AvoidUnnecessarySetStateRule(config),
3336
AvoidUnusedParametersRule.ruleId: (config) =>
3437
AvoidUnusedParametersRule(config),
3538
BinaryExpressionOperandOrderRule.ruleId: (config) =>

‎lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_setstate/avoid_unnecessary_setstate.dart

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import 'package:analyzer/dart/ast/ast.dart';
2+
import 'package:analyzer/dart/ast/visitor.dart';
3+
import 'package:analyzer/dart/element/type.dart';
4+
import 'package:collection/collection.dart';
5+
6+
import '../../../../../utils/node_utils.dart';
7+
import '../../../../models/internal_resolved_unit_result.dart';
8+
import '../../../../models/issue.dart';
9+
import '../../../../models/severity.dart';
10+
import '../../models/rule.dart';
11+
import '../../models/rule_documentation.dart';
12+
import '../../rule_utils.dart';
13+
14+
part 'visitor.dart';
15+
16+
class AvoidUnnecessarySetStateRule extends Rule {
17+
static const String ruleId = 'avoid-unnecessary-setstate';
18+
19+
static const _warningMessage =
20+
'Avoid calling unnecessary setState. Consider changing the state directly.';
21+
static const _methodWarningMessage = 'Avoid calling a method with setState.';
22+
23+
AvoidUnnecessarySetStateRule([Map<String, Object> config = const {}])
24+
: super(
25+
id: ruleId,
26+
documentation: const RuleDocumentation(
27+
name: 'Avoid returning widgets',
28+
brief:
29+
'Warns when `setState` is called inside `initState`, `didUpdateWidget` or `build` methods and when it is called from a `sync` method that is called inside those methods.',
30+
),
31+
severity: readSeverity(config, Severity.warning),
32+
excludes: readExcludes(config),
33+
);
34+
35+
@override
36+
Iterable<Issue> check(InternalResolvedUnitResult source) {
37+
final _visitor = _Visitor();
38+
39+
source.unit.visitChildren(_visitor);
40+
41+
return [
42+
..._visitor.setStateInvocations.map((invocation) => createIssue(
43+
rule: this,
44+
location: nodeLocation(
45+
node: invocation,
46+
source: source,
47+
),
48+
message: _warningMessage,
49+
)),
50+
..._visitor.classMethodsInvocations.map((invocation) => createIssue(
51+
rule: this,
52+
location: nodeLocation(
53+
node: invocation,
54+
source: source,
55+
),
56+
message: _methodWarningMessage,
57+
))
58+
];
59+
}
60+
}

‎lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_setstate/visitor.dart

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
part of 'avoid_unnecessary_setstate.dart';
2+
3+
class _Visitor extends RecursiveAstVisitor<void> {
4+
static const _checkedMethods = ['initState', 'didUpdateWidget', 'build'];
5+
6+
final _setStateInvocations = <MethodInvocation>[];
7+
final _classMethodsInvocations = <MethodInvocation>[];
8+
9+
Iterable<MethodInvocation> get setStateInvocations => _setStateInvocations;
10+
Iterable<MethodInvocation> get classMethodsInvocations =>
11+
_classMethodsInvocations;
12+
13+
@override
14+
void visitClassDeclaration(ClassDeclaration node) {
15+
super.visitClassDeclaration(node);
16+
17+
final type = node.extendsClause?.superclass.type;
18+
if (type == null || !_hasWidgetStateType(type)) {
19+
return;
20+
}
21+
22+
final methods = node.members
23+
.whereType<MethodDeclaration>()
24+
.where((member) => _checkedMethods.contains(member.name.name))
25+
.toList();
26+
final restMethods = node.members
27+
.whereType<MethodDeclaration>()
28+
.where((member) => !_checkedMethods.contains(member.name.name))
29+
.toList();
30+
final restMethodsNames =
31+
restMethods.map((method) => method.name.name).toList();
32+
33+
final visitedRestMethods = <String, bool>{};
34+
35+
for (final method in methods) {
36+
final visitor = _MethodVisitor(restMethodsNames);
37+
method.visitChildren(visitor);
38+
39+
_setStateInvocations.addAll(visitor.setStateInvocations);
40+
_classMethodsInvocations.addAll(
41+
visitor.classMethodsInvocations
42+
.where((invocation) => _containsSetState(
43+
visitedRestMethods,
44+
restMethods.firstWhere((method) =>
45+
method.name.name == invocation.methodName.name),
46+
))
47+
.toList(),
48+
);
49+
}
50+
}
51+
52+
bool _hasWidgetStateType(DartType type) =>
53+
_isWidgetState(type) || _isSubclassOfWidgetState(type);
54+
55+
bool _isSubclassOfWidgetState(DartType? type) =>
56+
type is InterfaceType &&
57+
type.allSupertypes.firstWhereOrNull(_isWidgetState) != null;
58+
59+
bool _isWidgetState(DartType? type) => type?.element?.displayName == 'State';
60+
61+
bool _containsSetState(
62+
Map<String, bool> visitedRestMethods,
63+
MethodDeclaration declaration,
64+
) {
65+
final type = declaration.returnType?.type;
66+
if (type != null && (type.isDartAsyncFuture || type.isDartAsyncFutureOr)) {
67+
return false;
68+
}
69+
70+
final name = declaration.name.name;
71+
if (visitedRestMethods.containsKey(name) && visitedRestMethods[name]!) {
72+
return true;
73+
}
74+
75+
final visitor = _MethodVisitor([]);
76+
declaration.visitChildren(visitor);
77+
78+
final hasSetState = visitor.setStateInvocations.isNotEmpty;
79+
80+
visitedRestMethods[name] = hasSetState;
81+
return hasSetState;
82+
}
83+
}
84+
85+
class _MethodVisitor extends RecursiveAstVisitor<void> {
86+
final Iterable<String> classMethodsNames;
87+
88+
final _setStateInvocations = <MethodInvocation>[];
89+
final _classMethodsInvocations = <MethodInvocation>[];
90+
91+
Iterable<MethodInvocation> get setStateInvocations => _setStateInvocations;
92+
Iterable<MethodInvocation> get classMethodsInvocations =>
93+
_classMethodsInvocations;
94+
95+
_MethodVisitor(this.classMethodsNames);
96+
97+
@override
98+
void visitMethodInvocation(MethodInvocation node) {
99+
super.visitMethodInvocation(node);
100+
101+
final name = node.methodName.name;
102+
103+
if (name == 'setState' &&
104+
node.thisOrAncestorOfType<ArgumentList>() == null) {
105+
_setStateInvocations.add(node);
106+
} else if (classMethodsNames.contains(name) &&
107+
node.thisOrAncestorOfType<ArgumentList>() == null &&
108+
node.realTarget == null) {
109+
_classMethodsInvocations.add(node);
110+
}
111+
}
112+
}

0 commit comments

Comments
 (0)