Skip to content

Commit f1c4687

Browse files
authored
added ConditionalCompilationCheck (#305)
1 parent 78b2c8d commit f1c4687

File tree

5 files changed

+317
-2
lines changed

5 files changed

+317
-2
lines changed

checkstyle.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
},
2020
"type": "CatchParameterName"
2121
},
22+
{
23+
"type": "ConditionalCompilation"
24+
},
2225
{
2326
"props": {
2427
"ignoreExtern": true,

resources/default-config.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@
3333
},
3434
"type": "CatchParameterName"
3535
},
36+
{
37+
"props": {
38+
"policy": "aligned",
39+
"allowSingleline": true
40+
},
41+
"type": "ConditionalCompilation"
42+
},
3643
{
3744
"props": {
3845
"ignoreExtern": true,
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
package checkstyle.checks.block;
2+
3+
import checkstyle.token.TokenTree;
4+
import checkstyle.Checker.LinePos;
5+
6+
@name("ConditionalCompilation")
7+
@desc("Checks placement and indentation of conditional compilation flags.")
8+
class ConditionalCompilationCheck extends Check {
9+
10+
public var policy:ConditionalCompilationPolicy;
11+
public var allowSingleline:Bool;
12+
13+
public function new() {
14+
super(TOKEN);
15+
policy = ALIGNED;
16+
allowSingleline = true;
17+
}
18+
19+
override function actualRun() {
20+
var root:TokenTree = checker.getTokenTree();
21+
checkSharpIf(root.filter([Sharp("if")], ALL));
22+
}
23+
24+
function checkSharpIf(tokens:Array<TokenTree>) {
25+
for (tok in tokens) {
26+
if (isPosSuppressed(tok.pos)) continue;
27+
var linePos:LinePos = checker.getLinePos(tok.pos.min);
28+
if (checkSingleLine(tok, linePos.line)) continue;
29+
checkMultiLine(tok, linePos);
30+
}
31+
}
32+
33+
function checkSingleLine(tok:TokenTree, line:Int):Bool {
34+
var endTok:TokenTree = null;
35+
36+
for (child in tok.childs) {
37+
switch (child.tok) {
38+
case Sharp("end"):
39+
endTok = child;
40+
break;
41+
default:
42+
}
43+
}
44+
if (endTok == null) return true;
45+
46+
var endPos:LinePos = checker.getLinePos(endTok.pos.min);
47+
48+
var singleLine:Bool = (endPos.line == line);
49+
if (!singleLine) return false;
50+
if (singleLine && allowSingleline) return true;
51+
52+
logPos("Single line #if…(#else/#elseif)…#end not allowed", tok.pos);
53+
return true;
54+
}
55+
56+
function checkMultiLine(tok:TokenTree, linePos:LinePos) {
57+
var line:String = checker.lines[linePos.line];
58+
var prefix:String = line.substr(0, linePos.ofs);
59+
if (checkLine(tok, linePos, line)) return;
60+
61+
switch (policy) {
62+
case START_OF_LINE:
63+
if (linePos.ofs != 0) {
64+
logPos("#if should start at beginning of line", tok.pos);
65+
return;
66+
}
67+
case ALIGNED:
68+
if (checkIndentation(tok, linePos)) return;
69+
}
70+
for (childTok in tok.childs) {
71+
switch (childTok.tok) {
72+
case Sharp("else"), Sharp("elseif"), Sharp("end"):
73+
var childLinePos:LinePos = checker.getLinePos(childTok.pos.min);
74+
var childLine:String = checker.lines[childLinePos.line];
75+
var childPrefix:String = childLine.substr(0, childLinePos.ofs);
76+
if (checkLine(childTok, childLinePos, childLine)) continue;
77+
if (childPrefix == prefix) continue;
78+
logPos('Indentation of $childTok must match corresponding #if', childTok.pos);
79+
default:
80+
}
81+
}
82+
}
83+
84+
function checkLine(tok:TokenTree, linePos:LinePos, line:String):Bool {
85+
var r:EReg = ~/^[ \t]*$/;
86+
var prefix:String = line.substr(0, linePos.ofs);
87+
if (!r.match(prefix)) {
88+
logPos('only whitespace allowed before $tok', tok.pos);
89+
return true;
90+
}
91+
var expr:TokenTree = tok.getFirstChild();
92+
if (expr == null) return false;
93+
var linePosAfter:LinePos = checker.getLinePos(expr.getPos().max);
94+
if (linePosAfter.line == linePos.line) {
95+
var postfix:String = line.substr(linePosAfter.ofs);
96+
if (!r.match(postfix)) {
97+
logPos('only whitespace allowed after $tok', tok.pos);
98+
return true;
99+
}
100+
}
101+
return false;
102+
}
103+
104+
function checkIndentation(tok:TokenTree, linePos:LinePos):Bool {
105+
var prevLen:Int = -1;
106+
var nextLen:Int = -1;
107+
108+
var lineIndex:Int = linePos.line - 1;
109+
while (lineIndex >= 0) {
110+
var line:String = checker.lines[lineIndex];
111+
prevLen = getIndentLength(line);
112+
if (prevLen >= 0) break;
113+
lineIndex--;
114+
}
115+
var lineIndex:Int = linePos.line + 1;
116+
while (lineIndex < checker.lines.length - 1) {
117+
var line:String = checker.lines[lineIndex];
118+
nextLen = getIndentLength(line);
119+
if (nextLen >= 0) break;
120+
lineIndex++;
121+
}
122+
if (prevLen < 0) prevLen = linePos.ofs;
123+
if (nextLen < 0) nextLen = linePos.ofs;
124+
if ((linePos.ofs >= prevLen) && (linePos.ofs <= nextLen)) return false;
125+
126+
logPos('Indentation of $tok should match surrounding lines', tok.pos);
127+
return true;
128+
}
129+
130+
function getIndentLength(line:String):Int {
131+
if (~/^[ \t]*$/.match(line)) {
132+
return -1;
133+
}
134+
var r:EReg = ~/^([ \t]*)/;
135+
if (r.match(line)) {
136+
var prefix:String = r.matched(1);
137+
return prefix.length;
138+
}
139+
return -1;
140+
}
141+
}
142+
143+
@:enum
144+
abstract ConditionalCompilationPolicy(String) {
145+
var START_OF_LINE = "startOfLine";
146+
var ALIGNED = "aligned";
147+
}

src/checkstyle/token/TokenStream.hx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ class TokenStream {
2727
if ((current < 0) || (current >= tokens.length)) throw NO_MORE_TOKENS;
2828
var token:Token = tokens[current];
2929
current++;
30-
#if debugTokenTree
30+
#if debugTokenTree
3131
Sys.println (token);
32-
#end
32+
#end
3333
return new TokenTree(token.tok, token.pos, current - 1);
3434
}
3535

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
package checks.block;
2+
3+
import checkstyle.checks.block.ConditionalCompilationCheck;
4+
5+
class ConditionalCompilationTest extends CheckTestCase<ConditionalCompilationTests> {
6+
7+
static inline var MSG_START_OF_LINE:String = "#if should start at beginning of line";
8+
static inline var MSG_NOT_ALIGNED:String = "Indentation of #if should match surrounding lines";
9+
static inline var MSG_NO_SINGLELINE:String = "Single line #if…(#else/#elseif)…#end not allowed";
10+
static inline var MSG_WRONG_INDENT:String = "Indentation of #else must match corresponding #if";
11+
static inline var MSG_WHITESPACE_BEFORE:String = "only whitespace allowed before #if";
12+
static inline var MSG_WHITESPACE_AFTER:String = "only whitespace allowed after #if";
13+
14+
public function testAlignedWithSingleline() {
15+
var check:ConditionalCompilationCheck = new ConditionalCompilationCheck();
16+
17+
assertNoMsg(check, ISSUE_76);
18+
assertNoMsg(check, ISSUE_79);
19+
assertNoMsg(check, ISSUE_252);
20+
21+
assertMsg(check, ISSUE_76_START_OF_LINE, MSG_NOT_ALIGNED);
22+
assertMsg(check, ISSUE_79_START_OF_LINE, MSG_NOT_ALIGNED);
23+
assertMsg(check, ISSUE_79_WRONG_INDENT, MSG_WRONG_INDENT);
24+
}
25+
26+
public function testAlignedWithNoSingleline() {
27+
var check:ConditionalCompilationCheck = new ConditionalCompilationCheck();
28+
check.allowSingleline = false;
29+
30+
assertNoMsg(check, ISSUE_76);
31+
assertNoMsg(check, ISSUE_79);
32+
assertMsg(check, ISSUE_252, MSG_NO_SINGLELINE);
33+
34+
assertMsg(check, ISSUE_76_START_OF_LINE, MSG_NOT_ALIGNED);
35+
assertMsg(check, ISSUE_79_START_OF_LINE, MSG_NOT_ALIGNED);
36+
assertMsg(check, ISSUE_79_WRONG_INDENT, MSG_WRONG_INDENT);
37+
}
38+
39+
public function testStartOfLine() {
40+
var check:ConditionalCompilationCheck = new ConditionalCompilationCheck();
41+
check.allowSingleline = false;
42+
check.policy = START_OF_LINE;
43+
44+
assertNoMsg(check, ISSUE_76_START_OF_LINE);
45+
assertNoMsg(check, ISSUE_79_START_OF_LINE);
46+
47+
assertMsg(check, ISSUE_76, MSG_START_OF_LINE);
48+
assertMsg(check, ISSUE_79, MSG_START_OF_LINE);
49+
assertMsg(check, ISSUE_252, MSG_NO_SINGLELINE);
50+
}
51+
52+
public function testWhitespace() {
53+
var check:ConditionalCompilationCheck = new ConditionalCompilationCheck();
54+
55+
assertMsg(check, ISSUE_79_WHITESPACE_BEFORE, MSG_WHITESPACE_BEFORE);
56+
assertMsg(check, ISSUE_79_WHITESPACE_AFTER, MSG_WHITESPACE_AFTER);
57+
58+
check.policy = START_OF_LINE;
59+
60+
assertMsg(check, ISSUE_79_WHITESPACE_BEFORE, MSG_WHITESPACE_BEFORE);
61+
assertMsg(check, ISSUE_79_WHITESPACE_AFTER, MSG_WHITESPACE_AFTER);
62+
}
63+
}
64+
65+
@:enum
66+
abstract ConditionalCompilationTests(String) to String {
67+
68+
var ISSUE_76 = "
69+
class Base {}
70+
71+
#if true
72+
73+
class Test extends Base
74+
#else
75+
class Test
76+
#end
77+
{
78+
}";
79+
80+
var ISSUE_79 = "
81+
class Test {
82+
function foo() {
83+
84+
#if true
85+
if (true) {
86+
#else
87+
if (true) {
88+
#end
89+
90+
}
91+
}
92+
}";
93+
94+
var ISSUE_79_WRONG_INDENT = "
95+
class Test {
96+
function foo() {
97+
#if true
98+
if (true) {
99+
#else
100+
if (true) {
101+
#end
102+
103+
}
104+
}
105+
}";
106+
107+
var ISSUE_79_WHITESPACE_BEFORE = "
108+
class Test {
109+
function foo() { #if true
110+
if (true) { #else
111+
if (true) {
112+
#end
113+
114+
}
115+
}
116+
}";
117+
118+
var ISSUE_79_WHITESPACE_AFTER = "
119+
class Test {
120+
function foo() {
121+
#if true if (true) {
122+
#else if (true) {
123+
#end
124+
125+
}
126+
}
127+
}";
128+
129+
var ISSUE_76_START_OF_LINE = "
130+
class Base {}
131+
132+
#if true
133+
134+
class Test extends Base
135+
#else
136+
class Test
137+
#end
138+
{
139+
}";
140+
141+
var ISSUE_79_START_OF_LINE = "
142+
class Test {
143+
function foo() {
144+
#if true
145+
if (true) {
146+
#else
147+
if (true) {
148+
#end
149+
150+
}
151+
}
152+
}";
153+
154+
var ISSUE_252 = "
155+
class Foo {
156+
var library = new #if haxe3 Map<String, #else Hash <#end String>();
157+
}";
158+
}

0 commit comments

Comments
 (0)