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

feat: add Avoid unnecessary setState #346

Merged
merged 9 commits into from
May 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

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

## 3.2.2
Expand All @@ -11,7 +12,7 @@

## 3.2.1

* Remove unnecessary scan by `Lines of Executable Code`
* Remove unnecessary scan by `Lines of Executable Code`.

## 3.2.0

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ Rules configuration is [described here](#configuring-a-rules-entry).
### Flutter specific

- [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)
- [avoid-unnecessary-setstate](https://github.com/dart-code-checker/dart-code-metrics/blob/master/doc/rules/avoid-unnecessary-setstate.md)

### Intl specific

Expand Down
151 changes: 151 additions & 0 deletions doc/rules/avoid-unnecessary-setstate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
# 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";
});
}

myStateUpdateMethod(); // LINT
}

@override
void didUpdateWidget(MyWidget oldWidget) {
// LINT
setState(() {
myString = "Hello";
});
}

void myStateUpdateMethod() {
setState(() {
myString = "Hello";
});
}

@override
Widget build(BuildContext context) {
// LINT
setState(() {
myString = "Hello";
});

if (condition) {
// LINT
setState(() {
myString = "Hello";
});
}

myStateUpdateMethod(); // LINT

return ElevatedButton(
onPressed: () => myStateUpdateMethod(),
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();

myString = "Hello";

classWithMethod.myMethod();
myAsyncMethod();
}

@override
void didUpdateWidget(MyWidget oldWidget) {
myString = "Hello";
}

void myStateUpdateMethod() {
setState(() {
myString = "Hello";
});
}

Future<void> myAsyncMethod() async {
final data = await service.fetchData();

setState(() {
myString = data;
});
}

@override
Widget build(BuildContext context) {
myAsyncMethod();

return ElevatedButton(
onPressed: () => myStateUpdateMethod(),
onLongPress: () {
setState(() {
myString = data;
});
},
child: Text('PRESS'),
);
}
}
```
3 changes: 3 additions & 0 deletions lib/src/analyzers/lint_analyzer/rules/rules_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'rules_list/avoid_late_keyword/avoid_late_keyword.dart';
import 'rules_list/avoid_non_null_assertion/avoid_non_null_assertion.dart';
import 'rules_list/avoid_preserve_whitespace_false/avoid_preserve_whitespace_false.dart';
import 'rules_list/avoid_returning_widgets/avoid_returning_widgets.dart';
import 'rules_list/avoid_unnecessary_setstate/avoid_unnecessary_setstate.dart';
import 'rules_list/avoid_unused_parameters/avoid_unused_parameters.dart';
import 'rules_list/binary_expression_operand_order/binary_expression_operand_order.dart';
import 'rules_list/component_annotation_arguments_ordering/component_annotation_arguments_ordering.dart';
Expand Down Expand Up @@ -30,6 +31,8 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
AvoidPreserveWhitespaceFalseRule(config),
AvoidReturningWidgetsRule.ruleId: (config) =>
AvoidReturningWidgetsRule(config),
AvoidUnnecessarySetStateRule.ruleId: (config) =>
AvoidUnnecessarySetStateRule(config),
AvoidUnusedParametersRule.ruleId: (config) =>
AvoidUnusedParametersRule(config),
BinaryExpressionOperandOrderRule.ruleId: (config) =>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
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. Consider changing the state directly.';
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 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.',
),
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,
)),
..._visitor.classMethodsInvocations.map((invocation) => createIssue(
rule: this,
location: nodeLocation(
node: invocation,
source: source,
),
message: _methodWarningMessage,
))
];
}
}
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);
}
}
}
Loading