From fd24216f46eef6dfaaedd957d30d6935c5a0606d Mon Sep 17 00:00:00 2001 From: Oliver Burn Date: Thu, 21 Feb 2002 22:43:55 +0000 Subject: [PATCH] Now have '{' checking for methods and types. Also put in place the logic to be able to check the others. --- build.xml | 3 +- .../tools/checkstyle/CheckStyleTask.java | 29 ++++- .../tools/checkstyle/Configuration.java | 17 +++ .../com/puppycrawl/tools/checkstyle/Defn.java | 2 + .../tools/checkstyle/LeftCurlyOption.java | 6 + .../puppycrawl/tools/checkstyle/Verifier.java | 113 +++++++++++------- .../com/puppycrawl/tools/checkstyle/java.g | 40 +++---- .../tools/checkstyle/CheckerTest.java | 1 + 8 files changed, 142 insertions(+), 69 deletions(-) diff --git a/build.xml b/build.xml index ae9ca0a87..fceecb553 100644 --- a/build.xml +++ b/build.xml @@ -159,7 +159,8 @@ memberPattern="^m[A-Z][a-zA-Z0-9]*$" staticPattern="^s[A-Z][a-zA-Z0-9]*$" paramPattern="^a[A-Z][a-zA-Z0-9]*$" - lcurlyMethod="nl"> + lcurlyMethod="nl" + lcurlyType="nl"> diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java index 85c79dacd..00e7ea5e6 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java @@ -309,12 +309,13 @@ public class CheckStyleTask /** @param aTo the lefy curly placement option for methods **/ public void setLCurlyMethod(String aTo) { - final LeftCurlyOption opt = LeftCurlyOption.decode(aTo); - if (opt == null) { - throw new BuildException("Unable to parse lcurlyMethod parameter,", - location); - } - mConfig.setLCurlyMethod(opt); + mConfig.setLCurlyMethod(extractLeftCurlyOption(aTo)); + } + + /** @param aTo the lefy curly placement option for types **/ + public void setLCurlyType(String aTo) + { + mConfig.setLCurlyType(extractLeftCurlyOption(aTo)); } //////////////////////////////////////////////////////////////////////////// @@ -522,4 +523,20 @@ public class CheckStyleTask return new FileOutputStream(mToFile); } } + + /** + * @param aFrom String to decode the option from + * @return the LeftCurlyOption represented by aFrom + * @throws BuildException if unable to decode aFrom + */ + private LeftCurlyOption extractLeftCurlyOption(String aFrom) + throws BuildException + { + final LeftCurlyOption opt = LeftCurlyOption.decode(aFrom); + if (opt == null) { + throw new BuildException("Unable to parse '" + aFrom + "'.", + location); + } + return opt; + } } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java index 6d5984b6c..1954d73a4 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java @@ -152,6 +152,8 @@ public class Configuration /** where to place left curlies on methods **/ private LeftCurlyOption mLCurlyMethod = LeftCurlyOption.EOL; + /** where to place left curlies on types **/ + private LeftCurlyOption mLCurlyType = LeftCurlyOption.EOL; //////////////////////////////////////////////////////////////////////////// // Constructors @@ -233,6 +235,9 @@ public class Configuration setLCurlyMethod(getLeftCurlyOptionProperty( aProps, LCURLY_METHOD_PROP, LeftCurlyOption.EOL, aLog)); + setLCurlyType(getLeftCurlyOptionProperty( + aProps, LCURLY_TYPE_PROP, + LeftCurlyOption.EOL, aLog)); } /** @@ -787,6 +792,18 @@ public class Configuration mLCurlyMethod = aTo; } + /** @return the left curly placement option for types **/ + public LeftCurlyOption getLCurlyType() + { + return mLCurlyType; + } + + /** @param aTo set the left curly placement option for types **/ + public void setLCurlyType(LeftCurlyOption aTo) + { + mLCurlyType = aTo; + } + //////////////////////////////////////////////////////////////////////////// // Private methods diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java index 445eecfd3..386b56adf 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java @@ -81,4 +81,6 @@ public interface Defn /** property name for lcurly placement for methods **/ String LCURLY_METHOD_PROP = "checkstyle.lcurly.method"; + /** property name for lcurly placement for types **/ + String LCURLY_TYPE_PROP = "checkstyle.lcurly.type"; } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/LeftCurlyOption.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/LeftCurlyOption.java index 3a4f59f1b..297c1b08b 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/LeftCurlyOption.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/LeftCurlyOption.java @@ -57,6 +57,12 @@ public final class LeftCurlyOption STR_TO_OPT.put(mStrRep, this); } + /** @see Object **/ + public String toString() + { + return mStrRep; + } + /** * Returns the LeftCurlyOption specified by a string representation. If no * option exists then null is returned. diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java index 2b7e6deba..eb3ce1b4d 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java @@ -572,55 +572,26 @@ class Verifier } } + /** * Verify that a method has correct placement of the left curly brace. * @param aMethodLine line the method starts on * @param aBrace location of the brace */ - void verifyMethodLCurly(int aMethodLine, MyCommonAST aBrace) + void verifyLCurlyMethod(int aMethodLine, MyCommonAST aBrace) { - final String prevLine = mLines[aBrace.getLineNo() - 2]; - final String braceLine = mLines[aBrace.getLineNo() - 1]; - final LeftCurlyOption option = mConfig.getLCurlyMethod(); + checkLCurly(aMethodLine, aBrace, mConfig.getLCurlyMethod()); + } - // Check for being told to ignore, or have '{}' which is a special case - if ((option == LeftCurlyOption.IGNORE) - || ((braceLine.length() > (aBrace.getColumnNo() + 1)) - && (braceLine.charAt(aBrace.getColumnNo() + 1) == '}'))) - { - // ignore - } - else if (option == LeftCurlyOption.NL) { - if (!Utils.whitespaceBefore(aBrace.getColumnNo(), braceLine)) { - log(aBrace.getLineNo(), "'{' should be on a new line."); - } - } - else if (option == LeftCurlyOption.EOL) { - if (Utils.whitespaceBefore(aBrace.getColumnNo(), braceLine) - && ((Utils.lengthMinusTrailingWhitespace(prevLine) + 2) - <= mConfig.getMaxLineLength())) - { - log(aBrace.getLineNo(), "'{' should be on the previous line."); - } - } - else if (option == LeftCurlyOption.NLOW) { - if (aMethodLine == aBrace.getLineNo()) { - // all ok as on the same line - } - else if ((aMethodLine + 1) == aBrace.getLineNo()) { - if (!Utils.whitespaceBefore(aBrace.getColumnNo(), braceLine)) { - log(aBrace.getLineNo(), "'{' should be on a new line."); - } - else if ((Utils.lengthMinusTrailingWhitespace(prevLine) + 2) - <= mConfig.getMaxLineLength()) { - log(aBrace.getLineNo(), - "'{' should be on the previous line."); - } - } - else if (!Utils.whitespaceBefore(aBrace.getColumnNo(), braceLine)) { - log(aBrace.getLineNo(), "'{' should be on a new line."); - } - } + + /** + * Verify that a type has correct placement of the left curly brace. + * @param aTypeLine line the type starts on + * @param aBrace location of the brace + */ + void verifyLCurlyType(int aTypeLine, MyCommonAST aBrace) + { + checkLCurly(aTypeLine, aBrace, mConfig.getLCurlyType()); } @@ -1214,5 +1185,63 @@ class Verifier return retVal; } + /** + * Verify the correct placement of the left curly brace. + * @param aStartLine line the construct starts on + * @param aBrace location of the brace + * @param aOption specifies where the brace should be + */ + private void checkLCurly(int aStartLine, + MyCommonAST aBrace, + LeftCurlyOption aOption) + { + final String braceLine = mLines[aBrace.getLineNo() - 1]; + + // calculate the previous line length without trailing whitespace. Need + // to handle the case where there is no previous line, cause the line + // being check is the first line in the file. + final int prevLineLen = (aBrace.getLineNo() == 1) + ? mConfig.getMaxLineLength() + : Utils.lengthMinusTrailingWhitespace( + mLines[aBrace.getLineNo() - 2]); + + // Check for being told to ignore, or have '{}' which is a special case + if ((aOption == LeftCurlyOption.IGNORE) + || ((braceLine.length() > (aBrace.getColumnNo() + 1)) + && (braceLine.charAt(aBrace.getColumnNo() + 1) == '}'))) + { + // ignore + } + else if (aOption == LeftCurlyOption.NL) { + if (!Utils.whitespaceBefore(aBrace.getColumnNo(), braceLine)) { + log(aBrace.getLineNo(), "'{' should be on a new line."); + } + } + else if (aOption == LeftCurlyOption.EOL) { + if (Utils.whitespaceBefore(aBrace.getColumnNo(), braceLine) + && ((prevLineLen + 2) <= mConfig.getMaxLineLength())) + { + log(aBrace.getLineNo(), "'{' should be on the previous line."); + } + } + else if (aOption == LeftCurlyOption.NLOW) { + if (aStartLine == aBrace.getLineNo()) { + // all ok as on the same line + } + else if ((aStartLine + 1) == aBrace.getLineNo()) { + if (!Utils.whitespaceBefore(aBrace.getColumnNo(), braceLine)) { + log(aBrace.getLineNo(), "'{' should be on a new line."); + } + else if ((prevLineLen + 2) <= mConfig.getMaxLineLength()) { + log(aBrace.getLineNo(), + "'{' should be on the previous line."); + } + } + else if (!Utils.whitespaceBefore(aBrace.getColumnNo(), braceLine)) { + log(aBrace.getLineNo(), "'{' should be on a new line."); + } + } + } + // }}} } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g b/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g index fe8e82f2d..14129a658 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g @@ -229,7 +229,7 @@ modifier // Definition of a Java class classDefinition![MyCommonAST modifiers, MyModifierSet modSet] - : "class" IDENT + : cc:"class" IDENT // it _might_ have a superclass... sc:superClassClause // it might implement some interfaces... @@ -239,7 +239,7 @@ classDefinition![MyCommonAST modifiers, MyModifierSet modSet] ver.verifyType(modSet, #IDENT); ver.reportStartTypeBlock(modSet.getVisibilityScope(), false); } - cb:classBlock + cb:classBlock[(modSet.size() == 0) ? #cc.getLineNo() : modSet.getFirstLineNo()] {#classDefinition = #(#[CLASS_DEF,"CLASS_DEF"], modifiers,IDENT,sc,ic,cb);} { @@ -254,7 +254,7 @@ superClassClause! // Definition of a Java Interface interfaceDefinition![MyCommonAST modifiers, MyModifierSet modSet] - : "interface" IDENT + : ii:"interface" IDENT // it might extend some other interfaces ie:interfaceExtends // now parse the body of the interface (looks like a class...) @@ -262,7 +262,7 @@ interfaceDefinition![MyCommonAST modifiers, MyModifierSet modSet] ver.verifyType(modSet, #IDENT); ver.reportStartTypeBlock(modSet.getVisibilityScope(), true); } - cb:classBlock + cb:classBlock[(modSet.size() == 0) ? #ii.getLineNo() : modSet.getFirstLineNo()] {#interfaceDefinition = #(#[INTERFACE_DEF,"INTERFACE_DEF"], modifiers,IDENT,ie,cb);} { @@ -273,8 +273,8 @@ interfaceDefinition![MyCommonAST modifiers, MyModifierSet modSet] // This is the body of a class. You can have fields and extra semicolons, // That's about it (until you see what a field is...) -classBlock - : LCURLY! +classBlock[int aLine] + : lc:LCURLY! { if (aLine != -1) ver.verifyLCurlyType(aLine, #lc); } ( field | SEMI! )* RCURLY! {#classBlock = #([OBJBLOCK, "OBJBLOCK"], #classBlock);} @@ -349,7 +349,7 @@ field! ( s2:compoundStatement[lcurls] { - ver.verifyMethodLCurly(msig.getLineNo(), lcurls[0]); + ver.verifyLCurlyMethod(msig.getLineNo(), lcurls[0]); ver.verifyMethodLength(#s2.getLineNo(), sCompoundLength); } @@ -379,13 +379,13 @@ field! ; constructorBody[int aLineNo] - : lc:LCURLY^ {#lc.setType(SLIST); ver.verifyMethodLCurly(aLineNo, #lc); } + : lc:LCURLY^ {#lc.setType(SLIST); ver.verifyLCurlyMethod(aLineNo, #lc); } // Predicate might be slow but only checked once per constructor def // not for general methods. ( (explicitConstructorInvocation) => explicitConstructorInvocation | ) - (statement[sIgnoreType])* + (statement[sIgnoreType, sIgnoreAST])* rc:RCURLY! {ver.verifyConstructorLength(#lc.getLineNo(), #rc.getLineNo() - #lc.getLineNo() + 1);} ; @@ -525,7 +525,7 @@ parameterModifier compoundStatement[MyCommonAST[] aCurlies] : lc:LCURLY^ {#lc.setType(SLIST); aCurlies[0] = #lc;} // include the (possibly-empty) list of statements - (statement[sIgnoreType])* + (statement[sIgnoreType, sIgnoreAST])* rc:RCURLY! { sCompoundLength = rc.getLine() - lc.getLine() + 1; @@ -534,14 +534,14 @@ compoundStatement[MyCommonAST[] aCurlies] ; -statement[int[] aType] +statement[int[] aType, MyCommonAST[] aCurlies] { final MyModifierSet modSet = new MyModifierSet(); final int[] stmtType = new int[1]; stmtType[0] = STMT_OTHER; } // A list of statements in curly braces -- start a new scope! - : compoundStatement[sIgnoreAST] { aType[0] = STMT_COMPOUND; } + : compoundStatement[aCurlies] { aType[0] = STMT_COMPOUND; } // declarations are ambiguous with "ID DOT" relative to expression // statements. Must backtrack to be sure. Could use a semantic @@ -558,10 +558,10 @@ statement[int[] aType] | m:modifiers[modSet]! classDefinition[#m, modSet] // Attach a label to the front of a statement - | IDENT c:COLON^ {#c.setType(LABELED_STAT);} statement[sIgnoreType] + | IDENT c:COLON^ {#c.setType(LABELED_STAT);} statement[sIgnoreType, sIgnoreAST] // If-else statement - | ii:"if"^ LPAREN! expression RPAREN! statement[stmtType] + | ii:"if"^ LPAREN! expression RPAREN! statement[stmtType, sIgnoreAST] { aType[0] = STMT_IF; ver.verifyWSAroundBegin(ii.getLine(), ii.getColumn(), ii.getText()); @@ -577,7 +577,7 @@ statement[int[] aType] warnWhenFollowAmbig = false; } : - ee:"else"! {stmtType[0] = STMT_OTHER; } statement[stmtType] + ee:"else"! {stmtType[0] = STMT_OTHER; } statement[stmtType, sIgnoreAST] { ver.verifyWSAroundBegin(ee.getLine(), ee.getColumn(), ee.getText()); if (stmtType[0] == STMT_OTHER) { @@ -593,7 +593,7 @@ statement[int[] aType] forCond s2:SEMI! // condition test forIter // updater RPAREN! - statement[stmtType] // statement to loop over + statement[stmtType, sIgnoreAST] // statement to loop over { ver.verifyWSAroundBegin(ff.getLine(), ff.getColumn(), ff.getText()); ver.verifyWSAfter(s1.getLine(), s1.getColumn(), MyToken.SEMI_COLON); @@ -604,7 +604,7 @@ statement[int[] aType] } // While statement - | ww:"while"^ LPAREN! expression RPAREN! statement[stmtType] + | ww:"while"^ LPAREN! expression RPAREN! statement[stmtType, sIgnoreAST] { ver.verifyWSAroundBegin(ww.getLine(), ww.getColumn(), ww.getText()); if (stmtType[0] != STMT_COMPOUND) { @@ -613,7 +613,7 @@ statement[int[] aType] } // do-while statement - | dd:"do"^ statement[stmtType] dw:"while"! LPAREN! expression RPAREN! SEMI! + | dd:"do"^ statement[stmtType, sIgnoreAST] dw:"while"! LPAREN! expression RPAREN! SEMI! { ver.verifyWSAroundBegin(dd.getLine(), dd.getColumn(), dd.getText()); ver.verifyWSAroundBegin(dw.getLine(), dw.getColumn(), dw.getText()); @@ -674,7 +674,7 @@ aCase ; caseSList - : (statement[sIgnoreType])* + : (statement[sIgnoreType, sIgnoreAST])* {#caseSList = #(#[SLIST,"SLIST"],#caseSList);} ; @@ -1004,7 +1004,7 @@ primaryExpression */ newExpression : "new"^ type - ( LPAREN! argList RPAREN! ({ver.reportStartTypeBlock(Scope.ANONINNER, false);} classBlock {ver.reportEndTypeBlock();})? + ( LPAREN! argList RPAREN! ({ver.reportStartTypeBlock(Scope.ANONINNER, false);} classBlock[-1] {ver.reportEndTypeBlock();})? //java 1.1 // Note: This will allow bad constructs like diff --git a/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java b/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java index d14144290..7e762719c 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java @@ -43,6 +43,7 @@ public class CheckerTest { mConfig.setHeaderFile(getPath("java.header")); mConfig.setLCurlyMethod(LeftCurlyOption.NL); + mConfig.setLCurlyType(LeftCurlyOption.NL); } protected String getPath(String aFilename)