From 57d8e4b099c3ff185e85b9633cb9a911bf37d8cc Mon Sep 17 00:00:00 2001 From: Oliver Burn Date: Sun, 13 Oct 2002 10:42:15 +0000 Subject: [PATCH] Implement left curly check for type definitions. Now about to implement for methods to refactor the common code. --- .../tools/checkstyle/Configuration.java | 44 +------ .../com/puppycrawl/tools/checkstyle/Defn.java | 3 - .../puppycrawl/tools/checkstyle/Verifier.java | 1 - .../checkstyle/checks/TypeLeftCurlyCheck.java | 113 ++++++++++++++++++ .../puppycrawl/tools/checkstyle/java_new.g | 2 +- .../tools/checkstyle/CheckerTest.java | 2 - .../tools/checkstyle/RightCurlyCheckTest.java | 3 +- .../checkstyle/TypeLeftCurlyCheckTest.java | 73 +++++++++++ 8 files changed, 192 insertions(+), 49 deletions(-) create mode 100644 src/checkstyle/com/puppycrawl/tools/checkstyle/checks/TypeLeftCurlyCheck.java create mode 100644 src/tests/com/puppycrawl/tools/checkstyle/TypeLeftCurlyCheckTest.java diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java index b2d7bb1fa..3e944fc39 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java @@ -18,6 +18,10 @@ //////////////////////////////////////////////////////////////////////////////// package com.puppycrawl.tools.checkstyle; +import com.puppycrawl.tools.checkstyle.api.Utils; +import org.apache.regexp.RE; +import org.apache.regexp.RESyntaxException; + import java.io.FileNotFoundException; import java.io.IOException; import java.io.InvalidObjectException; @@ -32,11 +36,6 @@ import java.util.Map; import java.util.Properties; import java.util.Set; -import com.puppycrawl.tools.checkstyle.api.Utils; -import com.puppycrawl.tools.checkstyle.checks.RightCurlyOption; -import org.apache.regexp.RE; -import org.apache.regexp.RESyntaxException; - /** * Represents the configuration that checkstyle uses when checking. The * configuration is Serializable, however the ClassLoader configuration is @@ -117,7 +116,6 @@ public class Configuration { mLCurliesProps.put(Defn.LCURLY_METHOD_PROP, LeftCurlyOption.EOL); - mLCurliesProps.put(Defn.LCURLY_TYPE_PROP, LeftCurlyOption.EOL); mLCurliesProps.put(Defn.LCURLY_OTHER_PROP, LeftCurlyOption.EOL); } @@ -560,12 +558,6 @@ public class Configuration return getLeftCurlyOptionProperty(Defn.LCURLY_METHOD_PROP); } - /** @return the left curly placement option for types **/ - LeftCurlyOption getLCurlyType() - { - return getLeftCurlyOptionProperty(Defn.LCURLY_TYPE_PROP); - } - /** @return the left curly placement option for others **/ LeftCurlyOption getLCurlyOther() { @@ -791,34 +783,6 @@ public class Configuration } } - /** - * @param aProps the properties set to use - * @param aLog where to log errors to - * @param aName the name of the property to parse - * @param aDefault the default value to use. - * - * @return the value of a RightCurlyOption property. If the property is not - * defined or cannot be decoded, then a default value is returned. - */ - private static RightCurlyOption getRightCurlyOptionProperty( - Properties aProps, - String aName, - RightCurlyOption aDefault, - PrintStream aLog) - { - RightCurlyOption retVal = aDefault; - final String strRep = aProps.getProperty(aName); - if (strRep != null) { - retVal = RightCurlyOption.decode(strRep); - if (retVal == null) { - aLog.println("Unable to parse " + aName - + " property with value " + strRep - + ", defaulting to " + aDefault + "."); - } - } - return retVal; - } - /** * Set the value of a BlockOption property. * @param aProps the properties set to use diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java index 43191437f..f48cc2634 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java @@ -83,8 +83,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"; /** property name for lcurly placement for others **/ String LCURLY_OTHER_PROP = "checkstyle.lcurly.other"; /** property name for padding around parenthesis **/ @@ -165,7 +163,6 @@ public interface Defn String[] ALL_LCURLY_PROPS = new String[] { LCURLY_METHOD_PROP, - LCURLY_TYPE_PROP, LCURLY_OTHER_PROP, }; diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java index 14a422be4..c1054c5d2 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java @@ -599,7 +599,6 @@ class Verifier */ void verifyLCurlyType(int aTypeLine, MyCommonAST aBrace) { - checkLCurly(aTypeLine, aBrace, mConfig.getLCurlyType()); } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/TypeLeftCurlyCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/TypeLeftCurlyCheck.java new file mode 100644 index 000000000..f2cdbfebe --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/TypeLeftCurlyCheck.java @@ -0,0 +1,113 @@ +//////////////////////////////////////////////////////////////////////////////// +// 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.checks; + +import com.puppycrawl.tools.checkstyle.JavaTokenTypes; +import com.puppycrawl.tools.checkstyle.LeftCurlyOption; +import com.puppycrawl.tools.checkstyle.api.Check; +import com.puppycrawl.tools.checkstyle.api.DetailAST; +import com.puppycrawl.tools.checkstyle.api.Utils; + +public class TypeLeftCurlyCheck + extends Check +{ + private LeftCurlyOption mOption = LeftCurlyOption.EOL; + // TODO: remove hack + private int mMaxLineLength = 80; + + /** @see Check */ + public int[] getDefaultTokens() + { + return new int[] {JavaTokenTypes.INTERFACE_DEF, + JavaTokenTypes.CLASS_DEF}; + } + + /** @see Check */ + public void visitToken(DetailAST aAST) + { + final DetailAST brace = (DetailAST) + Utils.getLastSibling(aAST.getFirstChild()) + .getFirstChild(); + // TODO: should check for modifiers + final DetailAST startToken = + (DetailAST) aAST.getFirstChild().getNextSibling(); + + final String braceLine = getLines()[brace.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 = (brace.getLineNo() == 1) + ? mMaxLineLength + : Utils.lengthMinusTrailingWhitespace( + getLines()[brace.getLineNo() - 2]); + + // Check for being told to ignore, or have '{}' which is a special case + if ((mOption == LeftCurlyOption.IGNORE) + || ((braceLine.length() > (brace.getColumnNo() + 1)) + && (braceLine.charAt(brace.getColumnNo() + 1) == '}'))) + { + // ignore + } + else if (mOption == LeftCurlyOption.NL) { + if (!Utils.whitespaceBefore(brace.getColumnNo(), braceLine)) { + log(brace.getLineNo(), brace.getColumnNo(), + "line.new", "{"); + } + } + else if (mOption == LeftCurlyOption.EOL) { + if (Utils.whitespaceBefore(brace.getColumnNo(), braceLine) + && ((prevLineLen + 2) <= mMaxLineLength)) + { + log(brace.getLineNo(), brace.getColumnNo(), + "line.previous", "{"); + } + } + else if (mOption == LeftCurlyOption.NLOW) { + if (startToken.getLineNo() == brace.getLineNo()) { + // all ok as on the same line + } + else if ((startToken.getLineNo() + 1) == brace.getLineNo()) { + if (!Utils.whitespaceBefore(brace.getColumnNo(), braceLine)) { + log(brace.getLineNo(), brace.getColumnNo(), + "line.new", "{"); + } + else if ((prevLineLen + 2) <= mMaxLineLength) { + log(brace.getLineNo(), brace.getColumnNo(), + "line.previous", "{"); + } + } + else if (!Utils.whitespaceBefore(brace.getColumnNo(), braceLine)) { + log(brace.getLineNo(), brace.getColumnNo(), + "line.new", "{"); + } + } + } + + public void setOption(String aFromStr) + { + mOption = LeftCurlyOption.decode(aFromStr); + } + + public void setMaxLineLength(int aMaxLineLength) + { + mMaxLineLength = aMaxLineLength; + } +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/java_new.g b/src/checkstyle/com/puppycrawl/tools/checkstyle/java_new.g index 9c9422dbc..c747f906e 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/java_new.g +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/java_new.g @@ -222,7 +222,7 @@ interfaceDefinition![DetailAST modifiers] // 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! + : LCURLY ( field | SEMI! )* RCURLY {#classBlock = #([OBJBLOCK, "OBJBLOCK"], #classBlock);} diff --git a/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java b/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java index ea336dd33..ff840d0dc 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/CheckerTest.java @@ -50,8 +50,6 @@ public class CheckerTest LeftCurlyOption.NL.toString()); mProps.setProperty(Defn.LCURLY_OTHER_PROP, LeftCurlyOption.NLOW.toString()); - mProps.setProperty(Defn.LCURLY_TYPE_PROP, - LeftCurlyOption.NL.toString()); mProps.setProperty(Defn.ALLOW_NO_AUTHOR_PROP, Boolean.TRUE.toString()); mProps.setProperty(Defn.LOCALE_COUNTRY_PROP, Locale.ENGLISH.getCountry()); diff --git a/src/tests/com/puppycrawl/tools/checkstyle/RightCurlyCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/RightCurlyCheckTest.java index 7067e79dc..363affb25 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/RightCurlyCheckTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/RightCurlyCheckTest.java @@ -1,11 +1,10 @@ package com.puppycrawl.tools.checkstyle; -import com.puppycrawl.tools.checkstyle.checks.UpperEllCheck; import com.puppycrawl.tools.checkstyle.checks.RightCurlyCheck; import com.puppycrawl.tools.checkstyle.checks.RightCurlyOption; public class RightCurlyCheckTest -extends BaseCheckTestCase + extends BaseCheckTestCase { public RightCurlyCheckTest(String aName) { diff --git a/src/tests/com/puppycrawl/tools/checkstyle/TypeLeftCurlyCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/TypeLeftCurlyCheckTest.java new file mode 100644 index 000000000..a786c5468 --- /dev/null +++ b/src/tests/com/puppycrawl/tools/checkstyle/TypeLeftCurlyCheckTest.java @@ -0,0 +1,73 @@ +package com.puppycrawl.tools.checkstyle; + +import com.puppycrawl.tools.checkstyle.checks.TypeLeftCurlyCheck; + +public class TypeLeftCurlyCheckTest + extends BaseCheckTestCase +{ + public TypeLeftCurlyCheckTest(String aName) + { + super(aName); + } + + public void testDefault() + throws Exception + { + final CheckConfiguration checkConfig = new CheckConfiguration(); + checkConfig.setClassname(TypeLeftCurlyCheck.class.getName()); + final Checker c = createChecker(checkConfig); + final String fname = getPath("InputScopeInnerInterfaces.java"); + final String[] expected = { + "8:1: '{' should be on the previous line.", + "12:5: '{' should be on the previous line.", + "21:5: '{' should be on the previous line.", + "30:5: '{' should be on the previous line.", + "39:5: '{' should be on the previous line.", + }; + verify(c, fname, expected); + } + + public void testNL() + throws Exception + { + final CheckConfiguration checkConfig = new CheckConfiguration(); + checkConfig.setClassname(TypeLeftCurlyCheck.class.getName()); + checkConfig.addProperty("option", LeftCurlyOption.NL.toString()); + final Checker c = createChecker(checkConfig); + final String fname = getPath("InputScopeInnerInterfaces.java"); + final String[] expected = { + }; + verify(c, fname, expected); + } + + public void testNLOW() + throws Exception + { + final CheckConfiguration checkConfig = new CheckConfiguration(); + checkConfig.setClassname(TypeLeftCurlyCheck.class.getName()); + checkConfig.addProperty("option", LeftCurlyOption.NLOW.toString()); + final Checker c = createChecker(checkConfig); + final String fname = getPath("InputScopeInnerInterfaces.java"); + final String[] expected = { + "8:1: '{' should be on the previous line.", + "12:5: '{' should be on the previous line.", + "21:5: '{' should be on the previous line.", + "30:5: '{' should be on the previous line.", + "39:5: '{' should be on the previous line.", + }; + verify(c, fname, expected); + } + + public void testIgnore() + throws Exception + { + final CheckConfiguration checkConfig = new CheckConfiguration(); + checkConfig.setClassname(TypeLeftCurlyCheck.class.getName()); + checkConfig.addProperty("option", LeftCurlyOption.IGNORE.toString()); + final Checker c = createChecker(checkConfig); + final String fname = getPath("InputScopeInnerInterfaces.java"); + final String[] expected = { + }; + verify(c, fname, expected); + } +}