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

fix: support assert(mounted) for use-setstate-synchronously #1181

Merged
merged 3 commits into from
Feb 5, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Unreleased

* fix: support `assert(mounted)` for [`use-setstate-synchronously`](https://dcm.dev/docs/individuals/rules/flutter/use-setstate-synchronously).

## 5.6.0-dev.1

* docs: remove old website
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
super.visitAwaitExpression(node);
}

@override
void visitAssertStatement(AssertStatement node) {
final newMounted = _extractMountedCheck(node.condition);
mounted = newMounted.or(mounted);
super.visitAssertStatement(node);
}

@override
void visitMethodInvocation(MethodInvocation node) {
if (!inAsync) {
Expand All @@ -65,7 +72,6 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
return node.visitChildren(this);
}

node.condition.visitChildren(this);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Desdaemon removing this looks safe or am I missing something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might cause lints to fail for e.g. if (await ..), but I was mostly being cautious when I wrote this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I'll add some tests to check this case

Copy link
Member Author

@incendial incendial Feb 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it was not being checked coz of .visitChildren, I've added a test and fixed the problem

final newMounted = _extractMountedCheck(node.condition);
mounted = newMounted.or(mounted);

Expand Down Expand Up @@ -153,9 +159,8 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
final oldMounted = mounted;
node.body.visitChildren(this);
final afterBody = mounted;
// ignore: omit_local_variable_types
final MountedFact beforeCatch =
mounted == oldMounted ? oldMounted : false.asFact();
final beforeCatch =
mounted == oldMounted ? oldMounted : false.asFact<BinaryExpression>();
for (final clause in node.catchClauses) {
mounted = beforeCatch;
clause.visitChildren(this);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
class _FooState extends State<StatefulWidget> {
Widget build(context) {
return FooWidget(
onChange: (value) async {
setState(() {});
await fetchData();
setState(() {}); // LINT

assert(mounted);
setState(() {});
},
);
}

void pathologicalCases() async {
setState(() {});

await fetch();
this.setState(() {}); // LINT

if (1 == 1) {
setState(() {}); // LINT
} else {
assert(mounted);
setState(() {});
return;
}
setState(() {}); // LINT

await fetch();
if (mounted && foo) {
assert(mounted);
} else {
return;
}
setState(() {}); // LINT

assert(mounted);
setState(() {});
}
}

class State {}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ const _trySwitchPath =
'use_setstate_synchronously/examples/extras_try_switch.dart';
const _contextMountedPath =
'use_setstate_synchronously/examples/context_mounted.dart';
const _assertExample =
'use_setstate_synchronously/examples/assert_example.dart';

void main() {
group('UseSetStateSynchronouslyTest', () {
Expand Down Expand Up @@ -118,6 +120,31 @@ void main() {
);
});

test('reports issues with assert statements', () async {
final unit = await RuleTestHelper.resolveFromFile(_assertExample);
final issues = UseSetStateSynchronouslyRule().check(unit);

RuleTestHelper.verifyIssues(
issues: issues,
startLines: [7, 19, 22, 28, 36],
startColumns: [9, 10, 7, 5, 5],
locationTexts: [
'setState',
'setState',
'setState',
'setState',
'setState',
],
messages: [
"Avoid calling 'setState' past an await point without checking if the widget is mounted.",
"Avoid calling 'setState' past an await point without checking if the widget is mounted.",
"Avoid calling 'setState' past an await point without checking if the widget is mounted.",
"Avoid calling 'setState' past an await point without checking if the widget is mounted.",
"Avoid calling 'setState' past an await point without checking if the widget is mounted.",
],
);
});

test('reports no issues for context.mounted', () async {
final unit = await RuleTestHelper.resolveFromFile(_contextMountedPath);
final issues = UseSetStateSynchronouslyRule().check(unit);
Expand Down