fix: additional cases for use-setstate-synchronously (#1136)

* feat: scan try-statements for async setState

* feat: scan switch-statements for async setState

* test: control flow in for- and while-statements

* docs: mention new heuristics

* chore: update changelog

Author: Desdaemon

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+* fix: handle try and switch statements for [`use-setstate-synchronously`](https://dartcodemetrics.dev/docs/rules/flutter/use-setstate-synchronously)
+
 ## 5.4.0
 
 * feat: ignore tear-off methods for [`avoid-unused-parameters`](https://dartcodemetrics.dev/docs/rules/common/avoid-unused-parameters).

lib/src/analyzers/lint_analyzer/rules/rules_list/use_setstate_synchronously/helpers.dart

@@ -85,8 +85,9 @@ bool _isIdentifier(Expression node, String ident) =>
     node is Identifier && node.name == ident;
 
 @pragma('vm:prefer-inline')
-bool _isDivergent(Statement node) =>
+bool _isDivergent(Statement node, {bool allowControlFlow = false}) =>
     node is ReturnStatement ||
+    allowControlFlow && (node is BreakStatement || node is ContinueStatement) ||
     node is ExpressionStatement && node.expression is ThrowExpression;
 
 @pragma('vm:prefer-inline')
@@ -95,5 +96,12 @@ Expression _thisOr(Expression node) =>
         ? node.propertyName
         : node;
 
-bool _blockDiverges(Statement block) =>
-    block is Block ? block.statements.any(_isDivergent) : _isDivergent(block);
+bool _blockDiverges(Statement block, {required bool allowControlFlow}) => block
+        is Block
+    ? block.statements
+        .any((node) => _isDivergent(node, allowControlFlow: allowControlFlow))
+    : _isDivergent(block, allowControlFlow: allowControlFlow);
+
+bool _caseDiverges(SwitchMember arm, {required bool allowControlFlow}) =>
+    arm.statements
+        .any((node) => _isDivergent(node, allowControlFlow: allowControlFlow));

lib/src/analyzers/lint_analyzer/rules/rules_list/use_setstate_synchronously/visitor.dart

@@ -31,7 +31,10 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
   _AsyncSetStateVisitor({this.validateMethod = _noop});
 
   MountedFact mounted = true.asFact();
+  bool inControlFlow = false;
   bool inAsync = true;
+
+  bool get isMounted => mounted.value ?? false;
   final nodes = <SimpleIdentifier>[];
 
   @override
@@ -47,8 +50,7 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
     }
 
     // [this.]setState()
-    final mounted_ = mounted.value ?? false;
-    if (!mounted_ &&
+    if (!isMounted &&
         validateMethod(node.methodName.name) &&
         node.target is ThisExpression?) {
       nodes.add(node.methodName);
@@ -74,12 +76,15 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
     var elseDiverges = false;
     final elseStatement = node.elseStatement;
     if (elseStatement != null) {
-      elseDiverges = _blockDiverges(elseStatement);
+      elseDiverges = _blockDiverges(
+        elseStatement,
+        allowControlFlow: inControlFlow,
+      );
       mounted = _tryInvert(newMounted).or(mounted);
       elseStatement.visitChildren(this);
     }
 
-    if (_blockDiverges(node.thenStatement)) {
+    if (_blockDiverges(node.thenStatement, allowControlFlow: inControlFlow)) {
       mounted = _tryInvert(newMounted).or(beforeThen);
     } else if (elseDiverges) {
       mounted = beforeThen != afterThen
@@ -95,14 +100,35 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
     }
 
     node.condition.visitChildren(this);
+
     final oldMounted = mounted;
     final newMounted = _extractMountedCheck(node.condition);
     mounted = newMounted.or(mounted);
+    final oldInControlFlow = inControlFlow;
+    inControlFlow = true;
     node.body.visitChildren(this);
 
-    if (_blockDiverges(node.body)) {
+    if (_blockDiverges(node.body, allowControlFlow: inControlFlow)) {
       mounted = _tryInvert(newMounted).or(oldMounted);
     }
+
+    inControlFlow = oldInControlFlow;
+  }
+
+  @override
+  void visitForStatement(ForStatement node) {
+    if (!inAsync) {
+      return node.visitChildren(this);
+    }
+
+    node.forLoopParts.visitChildren(this);
+
+    final oldInControlFlow = inControlFlow;
+    inControlFlow = true;
+
+    node.body.visitChildren(this);
+
+    inControlFlow = oldInControlFlow;
   }
 
   @override
@@ -117,4 +143,57 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
     mounted = oldMounted;
     inAsync = oldInAsync;
   }
+
+  @override
+  void visitTryStatement(TryStatement node) {
+    if (!inAsync) {
+      return node.visitChildren(this);
+    }
+
+    final oldMounted = mounted;
+    node.body.visitChildren(this);
+    final afterBody = mounted;
+    // ignore: omit_local_variable_types
+    final MountedFact beforeCatch =
+        mounted == oldMounted ? oldMounted : false.asFact();
+    for (final clause in node.catchClauses) {
+      mounted = beforeCatch;
+      clause.visitChildren(this);
+    }
+
+    final finallyBlock = node.finallyBlock;
+    if (finallyBlock != null) {
+      mounted = beforeCatch;
+      finallyBlock.visitChildren(this);
+    } else {
+      mounted = afterBody;
+    }
+  }
+
+  @override
+  void visitSwitchStatement(SwitchStatement node) {
+    if (!inAsync) {
+      return node.visitChildren(this);
+    }
+
+    node.expression.visitChildren(this);
+
+    final oldInControlFlow = inControlFlow;
+    inControlFlow = true;
+
+    final caseInvariant = mounted;
+    for (final arm in node.members) {
+      arm.visitChildren(this);
+      if (mounted != caseInvariant &&
+          !_caseDiverges(arm, allowControlFlow: false)) {
+        mounted = false.asFact();
+      }
+
+      if (_caseDiverges(arm, allowControlFlow: true)) {
+        mounted = caseInvariant;
+      }
+    }
+
+    inControlFlow = oldInControlFlow;
+  }
 }

test/src/analyzers/lint_analyzer/rules/rules_list/use_setstate_synchronously/examples/extras_try_switch.dart

@@ -0,0 +1,52 @@
+class MyState extends State {
+  void tryStatements() async {
+    try {
+      await doStuff();
+    } on FooException {
+      if (!mounted) return;
+      setState(() {});
+    } on BarException {
+      setState(() {}); // LINT
+    }
+    setState(() {}); // LINT
+  }
+
+  void tryFinally() async {
+    try {
+      await doStuff();
+    } on Exception {
+      await doStuff();
+    } finally {
+      if (!mounted) return;
+    }
+    setState(() {});
+  }
+
+  void switchStatements() async {
+    await doStuff();
+    switch (foobar) {
+      case 'foo':
+        if (!mounted) break;
+        setState(() {});
+        break;
+      case 'bar':
+        setState(() {}); // LINT
+        break;
+      case 'baz':
+        await doStuff();
+        break;
+    }
+    setState(() {}); // LINT
+  }
+
+  void switchAsync() async {
+    switch (foobar) {
+      case 'foo':
+        await doStuff();
+        return;
+    }
+    setState(() {});
+  }
+}
+
+class State {}

test/src/analyzers/lint_analyzer/rules/rules_list/use_setstate_synchronously/examples/known_errors.dart

@@ -8,6 +8,25 @@ class _MyState extends State {
     doStuff();
     setState(() {});
   }
+
+  void controlFlow() {
+    await doStuff();
+    for (;;) {
+      if (!mounted) break;
+      setState(() {});
+
+      await doStuff();
+      if (!mounted) continue;
+      setState(() {});
+    }
+
+    await doStuff();
+    while (true) {
+      if (!mounted) break;
+
+      setState(() {});
+    }
+  }
 }
 
 class State {}

test/src/analyzers/lint_analyzer/rules/rules_list/use_setstate_synchronously/use_setstate_synchronously_rule_test.dart

@@ -6,6 +6,8 @@ import '../../../../../helpers/rule_test_helper.dart';
 
 const _examplePath = 'use_setstate_synchronously/examples/example.dart';
 const _issuesPath = 'use_setstate_synchronously/examples/known_errors.dart';
+const _trySwitchPath =
+    'use_setstate_synchronously/examples/extras_try_switch.dart';
 
 void main() {
   group('UseSetStateSynchronouslyTest', () {
@@ -90,5 +92,28 @@ void main() {
         ],
       );
     });
+
+    test('reports issues with try- and switch-statements', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_trySwitchPath);
+      final issues = UseSetStateSynchronouslyRule().check(unit);
+
+      RuleTestHelper.verifyIssues(
+        issues: issues,
+        startLines: [9, 11, 33, 39],
+        startColumns: [7, 5, 9, 5],
+        locationTexts: [
+          '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.",
+        ],
+      );
+    });
   });
 }

website/docs/rules/flutter/use-setstate-synchronously.mdx

@@ -16,12 +16,13 @@ does not make use of [`mounted`], but rather revolves around refactoring your st
 
 :::info
 
-Only these patterns are considered for mounted-ness:
+The following patterns are recognized when statically determining mountedness:
 
 - `if (mounted)`
-- `if (!mounted)`
 - `if (mounted && ..)`
-- `if (!mounted || ..) return;`
+- `if (!mounted || ..)`
+- `try` and `switch` mountedness per branch
+- Divergence in `for`, `while` and `switch` statements using `break` or `continue`
 
 If a `!mounted` check diverges, i.e. ends in a `return` or `throw`, the outer scope is considered mounted and vice versa:
 
@@ -32,6 +33,13 @@ setState(() { ... });
 
 // After this statement, need to check 'mounted' again
 await fetch(...);
+
+// In control flow statements, 'break' and 'continue' diverges
+while (...) {
+  if (!mounted) break;
+  // Should be mounted right now
+  setState(() { ... });
+}
 ```
 
 :::