diff --git a/CHANGELOG.md b/CHANGELOG.md index 0559f1833a..4f60d96109 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/visitor.dart index 35418754e0..0ad9e422c7 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/visitor.dart @@ -108,6 +108,14 @@ class _BlockVisitor extends RecursiveAstVisitor { } 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(); @@ -153,4 +161,46 @@ class _BlockVisitor extends RecursiveAstVisitor { } } } + + 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(); + final parentWhile = node.thisOrAncestorOfType(); + + return visitedWhile != null && visitedWhile == parentWhile; + } + + bool _haveDifferentParameterTypes(AstNode visitedInvocation, AstNode node) => + visitedInvocation is Expression && + node is Expression && + visitedInvocation.staticParameterElement?.type != + node.staticParameterElement?.type; } diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/assignment_example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/assignment_example.dart index 4f214600a1..0a3f051a6d 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/assignment_example.dart +++ b/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'; + } +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/provider_example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/provider_example.dart new file mode 100644 index 0000000000..adeb7b5f04 --- /dev/null +++ b/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 { + 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() { + return 'value' as T; + } +} + +abstract class State { + void initState(); + + void setState(VoidCallback callback) => callback(); + + BuildContext get context => BuildContext(); +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/while_example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/while_example.dart new file mode 100644 index 0000000000..c6c74d7295 --- /dev/null +++ b/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? _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); +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule_test.dart index de9b23d450..f5e8ad3859 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule_test.dart +++ b/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 = {