Skip to content

Commit feefb13

Browse files
AlexHaxeAlexHaxe
committed
added tests for RightCurlyCheck
added support for switch/case blocks to LeftCurlyCheck
1 parent b6dcf73 commit feefb13

File tree

6 files changed

+515
-41
lines changed

6 files changed

+515
-41
lines changed

‎checkstyle/checks/LeftCurlyCheck.hx‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@ class LeftCurlyCheck extends Check {
155155
if (cases.length > 0) {
156156
firstCase = cases[0].values[0];
157157
}
158+
for (c in cases) {
159+
checkBlocks(c.expr, isListWrapped(c.values));
160+
}
161+
checkBlocks(edef);
158162
if (firstCase == null) {
159163
checkLinesBetween(e.pos.min, e.pos.max, isWrapped(expr), e.pos);
160164
return;
@@ -189,13 +193,22 @@ class LeftCurlyCheck extends Check {
189193
(functionDef.indexOf('\r') >= 0);
190194
}
191195

196+
function isListWrapped(es:Array<Expr>):Bool {
197+
if (es == null) return false;
198+
if (es.length <= 0) return false;
199+
var posMin:Int = es[0].pos.min;
200+
var posMax:Int = es[es.length - 1].pos.max;
201+
return (checker.getLinePos(posMin).line != checker.getLinePos(posMax).line);
202+
}
203+
192204
function isWrapped(e:Expr):Bool {
193205
if (e == null) return false;
194206
return (checker.getLinePos(e.pos.min).line != checker.getLinePos(e.pos.max).line);
195207
}
196208

197209
function checkBlocks(e:Expr, wrapped:Bool = false) {
198210
if ((e == null) || (e.expr == null)) return;
211+
if (checker.file.content.charAt(e.pos.min) != "{") return;
199212

200213
switch(e.expr) {
201214
case EBlock(_):

‎checkstyle/checks/RightCurlyCheck.hx‎

Lines changed: 27 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class RightCurlyCheck extends Check {
2828
public static inline var ALONE:String = "alone";
2929
public static inline var ALONE_OR_SINGLELINE:String = "aloneorsingle";
3030

31-
static var sameRegex:EReg;
31+
var sameRegex:EReg;
3232

3333
public var tokens:Array<String>;
3434
public var option:String;
@@ -41,7 +41,7 @@ class RightCurlyCheck extends Check {
4141
ABSTRACT_DEF,
4242
TYPEDEF_DEF,
4343
INTERFACE_DEF,
44-
//OBJECT_DECL, // => allow inline object declarations
44+
OBJECT_DECL,
4545
FUNCTION,
4646
FOR,
4747
IF,
@@ -52,6 +52,7 @@ class RightCurlyCheck extends Check {
5252
];
5353
option = ALONE_OR_SINGLELINE;
5454

55+
// only else and catch allowed on same line after a right curly
5556
sameRegex = ~/^\s*(else|catch)/;
5657
}
5758

@@ -113,7 +114,7 @@ class RightCurlyCheck extends Check {
113114
if (!hasToken(OBJECT_DECL)) return;
114115
switch(t) {
115116
case TAnonymous(_):
116-
checkLinesBetween(pos.min, pos.max, pos);
117+
checkPos(pos, isSingleLine(pos.min, pos.max));
117118
default:
118119
}
119120
});
@@ -134,7 +135,7 @@ class RightCurlyCheck extends Check {
134135
if (!hasToken(OBJECT_DECL)) return;
135136
if (fields.length <= 0) return;
136137
var lastExpr:Expr = fields[fields.length - 1].expr;
137-
checkBlocks(lastExpr, isSingleLine(e.pos.min, lastExpr.pos.max));
138+
checkBlocks(e, isSingleLine(e.pos.min, e.pos.max));
138139
case EFunction(_, f):
139140
if (!hasToken(FUNCTION)) return;
140141
checkBlocks(f.expr, isSingleLine(e.pos.min, f.expr.pos.max));
@@ -143,24 +144,22 @@ class RightCurlyCheck extends Check {
143144
checkBlocks(expr, isSingleLine(e.pos.min, expr.pos.max));
144145
case EIf(econd, eif, eelse):
145146
if (!hasToken(IF)) return;
146-
checkBlocks(eif, isSingleLine(e.pos.min, eif.pos.max));
147-
if (eelse != null) {
148-
checkBlocks(eelse, isSingleLine(e.pos.min, eelse.pos.max));
149-
}
147+
var singleLine:Bool;
148+
if (eelse != null) singleLine = isSingleLine(e.pos.min, eelse.pos.max);
149+
else singleLine = isSingleLine (e.pos.min, eif.pos.max);
150+
checkBlocks(eif, singleLine);
151+
if (eelse != null) checkBlocks(eelse, singleLine);
150152
case EWhile(econd, expr, _):
151153
if (!hasToken(WHILE)) return;
152154
checkBlocks(expr, isSingleLine(e.pos.min, expr.pos.max));
153155
case ESwitch(expr, cases, edef):
154156
if (!hasToken(SWITCH)) return;
155-
var firstCase:Expr = edef;
156-
if (cases.length > 0) {
157-
firstCase = cases[0].values[0];
158-
}
159-
if (firstCase == null) {
160-
checkLinesBetween(e.pos.min, e.pos.max, e.pos);
161-
return;
157+
checkPos(e.pos, isSingleLine(e.pos.min, e.pos.max));
158+
for (c in cases) {
159+
if (c.expr != null) checkBlocks(c.expr, isSingleLine(c.expr.pos.min, c.expr.pos.max));
162160
}
163-
checkLinesBetween(expr.pos.max, firstCase.pos.min, e.pos);
161+
if ((edef == null) || (edef.expr == null)) return;
162+
checkBlocks (edef, isSingleLine(edef.pos.min, edef.pos.max));
164163
case ETry(expr, catches):
165164
if (!hasToken(TRY)) return;
166165
checkBlocks(expr, isSingleLine(e.pos.min, expr.pos.max));
@@ -173,16 +172,21 @@ class RightCurlyCheck extends Check {
173172
}
174173

175174
function checkPos(pos:Position, singleLine:Bool) {
175+
var bracePos:Int = checker.file.content.lastIndexOf("}", pos.max);
176+
if (bracePos < 0 || bracePos < pos.min) return;
177+
176178
var linePos:Int = checker.getLinePos(pos.max).line;
177179
var line:String = checker.lines[linePos];
178180
checkRightCurly(line, singleLine, pos);
179181
}
180182

181183
function checkBlocks(e:Expr, singleLine:Bool) {
182184
if ((e == null) || (e.expr == null)) return;
185+
var bracePos:Int = checker.file.content.lastIndexOf("}", e.pos.max);
186+
if (bracePos < 0 || bracePos < e.pos.min) return;
183187

184188
switch(e.expr) {
185-
case EBlock(_):
189+
case EBlock(_), EObjectDecl(_):
186190
var linePos:Int = checker.getLinePos(e.pos.max).line;
187191
var line:String = checker.lines[linePos];
188192
checkRightCurly(line, singleLine, e.pos);
@@ -196,33 +200,15 @@ class RightCurlyCheck extends Check {
196200
return startLine == endLine;
197201
}
198202

199-
function isSameLine(pos:Int, regex:EReg):Bool {
200-
var linePos:LinePos = checker.getLinePos(pos);
201-
var line:String = checker.lines[linePos.line].substr(linePos.ofs);
202-
203-
return regex.match(line);
204-
}
205-
206-
function checkLinesBetween(min:Int, max:Int, pos:Position) {
207-
if (isPosSuppressed(pos)) return;
208-
var bracePos:Int = checker.file.content.lastIndexOf("{", max);
209-
if (bracePos < 0 || bracePos < min) return;
210-
211-
var lineNum:Int = checker.getLinePos(bracePos).line;
212-
var line:String = checker.lines[lineNum];
213-
//checkLeftCurly(line, pos);
214-
}
215-
216203
function checkRightCurly(line:String, singleLine:Bool, pos:Position) {
217204
try {
218-
219205
var linePos:LinePos = checker.getLinePos(pos.max);
220206
var afterCurly:String = checker.lines[linePos.line].substr(linePos.ofs);
221-
var needsSame:Bool = sameRegex.match(afterCurly);
222-
var couldBeSame:Bool = false;
207+
var needsSameOption:Bool = sameRegex.match(afterCurly);
208+
var shouldHaveSameOption:Bool = false;
223209
if (checker.lines.length > linePos.line + 1) {
224210
var nextLine:String = checker.lines[linePos.line + 1];
225-
couldBeSame = sameRegex.match(nextLine);
211+
shouldHaveSameOption = sameRegex.match(nextLine);
226212
}
227213
// adjust to show correct line number in log message
228214
pos.min = pos.max;
@@ -232,9 +218,9 @@ class RightCurlyCheck extends Check {
232218

233219
var curlyAlone:Bool = ~/^\s*\}[\);\s]*(|\/\/.*)$/.match(line);
234220
logErrorIf (!curlyAlone && (option == ALONE_OR_SINGLELINE || option == ALONE), 'Right curly should be alone on a new line', pos);
235-
logErrorIf (curlyAlone && needsSame, 'Right curly should be alone on a new line', pos);
236-
logErrorIf (needsSame && (option != SAME), 'Right curly must not be on same line as following block', pos);
237-
logErrorIf (couldBeSame && (option == SAME), 'Right curly should be on same line as following block (e.g. "} else {")', pos);
221+
logErrorIf (curlyAlone && needsSameOption, 'Right curly should be alone on a new line', pos);
222+
logErrorIf (needsSameOption && (option != SAME), 'Right curly must not be on same line as following block', pos);
223+
logErrorIf (shouldHaveSameOption && (option == SAME), 'Right curly should be on same line as following block (e.g. "} else" or "} catch")', pos);
238224
}
239225
catch (e:String) {
240226
// one of the error messages fired -> do nothing

‎resources/config.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@
247247
"TYPEDEF_DEF",
248248
"CLASS_DEF",
249249
"INTERFACE_DEF",
250+
"OBJECT_DECL",
250251
"FUNCTION",
251252
"FOR",
252253
"IF",

‎test/LeftCurlyCheckTest.hx‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ class LeftCurlyCheckTest extends CheckTestCase {
1212
assertMsg(check, LeftCurlyTests.TEST8, '');
1313
assertMsg(check, LeftCurlyTests.TEST9, '');
1414
assertMsg(check, LeftCurlyTests.TEST14, '');
15+
assertMsg(check, LeftCurlyTests.EOL_CASEBLOCK, '');
1516
}
1617

1718
public function testWrongBraces() {
@@ -23,6 +24,8 @@ class LeftCurlyCheckTest extends CheckTestCase {
2324
assertMsg(check, LeftCurlyTests.TEST5, 'Left curly should be at EOL (only linebreak or comment after curly)');
2425
assertMsg(check, LeftCurlyTests.TEST7, 'Left curly should be at EOL (only linebreak or comment after curly)');
2526
assertMsg(check, LeftCurlyTests.TEST10, 'Left curly should be at EOL (only linebreak or comment after curly)');
27+
assertMsg(check, LeftCurlyTests.NL_CASEBLOCK, 'Left curly should be at EOL (only linebreak or comment after curly)');
28+
assertMsg(check, LeftCurlyTests.NLOW_CASEBLOCK, 'Left curly should be at EOL (only linebreak or comment after curly)');
2629
}
2730

2831
public function testBraceOnNL() {
@@ -52,6 +55,9 @@ class LeftCurlyCheckTest extends CheckTestCase {
5255
var check = new LeftCurlyCheck();
5356
check.option = LeftCurlyCheck.NL;
5457
assertMsg(check, LeftCurlyTests.TEST15, '');
58+
assertMsg(check, LeftCurlyTests.NL_CASEBLOCK, '');
59+
assertMsg(check, LeftCurlyTests.EOL_CASEBLOCK, 'Left curly should be on new line (only whitespace before curly)');
60+
assertMsg(check, LeftCurlyTests.NLOW_CASEBLOCK, 'Left curly should be on new line (only whitespace before curly)');
5561
}
5662

5763
public function testNLOW() {
@@ -60,6 +66,7 @@ class LeftCurlyCheckTest extends CheckTestCase {
6066
assertMsg(check, LeftCurlyTests.TEST, '');
6167
assertMsg(check, LeftCurlyTests.TEST12, '');
6268
assertMsg(check, LeftCurlyTests.TEST16, '');
69+
assertMsg(check, LeftCurlyTests.NLOW_CASEBLOCK, '');
6370
assertMsg(check, LeftCurlyTests.TEST17, 'Left curly should be at EOL (previous expression is not split over muliple lines)');
6471
assertMsg(check, LeftCurlyTests.TEST18, 'Left curly should be on new line (previous expression is split over muliple lines)');
6572
assertMsg(check, LeftCurlyTests.TEST19, 'Left curly should be on new line (previous expression is split over muliple lines)');
@@ -307,4 +314,50 @@ class LeftCurlyTests {
307314
}
308315
}
309316
}";
317+
318+
public static inline var NL_CASEBLOCK:String = "
319+
class Test
320+
{
321+
public function test(val:Int,
322+
val2:Int):String
323+
{
324+
switch(val)
325+
{
326+
case 0:
327+
{
328+
// do nothing
329+
}
330+
default:
331+
}
332+
}
333+
}";
334+
335+
public static inline var EOL_CASEBLOCK:String = "
336+
class Test {
337+
public function test(val:Int,
338+
val2:Int):String {
339+
switch(val) {
340+
case 0: {
341+
// do nothing
342+
}
343+
default:
344+
}
345+
}
346+
}";
347+
348+
public static inline var NLOW_CASEBLOCK:String = "
349+
class Test {
350+
public function test(val:Int,
351+
val2:Int):String
352+
{
353+
switch(val) {
354+
case (true ||
355+
!false):
356+
{
357+
// do nothing
358+
}
359+
default:
360+
}
361+
}
362+
}";
310363
}

0 commit comments

Comments
 (0)