feat: add use-setstate-synchronously rule (#1120)

* feat: add avoid-async-setstate rule

* fix: avoid unnecessary work

* fix: reword documentation

* fix: unabbreviate variables

* feat: and-chain for use-setstate-synchronously

* chore: rename rule to use-setstate-synchronously

* feat: or-chains for use-setstate-synchronously

* fix: spurious false positives

* fix: skip analyzing sync blocks

* feat: method config for use-setstate-synchronously

* chore: report known lint issues

* test: known issues, config

Author: Desdaemon

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+* feat: add [`use-setstate-synchronously`](https://dartcodemetrics.dev/docs/rules/flutter/use-setstate-synchronously)
+
 ## 5.3.0
 
 * feat: add static code diagnostic [`list-all-equatable-fields`](https://dartcodemetrics.dev/docs/rules/common/list-all-equatable-fields).

lib/presets/flutter_all.yaml

@@ -7,6 +7,7 @@ dart_code_metrics:
     - avoid-unnecessary-setstate
     - avoid-expanded-as-spacer
     - avoid-wrapping-in-padding
+    - use-setstate-synchronously
     - check-for-equals-in-render-object-setters
     - consistent-update-render-object
     - prefer-const-border-radius

lib/src/analyzers/lint_analyzer/rules/rules_factory.dart

@@ -71,6 +71,7 @@ import 'rules_list/prefer_trailing_comma/prefer_trailing_comma_rule.dart';
 import 'rules_list/prefer_using_list_view/prefer_using_list_view_rule.dart';
 import 'rules_list/provide_correct_intl_args/provide_correct_intl_args_rule.dart';
 import 'rules_list/tag_name/tag_name_rule.dart';
+import 'rules_list/use_setstate_synchronously/use_setstate_synchronously_rule.dart';
 
 final _implementedRules = <String, Rule Function(Map<String, Object>)>{
   AlwaysRemoveListenerRule.ruleId: AlwaysRemoveListenerRule.new,
@@ -102,6 +103,7 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
   AvoidTopLevelMembersInTestsRule.ruleId: AvoidTopLevelMembersInTestsRule.new,
   AvoidUnnecessaryConditionalsRule.ruleId: AvoidUnnecessaryConditionalsRule.new,
   AvoidUnnecessarySetStateRule.ruleId: AvoidUnnecessarySetStateRule.new,
+  UseSetStateSynchronouslyRule.ruleId: UseSetStateSynchronouslyRule.new,
   AvoidUnnecessaryTypeAssertionsRule.ruleId:
       AvoidUnnecessaryTypeAssertionsRule.new,
   AvoidUnnecessaryTypeCastsRule.ruleId: AvoidUnnecessaryTypeCastsRule.new,

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

@@ -0,0 +1,9 @@
+part of 'use_setstate_synchronously_rule.dart';
+
+Set<String> readMethods(Map<String, Object> options) {
+  final methods = options['methods'];
+
+  return methods != null && methods is Iterable
+      ? methods.whereType<String>().toSet()
+      : {'setState'};
+}

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

@@ -0,0 +1,49 @@
+part of 'use_setstate_synchronously_rule.dart';
+
+/// Similar to a [bool], with an optional third indeterminate state and metadata.
+abstract class Fact<T> {
+  const factory Fact.maybe([T? info]) = _Maybe;
+  const Fact._();
+
+  T? get info => null;
+  bool? get value => null;
+
+  bool get isDefinite => this is! _Maybe;
+
+  Fact<U> _asValue<U>() => value! ? true.asFact() : false.asFact();
+
+  Fact<T> get not {
+    if (isDefinite) {
+      return value! ? false.asFact() : true.asFact();
+    }
+
+    return this;
+  }
+
+  Fact<U> or<U>(Fact<U> other) => isDefinite ? _asValue() : other;
+
+  Fact<U> orElse<U>(Fact<U> Function() other) =>
+      isDefinite ? _asValue() : other();
+
+  @override
+  String toString() => isDefinite ? '(definite: $value)' : '(maybe: $info)';
+}
+
+class _Bool<T> extends Fact<T> {
+  @override
+  final bool value;
+
+  // ignore: avoid_positional_boolean_parameters
+  const _Bool(this.value) : super._();
+}
+
+class _Maybe<T> extends Fact<T> {
+  @override
+  final T? info;
+
+  const _Maybe([this.info]) : super._();
+}
+
+extension _BoolExt on bool {
+  Fact<T> asFact<T>() => this ? const _Bool(true) : const _Bool(false);
+}

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

@@ -0,0 +1,99 @@
+// ignore_for_file: parameter_assignments
+
+part of 'use_setstate_synchronously_rule.dart';
+
+typedef MountedFact = Fact<BinaryExpression>;
+
+extension on BinaryExpression {
+  bool get isOr => operator.type == TokenType.BAR_BAR;
+  bool get isAnd => operator.type == TokenType.AMPERSAND_AMPERSAND;
+}
+
+MountedFact _extractMountedCheck(
+  Expression node, {
+  bool permitAnd = true,
+  bool expandOr = false,
+}) {
+  // ![this.]mounted
+  if (node is PrefixExpression &&
+      node.operator.type == TokenType.BANG &&
+      _isIdentifier(_thisOr(node.operand), 'mounted')) {
+    return false.asFact();
+  }
+
+  // [this.]mounted
+  if (_isIdentifier(_thisOr(node), 'mounted')) {
+    return true.asFact();
+  }
+
+  if (node is BinaryExpression) {
+    final right = node.rightOperand;
+    // mounted && ..
+    if (node.isAnd && permitAnd) {
+      return _extractMountedCheck(node.leftOperand)
+          .orElse(() => _extractMountedCheck(right));
+    }
+
+    if (node.isOr) {
+      if (!expandOr) {
+        // Or-chains don't indicate anything in the then-branch yet,
+        // but may yield information for the else-branch or divergence analysis.
+        return Fact.maybe(node);
+      }
+
+      return _extractMountedCheck(
+        node.leftOperand,
+        expandOr: expandOr,
+        permitAnd: permitAnd,
+      ).orElse(() => _extractMountedCheck(
+            right,
+            expandOr: expandOr,
+            permitAnd: permitAnd,
+          ));
+    }
+  }
+
+  return const Fact.maybe();
+}
+
+/// If [fact] is indeterminate, try to recover a fact from its metadata.
+MountedFact _tryInvert(MountedFact fact) {
+  final node = fact.info;
+
+  // a || b
+  if (node != null && node.isOr) {
+    return _extractMountedCheck(
+      node.leftOperand,
+      expandOr: true,
+      permitAnd: false,
+    )
+        .orElse(
+          () => _extractMountedCheck(
+            node.rightOperand,
+            expandOr: true,
+            permitAnd: false,
+          ),
+        )
+        .not;
+  }
+
+  return fact.not;
+}
+
+@pragma('vm:prefer-inline')
+bool _isIdentifier(Expression node, String ident) =>
+    node is Identifier && node.name == ident;
+
+@pragma('vm:prefer-inline')
+bool _isDivergent(Statement node) =>
+    node is ReturnStatement ||
+    node is ExpressionStatement && node.expression is ThrowExpression;
+
+@pragma('vm:prefer-inline')
+Expression _thisOr(Expression node) =>
+    node is PropertyAccess && node.target is ThisExpression
+        ? node.propertyName
+        : node;
+
+bool _blockDiverges(Statement block) =>
+    block is Block ? block.statements.any(_isDivergent) : _isDivergent(block);

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

@@ -0,0 +1,47 @@
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/token.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+
+import '../../../../../utils/flutter_types_utils.dart';
+import '../../../../../utils/node_utils.dart';
+import '../../../lint_utils.dart';
+import '../../../models/internal_resolved_unit_result.dart';
+import '../../../models/issue.dart';
+import '../../../models/severity.dart';
+import '../../models/flutter_rule.dart';
+import '../../rule_utils.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;
+
+  UseSetStateSynchronouslyRule([Map<String, Object> options = const {}])
+      : methods = readMethods(options),
+        super(
+          id: ruleId,
+          severity: readSeverity(options, Severity.warning),
+          excludes: readExcludes(options),
+          includes: readIncludes(options),
+        );
+
+  @override
+  Iterable<Issue> check(InternalResolvedUnitResult source) {
+    final visitor = _Visitor(methods: methods);
+    source.unit.visitChildren(visitor);
+
+    return visitor.nodes
+        .map((node) => createIssue(
+              rule: this,
+              location: nodeLocation(node: node, source: source),
+              message: "Avoid calling '${node.name}' past an await point "
+                  'without checking if the widget is mounted.',
+            ))
+        .toList(growable: false);
+  }
+}

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

@@ -0,0 +1,121 @@
+part of 'use_setstate_synchronously_rule.dart';
+
+class _Visitor extends RecursiveAstVisitor<void> {
+  final Set<String> methods;
+  _Visitor({required this.methods});
+
+  final nodes = <SimpleIdentifier>[];
+
+  @override
+  void visitClassDeclaration(ClassDeclaration node) {
+    if (isWidgetStateOrSubclass(node.extendsClause?.superclass.type)) {
+      node.visitChildren(this);
+    }
+  }
+
+  @override
+  void visitBlockFunctionBody(BlockFunctionBody node) {
+    if (!node.isAsynchronous) {
+      return node.visitChildren(this);
+    }
+    final visitor = _AsyncSetStateVisitor(validateMethod: methods.contains);
+    node.visitChildren(visitor);
+    nodes.addAll(visitor.nodes);
+  }
+}
+
+class _AsyncSetStateVisitor extends RecursiveAstVisitor<void> {
+  static bool _noop(String _) => false;
+
+  bool Function(String) validateMethod;
+  _AsyncSetStateVisitor({this.validateMethod = _noop});
+
+  MountedFact mounted = true.asFact();
+  bool inAsync = true;
+  final nodes = <SimpleIdentifier>[];
+
+  @override
+  void visitAwaitExpression(AwaitExpression node) {
+    mounted = false.asFact();
+    super.visitAwaitExpression(node);
+  }
+
+  @override
+  void visitMethodInvocation(MethodInvocation node) {
+    if (!inAsync) {
+      return node.visitChildren(this);
+    }
+
+    // [this.]setState()
+    final mounted_ = mounted.value ?? false;
+    if (!mounted_ &&
+        validateMethod(node.methodName.name) &&
+        node.target is ThisExpression?) {
+      nodes.add(node.methodName);
+    }
+
+    super.visitMethodInvocation(node);
+  }
+
+  @override
+  void visitIfStatement(IfStatement node) {
+    if (!inAsync) {
+      return node.visitChildren(this);
+    }
+
+    node.condition.visitChildren(this);
+    final oldMounted = mounted;
+    final newMounted = _extractMountedCheck(node.condition);
+
+    mounted = newMounted.or(mounted);
+    final beforeThen = mounted;
+    node.thenStatement.visitChildren(this);
+    final afterThen = mounted;
+
+    var elseDiverges = false;
+    final elseStatement = node.elseStatement;
+    if (elseStatement != null) {
+      elseDiverges = _blockDiverges(elseStatement);
+      mounted = _tryInvert(newMounted).or(mounted);
+      elseStatement.visitChildren(this);
+    }
+
+    if (_blockDiverges(node.thenStatement)) {
+      mounted = _tryInvert(newMounted).or(beforeThen);
+    } else if (elseDiverges) {
+      mounted = beforeThen != afterThen
+          ? afterThen
+          : _extractMountedCheck(node.condition, permitAnd: false);
+    } else {
+      mounted = oldMounted;
+    }
+  }
+
+  @override
+  void visitWhileStatement(WhileStatement node) {
+    if (!inAsync) {
+      return node.visitChildren(this);
+    }
+
+    node.condition.visitChildren(this);
+    final oldMounted = mounted;
+    final newMounted = _extractMountedCheck(node.condition);
+    mounted = newMounted.or(mounted);
+    node.body.visitChildren(this);
+
+    mounted = _blockDiverges(node.body) ? _tryInvert(newMounted) : oldMounted;
+  }
+
+  @override
+  void visitBlockFunctionBody(BlockFunctionBody node) {
+    final oldMounted = mounted;
+    final oldInAsync = inAsync;
+    mounted = true.asFact();
+    inAsync = node.isAsynchronous;
+
+    node.visitChildren(this);
+
+    mounted = oldMounted;
+    inAsync = oldInAsync;
+  }
+}