From 9a89487026df2335c18dd18e4bbb8a36b6e097fe Mon Sep 17 00:00:00 2001 From: Dmitry Zhifarsky Date: Wed, 4 May 2022 21:53:26 +0400 Subject: [PATCH 1/3] fix: remove duplicated and ignore void function calls --- .../prefer_moving_to_variable_rule.dart | 1 + .../prefer_moving_to_variable/visitor.dart | 7 +- .../examples/arguments_example.dart | 50 ++++++++ .../arguments_with_object_example.dart | 54 +++++++++ .../examples/example.dart | 13 ++- .../examples/scope_example.dart | 49 ++++++++ .../prefer_moving_to_variable_rule_test.dart | 108 +++++++++++++++++- 7 files changed, 270 insertions(+), 12 deletions(-) create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/arguments_example.dart create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/arguments_with_object_example.dart create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/scope_example.dart diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule.dart index 27c020e89e..915b6f1ee1 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule.dart @@ -2,6 +2,7 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/type.dart'; import '../../../../../utils/node_utils.dart'; import '../../../lint_utils.dart'; 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 f86ff0ac0f..405a4e2ec1 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 @@ -7,8 +7,6 @@ class _Visitor extends RecursiveAstVisitor { @override void visitBlockFunctionBody(BlockFunctionBody node) { - super.visitBlockFunctionBody(node); - final visitor = _BlockVisitor(); node.visitChildren(visitor); @@ -39,7 +37,8 @@ class _BlockVisitor extends RecursiveAstVisitor { @override void visitMethodInvocation(MethodInvocation node) { - if (node.parent is CascadeExpression) { + if (node.parent is CascadeExpression || + (node.staticType?.isVoid ?? false)) { return; } @@ -76,7 +75,7 @@ class _BlockVisitor extends RecursiveAstVisitor { final parentBlock = node.thisOrAncestorOfType(); // ignore: avoid-late-keyword - late final grandParentBlock = parentBlock?.thisOrAncestorMatching( + final grandParentBlock = parentBlock?.thisOrAncestorMatching( (block) => block is Block && block != parentBlock, ); diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/arguments_example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/arguments_example.dart new file mode 100644 index 0000000000..67ddbd1ac6 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/arguments_example.dart @@ -0,0 +1,50 @@ +void main() { + final someValue = 'value'; + + methodWithArguments('hello'); + methodWithArguments('hello', 'world'); + + methodWithArguments('world'); // LINT + methodWithArguments('world'); // LINT + + methodWithArguments(someValue); // LINT + methodWithArguments(someValue); // LINT + + final someOtherValue = 'otherValue'; + methodWithArguments(someOtherValue); + methodWithArguments(someValue); // LINT + + methodWithNamedArguments(age: 1); + methodWithNamedArguments(age: 1, firstName: ''); + + methodWithNamedArguments(firstName: 'hello'); // LINT + methodWithNamedArguments(firstName: 'hello'); // LINT + + methodWithNamedArguments(lastName: 'last'); // LINT + + if (true) { + methodWithNamedArguments(lastName: 'last'); // LINT + } + + methodWithMixedArguments(someValue); + methodWithMixedArguments(someOtherValue); + + methodWithMixedArguments(someValue, named: 'name'); // LINT + methodWithMixedArguments(someValue, named: 'name'); // LINT +} + +String methodWithArguments(String firstRegular, String secondRegular) { + return ''; +} + +String methodWithNamedArguments({ + String firstName, + String lastName, + int age, +}) { + return ''; +} + +String methodWithMixedArguments(String regular, {int? named}) { + return ''; +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/arguments_with_object_example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/arguments_with_object_example.dart new file mode 100644 index 0000000000..6fa133d974 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/arguments_with_object_example.dart @@ -0,0 +1,54 @@ +void main() { + final state = State(); + + final someValue = 'value'; + + state.methodWithArguments('hello'); + state.methodWithArguments('hello', 'world'); + + state.methodWithArguments('world'); // LINT + state.methodWithArguments('world'); // LINT + + state.methodWithArguments(someValue); // LINT + state.methodWithArguments(someValue); // LINT + + final someOtherValue = 'otherValue'; + state.methodWithArguments(someOtherValue); + state.methodWithArguments(someValue); // LINT + + state.methodWithNamedArguments(age: 1); + state.methodWithNamedArguments(age: 1, firstName: ''); + + state.methodWithNamedArguments(firstName: 'hello'); // LINT + state.methodWithNamedArguments(firstName: 'hello'); // LINT + + state.methodWithNamedArguments(lastName: 'last'); // LINT + + if (true) { + state.methodWithNamedArguments(lastName: 'last'); // LINT + } + + state.methodWithMixedArguments(someValue); + state.methodWithMixedArguments(someOtherValue); + + state.methodWithMixedArguments(someValue, named: 'name'); // LINT + state.methodWithMixedArguments(someValue, named: 'name'); // LINT +} + +class State { + String methodWithArguments(String firstRegular, String secondRegular) { + return ''; + } + + String methodWithNamedArguments({ + String firstName, + String lastName, + int age, + }) { + return ''; + } + + String methodWithMixedArguments(String regular, {int? named}) { + return ''; + } +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/example.dart index 4a1dfd6371..67a23f9ca2 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/example.dart @@ -22,8 +22,11 @@ void main() { Theme.from().value.runtimeType; // LINT Theme.from().value.length; // LINT - Theme.from().someMethod(); // LINT - Theme.from().someMethod(); // LINT + Theme.from().someMethod(); + Theme.from().someMethod(); + + Theme.from().notVoidMethod(); // LINT + Theme.from().notVoidMethod(); // LINT getValue(); // LINT getValue(); // LINT @@ -44,6 +47,12 @@ class Theme { string.indexOf('').sign.bitLength.isEven; // LINT string.indexOf('').sign.isOdd; // LINT } + + String notVoidMethod() { + final string = 'str'; + + return string; + } } String getValue() => 'hello'; diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/scope_example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/scope_example.dart new file mode 100644 index 0000000000..90550b1899 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/scope_example.dart @@ -0,0 +1,49 @@ +class BlocTest { + final state = State(); + + FutureOr mapEvent( + TestEvent event, + Emitter emit, + ) { + return event.map>( + init: (_) async { + emit(state.copyWith(isLoading: true)); // LINT + + const result = []; + + var newState = state.copyWith(dataList: result); // LINT + + if (result.isEmpty) { + newState = newState.copyWith(dataList: [], isLoading: false); + } else { + newState = newState.copyWith(dataList: result, isLoading: false); + } + + emit(state); + }, + checkProviders: (_) async { + emit(state.copyWith(isLoading: true)); // LINT + + const result = []; + + if (result.isEmpty) { + emit(state.copyWith(dataList: [])); // LINT + } else { + emit(state.copyWith(dataList: result)); + } + }, + ); + } + + void emit(Object value) {} +} + +class TestEvent { + FutureOr map({Function init, Function checkProviders}) {} +} + +class State { + State copyWith({List dataList, bool isLoading}) { + return State(); + } +} 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 c49f068d6b..ebb2c0ff96 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 @@ -5,6 +5,12 @@ import 'package:test/test.dart'; import '../../../../../helpers/rule_test_helper.dart'; const _examplePath = 'prefer_moving_to_variable/examples/example.dart'; +const _argumentsExamplePath = + 'prefer_moving_to_variable/examples/arguments_example.dart'; +const _argumentsWithObjectExamplePath = + 'prefer_moving_to_variable/examples/arguments_with_object_example.dart'; +const _scopeExamplePath = + 'prefer_moving_to_variable/examples/scope_example.dart'; const _cascadeExamplePath = 'prefer_moving_to_variable/examples/cascade_example.dart'; @@ -40,12 +46,12 @@ void main() { 20, 22, 23, - 25, - 26, 28, 29, - 44, - 45, + 31, + 32, + 47, + 48, ], startColumns: [ 19, @@ -80,8 +86,8 @@ void main() { 'Theme.after().value', 'Theme.from().value', 'Theme.from().value', - 'Theme.from().someMethod()', - 'Theme.from().someMethod()', + 'Theme.from().notVoidMethod()', + 'Theme.from().notVoidMethod()', 'getValue()', 'getValue()', "string.indexOf('').sign", @@ -110,6 +116,96 @@ void main() { ); }); + test( + 'reports about found issues for invocations with arguments example', + () async { + final unit = + await RuleTestHelper.resolveFromFile(_argumentsExamplePath); + final issues = PreferMovingToVariableRule().check(unit); + + RuleTestHelper.verifyIssues( + issues: issues, + startLines: [7, 8, 10, 11, 15, 20, 21, 23, 26, 32, 33], + startColumns: [3, 3, 3, 3, 3, 3, 3, 3, 5, 3, 3], + locationTexts: [ + "methodWithArguments('world')", + "methodWithArguments('world')", + 'methodWithArguments(someValue)', + 'methodWithArguments(someValue)', + 'methodWithArguments(someValue)', + "methodWithNamedArguments(firstName: 'hello')", + "methodWithNamedArguments(firstName: 'hello')", + "methodWithNamedArguments(lastName: 'last')", + "methodWithNamedArguments(lastName: 'last')", + "methodWithMixedArguments(someValue, named: 'name')", + "methodWithMixedArguments(someValue, named: 'name')", + ], + 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.', + '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.', + ], + ); + }, + ); + + test( + 'reports about found issues for invocations on object with arguments example', + () async { + final unit = await RuleTestHelper.resolveFromFile( + _argumentsWithObjectExamplePath, + ); + final issues = PreferMovingToVariableRule().check(unit); + + RuleTestHelper.verifyIssues( + issues: issues, + startLines: [9, 10, 12, 13, 17, 22, 23, 25, 28, 34, 35], + startColumns: [3, 3, 3, 3, 3, 3, 3, 3, 5, 3, 3], + locationTexts: [ + "state.methodWithArguments('world')", + "state.methodWithArguments('world')", + 'state.methodWithArguments(someValue)', + 'state.methodWithArguments(someValue)', + 'state.methodWithArguments(someValue)', + "state.methodWithNamedArguments(firstName: 'hello')", + "state.methodWithNamedArguments(firstName: 'hello')", + "state.methodWithNamedArguments(lastName: 'last')", + "state.methodWithNamedArguments(lastName: 'last')", + "state.methodWithMixedArguments(someValue, named: 'name')", + "state.methodWithMixedArguments(someValue, named: 'name')", + ], + 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.', + '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.', + ], + ); + }, + ); + + test('reports no issues for scope', () async { + final unit = await RuleTestHelper.resolveFromFile(_scopeExamplePath); + final issues = PreferMovingToVariableRule().check(unit); + + RuleTestHelper.verifyNoIssues(issues); + }); + test('reports no issues for cascade', () async { final unit = await RuleTestHelper.resolveFromFile(_cascadeExamplePath); final issues = PreferMovingToVariableRule().check(unit); From cc0eea438bfe12a0cacf86dfeabf8b011e1f2ad4 Mon Sep 17 00:00:00 2001 From: Dmitry Zhifarsky Date: Wed, 4 May 2022 21:55:32 +0400 Subject: [PATCH 2/3] chore: remove unused import --- CHANGELOG.md | 4 ++++ .../prefer_moving_to_variable_rule.dart | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cfe91a418..694433fa37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +* fix: remove duplicated and ignore void function calls for [`prefer-moving-to-variable`](https://dartcodemetrics.dev/docs/rules/common/prefer-moving-to-variable). + ## 4.15.0 * fix: [`format-comment`](https://dartcodemetrics.dev/docs/rules/common/format-comment) is listing the macros from dart doc. diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule.dart index 915b6f1ee1..27c020e89e 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule.dart @@ -2,7 +2,6 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; -import 'package:analyzer/dart/element/type.dart'; import '../../../../../utils/node_utils.dart'; import '../../../lint_utils.dart'; From b532ffad23664b85cacf808eb768df4f0cc9e886 Mon Sep 17 00:00:00 2001 From: Dmitry Zhifarsky Date: Wed, 11 May 2022 22:14:37 +0400 Subject: [PATCH 3/3] chore: remove unused ignore --- .../rules/rules_list/prefer_moving_to_variable/visitor.dart | 1 - 1 file changed, 1 deletion(-) 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 405a4e2ec1..94318606d7 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 @@ -74,7 +74,6 @@ class _BlockVisitor extends RecursiveAstVisitor { final visitedBlock = visitedInvocation.thisOrAncestorOfType(); final parentBlock = node.thisOrAncestorOfType(); - // ignore: avoid-late-keyword final grandParentBlock = parentBlock?.thisOrAncestorMatching( (block) => block is Block && block != parentBlock, );