Skip to content
This repository was archived by the owner on Jul 16, 2023. It is now read-only.

Commit ccbc0b5

Browse files
incendialdkrutskikh
incendial
and
dkrutskikh
authored
feat: add static code diagnostic prefer-moving-to-variable (#781)
* feat: add static code diagnostic prefer-moving-to-variable * fix: cover edge-cases * Update CHANGELOG.md Co-authored-by: Dmitry Krutskikh <dmitry.krutskikh@gmail.com>
1 parent e197826 commit ccbc0b5

File tree

8 files changed

+436
-0
lines changed

8 files changed

+436
-0
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## Unreleased
44

55
* feat: add static code diagnostic [`prefer-commenting-analyzer-ignores`](https://dartcodemetrics.dev/docs/rules/common/prefer-commenting-analyzer-ignores).
6+
* feat: add static code diagnostic [`prefer-moving-to-variable`](https://dartcodemetrics.dev/docs/rules/common/prefer-moving-to-variable).
67
* fix: add check for supertypes for [`avoid-non-null-assertions`](https://dartcodemetrics.dev/docs/rules/common/avoid-non-null-assertion) rule.
78
* fix: cover more cases in [`prefer-immediate-return`](https://dartcodemetrics.dev/docs/rules/common/prefer-immediate-return) rule
89
* fix: support index expressions for [`no-magic-number`](https://dartcodemetrics.dev/docs/rules/common/no-magic-number) rule.

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import 'rules_list/prefer_immediate_return/prefer_immediate_return_rule.dart';
4444
import 'rules_list/prefer_intl_name/prefer_intl_name_rule.dart';
4545
import 'rules_list/prefer_last/prefer_last_rule.dart';
4646
import 'rules_list/prefer_match_file_name/prefer_match_file_name_rule.dart';
47+
import 'rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule.dart';
4748
import 'rules_list/prefer_on_push_cd_strategy/prefer_on_push_cd_strategy_rule.dart';
4849
import 'rules_list/prefer_single_widget_per_file/prefer_single_widget_per_file_rule.dart';
4950
import 'rules_list/prefer_trailing_comma/prefer_trailing_comma_rule.dart';
@@ -121,6 +122,8 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
121122
PreferIntlNameRule.ruleId: (config) => PreferIntlNameRule(config),
122123
PreferLastRule.ruleId: (config) => PreferLastRule(config),
123124
PreferMatchFileNameRule.ruleId: (config) => PreferMatchFileNameRule(config),
125+
PreferMovingToVariableRule.ruleId: (config) =>
126+
PreferMovingToVariableRule(config),
124127
PreferOnPushCdStrategyRule.ruleId: (config) =>
125128
PreferOnPushCdStrategyRule(config),
126129
PreferSingleWidgetPerFileRule.ruleId: (config) =>

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// ignore_for_file: public_member_api_docs
2+
3+
import 'package:analyzer/dart/ast/ast.dart';
4+
import 'package:analyzer/dart/ast/visitor.dart';
5+
6+
import '../../../../../utils/node_utils.dart';
7+
import '../../../lint_utils.dart';
8+
import '../../../models/internal_resolved_unit_result.dart';
9+
import '../../../models/issue.dart';
10+
import '../../../models/severity.dart';
11+
import '../../models/common_rule.dart';
12+
import '../../rule_utils.dart';
13+
14+
part 'visitor.dart';
15+
16+
class PreferMovingToVariableRule extends CommonRule {
17+
static const String ruleId = 'prefer-moving-to-variable';
18+
19+
static const _warningMessage =
20+
'Prefer moving repeated invocations to variable and use it instead.';
21+
22+
PreferMovingToVariableRule([Map<String, Object> config = const {}])
23+
: super(
24+
id: ruleId,
25+
severity: readSeverity(config, Severity.warning),
26+
excludes: readExcludes(config),
27+
);
28+
29+
@override
30+
Iterable<Issue> check(InternalResolvedUnitResult source) {
31+
final visitor = _Visitor();
32+
33+
source.unit.visitChildren(visitor);
34+
35+
return visitor.nodes
36+
.map((node) => createIssue(
37+
rule: this,
38+
location: nodeLocation(node: node, source: source),
39+
message: _warningMessage,
40+
))
41+
.toList(growable: false);
42+
}
43+
}

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

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
part of 'prefer_moving_to_variable_rule.dart';
2+
3+
class _Visitor extends RecursiveAstVisitor<void> {
4+
final _nodes = <AstNode>[];
5+
6+
Iterable<AstNode> get nodes => _nodes;
7+
8+
@override
9+
void visitBlockFunctionBody(BlockFunctionBody node) {
10+
super.visitBlockFunctionBody(node);
11+
12+
final visitor = _BlockVisitor();
13+
node.visitChildren(visitor);
14+
15+
_nodes.addAll(visitor.duplicates);
16+
}
17+
}
18+
19+
class _BlockVisitor extends RecursiveAstVisitor<void> {
20+
final Map<String, AstNode> _visitedInvocations = {};
21+
final Set<AstNode> _visitedNodes = {};
22+
final Set<AstNode> _duplicates = {};
23+
24+
Set<AstNode> get duplicates => _duplicates;
25+
26+
_BlockVisitor();
27+
28+
@override
29+
void visitPropertyAccess(PropertyAccess node) {
30+
if (node.target == null) {
31+
return;
32+
}
33+
34+
final hasDuplicates = _checkForDuplicates(node, node.target);
35+
if (!hasDuplicates) {
36+
super.visitPropertyAccess(node);
37+
}
38+
}
39+
40+
@override
41+
void visitMethodInvocation(MethodInvocation node) {
42+
if (node.parent is CascadeExpression) {
43+
return;
44+
}
45+
46+
final hasDuplicates = _checkForDuplicates(node, node.target);
47+
if (!hasDuplicates) {
48+
super.visitMethodInvocation(node);
49+
}
50+
}
51+
52+
bool _checkForDuplicates(AstNode node, Expression? target) {
53+
final access = node.toString();
54+
final visitedInvocation = _visitedInvocations[access];
55+
final isDuplicate =
56+
visitedInvocation != null && _isDuplicate(visitedInvocation, node);
57+
if (isDuplicate) {
58+
_duplicates.addAll([visitedInvocation!, node]);
59+
}
60+
61+
if (_visitedNodes.contains(node)) {
62+
return isDuplicate;
63+
}
64+
65+
_visitedInvocations[access] = node;
66+
_visitAllTargets(target);
67+
68+
return false;
69+
}
70+
71+
bool _isDuplicate(AstNode visitedInvocation, AstNode node) {
72+
final visitedSwitch =
73+
visitedInvocation.thisOrAncestorOfType<SwitchStatement>();
74+
75+
final visitedBlock = visitedInvocation.thisOrAncestorOfType<Block>();
76+
final parentBlock = node.thisOrAncestorOfType<Block>();
77+
78+
// ignore: avoid-late-keyword
79+
late final grandParentBlock = parentBlock?.thisOrAncestorMatching(
80+
(block) => block is Block && block != parentBlock,
81+
);
82+
83+
final visitedFunctionExpression = visitedInvocation.thisOrAncestorMatching(
84+
(astNode) =>
85+
astNode is FunctionExpression || astNode is FunctionDeclaration,
86+
);
87+
final parentFunctionExpression = node.thisOrAncestorMatching((astNode) =>
88+
astNode is FunctionExpression || astNode is FunctionDeclaration);
89+
90+
return visitedInvocation != node &&
91+
visitedSwitch == null &&
92+
(visitedBlock == parentBlock || visitedBlock == grandParentBlock) &&
93+
(visitedFunctionExpression == null &&
94+
parentFunctionExpression == null ||
95+
visitedFunctionExpression == parentFunctionExpression);
96+
}
97+
98+
void _visitAllTargets(Expression? target) {
99+
var realTarget = target;
100+
101+
while (realTarget != null) {
102+
_visitedNodes.add(realTarget);
103+
104+
final access = realTarget.toString();
105+
if (!_visitedInvocations.containsKey(access)) {
106+
_visitedInvocations[access] = realTarget;
107+
}
108+
109+
if (realTarget is MethodInvocation) {
110+
realTarget = realTarget.realTarget;
111+
} else if (realTarget is PropertyAccess) {
112+
realTarget = realTarget.realTarget;
113+
} else {
114+
realTarget = null;
115+
}
116+
}
117+
}
118+
}

‎test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/cascade_example.dart‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
void main() {
2+
final cyclomaticValues = Element.tag('td')
3+
..classes.add('metrics-source-code__complexity');
4+
5+
final tooltip = Element.tag('div')
6+
..classes.add('metrics-source-code__tooltip')
7+
..append(Element.tag('div')
8+
..classes.add('metrics-source-code__tooltip-title')
9+
..text = title)
10+
..append(Element.tag('p')
11+
..classes.add('metrics-source-code__tooltip-section')
12+
..text = issue.message);
13+
14+
final html = Element.tag('html')
15+
..attributes['lang'] = 'en'
16+
..append(Element.tag('head')
17+
..append(Element.tag('title')..text = 'Metrics report')
18+
..append(Element.tag('meta')..attributes['charset'] = 'utf-8')
19+
..append(Element.tag('link')
20+
..attributes['rel'] = 'stylesheet'
21+
..attributes['href'] = 'variables.css')
22+
..append(Element.tag('link')
23+
..attributes['rel'] = 'stylesheet'
24+
..attributes['href'] = 'normalize.css')
25+
..append(Element.tag('link')
26+
..attributes['rel'] = 'stylesheet'
27+
..attributes['href'] = 'base.css')
28+
..append(Element.tag('link')
29+
..attributes['rel'] = 'stylesheet'
30+
..attributes['href'] = 'main.css'))
31+
..append(Element.tag('body')
32+
..append(Element.tag('h1')
33+
..classes.add('metric-header')
34+
..text = 'All files')
35+
..append(_generateTable('Directory', tableRecords)));
36+
}
37+
38+
class Element {
39+
final String localName;
40+
41+
final nodes = <Element>[];
42+
43+
Set<String> get classes => <String>{};
44+
45+
String text;
46+
47+
LinkedHashMap<Object, String> attributes = LinkedHashMap();
48+
49+
Element.tag(this.localName);
50+
51+
void append(Element node) => nodes.add(node);
52+
}

‎test/src/analyzers/lint_analyzer/rules/rules_list/prefer_moving_to_variable/examples/example.dart‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
void main() {
2+
final isEmpty = Theme.of('color').trim().isEmpty; // LINT
3+
final isNotEmpty = Theme.of('color').trim().isNotEmpty; // LINT
4+
5+
final string = 'str';
6+
7+
string.indexOf('').sign.bitLength.isEven; // LINT
8+
string.indexOf('').sign.isOdd; // LINT
9+
10+
getValue().isNotEmpty; // LINT
11+
getValue().length; // LINT
12+
13+
getValue().contains('').toString(); // LINT
14+
getValue().contains('asd').toString(); // LINT
15+
16+
string.length;
17+
string.isEmpty;
18+
19+
Theme.after().value.runtimeType; // LINT
20+
Theme.after().value.length; // LINT
21+
22+
Theme.from().value.runtimeType; // LINT
23+
Theme.from().value.length; // LINT
24+
25+
Theme.from().someMethod(); // LINT
26+
Theme.from().someMethod(); // LINT
27+
28+
getValue(); // LINT
29+
getValue(); // LINT
30+
}
31+
32+
class Theme {
33+
const value = '123';
34+
35+
static String of(String value) => value;
36+
37+
factory Theme.from() => Theme();
38+
39+
Theme.after() {}
40+
41+
void someMethod() {
42+
final string = 'str';
43+
44+
string.indexOf('').sign.bitLength.isEven; // LINT
45+
string.indexOf('').sign.isOdd; // LINT
46+
}
47+
}
48+
49+
String getValue() => 'hello';

0 commit comments

Comments
 (0)