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

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

* fix: handle await inside conditions

Author: incendial

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* fix: support `assert(mounted)` for [`use-setstate-synchronously`](https://dcm.dev/docs/individuals/rules/flutter/use-setstate-synchronously).
 * fix: correctly support dartdoc tags for [`format-comment`](https://dcm.dev/docs/individuals/rules/common/format-comment).
 
 ## 5.6.0-dev.1

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

@@ -11,15 +11,15 @@ import '../../../models/severity.dart';
 import '../../models/flutter_rule.dart';
 import '../../rule_utils.dart';
 
+part 'config.dart';
 part 'fact.dart';
 part 'helpers.dart';
-part 'config.dart';
 part 'visitor.dart';
 
 class UseSetStateSynchronouslyRule extends FlutterRule {
   static const ruleId = 'use-setstate-synchronously';
 
-  Set<String> methods;
+  final Set<String> methods;
 
   UseSetStateSynchronouslyRule([Map<String, Object> options = const {}])
       : methods = readMethods(options),

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

@@ -2,6 +2,7 @@ part of 'use_setstate_synchronously_rule.dart';
 
 class _Visitor extends RecursiveAstVisitor<void> {
   final Set<String> methods;
+
   _Visitor({required this.methods});
 
   final nodes = <SimpleIdentifier>[];
@@ -43,6 +44,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) {
@@ -65,7 +73,8 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
       return node.visitChildren(this);
     }
 
-    node.condition.visitChildren(this);
+    node.condition.accept(this);
+
     final newMounted = _extractMountedCheck(node.condition);
     mounted = newMounted.or(mounted);
 
@@ -99,7 +108,7 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
       return node.visitChildren(this);
     }
 
-    node.condition.visitChildren(this);
+    node.condition.accept(this);
 
     final oldMounted = mounted;
     final newMounted = _extractMountedCheck(node.condition);
@@ -121,7 +130,7 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
       return node.visitChildren(this);
     }
 
-    node.forLoopParts.visitChildren(this);
+    node.forLoopParts.accept(this);
 
     final oldInControlFlow = inControlFlow;
     inControlFlow = true;
@@ -153,9 +162,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);
@@ -176,7 +184,7 @@ class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
       return node.visitChildren(this);
     }
 
-    node.expression.visitChildren(this);
+    node.expression.accept(this);
 
     final oldInControlFlow = inControlFlow;
     inControlFlow = true;

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

@@ -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 {}

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

@@ -95,7 +95,17 @@ class _FooState extends State<StatefulWidget> {
       await fetch();
     }
     setState(() {}); // LINT
+
+    if (!mounted) return;
+
+    if (await condition()) {
+      setState(() {}); // LINT
+    }
   }
 }
 
 class State {}
+
+Future<void> fetch() {}
+
+Future<bool> condition() {}

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

@@ -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', () {
@@ -30,8 +32,8 @@ void main() {
 
       RuleTestHelper.verifyIssues(
         issues: issues,
-        startLines: [7, 24, 29, 36, 51, 66, 70, 76, 82, 92, 97],
-        startColumns: [9, 10, 7, 7, 5, 7, 5, 5, 5, 5, 5],
+        startLines: [7, 24, 29, 36, 51, 66, 70, 76, 82, 92, 97, 102],
+        startColumns: [9, 10, 7, 7, 5, 7, 5, 5, 5, 5, 5, 7],
         locationTexts: [
           'setState',
           'setState',
@@ -44,6 +46,7 @@ void main() {
           'setState',
           'setState',
           'setState',
+          'setState',
         ],
         messages: [
           "Avoid calling 'setState' past an await point without checking if the widget is mounted.",
@@ -57,6 +60,7 @@ void main() {
           "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.",
         ],
       );
     });
@@ -118,6 +122,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);