feat: add configuration to prefer-moving-to-variable (#934)

* feat: add configuration to prefer-moving-to-variable

* test: update tests

* chore: add missed trailing comma

Co-authored-by: Dmitry Krutskikh <dmitry.krutskikh@gmail.com>

Author: incendial

CHANGELOG.md

@@ -2,13 +2,14 @@
 
 ## Unreleased
 
+* feat: add configuration to [`prefer-moving-to-variable`](https://dartcodemetrics.dev/docs/rules/common/prefer-moving-to-variable).
 * feat: add static code diagnostic [`avoid-duplicate-exports`](https://dartcodemetrics.dev/docs/rules/common/avoid-duplicate-exports).
 * feat: add static code diagnostic [`avoid-shrink-wrap-in-lists`](https://dartcodemetrics.dev/docs/rules/flutter/avoid-shrink-wrap-in-lists).
 * feat: add static code diagnostic [`avoid-top-level-members-in-tests`](https://dartcodemetrics.dev/docs/rules/common/avoid-top-level-members-in-tests).
 * feat: add static code diagnostic [`prefer-correct-edge-insets-constructor-rule`](https://dartcodemetrics.dev/docs/rules/flutter/prefer-correct-edge-insets-constructor).
 * feat: add static code diagnostic [`prefer-enums-by-name`](https://dartcodemetrics.dev/docs/rules/common/prefer-enums-by-name).
-* fix: add zero exit to command runner.
 * feat: add suppressions for `check-unused-files`, `check-unused-code`, `check-unnecessary-nullable` commands.
+* fix: add zero exit to command runner
 
 ## 4.17.0-dev.1
 

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

@@ -0,0 +1,11 @@
+part of 'prefer_moving_to_variable_rule.dart';
+
+class _ConfigParser {
+  static const _allowedDuplicatedChains = 'allowed-duplicated-chains';
+
+  static int? parseAllowedDuplicatedChains(Map<String, Object> config) {
+    final raw = config[_allowedDuplicatedChains];
+
+    return raw is int? ? raw : null;
+  }
+}

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

@@ -11,6 +11,7 @@ import '../../../models/severity.dart';
 import '../../models/common_rule.dart';
 import '../../rule_utils.dart';
 
+part 'config_parser.dart';
 part 'visitor.dart';
 
 class PreferMovingToVariableRule extends CommonRule {
@@ -19,16 +20,20 @@ class PreferMovingToVariableRule extends CommonRule {
   static const _warningMessage =
       'Prefer moving repeated invocations to variable and use it instead.';
 
+  final int? _duplicatesThreshold;
+
   PreferMovingToVariableRule([Map<String, Object> config = const {}])
-      : super(
+      : _duplicatesThreshold =
+            _ConfigParser.parseAllowedDuplicatedChains(config),
+        super(
           id: ruleId,
           severity: readSeverity(config, Severity.warning),
           excludes: readExcludes(config),
         );
 
   @override
   Iterable<Issue> check(InternalResolvedUnitResult source) {
-    final visitor = _Visitor();
+    final visitor = _Visitor(_duplicatesThreshold);
 
     source.unit.visitChildren(visitor);
 

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

@@ -3,11 +3,15 @@ part of 'prefer_moving_to_variable_rule.dart';
 class _Visitor extends RecursiveAstVisitor<void> {
   final _nodes = <AstNode>[];
 
+  final int? _duplicatesThreshold;
+
   Iterable<AstNode> get nodes => _nodes;
 
+  _Visitor(this._duplicatesThreshold);
+
   @override
   void visitBlockFunctionBody(BlockFunctionBody node) {
-    final visitor = _BlockVisitor();
+    final visitor = _BlockVisitor(_duplicatesThreshold);
     node.visitChildren(visitor);
 
     _nodes.addAll(visitor.duplicates);
@@ -17,11 +21,22 @@ class _Visitor extends RecursiveAstVisitor<void> {
 class _BlockVisitor extends RecursiveAstVisitor<void> {
   final Map<String, AstNode> _visitedInvocations = {};
   final Set<AstNode> _visitedNodes = {};
-  final Set<AstNode> _duplicates = {};
+  final Map<String, Set<AstNode>> _duplicates = {};
+
+  final int? _duplicatesThreshold;
+
+  Set<AstNode> get duplicates =>
+      _duplicates.entries.fold(<AstNode>{}, (previousValue, element) {
+        final duplicatesThreshold = _duplicatesThreshold;
+        if (duplicatesThreshold == null ||
+            element.value.length >= duplicatesThreshold) {
+          previousValue.addAll(element.value);
+        }
 
-  Set<AstNode> get duplicates => _duplicates;
+        return previousValue;
+      });
 
-  _BlockVisitor();
+  _BlockVisitor(this._duplicatesThreshold);
 
   @override
   void visitPropertyAccess(PropertyAccess node) {
@@ -54,7 +69,11 @@ class _BlockVisitor extends RecursiveAstVisitor<void> {
     final isDuplicate =
         visitedInvocation != null && _isDuplicate(visitedInvocation, node);
     if (isDuplicate) {
-      _duplicates.addAll([visitedInvocation, node]);
+      _duplicates.update(
+        access,
+        (value) => value..addAll([visitedInvocation, node]),
+        ifAbsent: () => {visitedInvocation, node},
+      );
     }
 
     if (_visitedNodes.contains(node)) {

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

@@ -42,14 +42,14 @@ void main() {
           11,
           13,
           14,
+          31,
+          32,
           19,
           20,
           22,
           23,
           28,
           29,
-          31,
-          32,
           47,
           48,
         ],
@@ -82,14 +82,14 @@ void main() {
           'getValue()',
           'getValue()',
           'getValue()',
+          'getValue()',
+          'getValue()',
           'Theme.after().value',
           'Theme.after().value',
           'Theme.from().value',
           'Theme.from().value',
           'Theme.from().notVoidMethod()',
           'Theme.from().notVoidMethod()',
-          'getValue()',
-          'getValue()',
           "string.indexOf('').sign",
           "string.indexOf('').sign",
         ],
@@ -212,5 +212,35 @@ void main() {
 
       RuleTestHelper.verifyNoIssues(issues);
     });
+
+    test('reports issues with custom config', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_examplePath);
+      final config = {
+        'allowed-duplicated-chains': 3,
+      };
+      final issues = PreferMovingToVariableRule(config).check(unit);
+
+      RuleTestHelper.verifyIssues(
+        issues: issues,
+        startLines: [10, 11, 13, 14, 31, 32],
+        startColumns: [3, 3, 3, 3, 3, 3],
+        locationTexts: [
+          'getValue()',
+          'getValue()',
+          'getValue()',
+          'getValue()',
+          'getValue()',
+          'getValue()',
+        ],
+        messages: [
+          'Prefer moving repeated invocations to variable and use it instead.',
+          'Prefer moving repeated invocations to variable and use it instead.',
+          'Prefer moving repeated invocations to variable and use it instead.',
+          'Prefer moving repeated invocations to variable and use it instead.',
+          'Prefer moving repeated invocations to variable and use it instead.',
+          'Prefer moving repeated invocations to variable and use it instead.',
+        ],
+      );
+    });
   });
 }

website/docs/rules/common/prefer-moving-to-variable.md

@@ -1,5 +1,7 @@
 # Prefer moving to variable
 
+![Configurable](https://img.shields.io/badge/-configurable-informational)
+
 ## Rule id {#rule-id}
 
 prefer-moving-to-variable
@@ -21,6 +23,19 @@ final age = getUser().age;
 
 the rule will suggest to move `getUser()` call to a single variable.
 
+Use `allowed-duplicated-chains` configuration, if you want to set a threshold after which the rule should trigger on duplicated lines.
+
+### Config example {#config-example}
+
+```yaml
+dart_code_metrics:
+  ...
+  rules:
+    ...
+    - prefer-moving-to-variable:
+        allowed-duplicated-chains: 3
+```
+
 ### Example {#example}
 
 Bad:

website/docs/rules/flutter/prefer-extracting-callbacks.md

@@ -14,10 +14,11 @@ Style
 
 Warns about inline callbacks in a widget tree and suggests to extract them to widget methods in order to make a `build` method more readable. In addition extracting can help test those methods separately as well.
 
-**NOTE** the rule will not trigger on: 
- - arrow functions like `onPressed: () => _handler(...)` in order to cover cases when a callback needs a variable from the outside;
- - empty blocks.
- - Flutter specific: arguments with functions returning `Widget` type (or its subclass) and with first parameter of type `BuildContext`. "Builder" functions is a common pattern in Flutter, for example, [IndexedWidgetBuilder typedef](https://api.flutter.dev/flutter/widgets/IndexedWidgetBuilder.html) is used in [ListView.builder](https://api.flutter.dev/flutter/widgets/ListView/ListView.builder.html).
+**NOTE** the rule will not trigger on:
+
+- arrow functions like `onPressed: () => _handler(...)` in order to cover cases when a callback needs a variable from the outside;
+- empty blocks.
+- Flutter specific: arguments with functions returning `Widget` type (or its subclass) and with first parameter of type `BuildContext`. "Builder" functions is a common pattern in Flutter, for example, [IndexedWidgetBuilder typedef](https://api.flutter.dev/flutter/widgets/IndexedWidgetBuilder.html) is used in [ListView.builder](https://api.flutter.dev/flutter/widgets/ListView/ListView.builder.html).
 
 Use `ignored-named-arguments` configuration, if you want to ignore specific named parameters.
 
@@ -45,7 +46,7 @@ class MyWidget extends StatelessWidget {
     return TextButton(
       style: ...,
       onPressed: () { // LINT
-        // Some 
+        // Some
         // Huge
         // Callback
       },
@@ -67,7 +68,7 @@ class MyWidget extends StatelessWidget {
       child: ...
     );
   }
-  
+
   void handlePressed(BuildContext context) {
     ...
   }
@@ -82,7 +83,7 @@ class MyWidget extends StatelessWidget {
       child: ...
     );
   }
-  
+
   void handlePressed() {
     ...
   }