From e10faf33158e0ffbbf8bbe4ed2e32d744b2b1acb Mon Sep 17 00:00:00 2001 From: Oliver Burn Date: Wed, 20 Feb 2002 23:08:21 +0000 Subject: [PATCH] First cut at doing left curly checking for methods. This is definitely a work in progress that will change a lot as I add checking for other constructs. I always tend to evolve to the general solution. :-) --- .../tools/checkstyle/CheckStyleTask.java | 11 +++ .../puppycrawl/tools/checkstyle/Utils.java | 70 +++++++++++++++++++ .../puppycrawl/tools/checkstyle/Verifier.java | 51 ++++++++++++++ .../com/puppycrawl/tools/checkstyle/java.g | 43 ++++++++---- .../tools/checkstyle/CheckerTest.java | 30 ++++++++ .../checkstyle/InputLeftCurlyMethod.java | 37 ++++++++++ 6 files changed, 228 insertions(+), 14 deletions(-) create mode 100644 src/checkstyle/com/puppycrawl/tools/checkstyle/Utils.java create mode 100644 src/tests/com/puppycrawl/tools/checkstyle/InputLeftCurlyMethod.java diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java index 6d24f2e47..d55050150 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java @@ -306,6 +306,17 @@ public class CheckStyleTask mConfig.setCacheFile(aCacheFile.getAbsolutePath()); } + /** @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); + } + //////////////////////////////////////////////////////////////////////////// // The doers //////////////////////////////////////////////////////////////////////////// diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Utils.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Utils.java new file mode 100644 index 000000000..a880d2f8c --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Utils.java @@ -0,0 +1,70 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2002 Oliver Burn +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; either +// version 2.1 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +//////////////////////////////////////////////////////////////////////////////// +package com.puppycrawl.tools.checkstyle; + +/** + * Contains utility methods. + * + * @author Oliver Burn + * @version 1.0 + */ +final class Utils +{ + /** stop instances being created **/ + private Utils() + { + } + + /** + * Returns whether the specified string contains only whitespace up to the + * specified index. + * + * @param aIndex index to check up to + * @param aLine the line to check + * @return whether there is only whitespace + */ + static boolean whitespaceBefore(int aIndex, String aLine) + { + for (int i = 0; i < aIndex; i++) { + if (!Character.isWhitespace(aLine.charAt(i))) { + return false; + } + } + return true; + } + + /** + * Returns the length of a string ignoring all trailing whitespace. It is a + * pity that there is not a trim() like method that only removed the + * trailing whitespace. + * @param aLine the string to process + * @return the length of the string ignoring all trailing whitespace + **/ + static int lengthMinusTrailingWhitespace(String aLine) + { + int len = aLine.length(); + for (int i = len - 1; i >= 0; i--) { + if (!Character.isWhitespace(aLine.charAt(i))) { + break; + } + len--; + } + return len; + } +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java index e6dd637b8..2b7e6deba 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java @@ -572,6 +572,57 @@ 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) + { + final String prevLine = mLines[aBrace.getLineNo() - 2]; + final String braceLine = mLines[aBrace.getLineNo() - 1]; + final LeftCurlyOption option = 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 constructor length is ok. diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g b/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g index 4db04ccb0..fe8e82f2d 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g @@ -64,6 +64,7 @@ tokens { final Verifier ver = VerifierSingleton.getInstance(); private static String sFirstIdent = ""; private static int sCompoundLength = -1; + private static final MyCommonAST[] sIgnoreAST = new MyCommonAST[2]; private static LineText sLastIdentifier; private static final int[] sIgnoreType = new int[1]; @@ -306,6 +307,7 @@ field! { java.util.List exs = new java.util.ArrayList(); MethodSignature msig = new MethodSignature(); + final MyCommonAST[] lcurls = new MyCommonAST[2]; } : // method, constructor, or variable declaration mods:modifiers[msig.getModSet()] @@ -315,7 +317,7 @@ field! ver.verifyMethod(msig); ver.reportStartMethodBlock(); } - s:constructorBody // constructor + s:constructorBody[msig.getLineNo()] // constructor {#field = #(#[CTOR_DEF,"CTOR_DEF"], mods, h, s);} {ver.reportEndMethodBlock();} @@ -344,7 +346,16 @@ field! ver.verifyMethod(msig); ver.reportStartMethodBlock(); } - ( s2:compoundStatement {ver.verifyMethodLength(#s2.getLineNo(), sCompoundLength);} | SEMI ) + ( + s2:compoundStatement[lcurls] + { + ver.verifyMethodLCurly(msig.getLineNo(), lcurls[0]); + ver.verifyMethodLength(#s2.getLineNo(), + sCompoundLength); + } + | + SEMI + ) {#field = #(#[METHOD_DEF,"METHOD_DEF"], mods, #(#[TYPE,"TYPE"],rt), @@ -359,16 +370,16 @@ field! ) // "static { ... }" class initializer - | "static" {ver.reportStartMethodBlock();} s3:compoundStatement {ver.reportEndMethodBlock();} + | "static" {ver.reportStartMethodBlock();} s3:compoundStatement[sIgnoreAST] {ver.reportEndMethodBlock();} {#field = #(#[STATIC_INIT,"STATIC_INIT"], s3);} // "{ ... }" instance initializer - | {ver.reportStartMethodBlock();} s4:compoundStatement {ver.reportEndMethodBlock();} + | {ver.reportStartMethodBlock();} s4:compoundStatement[sIgnoreAST] {ver.reportEndMethodBlock();} {#field = #(#[INSTANCE_INIT,"INSTANCE_INIT"], s4);} ; -constructorBody - : lc:LCURLY^ {#lc.setType(SLIST);} +constructorBody[int aLineNo] + : lc:LCURLY^ {#lc.setType(SLIST); ver.verifyMethodLCurly(aLineNo, #lc); } // Predicate might be slow but only checked once per constructor def // not for general methods. ( (explicitConstructorInvocation) => explicitConstructorInvocation @@ -511,11 +522,15 @@ parameterModifier // As a completely indepdent braced block of code inside a method // it starts a new scope for variable definitions -compoundStatement - : lc:LCURLY^ {#lc.setType(SLIST);} +compoundStatement[MyCommonAST[] aCurlies] + : lc:LCURLY^ {#lc.setType(SLIST); aCurlies[0] = #lc;} // include the (possibly-empty) list of statements (statement[sIgnoreType])* - rc:RCURLY! { sCompoundLength = rc.getLine() - lc.getLine() + 1; } + rc:RCURLY! + { + sCompoundLength = rc.getLine() - lc.getLine() + 1; + aCurlies[1] = #rc; + } ; @@ -526,7 +541,7 @@ statement[int[] aType] stmtType[0] = STMT_OTHER; } // A list of statements in curly braces -- start a new scope! - : compoundStatement { aType[0] = STMT_COMPOUND; } + : compoundStatement[sIgnoreAST] { aType[0] = STMT_COMPOUND; } // declarations are ambiguous with "ID DOT" relative to expression // statements. Must backtrack to be sure. Could use a semantic @@ -628,7 +643,7 @@ statement[int[] aType] | "throw"^ expression SEMI! // synchronize a statement - | ss:"synchronized"^ LPAREN! expression RPAREN! compoundStatement + | ss:"synchronized"^ LPAREN! expression RPAREN! compoundStatement[sIgnoreAST] {ver.verifyWSAroundBegin(ss.getLine(), ss.getColumn(), ss.getText());} // empty statement @@ -685,16 +700,16 @@ forIter // an exception handler try/catch block tryBlock - : t:"try"^ compoundStatement + : t:"try"^ compoundStatement[sIgnoreAST] { ver.verifyWSAroundBegin(t.getLine(), t.getColumn(), t.getText()); } (handler)* - ( "finally"^ compoundStatement )? + ( "finally"^ compoundStatement[sIgnoreAST] )? ; // an exception handler handler - : c:"catch"^ LPAREN! parameterDeclaration[new MethodSignature()] RPAREN! compoundStatement + : c:"catch"^ LPAREN! parameterDeclaration[new MethodSignature()] RPAREN! compoundStatement[sIgnoreAST] {ver.verifyWSAroundBegin(c.getLine(), c.getColumn(), c.getText());} ; diff --git a/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java b/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java index 61a2e3eab..d14144290 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java @@ -42,6 +42,7 @@ public class CheckerTest throws Exception { mConfig.setHeaderFile(getPath("java.header")); + mConfig.setLCurlyMethod(LeftCurlyOption.NL); } protected String getPath(String aFilename) @@ -591,4 +592,33 @@ public class CheckerTest }; verify(c, filepath, expected); } + + public void testLCurlyMethodIgnore() + throws Exception + { + mConfig.setLCurlyMethod(LeftCurlyOption.IGNORE); + mConfig.setJavadocScope(Scope.NOTHING); + final Checker c = createChecker(); + final String filepath = getPath("InputLeftCurlyMethod.java"); + assertNotNull(c); + final String[] expected = { + }; + verify(c, filepath, expected); + } + + public void testLCurlyMethodNL() + throws Exception + { + mConfig.setLCurlyMethod(LeftCurlyOption.NL); + mConfig.setJavadocScope(Scope.NOTHING); + final Checker c = createChecker(); + final String filepath = getPath("InputLeftCurlyMethod.java"); + assertNotNull(c); + final String[] expected = { + filepath + ":14: '{' should be on a new line.", + filepath + ":21: '{' should be on a new line.", + filepath + ":34: '{' should be on a new line.", + }; + verify(c, filepath, expected); + } } diff --git a/src/tests/com/puppycrawl/tools/checkstyle/InputLeftCurlyMethod.java b/src/tests/com/puppycrawl/tools/checkstyle/InputLeftCurlyMethod.java new file mode 100644 index 000000000..b201c0545 --- /dev/null +++ b/src/tests/com/puppycrawl/tools/checkstyle/InputLeftCurlyMethod.java @@ -0,0 +1,37 @@ +//////////////////////////////////////////////////////////////////////////////// +// Test case file for checkstyle. +// Created: 2001 +//////////////////////////////////////////////////////////////////////////////// +package com.puppycrawl.tools.checkstyle; + +/** + * Test case for correct use of braces. + * @author Oliver Burn + **/ +class InputLeftCurlyMethod +{ + InputLeftCurlyMethod() {} + InputLeftCurlyMethod(String aOne) { + } + InputLeftCurlyMethod(int aOne) + { + } + + void method1() {} + void method2() { + } + void method3() + { + } + void method4() + { + } + void method5(String aOne, + String aTwo) + { + } + void method6(String aOne, + String aTwo) { + } +} +