fix: fix moving to variable false positives (#1186)

* fix: correctly handle implicit type parameters

* fix: exclude mutable arguments and duplicates inside while statement

* fix: handle setters

* chore: update changelog

* fix: fix comparison

Author: incendial

CHANGELOG.md

@@ -5,6 +5,7 @@
 * feat: add `allow-nullable` config option for [`avoid-returning-widgets`](https://dcm.dev/docs/individuals/rules/common/avoid-returning-widgets).
 * 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).
+* fix: resolve several false-positives with while loops, setters and implicit type parameters for [`prefer-moving-to-variable`](https://dcm.dev/docs/individuals/rules/common/prefer-moving-to-variable).
 
 ## 5.6.0-dev.1
 

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

@@ -108,6 +108,14 @@ class _BlockVisitor extends RecursiveAstVisitor<void> {
   }
 
   bool _isDuplicate(AstNode visitedInvocation, AstNode node) {
+    if (_haveDifferentParameterTypes(visitedInvocation, node) ||
+        _isSetter(node) ||
+        _isSetter(visitedInvocation) ||
+        _hasMutableArguments(visitedInvocation, node) ||
+        _bothInsideWhile(visitedInvocation, node)) {
+      return false;
+    }
+
     final visitedSwitch =
         visitedInvocation.thisOrAncestorOfType<SwitchStatement>();
 
@@ -153,4 +161,46 @@ class _BlockVisitor extends RecursiveAstVisitor<void> {
       }
     }
   }
+
+  bool _hasMutableArguments(AstNode visitedInvocation, AstNode node) =>
+      visitedInvocation is MethodInvocation &&
+      node is MethodInvocation &&
+      visitedInvocation.argumentList.arguments.any((argument) {
+        final expression =
+            argument is NamedExpression ? argument.expression : argument;
+
+        if (expression is SimpleIdentifier) {
+          final element = expression.staticElement;
+
+          return element is VariableElement &&
+              !element.isConst &&
+              !element.isFinal;
+        }
+
+        return false;
+      });
+
+  bool _isSetter(AstNode node) {
+    if (node is! PropertyAccess) {
+      return false;
+    }
+
+    final parent = node.parent;
+
+    return parent is AssignmentExpression && parent.leftHandSide == node;
+  }
+
+  bool _bothInsideWhile(AstNode visitedInvocation, AstNode node) {
+    final visitedWhile =
+        visitedInvocation.thisOrAncestorOfType<WhileStatement>();
+    final parentWhile = node.thisOrAncestorOfType<WhileStatement>();
+
+    return visitedWhile != null && visitedWhile == parentWhile;
+  }
+
+  bool _haveDifferentParameterTypes(AstNode visitedInvocation, AstNode node) =>
+      visitedInvocation is Expression &&
+      node is Expression &&
+      visitedInvocation.staticParameterElement?.type !=
+          node.staticParameterElement?.type;
 }

test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/assignment_example.dart

@@ -6,3 +6,19 @@ void main() {
 String someFunction() {
   return 'qwe';
 }
+
+class SomeClass {
+  final SomeClass inner;
+
+  String _value = '123';
+
+  String get value => _value;
+  set(String value) {
+    _value = value;
+  }
+
+  void method() {
+    final first = inner.inner.value;
+    inner.inner.value = 'hello';
+  }
+}

test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/provider_example.dart

@@ -0,0 +1,49 @@
+class MyApp extends StatefulWidget {
+  const MyApp();
+}
+
+class _MyAppState extends State<MyApp> {
+  late final A a;
+  late final B b;
+  late final C c;
+
+  @override
+  void initState() {
+    super.initState();
+
+    a = context.read();
+    b = context.read();
+    c = context.read();
+  }
+
+  @override
+  Widget build(BuildContext context) {
+    return Container();
+  }
+}
+
+class A {}
+
+class B {}
+
+class C {}
+
+class Widget {}
+
+class StatefulWidget extends Widget {}
+
+class BuildContext {}
+
+extension ReadContext on BuildContext {
+  T read<T>() {
+    return 'value' as T;
+  }
+}
+
+abstract class State<T> {
+  void initState();
+
+  void setState(VoidCallback callback) => callback();
+
+  BuildContext get context => BuildContext();
+}

test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/while_example.dart

@@ -0,0 +1,36 @@
+class SomeClass {
+  final _addedComments = <Token>{};
+
+  Token? _latestCommentToken(Token token) {
+    Token? latestCommentToken = token.precedingComments;
+    while (latestCommentToken?.next != null) {
+      latestCommentToken = latestCommentToken?.next;
+    }
+
+    return latestCommentToken;
+  }
+
+  void method(Token token) {
+    Token? commentToken = token.precedingComments;
+
+    if (commentToken != null && !_addedComments.contains(commentToken)) {
+      if (!fromEnd) {
+        sink.write('\n');
+      }
+    }
+
+    while (commentToken != null) {
+      if (_addedComments.contains(commentToken)) {
+        commentToken = commentToken.next;
+        continue;
+      }
+      _addedComments.add(commentToken);
+    }
+  }
+}
+
+class Token {
+  final Token precedingComments;
+
+  const Token(this.precedingComments);
+}

test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule_test.dart

@@ -19,6 +19,10 @@ const _assignmentExamplePath =
     'prefer_moving_to_variable/examples/assignment_example.dart';
 const _prefixExamplePath =
     'prefer_moving_to_variable/examples/prefix_example.dart';
+const _providerExamplePath =
+    'prefer_moving_to_variable/examples/provider_example.dart';
+const _whileExamplePath =
+    'prefer_moving_to_variable/examples/while_example.dart';
 
 void main() {
   group('PreferMovingToVariableRule', () {
@@ -240,6 +244,20 @@ void main() {
       RuleTestHelper.verifyNoIssues(issues);
     });
 
+    test('reports no issues for provider read', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_providerExamplePath);
+      final issues = PreferMovingToVariableRule().check(unit);
+
+      RuleTestHelper.verifyNoIssues(issues);
+    });
+
+    test('reports no issues for while', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_whileExamplePath);
+      final issues = PreferMovingToVariableRule().check(unit);
+
+      RuleTestHelper.verifyNoIssues(issues);
+    });
+
     test('reports issues with custom config', () async {
       final unit = await RuleTestHelper.resolveFromFile(_examplePath);
       final config = {