From ad34b2a35eff0dba70ef5b27a0ff361520b66bea Mon Sep 17 00:00:00 2001 From: Oliver Burn Date: Sun, 8 Mar 2009 10:30:30 +0000 Subject: [PATCH] Fixed the LeftCurly check to ignore leading annotations. Thanks to Tim Carpenter for patch #2506439. An excellent quality patch. --- .../checks/blocks/LeftCurlyCheck.java | 68 +++++++++++++++++-- .../checkstyle/InputLeftCurlyAnnotations.java | 59 ++++++++++++++++ .../checks/blocks/LeftCurlyCheckTest.java | 26 ++++++- src/xdocs/releasenotes.xml | 5 ++ 4 files changed, 153 insertions(+), 5 deletions(-) create mode 100644 src/testinputs/com/puppycrawl/tools/checkstyle/InputLeftCurlyAnnotations.java diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/blocks/LeftCurlyCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/blocks/LeftCurlyCheck.java index adcc3d290..8899dab32 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/blocks/LeftCurlyCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/blocks/LeftCurlyCheck.java @@ -126,7 +126,11 @@ public class LeftCurlyCheck switch (aAST.getType()) { case TokenTypes.CTOR_DEF : case TokenTypes.METHOD_DEF : - startToken = aAST; + // Orig + //startToken = aAST; + // New + startToken = skipAnnotationOnlyLines(aAST); + // End brace = aAST.findFirstToken(TokenTypes.SLIST); break; @@ -135,11 +139,11 @@ public class LeftCurlyCheck case TokenTypes.ANNOTATION_DEF : case TokenTypes.ENUM_DEF : case TokenTypes.ENUM_CONSTANT_DEF : - startToken = aAST.getFirstChild(); + startToken = (DetailAST) skipAnnotationOnlyLines(aAST); final DetailAST objBlock = aAST.findFirstToken(TokenTypes.OBJBLOCK); brace = (objBlock == null) ? null - : objBlock.getFirstChild(); + : (DetailAST) objBlock.getFirstChild(); break; case TokenTypes.LITERAL_WHILE: @@ -156,7 +160,7 @@ public class LeftCurlyCheck case TokenTypes.LITERAL_ELSE : startToken = aAST; - final DetailAST candidate = aAST.getFirstChild(); + final DetailAST candidate = (DetailAST) aAST.getFirstChild(); brace = (candidate.getType() == TokenTypes.SLIST) ? candidate @@ -178,6 +182,62 @@ public class LeftCurlyCheck } } + /** + * Skip lines that only contain TokenTypes.ANNOTATIONs. + * If the received DetailAST + * has annotations within its modifiers then first token on the line + * of the first token afer all annotations is return. This might be + * an annotation. + * Otherwise, the received DetailAST is returned. + * @param aAST DetailAST. + * @return DetailAST. + */ + private DetailAST skipAnnotationOnlyLines(DetailAST aAST) + { + final DetailAST modifiers = aAST.findFirstToken(TokenTypes.MODIFIERS); + if (modifiers == null) { + return aAST; + } + DetailAST lastAnnot = findLastAnnotation(modifiers); + if (lastAnnot == null) { + // There are no annotations. + return aAST; + } + final DetailAST tokenAfterLast = lastAnnot.getNextSibling() != null + ? lastAnnot.getNextSibling() + : modifiers.getNextSibling(); + if (tokenAfterLast.getLineNo() > lastAnnot.getLineNo()) { + return tokenAfterLast; + } + else { + final int lastAnnotLineNumber = lastAnnot.getLineNo(); + while (lastAnnot.getPreviousSibling() != null + && (lastAnnot.getPreviousSibling().getLineNo() + == lastAnnotLineNumber)) + { + lastAnnot = lastAnnot.getPreviousSibling(); + } + return lastAnnot; + } + } + + /** + * Find the last token of type TokenTypes.ANNOTATION + * under the given set of modifiers. + * @param aModifiers DetailAST. + * @return DetailAST or null if there are no annotations. + */ + private DetailAST findLastAnnotation(DetailAST aModifiers) + { + DetailAST aAnnot = aModifiers.findFirstToken(TokenTypes.ANNOTATION); + while (aAnnot != null && aAnnot.getNextSibling() != null + && aAnnot.getNextSibling().getType() == TokenTypes.ANNOTATION) + { + aAnnot = aAnnot.getNextSibling(); + } + return aAnnot; + } + /** * Verifies that a specified left curly brace is placed correctly * according to policy. diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/InputLeftCurlyAnnotations.java b/src/testinputs/com/puppycrawl/tools/checkstyle/InputLeftCurlyAnnotations.java new file mode 100644 index 000000000..43d1b3887 --- /dev/null +++ b/src/testinputs/com/puppycrawl/tools/checkstyle/InputLeftCurlyAnnotations.java @@ -0,0 +1,59 @@ +package com.puppycrawl.tools.checkstyle; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +@TestClassAnnotation +class InputLeftCurlyAnnotations +{ + private static final int X = 10; + @Override + public boolean equals(Object other) + { + return false; + } + + @Override + @SuppressWarnings("unused") + public int hashCode() + { + int a = 10; + return 1; + } + + @Override @SuppressWarnings({"unused", "unchecked", "static-access"}) public String toString() + { + Integer i = this.X; + List l = new ArrayList(); + return "SomeString"; + } +} + +@TestClassAnnotation +class InputLeftCurlyAnnotations2 { + private static final int X = 10; + @Override + public boolean equals(Object other) { + return false; + } + + @Override + @SuppressWarnings("unused") + public int hashCode() { + int a = 10; + return 1; + } + + @Override @SuppressWarnings({"unused", "unchecked", "static-access"}) public String toString() + { + Integer i = this.X; + List l = new ArrayList(); + return "SomeString"; + } +} + +@Target(ElementType.TYPE) +@interface TestClassAnnotation { +} diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/blocks/LeftCurlyCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/blocks/LeftCurlyCheckTest.java index 65c3ad483..366a87285 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/checks/blocks/LeftCurlyCheckTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/blocks/LeftCurlyCheckTest.java @@ -119,7 +119,7 @@ public class LeftCurlyCheckTest extends BaseCheckTestSupport }; verify(mCheckConfig, getPath("InputLeftCurlyOther.java"), expected); } - + @Test public void testNL3() throws Exception { @@ -146,4 +146,28 @@ public class LeftCurlyCheckTest extends BaseCheckTestSupport }; verify(mCheckConfig, getPath("InputBraces.java"), expected); } + + @Test + public void testDefaultWithAnnotations() throws Exception + { + final String[] expected = { + "10:1: '{' should be on the previous line.", + "14:5: '{' should be on the previous line.", + "21:5: '{' should be on the previous line." + }; + verify(mCheckConfig, getPath("InputLeftCurlyAnnotations.java"), expected); + } + + @Test + public void testNLWithAnnotations() throws Exception + { + mCheckConfig.addAttribute("option", LeftCurlyOption.NL.toString()); + final String[] expected = { + "35:34: '{' should be on a new line.", + "38:41: '{' should be on a new line.", + "44:27: '{' should be on a new line.", + "58:32: '{' should be on a new line." + }; + verify(mCheckConfig, getPath("InputLeftCurlyAnnotations.java"), expected); + } } diff --git a/src/xdocs/releasenotes.xml b/src/xdocs/releasenotes.xml index 553bc4d77..335735c7e 100755 --- a/src/xdocs/releasenotes.xml +++ b/src/xdocs/releasenotes.xml @@ -112,6 +112,11 @@ user specifies a directory with the -r option that does not contain any files. Thanks to Florian for patch #2151706. +
  • + Fixed the LeftCurly check + to ignore leading annotations. Thanks to Tim Carpenter for patch + #2506439. +

    • Notes: