Now have '{' checking for methods and types. Also put in place the logic to

be able to check the others.
This commit is contained in:
Oliver Burn 2002-02-21 22:43:55 +00:00
parent c85ec6f26a
commit fd24216f46
8 changed files with 142 additions and 69 deletions

View File

@ -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">
<fileset dir="src/checkstyle"
includes="**/*.java"
excludes="**/Generated*.java"/>

View File

@ -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;
}
}

View File

@ -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

View File

@ -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";
}

View File

@ -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.

View File

@ -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.");
}
}
}
// }}}
}

View File

@ -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

View File

@ -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)