diff --git a/CHANGES.md b/CHANGES.md index e5c5c8c6..9843db34 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,7 @@ - Improved support for eval target and Haxe 4 [#423](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/423) - Refactored configuration parser [#420](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/420) - Refactored check exclusion handling [#424](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/424) +- Refactored comment placement in token tree [#429](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/429) - Cleanup `checkstyle.json` [#427](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/427) ## version 2.3.1 (2018-05-16) diff --git a/src/checkstyle/checks/whitespace/ExtendedEmptyLinesCheck.hx b/src/checkstyle/checks/whitespace/ExtendedEmptyLinesCheck.hx index a4cc0f73..c752923f 100644 --- a/src/checkstyle/checks/whitespace/ExtendedEmptyLinesCheck.hx +++ b/src/checkstyle/checks/whitespace/ExtendedEmptyLinesCheck.hx @@ -194,12 +194,12 @@ class ExtendedEmptyLinesCheck extends Check { checkType(emptyLines, typeToken, getPolicy(BEGINABSTRACT), getPolicy(ENDABSTRACT), function(child:TokenTree, next:TokenTree):PolicyAndWhat { var isFuncChild:Bool = child.is(Kwd(KwdFunction)); var isVarChild:Bool = child.is(Kwd(KwdVar)); - var isFuncNext:Bool = next.is(Kwd(KwdFunction)); - var isVarNext:Bool = next.is(Kwd(KwdVar)); - - if (isFuncChild && isFuncNext) return makePolicyAndWhat(getPolicy(BETWEENABSTRACTMETHODS), "between abstract functions"); - if (isVarChild && isFuncNext) return makePolicyAndWhat(getPolicy(AFTERABSTRACTVARS), "after abstract vars"); - if (isFuncChild && isVarNext) return makePolicyAndWhat(getPolicy(AFTERABSTRACTVARS), "after abstract vars"); + if (!isVarChild && !isFuncChild) return null; + var type:EmptyLinesFieldType = detectNextFieldType(next); + if (type == OTHER) return null; + if (isFuncChild && (type == FUNCTION)) return makePolicyAndWhat(getPolicy(BETWEENABSTRACTMETHODS), "between abstract functions"); + if (isVarChild && (type == FUNCTION)) return makePolicyAndWhat(getPolicy(AFTERABSTRACTVARS), "after abstract vars"); + if (isFuncChild && (type == VAR)) return makePolicyAndWhat(getPolicy(AFTERABSTRACTVARS), "after abstract vars"); return makePolicyAndWhat(getPolicy(BETWEENABSTRACTVARS), "between abstract vars"); }); } @@ -220,12 +220,12 @@ class ExtendedEmptyLinesCheck extends Check { checkType(emptyLines, typeToken, getPolicy(BEGINCLASS), getPolicy(ENDCLASS), function(child:TokenTree, next:TokenTree):PolicyAndWhat { var isFuncChild:Bool = child.is(Kwd(KwdFunction)); var isVarChild:Bool = child.is(Kwd(KwdVar)); - var isFuncNext:Bool = next.is(Kwd(KwdFunction)); - var isVarNext:Bool = next.is(Kwd(KwdVar)); - - if (isFuncChild && isFuncNext) return makePolicyAndWhat(getPolicy(BETWEENCLASSMETHODS), "between class methods"); - if (isVarChild && isFuncNext) return makePolicyAndWhat(getPolicy(AFTERCLASSVARS), "after class vars"); - if (isFuncChild && isVarNext) return makePolicyAndWhat(getPolicy(AFTERCLASSVARS), "after class vars"); + if (!isVarChild && !isFuncChild) return null; + var type:EmptyLinesFieldType = detectNextFieldType(next); + if (type == OTHER) return null; + if (isFuncChild && (type == FUNCTION)) return makePolicyAndWhat(getPolicy(BETWEENCLASSMETHODS), "between class methods"); + if (isVarChild && (type == FUNCTION)) return makePolicyAndWhat(getPolicy(AFTERCLASSVARS), "after class vars"); + if (isFuncChild && (type == VAR)) return makePolicyAndWhat(getPolicy(AFTERCLASSVARS), "after class vars"); var isStaticChild:Bool = (child.filter([Kwd(KwdStatic)], FIRST).length > 0); var isStaticNext:Bool = (next.filter([Kwd(KwdStatic)], FIRST).length > 0); @@ -236,6 +236,24 @@ class ExtendedEmptyLinesCheck extends Check { }); } + function detectNextFieldType(field:TokenTree):EmptyLinesFieldType { + if (field.is(Kwd(KwdFunction))) return FUNCTION; + if (field.is(Kwd(KwdVar))) return VAR; + if (!field.isComment()) return OTHER; + + var after:TokenTree = field.nextSibling; + while (after != null) { + if (after.is(Kwd(KwdFunction))) return FUNCTION; + if (after.is(Kwd(KwdVar))) return VAR; + if (after.isComment()) { + after = after.nextSibling; + continue; + } + return OTHER; + } + return OTHER; + } + function checkType(emptyLines:ListOfEmptyLines, typeToken:TokenTree, beginPolicy:EmptyLinesPolicy, @@ -478,6 +496,12 @@ abstract EmptyLinesPolicy(String) { var ATLEAST = "atleast"; } +enum EmptyLinesFieldType { + VAR; + FUNCTION; + OTHER; +} + @:enum abstract EmptyLinesPlace(String) { var BEFOREPACKAGE = "beforePackage"; diff --git a/src/checkstyle/token/TokenStream.hx b/src/checkstyle/token/TokenStream.hx index 42a18ba6..adfcc533 100644 --- a/src/checkstyle/token/TokenStream.hx +++ b/src/checkstyle/token/TokenStream.hx @@ -26,7 +26,7 @@ class TokenStream { var token:Token = tokens[current]; current++; #if debugTokenTree - Sys.println (token); + Sys.println(token); #end return new TokenTree(token.tok, token.pos, current - 1); } diff --git a/src/checkstyle/token/TokenTree.hx b/src/checkstyle/token/TokenTree.hx index 17f7f0fb..6a0f3dec 100644 --- a/src/checkstyle/token/TokenTree.hx +++ b/src/checkstyle/token/TokenTree.hx @@ -20,6 +20,14 @@ class TokenTree extends Token { return Type.enumEq(tokenDef, tok); } + public function isComment():Bool { + if (tok == null) return false; + return switch (tok) { + case Comment(_), CommentLine(_): true; + default: false; + } + } + public function addChild(child:TokenTree) { if (children == null) children = []; if (children.length > 0) { diff --git a/src/checkstyle/token/walk/WalkClass.hx b/src/checkstyle/token/walk/WalkClass.hx index b4aef4ec..8d989848 100644 --- a/src/checkstyle/token/walk/WalkClass.hx +++ b/src/checkstyle/token/walk/WalkClass.hx @@ -49,16 +49,13 @@ class WalkClass { case Kwd(KwdPublic), Kwd(KwdPrivate), Kwd(KwdStatic), Kwd(KwdInline), Kwd(KwdMacro), Kwd(KwdOverride), Kwd(KwdDynamic): tempStore.push(stream.consumeToken()); case Comment(_), CommentLine(_): - tempStore.push(stream.consumeToken()); + parent.addChild(stream.consumeToken()); default: throw "invalid token tree structure"; } } - for (tok in tempStore) { - switch (tok.tok) { - case Comment(_), CommentLine(_): parent.addChild(tok); - default: throw "invalid token tree structure"; - } + if (tempStore.length > 0) { + throw "invalid token tree structure"; } } diff --git a/src/checkstyle/token/walk/WalkVar.hx b/src/checkstyle/token/walk/WalkVar.hx index 31385e55..33906507 100644 --- a/src/checkstyle/token/walk/WalkVar.hx +++ b/src/checkstyle/token/walk/WalkVar.hx @@ -24,7 +24,6 @@ class WalkVar { if (stream.is(Binop(OpAssign))) { WalkStatement.walkStatement(stream, name); } - WalkComment.walkComment(stream, name); if (stream.is(Comma)) { var comma:TokenTree = stream.consumeTokenDef(Comma); name.addChild(comma); diff --git a/test/checks/whitespace/ExtendedEmptyLinesCheckTest.hx b/test/checks/whitespace/ExtendedEmptyLinesCheckTest.hx index 4bf23ff8..c4cbd855 100644 --- a/test/checks/whitespace/ExtendedEmptyLinesCheckTest.hx +++ b/test/checks/whitespace/ExtendedEmptyLinesCheckTest.hx @@ -280,6 +280,8 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { static var s2:String = 'xx'; var a:Int; var b:String = 'x'; + // var c:Int = 5; + var c:String = 'y'; function a() { callA(); callB(); @@ -287,11 +289,17 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { callC(); } } - // test comment function b() { callA(); callB(); } + // test comment + // test comment + function c() { + callA(); + callB(); + } + // test comment } interface ITest { function a(); @@ -307,12 +315,18 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { @:enum abstract Test(String) { var val1 = 'test'; - var val2 = 'test'; + var val2 = 'test2'; + // var val3 = 'test3'; + var val4 = 'test4'; function test() { } function test2() { } + // comment + function test3() { + } } + abstract OneOfTwo(Dynamic) from T1 from T2 to T1 to T2 {} typedef Any = Dynamic; typedef Struct = { var fieldA:String; @@ -345,6 +359,10 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { var b:String = 'x'; + // var c:Int = 5; + + var c:String = 'y'; + function a() { callA(); @@ -359,9 +377,19 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { } + function b() { + + callA(); + + callB(); + + } + // test comment - function b() { + // test comment + + function c() { callA(); @@ -369,6 +397,8 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { } + // test comment + } interface ITest @@ -404,6 +434,10 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { var val2 = 'test'; + // var val3 = 'test3'; + + var val4 = 'test4'; + function test() { } @@ -412,8 +446,16 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { } + // comment + + function test3() { + + } + } + abstract OneOfTwo(Dynamic) from T1 from T2 to T1 to T2 {} + typedef Any = Dynamic; typedef Struct = @@ -467,6 +509,12 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { var b:String = 'x'; + // var c:Int = 5; + + + var c:String = 'y'; + + function a() { @@ -488,10 +536,25 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { } + function b() { + + + callA(); + + + callB(); + + + } + + // test comment - function b() { + // test comment + + + function c() { callA(); @@ -503,6 +566,9 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { } + // test comment + + } @@ -554,6 +620,12 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { var val2 = 'test'; + // var val3 = 'test3'; + + + var val4 = 'test4'; + + function test() { @@ -566,9 +638,21 @@ abstract ExtendedEmptyLinesCheckTests(String) to String { } + // comment + + + function test3() { + + + } + + } + abstract OneOfTwo(Dynamic) from T1 from T2 to T1 to T2 {} + + typedef Any = Dynamic;