Skip to content

Commit cdf881c

Browse files
WebFreak001WebFreak001
committed
add cyclomatic complexity check
Fix #684 Disabled by default.
1 parent 93f614a commit cdf881c

File tree

5 files changed

+278
-0
lines changed

5 files changed

+278
-0
lines changed

‎.dscanner.ini‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,7 @@ allman_braces_check="disabled"
9393
redundant_attributes_check="enabled"
9494
; Check for unused function return values
9595
unused_result="enabled"
96+
; Enable cyclomatic complexity check
97+
cyclomatic_complexity="disabled"
98+
; Maximum cyclomatic complexity after which to issue warnings
99+
max_cyclomatic_complexity="50"

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ Note that the "--skipTests" option is the equivalent of changing each
163163
* Indentation of if constraints
164164
* Check that `@trusted` is not applied to a whole scope. Trusting a whole scope can be a problem when new declarations are added and if they are not verified manually to be trustable.
165165
* Redundant storage class attributes
166+
* Cyclomatic complexity threshold per function and unittest (starts at 1, increased by 1 at each `if`, switch `case`, loop, `&&`, `||`, `?:`, `throw`, `catch`, `return`, `break`, `continue`, `goto` and function literal)
166167

167168
#### Wishlist
168169

‎src/dscanner/analysis/config.d‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,12 @@ struct StaticAnalysisConfig
209209
@INI("Check for unused function return values")
210210
string unused_result = Check.enabled;
211211

212+
@INI("Enable cyclomatic complexity check")
213+
string cyclomatic_complexity = Check.disabled;
214+
215+
@INI("Maximum cyclomatic complexity after which to issue warnings")
216+
int max_cyclomatic_complexity = 50;
217+
212218
@INI("Module-specific filters")
213219
ModuleFilters filters;
214220
}

‎src/dscanner/analysis/cyclomatic_complexity.d‎

Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
// Distributed under the Boost Software License, Version 1.0.
2+
// (See accompanying file LICENSE_1_0.txt or copy at
3+
// http://www.boost.org/LICENSE_1_0.txt)
4+
5+
module dscanner.analysis.cyclomatic_complexity;
6+
7+
import dparse.ast;
8+
import dparse.lexer;
9+
import dsymbol.scope_ : Scope;
10+
import dscanner.analysis.base;
11+
import dscanner.analysis.helpers;
12+
13+
import std.format;
14+
15+
/// Implements a basic cyclomatic complexity algorithm using the AST.
16+
///
17+
/// Issues a warning on functions whenever the cyclomatic complexity of them
18+
/// passed over a configurable threshold.
19+
///
20+
/// The complexity score starts at 1 and is increased each time on
21+
/// - `if`
22+
/// - switch `case`
23+
/// - any loop
24+
/// - `&&`
25+
/// - `||`
26+
/// - `?:` (ternary operator)
27+
/// - `throw`
28+
/// - `catch`
29+
/// - `return`
30+
/// - `break` (unless in case)
31+
/// - `continue`
32+
/// - `goto`
33+
/// - function literals
34+
///
35+
/// See: https://en.wikipedia.org/wiki/Cyclomatic_complexity
36+
/// Rules based on http://cyvis.sourceforge.net/cyclomatic_complexity.html
37+
/// and https://github.com/fzipp/gocyclo
38+
final class CyclomaticComplexityCheck : BaseAnalyzer
39+
{
40+
/// Message key emitted when the threshold is reached
41+
enum string KEY = "dscanner.metric.cyclomatic_complexity";
42+
/// Human readable message emitted when the threshold is reached
43+
enum string MESSAGE = "Cyclomatic complexity of this function is %s.";
44+
mixin AnalyzerInfo!"cyclomatic_complexity";
45+
46+
/// Maximum cyclomatic complexity. Once the cyclomatic complexity is greater
47+
/// than this threshold, a warning is issued.
48+
///
49+
/// By default 50 is used as threshold, which is considered almost
50+
/// unmaintainable / untestable.
51+
///
52+
/// For clean development a threshold like 20 can be used instead.
53+
int maxCyclomaticComplexity;
54+
55+
///
56+
this(string fileName, const(Scope)* sc, bool skipTests = false,
57+
int maxCyclomaticComplexity = 50)
58+
{
59+
super(fileName, sc, skipTests);
60+
this.maxCyclomaticComplexity = maxCyclomaticComplexity;
61+
}
62+
63+
mixin VisitComplex!IfStatement;
64+
mixin VisitComplex!CaseStatement;
65+
mixin VisitComplex!CaseRangeStatement;
66+
mixin VisitLoop!DoStatement;
67+
mixin VisitLoop!WhileStatement;
68+
mixin VisitLoop!ForStatement;
69+
mixin VisitLoop!ForeachStatement;
70+
mixin VisitComplex!AndAndExpression;
71+
mixin VisitComplex!OrOrExpression;
72+
mixin VisitComplex!TernaryExpression;
73+
mixin VisitComplex!ThrowStatement;
74+
mixin VisitComplex!Catch;
75+
mixin VisitComplex!LastCatch;
76+
mixin VisitComplex!ReturnStatement;
77+
mixin VisitComplex!FunctionLiteralExpression;
78+
mixin VisitComplex!GotoStatement;
79+
mixin VisitComplex!ContinueStatement;
80+
81+
override void visit(const SwitchStatement n)
82+
{
83+
inLoop.assumeSafeAppend ~= false;
84+
scope (exit)
85+
inLoop.length--;
86+
n.accept(this);
87+
}
88+
89+
override void visit(const BreakStatement b)
90+
{
91+
if (b.label !is Token.init || inLoop[$ - 1])
92+
complexityStack[$ - 1]++;
93+
}
94+
95+
override void visit(const FunctionDeclaration fun)
96+
{
97+
if (!fun.functionBody)
98+
return;
99+
100+
complexityStack.assumeSafeAppend ~= 1;
101+
inLoop.assumeSafeAppend ~= false;
102+
scope (exit)
103+
{
104+
complexityStack.length--;
105+
inLoop.length--;
106+
}
107+
fun.functionBody.accept(this);
108+
testComplexity(fun.name.line, fun.name.column);
109+
}
110+
111+
override void visit(const Unittest unittest_)
112+
{
113+
if (!skipTests)
114+
{
115+
complexityStack.assumeSafeAppend ~= 1;
116+
inLoop.assumeSafeAppend ~= false;
117+
scope (exit)
118+
{
119+
complexityStack.length--;
120+
inLoop.length--;
121+
}
122+
unittest_.accept(this);
123+
testComplexity(unittest_.line, unittest_.column);
124+
}
125+
}
126+
127+
alias visit = BaseAnalyzer.visit;
128+
private:
129+
int[] complexityStack = [0];
130+
bool[] inLoop = [false];
131+
132+
void testComplexity(size_t line, size_t column)
133+
{
134+
auto complexity = complexityStack[$ - 1];
135+
if (complexity > maxCyclomaticComplexity)
136+
{
137+
addErrorMessage(line, column, KEY, format!MESSAGE(complexity));
138+
}
139+
}
140+
141+
template VisitComplex(NodeType, int increase = 1)
142+
{
143+
override void visit(const NodeType n)
144+
{
145+
complexityStack[$ - 1] += increase;
146+
n.accept(this);
147+
}
148+
}
149+
150+
template VisitLoop(NodeType, int increase = 1)
151+
{
152+
override void visit(const NodeType n)
153+
{
154+
inLoop.assumeSafeAppend ~= true;
155+
scope (exit)
156+
inLoop.length--;
157+
complexityStack[$ - 1] += increase;
158+
n.accept(this);
159+
}
160+
}
161+
}
162+
163+
unittest
164+
{
165+
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
166+
import std.stdio : stderr;
167+
168+
StaticAnalysisConfig sac = disabledConfig();
169+
sac.cyclomatic_complexity = Check.enabled;
170+
sac.max_cyclomatic_complexity = 0;
171+
assertAnalyzerWarnings(q{
172+
unittest // [warn]: Cyclomatic complexity of this function is 1.
173+
{
174+
}
175+
176+
unittest // [warn]: Cyclomatic complexity of this function is 1.
177+
{
178+
writeln("hello");
179+
writeln("world");
180+
}
181+
182+
void main(string[] args) // [warn]: Cyclomatic complexity of this function is 3.
183+
{
184+
if (!args.length)
185+
return;
186+
writeln("hello ", args);
187+
}
188+
189+
unittest // [warn]: Cyclomatic complexity of this function is 1.
190+
{
191+
// static if / static foreach does not increase cyclomatic complexity
192+
static if (stuff)
193+
int a;
194+
int a;
195+
}
196+
197+
unittest // [warn]: Cyclomatic complexity of this function is 2.
198+
{
199+
foreach (i; 0 .. 2)
200+
{
201+
}
202+
int a;
203+
}
204+
205+
unittest // [warn]: Cyclomatic complexity of this function is 3.
206+
{
207+
foreach (i; 0 .. 2)
208+
{
209+
break;
210+
}
211+
int a;
212+
}
213+
214+
unittest // [warn]: Cyclomatic complexity of this function is 2.
215+
{
216+
switch (x)
217+
{
218+
case 1:
219+
break;
220+
default:
221+
break;
222+
}
223+
int a;
224+
}
225+
226+
bool shouldRun(check : BaseAnalyzer)( // [warn]: Cyclomatic complexity of this function is 20.
227+
string moduleName, const ref StaticAnalysisConfig config)
228+
{
229+
enum string a = check.name;
230+
231+
if (mixin("config." ~ a) == Check.disabled)
232+
return false;
233+
234+
// By default, run the check
235+
if (!moduleName.length)
236+
return true;
237+
238+
auto filters = mixin("config.filters." ~ a);
239+
240+
// Check if there are filters are defined
241+
// filters starting with a comma are invalid
242+
if (filters.length == 0 || filters[0].length == 0)
243+
return true;
244+
245+
auto includers = filters.filter!(f => f[0] == '+').map!(f => f[1..$]);
246+
auto excluders = filters.filter!(f => f[0] == '-').map!(f => f[1..$]);
247+
248+
// exclusion has preference over inclusion
249+
if (!excluders.empty && excluders.any!(s => moduleName.canFind(s)))
250+
return false;
251+
252+
if (!includers.empty)
253+
return includers.any!(s => moduleName.canFind(s));
254+
255+
// by default: include all modules
256+
return true;
257+
}
258+
259+
}c, sac);
260+
stderr.writeln("Unittest for CyclomaticComplexityCheck passed.");
261+
}

‎src/dscanner/analysis/run.d‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ import dscanner.analysis.if_constraints_indent;
8080
import dscanner.analysis.trust_too_much;
8181
import dscanner.analysis.redundant_storage_class;
8282
import dscanner.analysis.unused_result;
83+
import dscanner.analysis.cyclomatic_complexity;
8384

8485
import dsymbol.string_interning : internString;
8586
import dsymbol.scope_;
@@ -589,6 +590,11 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
589590
checks ~= new UnusedResultChecker(fileName, moduleScope,
590591
analysisConfig.unused_result == Check.skipTests && !ut);
591592

593+
if (moduleName.shouldRun!CyclomaticComplexityCheck(analysisConfig))
594+
checks ~= new CyclomaticComplexityCheck(fileName, moduleScope,
595+
analysisConfig.cyclomatic_complexity == Check.skipTests && !ut,
596+
analysisConfig.max_cyclomatic_complexity.to!int);
597+
592598
version (none)
593599
if (moduleName.shouldRun!IfStatementCheck(analysisConfig))
594600
checks ~= new IfStatementCheck(fileName, moduleScope,

0 commit comments

Comments
 (0)