Skip to content

Commit 1940463

Browse files
solid-illiaaihistovIllia Aihistov
andauthored
refactor: migrate cyclomatic_complexity (#294)
* refactor: migrate cyclomatic_complexity (#250) * test: add coverage for nested closures in cyclomatic complexity rule and remove obsolete issue link * fix: update cyclomatic complexity rule to support constructors and do-statements * refactor: migrate CyclomaticComplexityRuleTest to AutoTestLintOffsets and add additional test cases * feat: include collection elements, logical patterns, and null-aware operators in cyclomatic complexity calculation * feat: include null check and null assert patterns in cyclomatic complexity analysis * refactor: migrate cyclomatic complexity visitor to SimpleAstVisitor and target individual declaration types * chore: refactor test files formatting * test: add cyclomatic complexity test case for nested for-in loops * refactor: use inclusive comparison for cyclomatic complexity threshold --------- Co-authored-by: Illia Aihistov <illia.aihistov-us@solid.software>
1 parent 8736521 commit 1940463

8 files changed

Lines changed: 667 additions & 127 deletions

File tree

lib/analysis_options_test.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ custom_lint:
1212
# Since we're not using the source-lines-of-code rule, `main()` function in test can
1313
# have high cyclomatic complexity.
1414
# For rationale against splitting up `main()` in tests, see `source-lines-of-code` comments.
15-
# Also, there is a bug in metric calculation: https://github.com/dart-code-checker/dart-code-metrics/issues/663
1615
- cyclomatic_complexity: false
1716

1817
# Late keyword is allowed in tests in order to enable the use of custom mocks and

lib/main.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import 'package:solid_lints/src/lints/avoid_returning_widgets/avoid_returning_wi
1010
import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart';
1111
import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/fixes/avoid_unnecessary_type_assertions_fix.dart';
1212
import 'package:solid_lints/src/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart';
13+
import 'package:solid_lints/src/lints/cyclomatic_complexity/cyclomatic_complexity_rule.dart';
14+
import 'package:solid_lints/src/lints/cyclomatic_complexity/models/cyclomatic_complexity_parameters.dart';
1315
import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_rule.dart';
1416
import 'package:solid_lints/src/lints/double_literal_format/fixes/double_literal_format_fix.dart';
1517
import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart';
@@ -53,6 +55,10 @@ class SolidLintsPlugin extends Plugin {
5355
AvoidUnusedParametersRule(
5456
analysisOptionsLoader: analysisLoader,
5557
),
58+
CyclomaticComplexityRule(
59+
analysisOptionsLoader: analysisLoader,
60+
parametersParser: CyclomaticComplexityParameters.fromJson,
61+
),
5662
UseNearestContextRule(),
5763
];
5864

Lines changed: 38 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import 'package:analyzer/error/listener.dart';
2-
import 'package:custom_lint_builder/custom_lint_builder.dart';
1+
import 'package:analyzer/analysis_rule/rule_context.dart';
2+
import 'package:analyzer/analysis_rule/rule_visitor_registry.dart';
3+
import 'package:analyzer/error/error.dart';
34
import 'package:solid_lints/src/lints/cyclomatic_complexity/models/cyclomatic_complexity_parameters.dart';
4-
import 'package:solid_lints/src/lints/cyclomatic_complexity/visitors/cyclomatic_complexity_flow_visitor.dart';
5-
import 'package:solid_lints/src/models/rule_config.dart';
5+
import 'package:solid_lints/src/lints/cyclomatic_complexity/visitors/cyclomatic_complexity_visitor.dart';
66
import 'package:solid_lints/src/models/solid_lint_rule.dart';
77

88
/// Limit for the number of linearly independent paths through a program's
@@ -17,54 +17,51 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart';
1717
/// triggering a warning.
1818
///
1919
/// ```yaml
20-
/// custom_lint:
21-
/// rules:
22-
/// - cyclomatic_complexity:
23-
/// max_complexity: 10
20+
/// plugins:
21+
/// solid_lints:
22+
/// diagnostics:
23+
/// cyclomatic_complexity:
24+
/// max_complexity: 10
2425
/// ```
2526
class CyclomaticComplexityRule
2627
extends SolidLintRule<CyclomaticComplexityParameters> {
27-
/// This lint rule represents the error if complexity
28-
/// reaches maximum value.
28+
/// Name of the lint.
2929
static const lintName = 'cyclomatic_complexity';
3030

31-
CyclomaticComplexityRule._(super.rule);
31+
static const _code = LintCode(
32+
lintName,
33+
'The maximum allowed complexity of a function is {0}. Please decrease it.',
34+
);
3235

33-
/// Creates a new instance of [CyclomaticComplexityRule]
34-
/// based on the lint configuration.
35-
factory CyclomaticComplexityRule.createRule(CustomLintConfigs configs) {
36-
final rule = RuleConfig(
37-
configs: configs,
38-
name: lintName,
39-
paramsParser: CyclomaticComplexityParameters.fromJson,
40-
problemMessage: (value) =>
41-
'The maximum allowed complexity of a function is '
42-
'${value.maxComplexity}. Please decrease it.',
43-
);
36+
@override
37+
DiagnosticCode get diagnosticCode => _code;
4438

45-
return CyclomaticComplexityRule._(rule);
46-
}
39+
/// Creates a new instance of [CyclomaticComplexityRule].
40+
CyclomaticComplexityRule({
41+
required super.analysisOptionsLoader,
42+
required super.parametersParser,
43+
}) : super.withParameters(
44+
name: lintName,
45+
description:
46+
'Limit for the number of linearly independent paths '
47+
"through a program's source code.",
48+
);
4749

4850
@override
49-
void run(
50-
CustomLintResolver resolver,
51-
DiagnosticReporter reporter,
52-
CustomLintContext context,
51+
void registerNodeProcessors(
52+
RuleVisitorRegistry registry,
53+
RuleContext context,
5354
) {
54-
context.registry.addBlockFunctionBody((node) {
55-
context.registry.addDeclaration((declarationNode) {
56-
final isIgnored =
57-
config.parameters.exclude.shouldIgnore(declarationNode);
58-
if (isIgnored) return;
55+
super.registerNodeProcessors(registry, context);
56+
57+
final parameters =
58+
getParametersForContext(context) ??
59+
CyclomaticComplexityParameters.empty();
5960

60-
final visitor = CyclomaticComplexityFlowVisitor();
61-
node.visitChildren(visitor);
61+
final visitor = CyclomaticComplexityVisitor(this, parameters);
6262

63-
if (visitor.complexityEntities.length + 1 >
64-
config.parameters.maxComplexity) {
65-
reporter.atNode(node, code);
66-
}
67-
});
68-
});
63+
registry.addFunctionDeclaration(this, visitor);
64+
registry.addConstructorDeclaration(this, visitor);
65+
registry.addMethodDeclaration(this, visitor);
6966
}
7067
}

lib/src/lints/cyclomatic_complexity/models/cyclomatic_complexity_parameters.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ class CyclomaticComplexityParameters {
1717
required this.exclude,
1818
});
1919

20+
/// Empty [CyclomaticComplexityParameters] model.
21+
factory CyclomaticComplexityParameters.empty() =>
22+
CyclomaticComplexityParameters(
23+
maxComplexity: _defaultMaxComplexity,
24+
exclude: ExcludedIdentifiersListParameter(exclude: []),
25+
);
26+
2027
/// Method for creating from json data
2128
factory CyclomaticComplexityParameters.fromJson(Map<String, Object?> json) =>
2229
CyclomaticComplexityParameters(

lib/src/lints/cyclomatic_complexity/visitors/cyclomatic_complexity_flow_visitor.dart

Lines changed: 127 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,6 @@ import 'package:analyzer/dart/ast/visitor.dart';
3131
/// The AST visitor that will collect cyclomatic complexity of visit nodes in an
3232
/// AST structure.
3333
class CyclomaticComplexityFlowVisitor extends RecursiveAstVisitor<void> {
34-
static const _complexityTokenTypes = [
35-
TokenType.AMPERSAND_AMPERSAND,
36-
TokenType.BAR_BAR,
37-
TokenType.QUESTION_PERIOD,
38-
TokenType.QUESTION_QUESTION,
39-
TokenType.QUESTION_QUESTION_EQ,
40-
];
41-
4234
final _complexityEntities = <SyntacticEntity>{};
4335

4436
/// Returns an array of entities that increase cyclomatic complexity.
@@ -52,13 +44,46 @@ class CyclomaticComplexityFlowVisitor extends RecursiveAstVisitor<void> {
5244
}
5345

5446
@override
55-
void visitBlockFunctionBody(BlockFunctionBody node) {
56-
_visitBlock(
57-
node.block.leftBracket.next,
58-
node.block.rightBracket,
59-
);
47+
void visitBinaryExpression(BinaryExpression node) {
48+
final type = node.operator.type;
49+
if (type == TokenType.AMPERSAND_AMPERSAND ||
50+
type == TokenType.BAR_BAR ||
51+
type == TokenType.QUESTION_QUESTION) {
52+
_increaseComplexity(node);
53+
}
54+
super.visitBinaryExpression(node);
55+
}
56+
57+
@override
58+
void visitAssignmentExpression(AssignmentExpression node) {
59+
if (node.operator.type == TokenType.QUESTION_QUESTION_EQ) {
60+
_increaseComplexity(node);
61+
}
62+
super.visitAssignmentExpression(node);
63+
}
64+
65+
@override
66+
void visitPropertyAccess(PropertyAccess node) {
67+
if (node.operator.type == TokenType.QUESTION_PERIOD) {
68+
_increaseComplexity(node);
69+
}
70+
super.visitPropertyAccess(node);
71+
}
6072

61-
super.visitBlockFunctionBody(node);
73+
@override
74+
void visitMethodInvocation(MethodInvocation node) {
75+
if (node.operator?.type == TokenType.QUESTION_PERIOD) {
76+
_increaseComplexity(node);
77+
}
78+
super.visitMethodInvocation(node);
79+
}
80+
81+
@override
82+
void visitIndexExpression(IndexExpression node) {
83+
if (node.question != null) {
84+
_increaseComplexity(node);
85+
}
86+
super.visitIndexExpression(node);
6287
}
6388

6489
@override
@@ -76,13 +101,10 @@ class CyclomaticComplexityFlowVisitor extends RecursiveAstVisitor<void> {
76101
}
77102

78103
@override
79-
void visitExpressionFunctionBody(ExpressionFunctionBody node) {
80-
_visitBlock(
81-
node.expression.beginToken.previous,
82-
node.expression.endToken.next,
83-
);
104+
void visitDoStatement(DoStatement node) {
105+
_increaseComplexity(node);
84106

85-
super.visitExpressionFunctionBody(node);
107+
super.visitDoStatement(node);
86108
}
87109

88110
@override
@@ -113,6 +135,79 @@ class CyclomaticComplexityFlowVisitor extends RecursiveAstVisitor<void> {
113135
super.visitSwitchDefault(node);
114136
}
115137

138+
@override
139+
void visitSwitchExpressionCase(SwitchExpressionCase node) {
140+
_increaseComplexity(node);
141+
142+
super.visitSwitchExpressionCase(node);
143+
}
144+
145+
@override
146+
void visitSwitchPatternCase(SwitchPatternCase node) {
147+
_increaseComplexity(node);
148+
149+
super.visitSwitchPatternCase(node);
150+
}
151+
152+
@override
153+
void visitWhenClause(WhenClause node) {
154+
_increaseComplexity(node);
155+
156+
super.visitWhenClause(node);
157+
}
158+
159+
@override
160+
void visitIfElement(IfElement node) {
161+
_increaseComplexity(node);
162+
super.visitIfElement(node);
163+
}
164+
165+
@override
166+
void visitForElement(ForElement node) {
167+
_increaseComplexity(node);
168+
super.visitForElement(node);
169+
}
170+
171+
@override
172+
void visitLogicalAndPattern(LogicalAndPattern node) {
173+
_increaseComplexity(node);
174+
super.visitLogicalAndPattern(node);
175+
}
176+
177+
@override
178+
void visitLogicalOrPattern(LogicalOrPattern node) {
179+
_increaseComplexity(node);
180+
super.visitLogicalOrPattern(node);
181+
}
182+
183+
@override
184+
void visitNullCheckPattern(NullCheckPattern node) {
185+
_increaseComplexity(node);
186+
super.visitNullCheckPattern(node);
187+
}
188+
189+
@override
190+
void visitNullAssertPattern(NullAssertPattern node) {
191+
_increaseComplexity(node);
192+
super.visitNullAssertPattern(node);
193+
}
194+
195+
@override
196+
void visitCascadeExpression(CascadeExpression node) {
197+
if (node.isNullAware) {
198+
_increaseComplexity(node);
199+
}
200+
super.visitCascadeExpression(node);
201+
}
202+
203+
@override
204+
void visitSpreadElement(SpreadElement node) {
205+
if (node.isNullAware) {
206+
_increaseComplexity(node);
207+
}
208+
super.visitSpreadElement(node);
209+
}
210+
116211
@override
117212
void visitWhileStatement(WhileStatement node) {
118213
_increaseComplexity(node);
@@ -127,15 +222,19 @@ class CyclomaticComplexityFlowVisitor extends RecursiveAstVisitor<void> {
127222
super.visitYieldStatement(node);
128223
}
129224

130-
void _visitBlock(Token? firstToken, Token? lastToken) {
131-
var token = firstToken;
132-
while (token != lastToken && token != null) {
133-
if (token.matchesAny(_complexityTokenTypes)) {
134-
_increaseComplexity(token);
135-
}
225+
@override
226+
void visitFunctionDeclaration(FunctionDeclaration node) {
227+
// Stop recursion into nested function declarations.
228+
}
136229

137-
token = token.next;
138-
}
230+
@override
231+
void visitMethodDeclaration(MethodDeclaration node) {
232+
// Stop recursion into nested method declarations.
233+
}
234+
235+
@override
236+
void visitFunctionExpression(FunctionExpression node) {
237+
// Stop recursion into nested function expressions (closures).
139238
}
140239

141240
void _increaseComplexity(SyntacticEntity entity) {
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import 'package:analyzer/dart/ast/ast.dart';
2+
import 'package:analyzer/dart/ast/visitor.dart';
3+
import 'package:solid_lints/src/lints/cyclomatic_complexity/cyclomatic_complexity_rule.dart';
4+
import 'package:solid_lints/src/lints/cyclomatic_complexity/models/cyclomatic_complexity_parameters.dart';
5+
import 'package:solid_lints/src/lints/cyclomatic_complexity/visitors/cyclomatic_complexity_flow_visitor.dart';
6+
7+
/// Visitor that runs [CyclomaticComplexityFlowVisitor] on function,
8+
/// constructor, and method bodies to count their cyclomatic complexity.
9+
class CyclomaticComplexityVisitor extends SimpleAstVisitor<void> {
10+
final CyclomaticComplexityRule _rule;
11+
final CyclomaticComplexityParameters _parameters;
12+
13+
/// Creates a new instance of [CyclomaticComplexityVisitor].
14+
CyclomaticComplexityVisitor(this._rule, this._parameters);
15+
16+
void _checkBody(Declaration declaration, FunctionBody? body) {
17+
if (body == null) return;
18+
19+
final isIgnored = _parameters.exclude.shouldIgnore(declaration);
20+
if (isIgnored) return;
21+
22+
final visitor = CyclomaticComplexityFlowVisitor();
23+
body.accept(visitor);
24+
25+
if (visitor.complexityEntities.length >= _parameters.maxComplexity) {
26+
_rule.reportAtNode(
27+
body,
28+
arguments: [_parameters.maxComplexity.toString()],
29+
);
30+
}
31+
}
32+
33+
@override
34+
void visitFunctionDeclaration(FunctionDeclaration node) {
35+
_checkBody(node, node.functionExpression.body);
36+
}
37+
38+
@override
39+
void visitConstructorDeclaration(ConstructorDeclaration node) {
40+
_checkBody(node, node.body);
41+
}
42+
43+
@override
44+
void visitMethodDeclaration(MethodDeclaration node) {
45+
_checkBody(node, node.body);
46+
}
47+
}

0 commit comments

Comments
 (0)