This repository was archived by the owner on Jul 16, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 272
feat: add Avoid unnecessary setState #346
Merged
dkrutskikh
merged 9 commits into
dart-code-checker:master
from
incendial:implement-avoid-unnecessary-setstate
May 26, 2021
Merged
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
7b7a9fc
chore: fix imports
incendial e772722
Merge remote-tracking branch 'upstream/master'
incendial 4ee587e
Merge remote-tracking branch 'upstream/master'
incendial 6eda271
Merge remote-tracking branch 'upstream/master'
incendial 2a48724
fear: add Avoid unnecessary setstate rule
incendial 48b53eb
Merge remote-tracking branch 'upstream/master' into implement-avoid-u…
incendial e8a40d9
chore: review fixes
incendial db03d69
Merge remote-tracking branch 'upstream/master' into implement-avoid-u…
incendial e484bbd
test: update rule tests
incendial File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
# Avoid unnecessary setState | ||
|
||
## Rule id | ||
|
||
avoid-unnecessary-setstate | ||
|
||
## Description | ||
|
||
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. | ||
|
||
Calling setState in those cases will lead to an additional widget rerender which is bad for performance. | ||
|
||
Consider changing state directly without calling `setState`. | ||
|
||
Additional resources: | ||
|
||
* <https://stackoverflow.com/questions/53363774/importance-of-calling-setstate-inside-initstate/53373017#53373017> | ||
|
||
### Example | ||
|
||
Bad: | ||
|
||
```dart | ||
class MyWidget extends StatefulWidget { | ||
@override | ||
_MyWidgetState createState() => _MyWidgetState(); | ||
} | ||
|
||
class _MyWidgetState extends State<MyWidget> { | ||
String myString = ''; | ||
|
||
@override | ||
void initState() { | ||
super.initState(); | ||
|
||
// LINT | ||
setState(() { | ||
myString = "Hello"; | ||
}); | ||
|
||
if (condition) { | ||
// LINT | ||
setState(() { | ||
myString = "Hello"; | ||
}); | ||
} | ||
|
||
myMethod(); // LINT | ||
} | ||
|
||
@override | ||
void didUpdateWidget(MyWidget oldWidget) { | ||
// LINT | ||
setState(() { | ||
myString = "Hello"; | ||
}); | ||
} | ||
|
||
void myMethod() { | ||
setState(() { | ||
myString = "Hello"; | ||
}); | ||
} | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
// LINT | ||
setState(() { | ||
myString = "Hello"; | ||
}); | ||
|
||
if (condition) { | ||
// LINT | ||
setState(() { | ||
myString = "Hello"; | ||
}); | ||
} | ||
|
||
myMethod(); // LINT | ||
|
||
return ElevatedButton( | ||
onPressed: () => myMethod(), | ||
onLongPress: () { | ||
setState(() { | ||
myString = data; | ||
}); | ||
}, | ||
child: Text('PRESS'), | ||
); | ||
} | ||
} | ||
``` | ||
|
||
Good: | ||
|
||
```dart | ||
class MyWidget extends StatefulWidget { | ||
@override | ||
_MyWidgetState createState() => _MyWidgetState(); | ||
} | ||
|
||
class _MyWidgetState extends State<MyWidget> { | ||
String myString = ''; | ||
|
||
final classWithMethod = SomeClassWithMethod(); | ||
|
||
@override | ||
void initState() { | ||
super.initState(); | ||
|
||
classWithMethod.myMethod(); | ||
myAsyncMethod(); | ||
} | ||
|
||
@override | ||
void didUpdateWidget(MyWidget oldWidget) { | ||
... | ||
} | ||
|
||
void myMethod() { | ||
setState(() { | ||
myString = "Hello"; | ||
}); | ||
} | ||
|
||
Future<void> myAsyncMethod() async { | ||
final data = await service.fetchData(); | ||
|
||
setState(() { | ||
myString = data; | ||
}); | ||
} | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
myAsyncMethod(); | ||
|
||
return ElevatedButton( | ||
onPressed: () => myMethod(), | ||
onLongPress: () { | ||
setState(() { | ||
myString = data; | ||
}); | ||
}, | ||
child: Text('PRESS'), | ||
); | ||
} | ||
} | ||
``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
export 'package:dart_code_metrics/src/config_builder/config_builder.dart'; | ||
export 'package:dart_code_metrics/src/config_builder/models/config.dart'; | ||
export 'package:dart_code_metrics/src/config_builder/models/analysis_options.dart'; | ||
export 'package:dart_code_metrics/src/config_builder/config_builder.dart'; | ||
export 'package:dart_code_metrics/src/config_builder/models/config.dart'; | ||
export 'package:dart_code_metrics/src/config_builder/models/analysis_options.dart'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
62 changes: 62 additions & 0 deletions
62
...lint_analyzer/rules/rules_list/avoid_unnecessary_setstate/avoid_unnecessary_setstate.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
import 'package:analyzer/dart/ast/ast.dart'; | ||
import 'package:analyzer/dart/ast/visitor.dart'; | ||
import 'package:analyzer/dart/element/type.dart'; | ||
import 'package:collection/collection.dart'; | ||
|
||
import '../../../../../utils/node_utils.dart'; | ||
import '../../../../models/internal_resolved_unit_result.dart'; | ||
import '../../../../models/issue.dart'; | ||
import '../../../../models/severity.dart'; | ||
import '../../models/rule.dart'; | ||
import '../../models/rule_documentation.dart'; | ||
import '../../rule_utils.dart'; | ||
|
||
part 'visitor.dart'; | ||
|
||
class AvoidUnnecessarySetStateRule extends Rule { | ||
static const String ruleId = 'avoid-unnecessary-setstate'; | ||
|
||
static const _warningMessage = 'Avoid calling unnecessary setState.'; | ||
static const _methodWarningMessage = 'Avoid calling a method with setState.'; | ||
|
||
AvoidUnnecessarySetStateRule([Map<String, Object> config = const {}]) | ||
: super( | ||
id: ruleId, | ||
documentation: const RuleDocumentation( | ||
name: 'Avoid returning widgets', | ||
brief: 'Warns ', | ||
), | ||
severity: readSeverity(config, Severity.warning), | ||
excludes: readExcludes(config), | ||
); | ||
|
||
@override | ||
Iterable<Issue> check(InternalResolvedUnitResult source) { | ||
final _visitor = _Visitor(); | ||
|
||
source.unit.visitChildren(_visitor); | ||
|
||
return [ | ||
..._visitor.setStateInvocations | ||
.map((invocation) => createIssue( | ||
rule: this, | ||
location: nodeLocation( | ||
node: invocation, | ||
source: source, | ||
), | ||
message: _warningMessage, | ||
)) | ||
.toList(growable: false), | ||
dkrutskikh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
..._visitor.classMethodsInvocations | ||
.map((invocation) => createIssue( | ||
rule: this, | ||
location: nodeLocation( | ||
node: invocation, | ||
source: source, | ||
), | ||
message: _methodWarningMessage, | ||
)) | ||
.toList(growable: false) | ||
dkrutskikh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
]; | ||
} | ||
} |
112 changes: 112 additions & 0 deletions
112
lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_setstate/visitor.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
part of 'avoid_unnecessary_setstate.dart'; | ||
|
||
class _Visitor extends RecursiveAstVisitor<void> { | ||
static const _checkedMethods = ['initState', 'didUpdateWidget', 'build']; | ||
|
||
final _setStateInvocations = <MethodInvocation>[]; | ||
final _classMethodsInvocations = <MethodInvocation>[]; | ||
|
||
Iterable<MethodInvocation> get setStateInvocations => _setStateInvocations; | ||
Iterable<MethodInvocation> get classMethodsInvocations => | ||
_classMethodsInvocations; | ||
|
||
@override | ||
void visitClassDeclaration(ClassDeclaration node) { | ||
super.visitClassDeclaration(node); | ||
|
||
final type = node.extendsClause?.superclass.type; | ||
if (type == null || !_hasWidgetStateType(type)) { | ||
return; | ||
} | ||
|
||
final methods = node.members | ||
.whereType<MethodDeclaration>() | ||
.where((member) => _checkedMethods.contains(member.name.name)) | ||
.toList(); | ||
final restMethods = node.members | ||
.whereType<MethodDeclaration>() | ||
.where((member) => !_checkedMethods.contains(member.name.name)) | ||
.toList(); | ||
final restMethodsNames = | ||
restMethods.map((method) => method.name.name).toList(); | ||
|
||
final visitedRestMethods = <String, bool>{}; | ||
|
||
for (final method in methods) { | ||
final visitor = _MethodVisitor(restMethodsNames); | ||
method.visitChildren(visitor); | ||
|
||
_setStateInvocations.addAll(visitor.setStateInvocations); | ||
_classMethodsInvocations.addAll( | ||
visitor.classMethodsInvocations | ||
.where((invocation) => _containsSetState( | ||
visitedRestMethods, | ||
restMethods.firstWhere((method) => | ||
method.name.name == invocation.methodName.name), | ||
)) | ||
.toList(), | ||
); | ||
} | ||
} | ||
|
||
bool _hasWidgetStateType(DartType type) => | ||
_isWidgetState(type) || _isSubclassOfWidgetState(type); | ||
|
||
bool _isSubclassOfWidgetState(DartType? type) => | ||
type is InterfaceType && | ||
type.allSupertypes.firstWhereOrNull(_isWidgetState) != null; | ||
|
||
bool _isWidgetState(DartType? type) => type?.element?.displayName == 'State'; | ||
|
||
bool _containsSetState( | ||
Map<String, bool> visitedRestMethods, | ||
MethodDeclaration declaration, | ||
) { | ||
final type = declaration.returnType?.type; | ||
if (type != null && (type.isDartAsyncFuture || type.isDartAsyncFutureOr)) { | ||
return false; | ||
} | ||
|
||
final name = declaration.name.name; | ||
if (visitedRestMethods.containsKey(name) && visitedRestMethods[name]!) { | ||
return true; | ||
} | ||
|
||
final visitor = _MethodVisitor([]); | ||
declaration.visitChildren(visitor); | ||
|
||
final hasSetState = visitor.setStateInvocations.isNotEmpty; | ||
|
||
visitedRestMethods[name] = hasSetState; | ||
return hasSetState; | ||
} | ||
} | ||
|
||
class _MethodVisitor extends RecursiveAstVisitor<void> { | ||
final Iterable<String> classMethodsNames; | ||
|
||
final _setStateInvocations = <MethodInvocation>[]; | ||
final _classMethodsInvocations = <MethodInvocation>[]; | ||
|
||
Iterable<MethodInvocation> get setStateInvocations => _setStateInvocations; | ||
Iterable<MethodInvocation> get classMethodsInvocations => | ||
_classMethodsInvocations; | ||
|
||
_MethodVisitor(this.classMethodsNames); | ||
|
||
@override | ||
void visitMethodInvocation(MethodInvocation node) { | ||
super.visitMethodInvocation(node); | ||
|
||
final name = node.methodName.name; | ||
|
||
if (name == 'setState' && | ||
node.thisOrAncestorOfType<ArgumentList>() == null) { | ||
_setStateInvocations.add(node); | ||
} else if (classMethodsNames.contains(name) && | ||
node.thisOrAncestorOfType<ArgumentList>() == null && | ||
node.realTarget == null) { | ||
_classMethodsInvocations.add(node); | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.