Skip to content

Commit 7d4e708

Browse files
Initial implementation for #302
1 parent c307d13 commit 7d4e708

File tree

9 files changed

+132
-22
lines changed

9 files changed

+132
-22
lines changed

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,5 @@ target/
1818
check-style-report.json
1919
check-style-report.xml
2020
coverage.json
21+
22+
display.hxml

resources/default-config.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -442,11 +442,11 @@
442442
},
443443
{
444444
"props": {
445-
"spaceIfCondition": true,
445+
"spaceIfCondition": "should",
446446
"spaceAroundBinop": true,
447-
"spaceForLoop": true,
447+
"spaceForLoop": "should",
448448
"ignoreRangeOperator": true,
449-
"spaceWhileLoop": true,
449+
"spaceWhileLoop": "should",
450450
"spaceCatch": true,
451451
"spaceSwitchCase": true,
452452
"noSpaceAroundUnop": true

src/checkstyle/Main.hx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import checkstyle.reporter.TextReporter;
1111
import checkstyle.reporter.XMLReporter;
1212
import checkstyle.reporter.CodeClimateReporter;
1313
import checkstyle.reporter.ExitCodeReporter;
14+
import checkstyle.errors.Error;
1415
import haxe.CallStack;
1516
import haxe.Json;
1617
import hxargs.Args;
@@ -181,7 +182,14 @@ class Main {
181182
if (!checkFields.contains(prop)) {
182183
failWith('Check ${check.getModuleName()} has no property named \'$prop\'');
183184
}
184-
Reflect.setField(check, prop, val);
185+
try {
186+
check.configureProperty(prop, val);
187+
}
188+
catch (e:Dynamic) {
189+
var message = 'Failed to configure $prop setting for ${check.getModuleName()}: ';
190+
message += (Std.is(e, Error) ? (e:Error).message : Std.string(message));
191+
failWith(e.message);
192+
}
185193
}
186194
if (defaultSeverity != null && !props.contains("severity")) check.severity = defaultSeverity;
187195
}

src/checkstyle/checks/Check.hx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ class Check {
3131
desc = haxe.rtti.Meta.getType(Type.getClass(this)).desc[0];
3232
}
3333

34+
public function configureProperty(name:String, value:Any) {
35+
Reflect.setField(this, name, value);
36+
}
37+
3438
public function run(checker:Checker):Array<CheckMessage> {
3539
this.checker = checker;
3640
messages = [];

src/checkstyle/checks/Directive.hx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package checkstyle.checks;
2+
3+
import Type.ValueType;
4+
import checkstyle.errors.InvalidDirectiveError;
5+
6+
@:enum
7+
abstract Directive(String) to String {
8+
var SHOULD = "should";
9+
var SHOULD_NOT = "shouldNot";
10+
var ANY = "any";
11+
12+
@:from
13+
public static function fromAny(value:Any):Directive {
14+
return switch (Type.typeof(value)) {
15+
case ValueType.TClass(String): getValidated(value);
16+
//support for legacy configs when such settings were boolean
17+
case ValueType.TBool: (value ? SHOULD : ANY);
18+
case _: ANY;
19+
}
20+
}
21+
22+
@:from
23+
static inline function fromString(value:String):Directive {
24+
return getValidated(value);
25+
}
26+
27+
static function getValidated(value:String):Directive {
28+
switch (value:Directive) {
29+
case SHOULD, SHOULD_NOT, ANY:
30+
return value;
31+
}
32+
throw new InvalidDirectiveError('Invalid directive: $value');
33+
}
34+
}

src/checkstyle/checks/whitespace/SpacingCheck.hx

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package checkstyle.checks.whitespace;
22

3+
import Type.ValueType;
34
import checkstyle.utils.ExprUtils;
45
import haxe.macro.Expr;
56
import haxe.macro.Printer;
@@ -12,9 +13,9 @@ class SpacingCheck extends Check {
1213

1314
public var spaceAroundBinop:Bool;
1415
public var noSpaceAroundUnop:Bool;
15-
public var spaceIfCondition:Bool;
16-
public var spaceForLoop:Bool;
17-
public var spaceWhileLoop:Bool;
16+
public var spaceIfCondition:Directive;
17+
public var spaceForLoop:Directive;
18+
public var spaceWhileLoop:Directive;
1819
public var spaceSwitchCase:Bool;
1920
public var spaceCatch:Bool;
2021
public var ignoreRangeOperator:Bool;
@@ -23,15 +24,26 @@ class SpacingCheck extends Check {
2324
super(AST);
2425
spaceAroundBinop = true;
2526
noSpaceAroundUnop = true;
26-
spaceIfCondition = true;
27-
spaceForLoop = true;
28-
spaceWhileLoop = true;
27+
spaceIfCondition = SHOULD;
28+
spaceForLoop = SHOULD;
29+
spaceWhileLoop = SHOULD;
2930
spaceSwitchCase = true;
3031
spaceCatch = true;
3132
ignoreRangeOperator = true;
3233
categories = [Category.STYLE, Category.CLARITY];
3334
}
3435

36+
override public function configureProperty(name:String, value:Any) {
37+
var currentValue = Reflect.field(this, name);
38+
39+
switch (Type.typeof(currentValue)) {
40+
case ValueType.TClass(String):
41+
Reflect.setField(this, name, (value:Directive));
42+
case _:
43+
super.configureProperty(name, value);
44+
}
45+
}
46+
3547
override function actualRun() {
3648
var lastExpr = null;
3749

@@ -50,12 +62,12 @@ class SpacingCheck extends Check {
5062
if (post) dist = e.pos.max - e2.pos.max;
5163
else dist = e2.pos.min - e.pos.min;
5264
if (dist > unopSize(uo)) logPos('Space around "${unopString(uo)}"', e.pos);
53-
case EIf(econd, _, _) if (spaceIfCondition):
54-
checkSpaceBetweenExpressions("if", e, econd);
55-
case EFor(it, _) if (spaceForLoop):
56-
checkSpaceBetweenExpressions("for", e, it);
57-
case EWhile(econd, _, true) if (spaceWhileLoop):
58-
checkSpaceBetweenExpressions("while", e, econd);
65+
case EIf(econd, _, _):
66+
checkSpaceBetweenExpressions("if", e, econd, spaceIfCondition);
67+
case EFor(it, _):
68+
checkSpaceBetweenExpressions("for", e, it, spaceForLoop);
69+
case EWhile(econd, _, true):
70+
checkSpaceBetweenExpressions("while", e, econd, spaceWhileLoop);
5971
case ESwitch(eswitch, _, _) if (spaceSwitchCase):
6072
checkSpaceBetweenManually("switch", lastExpr, eswitch);
6173
case ETry(etry, catches) if (spaceCatch):
@@ -87,9 +99,17 @@ class SpacingCheck extends Check {
8799
return (new Printer()).printUnop(uo);
88100
}
89101

90-
function checkSpaceBetweenExpressions(name:String, e1:Expr, e2:Expr) {
91-
if (e2.pos.min - e1.pos.min < '$name ('.length) {
92-
logRange('No space between "$name" and "("', e1.pos.max, e2.pos.min);
102+
function checkSpaceBetweenExpressions(name:String, e1:Expr, e2:Expr, directive:Directive = SHOULD) {
103+
switch (directive) {
104+
case ANY:
105+
case SHOULD_NOT:
106+
if (e2.pos.min - e1.pos.min > '$name('.length) {
107+
logRange('Space between "$name" and "("', e1.pos.max, e2.pos.min);
108+
}
109+
case SHOULD:
110+
if (e2.pos.min - e1.pos.min < '$name ('.length) {
111+
logRange('No space between "$name" and "("', e1.pos.max, e2.pos.min);
112+
}
93113
}
94114
}
95115

src/checkstyle/errors/Error.hx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package checkstyle.errors;
2+
3+
class Error {
4+
public var message (default, null):String;
5+
6+
public function new(message:String) {
7+
this.message = message;
8+
}
9+
10+
public function toString():String {
11+
return message;
12+
}
13+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package checkstyle.errors;
2+
3+
class InvalidDirectiveError extends Error {}

test/checks/whitespace/SpacingCheckTest.hx

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,23 @@
11
package checks.whitespace;
22

33
import checkstyle.checks.whitespace.SpacingCheck;
4+
import checkstyle.checks.Directive;
45

56
class SpacingCheckTest extends CheckTestCase<SpacingCheckTests> {
67

7-
public function testIf() {
8+
public function testIfShouldContainSpace() {
89
assertMsg(new SpacingCheck(), TEST1A, 'No space between "if" and "("');
910
assertNoMsg(new SpacingCheck(), TEST1B);
1011
}
1112

13+
public function testIfShouldNotContainSpace() {
14+
var check = new SpacingCheck();
15+
check.spaceIfCondition = Directive.SHOULD_NOT;
16+
17+
assertMsg(check, TEST1B, 'Space between "if" and "("');
18+
assertNoMsg(check, TEST1A);
19+
}
20+
1221
public function testBinaryOperator() {
1322
assertMsg(new SpacingCheck(), TEST2, 'No space around "+"');
1423
}
@@ -17,16 +26,32 @@ class SpacingCheckTest extends CheckTestCase<SpacingCheckTests> {
1726
assertMsg(new SpacingCheck(), TEST3, 'Space around "++"');
1827
}
1928

20-
public function testFor() {
29+
public function testForShouldContainSpace() {
2130
assertMsg(new SpacingCheck(), TEST4A, 'No space between "for" and "("');
2231
assertNoMsg(new SpacingCheck(), TEST4B);
2332
}
2433

25-
public function testWhile() {
34+
public function testForShouldNotContainSpace() {
35+
var check = new SpacingCheck();
36+
check.spaceForLoop = Directive.SHOULD_NOT;
37+
38+
assertMsg(check, TEST4B, 'Space between "for" and "("');
39+
assertNoMsg(check, TEST4A);
40+
}
41+
42+
public function testWhileShouldContainSpace() {
2643
assertMsg(new SpacingCheck(), TEST5A, 'No space between "while" and "("');
2744
assertNoMsg(new SpacingCheck(), TEST5B);
2845
}
2946

47+
public function testWhileShouldNotContainSpace() {
48+
var check = new SpacingCheck();
49+
check.spaceWhileLoop = Directive.SHOULD_NOT;
50+
51+
assertMsg(check, TEST5B, 'Space between "while" and "("');
52+
assertNoMsg(check, TEST5A);
53+
}
54+
3055
public function testSwitch() {
3156
assertMsg(new SpacingCheck(), TEST6A, 'No space between "switch" and "("');
3257
assertNoMsg(new SpacingCheck(), TEST6B);
@@ -54,6 +79,7 @@ abstract SpacingCheckTests(String) to String {
5479
}
5580
}";
5681

82+
5783
var TEST2 =
5884
"class Test {
5985
public function test() {

0 commit comments

Comments
 (0)