Skip to content

Commit 1f2268c

Browse files
committed
Merge pull request #23 from AlexHaxe/dev
added NeedBracesCheck
2 parents b9b8bdf + 3e87f4c commit 1f2268c

File tree

10 files changed

+455
-33
lines changed

10 files changed

+455
-33
lines changed

README.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,14 @@ More information in [wiki page](https://github.com/adireddy/haxe-checkstyle/wiki
175175
"tokens": []
176176
}
177177
},
178+
{
179+
"type": "NeedBraces",
180+
"props": {
181+
"severity": "WARNING",
182+
"allowSingleLineStatement": true,
183+
"tokens": []
184+
}
185+
},
178186
{
179187
"type": "NestedForDepth",
180188
"props": {

checkstyle/Checker.hx

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -121,15 +121,17 @@ class Checker {
121121
makeAST();
122122
}
123123
catch (e:Dynamic) {
124-
for (reporter in reporters) reporter.addMessage({
125-
fileName:file.name,
126-
message: "Parsing failed: " + e + "\nStacktrace: " +
127-
CallStack.toString(CallStack.exceptionStack()),
128-
line:1,
129-
column:1,
130-
severity:ERROR,
131-
moduleName:"Checker"
132-
});
124+
for (reporter in reporters) {
125+
reporter.addMessage({
126+
fileName:file.name,
127+
message: "Parsing failed: " + e + "\nStacktrace: " +
128+
CallStack.toString(CallStack.exceptionStack()),
129+
line:1,
130+
column:1,
131+
severity:ERROR,
132+
moduleName:"Checker"
133+
});
134+
}
133135
for (reporter in reporters) reporter.fileFinish(file);
134136
return;
135137
}
@@ -141,15 +143,17 @@ class Checker {
141143
for (reporter in reporters) for (m in messages) reporter.addMessage(m);
142144
}
143145
catch (e:Dynamic) {
144-
for (reporter in reporters) reporter.addMessage({
145-
fileName:file.name,
146-
message:"Check " + check.getModuleName() + " failed: " +
147-
e + "\nStacktrace: " + CallStack.toString(CallStack.exceptionStack()),
148-
line:1,
149-
column:1,
150-
severity:ERROR,
151-
moduleName:"Checker"
152-
});
146+
for (reporter in reporters) {
147+
reporter.addMessage({
148+
fileName:file.name,
149+
message:"Check " + check.getModuleName() + " failed: " +
150+
e + "\nStacktrace: " + CallStack.toString(CallStack.exceptionStack()),
151+
line:1,
152+
column:1,
153+
severity:ERROR,
154+
moduleName:"Checker"
155+
});
156+
}
153157
}
154158
}
155159

checkstyle/ComplexTypeUtils.hx

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,11 @@ class ComplexTypeUtils {
4242

4343
public static function walkClass(d:Definition<ClassFlag, Array<Field>>, pos:Position, cb:ComplexTypeCallback) {
4444
walkCommonDefinition(d, pos, cb);
45-
for (f in d.flags) switch f {
46-
case HExtends(t) | HImplements(t): walkTypePath(t, d.name, pos, cb);
47-
default:
45+
for (f in d.flags) {
46+
switch f {
47+
case HExtends(t) | HImplements(t): walkTypePath(t, d.name, pos, cb);
48+
default:
49+
}
4850
}
4951
for (f in d.data) walkField(f, cb);
5052
}
@@ -61,9 +63,11 @@ class ComplexTypeUtils {
6163

6264
public static function walkAbstract(d:Definition<AbstractFlag, Array<Field>>, pos:Position, cb:ComplexTypeCallback) {
6365
walkCommonDefinition(d, pos, cb);
64-
for (f in d.flags) switch f {
65-
case AFromType(ct) | AToType(ct) | AIsType(ct): walkComplexType(ct, f.getName(), pos, cb);
66-
default:
66+
for (f in d.flags) {
67+
switch f {
68+
case AFromType(ct) | AToType(ct) | AIsType(ct): walkComplexType(ct, f.getName(), pos, cb);
69+
default:
70+
}
6771
}
6872
for (f in d.data) walkField(f, cb);
6973
}

checkstyle/ExprUtils.hx

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,11 @@ class ExprUtils {
4242

4343
public static function walkClass(d:Definition<ClassFlag, Array<Field>>, cb:Expr -> Void) {
4444
walkCommonDefinition(d, cb);
45-
for (f in d.flags) switch f {
46-
case HExtends(t) | HImplements(t): walkTypePath(t,cb);
47-
default:
45+
for (f in d.flags) {
46+
switch f {
47+
case HExtends(t) | HImplements(t): walkTypePath(t,cb);
48+
default:
49+
}
4850
}
4951
for (f in d.data) walkField(f,cb);
5052
}
@@ -61,9 +63,11 @@ class ExprUtils {
6163

6264
public static function walkAbstract(d:Definition<AbstractFlag, Array<Field>>, cb:Expr -> Void) {
6365
walkCommonDefinition(d, cb);
64-
for (f in d.flags) switch f {
65-
case AFromType(ct) | AToType(ct) | AIsType(ct): walkComplexType(ct,cb);
66-
default:
66+
for (f in d.flags) {
67+
switch f {
68+
case AFromType(ct) | AToType(ct) | AIsType(ct): walkComplexType(ct,cb);
69+
default:
70+
}
6771
}
6872
for (f in d.data) walkField(f,cb);
6973
}

checkstyle/checks/BlockFormatCheck.hx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ class BlockFormatCheck extends Check {
2222
ExprUtils.walkFile(checker.ast, function(e) {
2323
switch(e.expr){
2424
case EBlock([]) | EObjectDecl([]):
25-
if (emptyBlockCheck && e.pos.max - e.pos.min > "{}".length)
25+
if (emptyBlockCheck && e.pos.max - e.pos.min > "{}".length) {
2626
logPos("Empty block should be written as {}", e.pos, Reflect.field(SeverityLevel, severity));
27+
}
2728
case EBlock(_) | EObjectDecl(_):
2829
var lmin = checker.getLinePos(e.pos.min).line;
2930
var lmax = checker.getLinePos(e.pos.max).line;

checkstyle/checks/CyclomaticComplexityCheck.hx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ class CyclomaticComplexityCheck extends Check {
3838
definition.data.map(function(field:Field):Null<Target> {
3939
return switch (field.kind) {
4040
case FieldType.FFun(f):
41-
if (isCheckSuppressed(field))
42-
null;
41+
if (isCheckSuppressed(field)) null;
4342
else { name:field.name, expr:f.expr, pos:field.pos};
4443
default: null;
4544
}
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package checkstyle.checks;
2+
3+
import checkstyle.Checker.LinePos;
4+
import checkstyle.LintMessage.SeverityLevel;
5+
import haxeparser.Data;
6+
import haxe.macro.Expr;
7+
8+
@name("NeedBraces")
9+
@desc("Checks for braces on if, if else, for and while statements")
10+
class NeedBracesCheck extends Check {
11+
12+
public static inline var FOR:String = "FOR";
13+
public static inline var IF:String = "IF";
14+
public static inline var ELSE_IF:String = "ELSE_IF";
15+
public static inline var WHILE:String = "WHILE";
16+
17+
public var tokens:Array<String>;
18+
public var allowSingleLineStatement:Bool;
19+
20+
public function new() {
21+
super();
22+
tokens = [];
23+
allowSingleLineStatement = true;
24+
}
25+
26+
function hasToken(token:String):Bool {
27+
if (tokens.length == 0) return true;
28+
if (tokens.indexOf (token) > -1) return true;
29+
return false;
30+
}
31+
32+
override function actualRun() {
33+
ExprUtils.walkFile(checker.ast, function(e) {
34+
if (isPosSuppressed(e.pos)) return;
35+
switch(e.expr) {
36+
case EFor (it, expr):
37+
if (!hasToken(FOR)) return;
38+
var itLine:LinePos = checker.getLinePos(it.pos.max);
39+
var exprLine:LinePos = checker.getLinePos(expr.pos.min);
40+
checkBraces(expr, 'for loop', itLine.line == exprLine.line, false);
41+
case EIf(econd, eif, eelse):
42+
if (!hasToken(IF)) return;
43+
var condLine:LinePos = checker.getLinePos(econd.pos.max);
44+
var ifLine:LinePos = checker.getLinePos(eif.pos.min);
45+
var elseSameLine:Bool = false;
46+
if (eelse != null){
47+
var elseLine:LinePos = checker.getLinePos(eelse.pos.min);
48+
var line:String = checker.lines[elseLine.line];
49+
if (StringTools.startsWith(StringTools.trim(line), "else")) elseSameLine = true;
50+
}
51+
checkBraces(eif, 'if branch', condLine.line == ifLine.line, true);
52+
checkBraces(eelse, 'else branch', elseSameLine, true);
53+
case EWhile(econd, expr, _):
54+
if (!hasToken(WHILE)) return;
55+
var condLine:LinePos = checker.getLinePos(econd.pos.max);
56+
var exprLine:LinePos = checker.getLinePos(expr.pos.min);
57+
checkBraces(expr, 'while loop', condLine.line == exprLine.line, false);
58+
default:
59+
}
60+
});
61+
}
62+
63+
@SuppressWarnings("checkstyle:CyclomaticComplexity")
64+
function checkBraces(e:Expr, name:String, sameLine:Bool, parentIsIf:Bool) {
65+
if ((e == null) || (e.expr == null)) return;
66+
67+
var minLine:LinePos = checker.getLinePos(e.pos.min);
68+
var maxLine:LinePos = checker.getLinePos(e.pos.max);
69+
var multiLine:Bool = (minLine.line < maxLine.line);
70+
switch (e.expr) {
71+
case EBlock(_):
72+
if (!multiLine && !allowSingleLineStatement) {
73+
logPos('Single line Block detected', e.pos, Reflect.field(SeverityLevel, severity));
74+
}
75+
return;
76+
case EIf(_, _, _):
77+
if (!parentIsIf || !hasToken(ELSE_IF)) {
78+
if (multiLine) sameLine = false;
79+
}
80+
if (sameLine && allowSingleLineStatement) return;
81+
if (sameLine && !allowSingleLineStatement) {
82+
logPos('Body of $name on same line', e.pos, Reflect.field(SeverityLevel, severity));
83+
return;
84+
}
85+
logPos('No braces used for body of $name', e.pos, Reflect.field(SeverityLevel, severity));
86+
default:
87+
if (multiLine) sameLine = false;
88+
if (sameLine && allowSingleLineStatement) return;
89+
if (sameLine && !allowSingleLineStatement) {
90+
logPos('Body of $name on same line', e.pos, Reflect.field(SeverityLevel, severity));
91+
return;
92+
}
93+
logPos('No braces used for body of $name', e.pos, Reflect.field(SeverityLevel, severity));
94+
}
95+
}
96+
}

resources/config.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,14 @@
152152
"tokens": []
153153
}
154154
},
155+
{
156+
"type": "NeedBraces",
157+
"props": {
158+
"severity": "WARNING",
159+
"allowSingleLineStatement": true,
160+
"tokens": []
161+
}
162+
},
155163
{
156164
"type": "NestedForDepth",
157165
"props": {

0 commit comments

Comments
 (0)