Skip to content

Commit 9fa8756

Browse files
dlang-botdlang-bot
authored
Merge pull request #450 from wilzbach/if_constraints_indent
Add check for if constraint indendation merged-on-behalf-of: BBasile <BBasile@users.noreply.github.com>
2 parents 27b09ee + e807bd7 commit 9fa8756

File tree

4 files changed

+242
-0
lines changed

4 files changed

+242
-0
lines changed

‎README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ Note that the "--skipTests" option is the equivalent of changing each
140140
* Redundant visibility attributes
141141
* Public declarations without a documented unittest. By default disabled.
142142
* Asserts without an explanatory message. By default disabled.
143+
* Indentation of if constraints
143144

144145
#### Wishlist
145146

‎src/dscanner/analysis/config.d

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,9 @@ struct StaticAnalysisConfig
192192
@INI("Check for asserts without an explanatory message")
193193
string assert_without_msg = Check.disabled;
194194

195+
@INI("Check indent of if constraints")
196+
string if_constraints_indent = Check.disabled;
197+
195198
@INI("Module-specific filters")
196199
ModuleFilters filters;
197200
}

‎src/dscanner/analysis/if_constraints_indent.d

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
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.if_constraints_indent;
6+
7+
import dparse.lexer;
8+
import dparse.ast;
9+
import dscanner.analysis.base : BaseAnalyzer, Message;
10+
import dsymbol.scope_ : Scope;
11+
12+
import std.algorithm.iteration : filter;
13+
import std.range;
14+
15+
/**
16+
Checks whether all if constraints have the same indention as their declaration.
17+
*/
18+
class IfConstraintsIndentCheck : BaseAnalyzer
19+
{
20+
///
21+
this(string fileName, const(Token)[] tokens, bool skipTests = false)
22+
{
23+
super(fileName, null, skipTests);
24+
25+
// libdparse columns start at 1
26+
foreach (t; tokens)
27+
{
28+
while (firstSymbolAtLine.length < t.line - 1)
29+
firstSymbolAtLine ~= Pos(1);
30+
31+
if (firstSymbolAtLine.length < t.line)
32+
firstSymbolAtLine ~= Pos(t.column, t.type == tok!"if");
33+
}
34+
}
35+
36+
override void visit(const FunctionDeclaration decl)
37+
{
38+
if (decl.constraint !is null)
39+
checkConstraintSpace(decl.constraint, decl.name);
40+
}
41+
42+
override void visit(const InterfaceDeclaration decl)
43+
{
44+
if (decl.constraint !is null)
45+
checkConstraintSpace(decl.constraint, decl.name);
46+
}
47+
48+
49+
override void visit(const ClassDeclaration decl)
50+
{
51+
if (decl.constraint !is null)
52+
checkConstraintSpace(decl.constraint, decl.name);
53+
}
54+
55+
override void visit(const TemplateDeclaration decl)
56+
{
57+
if (decl.constraint !is null)
58+
checkConstraintSpace(decl.constraint, decl.name);
59+
}
60+
61+
override void visit(const UnionDeclaration decl)
62+
{
63+
if (decl.constraint !is null)
64+
checkConstraintSpace(decl.constraint, decl.name);
65+
}
66+
67+
override void visit(const StructDeclaration decl)
68+
{
69+
if (decl.constraint !is null)
70+
checkConstraintSpace(decl.constraint, decl.name);
71+
}
72+
73+
override void visit(const Constructor decl)
74+
{
75+
if (decl.constraint !is null)
76+
checkConstraintSpace(decl.constraint, decl.line);
77+
}
78+
79+
alias visit = ASTVisitor.visit;
80+
81+
private:
82+
83+
enum string KEY = "dscanner.style.if_constraints_indent";
84+
enum string MESSAGE = "If constraints should have the same indentation as the function";
85+
86+
Pos[] firstSymbolAtLine;
87+
static struct Pos
88+
{
89+
size_t column;
90+
bool isIf;
91+
}
92+
93+
/**
94+
Check indentation of constraints
95+
*/
96+
void checkConstraintSpace(const Constraint constraint, const Token token)
97+
{
98+
checkConstraintSpace(constraint, token.line);
99+
}
100+
101+
void checkConstraintSpace(const Constraint constraint, size_t line)
102+
{
103+
// dscanner lines start at 1
104+
auto pDecl = firstSymbolAtLine[line - 1];
105+
106+
// search for constraint if (might not be on the same line as the expression)
107+
auto r = firstSymbolAtLine[line .. constraint.expression.line].retro.filter!(s => s.isIf);
108+
109+
// no hit = constraint is on the same line
110+
if (r.empty)
111+
addErrorMessage(constraint.expression.line, constraint.expression.column, KEY, MESSAGE);
112+
else if (pDecl.column != r.front.column)
113+
addErrorMessage(constraint.expression.line, constraint.expression.column, KEY, MESSAGE);
114+
}
115+
}
116+
117+
unittest
118+
{
119+
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
120+
import dscanner.analysis.helpers : assertAnalyzerWarnings;
121+
import std.format : format;
122+
import std.stdio : stderr;
123+
124+
StaticAnalysisConfig sac = disabledConfig();
125+
sac.if_constraints_indent = Check.enabled;
126+
127+
assertAnalyzerWarnings(q{
128+
void foo(R)(R r)
129+
if (R == int)
130+
{}
131+
132+
void foo(R)(R r)
133+
if (R == int) // [warn]: %s
134+
{}
135+
}c.format(
136+
IfConstraintsIndentCheck.MESSAGE,
137+
), sac);
138+
139+
assertAnalyzerWarnings(q{
140+
void foo(R)(R r)
141+
if (R == int)
142+
{}
143+
144+
void foo(R)(R r)
145+
if (R == int) // [warn]: %s
146+
{}
147+
148+
void foo(R)(R r)
149+
if (R == int) // [warn]: %s
150+
{}
151+
}c.format(
152+
IfConstraintsIndentCheck.MESSAGE,
153+
IfConstraintsIndentCheck.MESSAGE,
154+
), sac);
155+
156+
assertAnalyzerWarnings(q{
157+
struct Foo(R)
158+
if (R == int)
159+
{}
160+
161+
struct Foo(R)
162+
if (R == int) // [warn]: %s
163+
{}
164+
165+
struct Foo(R)
166+
if (R == int) // [warn]: %s
167+
{}
168+
}c.format(
169+
IfConstraintsIndentCheck.MESSAGE,
170+
IfConstraintsIndentCheck.MESSAGE,
171+
), sac);
172+
173+
// test example from Phobos
174+
assertAnalyzerWarnings(q{
175+
Num abs(Num)(Num x) @safe pure nothrow
176+
if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) &&
177+
!(is(Num* : const(ifloat*)) || is(Num* : const(idouble*))
178+
|| is(Num* : const(ireal*))))
179+
{
180+
static if (isFloatingPoint!(Num))
181+
return fabs(x);
182+
else
183+
return x >= 0 ? x : -x;
184+
}
185+
}, sac);
186+
187+
// weird constraint formatting
188+
assertAnalyzerWarnings(q{
189+
struct Foo(R)
190+
if
191+
(R == int)
192+
{}
193+
194+
struct Foo(R)
195+
if
196+
(R == int)
197+
{}
198+
199+
struct Foo(R)
200+
if
201+
(R == int) // [warn]: %s
202+
{}
203+
204+
struct Foo(R)
205+
if (
206+
R == int)
207+
{}
208+
209+
struct Foo(R)
210+
if (
211+
R == int
212+
)
213+
{}
214+
215+
struct Foo(R)
216+
if (
217+
R == int // [warn]: %s
218+
) {}
219+
}c.format(
220+
IfConstraintsIndentCheck.MESSAGE,
221+
IfConstraintsIndentCheck.MESSAGE,
222+
), sac);
223+
224+
// constraint on the same line
225+
assertAnalyzerWarnings(q{
226+
struct CRC(uint N, ulong P) if (N == 32 || N == 64) // [warn]: %s
227+
{}
228+
}c.format(
229+
IfConstraintsIndentCheck.MESSAGE,
230+
), sac);
231+
232+
stderr.writeln("Unittest for IfConstraintsIndentCheck passed.");
233+
}

‎src/dscanner/analysis/run.d

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ import dscanner.analysis.allman;
7171
import dscanner.analysis.redundant_attributes;
7272
import dscanner.analysis.has_public_example;
7373
import dscanner.analysis.assert_without_msg;
74+
import dscanner.analysis.if_constraints_indent;
7475

7576
import dsymbol.string_interning : internString;
7677
import dsymbol.scope_;
@@ -506,6 +507,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
506507
checks ~= new AssertWithoutMessageCheck(fileName, moduleScope,
507508
analysisConfig.assert_without_msg == Check.skipTests && !ut);
508509

510+
if (moduleName.shouldRun!"if_constraints_indent"(analysisConfig))
511+
checks ~= new IfConstraintsIndentCheck(fileName, tokens,
512+
analysisConfig.if_constraints_indent == Check.skipTests && !ut);
513+
509514
version (none)
510515
if (moduleName.shouldRun!"redundant_if_check"(analysisConfig))
511516
checks ~= new IfStatementCheck(fileName, moduleScope,

0 commit comments

Comments
 (0)