Major refactoring to support correct column numbers. Also removed all the old

code for whitespace checking around things. This required fixing the grammar
to not consume COLON tokens. Looks like you are onto something Lars with your
previous email (made it easy to find the bug).
This commit is contained in:
Oliver Burn 2002-10-01 08:47:26 +00:00
parent 81dab0c141
commit eb7b7bfd3e
14 changed files with 91 additions and 128 deletions

View File

@ -176,7 +176,7 @@ public class Checker
LocalizedMessage.setLocale(new Locale(mConfig.getLocaleLanguage(),
mConfig.getLocaleCountry()));
mMessages = new LocalizedMessages(mConfig.getTabWidth());
mWalker = new TreeWalker(mMessages);
mWalker = new TreeWalker(mMessages, mConfig.getTabWidth());
// TODO: improve the error handing
for (int i = 0; i < aConfigs.length; i++) {
final CheckConfiguration config = aConfigs[i];

View File

@ -52,6 +52,8 @@ class TreeWalker
private final Set mAllChecks = new HashSet();
/** collects the error messages */
private final LocalizedMessages mMessages;
/** the tab width for error reporting */
private final int mTabWidth;
// initialise the constants
static {
@ -82,9 +84,10 @@ class TreeWalker
*
* @param aMessages used to collect messages
*/
public TreeWalker(LocalizedMessages aMessages)
public TreeWalker(LocalizedMessages aMessages, int aTabWidth)
{
mMessages = aMessages;
mTabWidth = aTabWidth;
}
/**
@ -109,6 +112,7 @@ class TreeWalker
void registerCheck(Check aCheck, CheckConfiguration aConfig)
{
aCheck.setMessages(mMessages);
aCheck.setTabWidth(mTabWidth);
if (!aConfig.getTokens().isEmpty()) {
final Iterator it = aConfig.getTokens().iterator();
while (it.hasNext()) {

View File

@ -444,7 +444,6 @@ class Verifier
*/
void verifyWSAroundEnd(int aLineNo, int aColNo, String aText)
{
verifyWSAroundBegin(aLineNo, aColNo - aText.length(), aText);
}
@ -456,23 +455,6 @@ class Verifier
*/
void verifyWSAroundBegin(int aLineNo, int aColNo, String aText)
{
if (mConfig.isIgnoreWhitespace()) {
return;
}
final String line = mLines[aLineNo - 1];
final int before = aColNo - 2;
final int after = aColNo + aText.length() - 1;
if ((before >= 0) && !Character.isWhitespace(line.charAt(before))) {
mMessages.add(aLineNo, before + 1, "ws.notPreceeded", aText);
}
if ((after < line.length())
&& !Character.isWhitespace(line.charAt(after)))
{
mMessages.add(aLineNo, after, "ws.notFollowed", aText);
}
}

View File

@ -47,6 +47,8 @@ public abstract class Check
private Map mTreeContext;
/** the context for a check across a token. */
private Map mTokenContext;
/** the tab with for column reporting */
private int mTabWidth = 8; // meaningful default
/**
* Returns the default token a check is interested in. Only used if the
@ -182,7 +184,7 @@ public abstract class Check
* Returns the lines associated with the tree.
* @return the file contents
*/
protected final String[] getLines()
public final String[] getLines()
{
return (String[]) getTreeContext().get(LINES_ATTRIBUTE);
}
@ -196,11 +198,26 @@ public abstract class Check
getTreeContext().put(FILENAME_ATTRIBUTE, aFilename);
}
/** @return the tab width to report errors with */
public int getTabWidth()
{
return mTabWidth;
}
/**
* Set the tab width to report errors with
* @param aTabWidth an <code>int</code> value
*/
public void setTabWidth(int aTabWidth)
{
mTabWidth = aTabWidth;
}
/**
* Returns the filename associated with the tree.
* @return the file name
*/
protected final String getFilename()
public final String getFilename()
{
return (String) getTreeContext().get(FILENAME_ATTRIBUTE);
}
@ -211,7 +228,7 @@ public abstract class Check
* @param aLine the line number where the error was found
* @param aKey the message that describes the error
*/
protected final void log(int aLine, String aKey)
public final void log(int aLine, String aKey)
{
log(aLine, aKey, EMPTY_OBJECT_ARRAY);
}
@ -227,7 +244,8 @@ public abstract class Check
*/
protected final void log(int aLine, String aKey, Object aArgs[])
{
log(aLine, 0, aKey, aArgs);
mMessages.add(new LocalizedMessage(
aLine, getResourceBundle(), aKey, aArgs));
}
@ -323,12 +341,10 @@ public abstract class Check
*/
public void log(int aLineNo, int aColNo, String aKey, Object[] aArgs)
{
// TODO: need to fix this ugly hack below!!!!!!
final int col = aColNo + 1;
// final int col = 1 + Utils.lengthExpandedTabs(
// mLines[aLineNo - 1], aColNo, mTabWidth);
final int col = 1 + Utils.lengthExpandedTabs(
getLines()[aLineNo - 1], aColNo, getTabWidth());
mMessages.add(new LocalizedMessage(
aLineNo, col, getResourceBundle(), aKey, aArgs));
aLineNo, col, getResourceBundle(), aKey, aArgs));
}

View File

@ -34,9 +34,9 @@ public class LocalizedMessages
{
/** contains the messages logged **/
private final ArrayList mMessages = new ArrayList();
/** the tabwidth to calculate columns **/
/** the tabwidth to calculate columns TODO: remove */
private final int mTabWidth;
/** the lines of the file being checked **/
/** the lines of the file being checked, TODO: remove */
private String[] mLines;
/** Name of the exising resource bundle
* TODO: remove this

View File

@ -32,7 +32,7 @@ public class WhitespaceAroundCheck
{
return new int[] {
QUESTION, // '?'
// COLON, // ':' TODO: dont flag after "case"
COLON, // ':' TODO: dont flag after "case"
ASSIGN, // '='
EQUAL, // "=="
NOT_EQUAL, // "!="
@ -64,15 +64,16 @@ public class WhitespaceAroundCheck
BAND, // '&'
BAND_ASSIGN, // "&="
LAND, // "&&"
LITERAL_if,
LITERAL_else,
LITERAL_for,
LITERAL_do,
LITERAL_return,
LITERAL_try,
LITERAL_catch,
LITERAL_do,
LITERAL_else,
LITERAL_finally,
LITERAL_for,
LITERAL_if,
LITERAL_return,
LITERAL_synchronized,
LITERAL_try,
LITERAL_while,
ASSERT // TODO: why is it not LITERAL_assert?
// maybe it's a bug in the grammar?
};

View File

@ -49,7 +49,7 @@ statement
// assert statement, available since JDK 1.4
assertStatement
: ASSERT^ expression ( COLON! expression )? SEMI!
: ASSERT^ expression ( COLON expression )? SEMI!
;
class Java14Lexer extends JavaLexer;

View File

@ -655,7 +655,7 @@ assignmentExpression
// conditional test (level 12)
conditionalExpression
: logicalOrExpression
( QUESTION^ assignmentExpression COLON! conditionalExpression )?
( QUESTION^ assignmentExpression COLON conditionalExpression )?
;

View File

@ -18,9 +18,9 @@ extends BaseCheckTestCase
final Checker c = createChecker(checkConfig);
final String fname = getPath("InputImport.java");
final String[] expected = {
"7:1: Avoid using the '.*' form of import.",
"9:1: Avoid using the '.*' form of import.",
"10:1: Avoid using the '.*' form of import.",
"7: Avoid using the '.*' form of import.",
"9: Avoid using the '.*' form of import.",
"10: Avoid using the '.*' form of import.",
};
verify(c, fname, expected);
}

View File

@ -119,27 +119,12 @@ public class CheckerTest
filepath + ":5:12: '.' is preceeded with whitespace.",
filepath + ":5:14: '.' is followed by whitespace.",
filepath + ":13: Type Javadoc comment is missing an @author tag.",
filepath + ":16:22: '=' is not preceeded with whitespace.",
filepath + ":16:23: '=' is not followed by whitespace.",
filepath + ":18:24: '=' is not followed by whitespace.",
filepath + ":26:14: '=' is not preceeded with whitespace.",
filepath + ":27:10: '=' is not preceeded with whitespace.",
filepath + ":27:11: '=' is not followed by whitespace.",
filepath + ":28:10: '+=' is not preceeded with whitespace.",
filepath + ":28:12: '+=' is not followed by whitespace.",
filepath + ":29:13: '-=' is not followed by whitespace.",
filepath + ":29:14: '-' is followed by whitespace.",
filepath + ":29:21: '+' is followed by whitespace.",
filepath + ":30:14: '++' is preceeded with whitespace.",
filepath + ":30:21: '--' is preceeded with whitespace.",
filepath + ":31:15: '++' is followed by whitespace.",
filepath + ":31:22: '--' is followed by whitespace.",
filepath + ":37:21: 'synchronized' is not followed by whitespace.",
filepath + ":39:12: 'try' is not followed by whitespace.",
filepath + ":39:12: '{' is not preceeded with whitespace.",
filepath + ":41:14: 'catch' is not followed by whitespace.",
filepath + ":41:34: '{' is not preceeded with whitespace.",
filepath + ":58:11: 'if' is not followed by whitespace.",
filepath + ":58:12: '(' is followed by whitespace.",
filepath + ":58:36: ')' is preceeded with whitespace.",
filepath + ":59:9: '{' should be on the previous line.",
@ -147,34 +132,14 @@ public class CheckerTest
filepath + ":74:13: '(' is followed by whitespace.",
filepath + ":74:18: ')' is preceeded with whitespace.",
filepath + ":75:9: '{' should be on the previous line.",
filepath + ":76:19: 'return' is not followed by whitespace.",
filepath + ":79:9: '{' should be on the previous line.",
filepath + ":88:21: 'cast' is not followed by whitespace.",
filepath + ":97:29: '?' is not preceeded with whitespace.",
filepath + ":97:30: '?' is not followed by whitespace.",
filepath + ":97:34: ':' is not preceeded with whitespace.",
filepath + ":97:35: ':' is not followed by whitespace.",
filepath + ":98:15: '==' is not preceeded with whitespace.",
filepath + ":98:17: '==' is not followed by whitespace.",
filepath + ":104:20: '*' is not followed by whitespace.",
filepath + ":104:21: '*' is not preceeded with whitespace.",
filepath + ":111:22: '!' is followed by whitespace.",
filepath + ":112:23: '~' is followed by whitespace.",
filepath + ":119:18: '%' is not preceeded with whitespace.",
filepath + ":120:20: '%' is not followed by whitespace.",
filepath + ":121:18: '%' is not preceeded with whitespace.",
filepath + ":121:19: '%' is not followed by whitespace.",
filepath + ":123:18: '/' is not preceeded with whitespace.",
filepath + ":124:20: '/' is not followed by whitespace.",
filepath + ":125:18: '/' is not preceeded with whitespace.",
filepath + ":125:19: '/' is not followed by whitespace.",
filepath + ":129:17: '.' is preceeded with whitespace.",
filepath + ":129:24: '.' is followed by whitespace.",
filepath + ":136:10: '.' is preceeded with whitespace.",
filepath + ":136:12: '.' is followed by whitespace.",
filepath + ":153:15: 'assert' is not followed by whitespace.",
filepath + ":156:20: ':' is not preceeded with whitespace.",
filepath + ":156:21: ':' is not followed by whitespace.",
};
verify(c, filepath, expected);
c.destroy();
@ -195,15 +160,6 @@ public class CheckerTest
filepath + ":5:12: '.' is preceeded with whitespace.",
filepath + ":5:14: '.' is followed by whitespace.",
filepath + ":13: Type Javadoc comment is missing an @author tag.",
filepath + ":16:22: '=' is not preceeded with whitespace.",
filepath + ":16:23: '=' is not followed by whitespace.",
filepath + ":18:24: '=' is not followed by whitespace.",
filepath + ":26:14: '=' is not preceeded with whitespace.",
filepath + ":27:10: '=' is not preceeded with whitespace.",
filepath + ":27:11: '=' is not followed by whitespace.",
filepath + ":28:10: '+=' is not preceeded with whitespace.",
filepath + ":28:12: '+=' is not followed by whitespace.",
filepath + ":29:13: '-=' is not followed by whitespace.",
filepath + ":29:14: '-' is followed by whitespace.",
filepath + ":29:20: '(' is not followed by whitespace.",
filepath + ":29:21: '+' is followed by whitespace.",
@ -212,20 +168,13 @@ public class CheckerTest
filepath + ":30:21: '--' is preceeded with whitespace.",
filepath + ":31:15: '++' is followed by whitespace.",
filepath + ":31:22: '--' is followed by whitespace.",
filepath + ":37:21: 'synchronized' is not followed by whitespace.",
filepath + ":37:22: '(' is not followed by whitespace.",
filepath + ":37:25: ')' is not preceeded with whitespace.",
filepath + ":39:12: 'try' is not followed by whitespace.",
filepath + ":39:12: '{' is not preceeded with whitespace.",
filepath + ":41:14: 'catch' is not followed by whitespace.",
filepath + ":41:15: '(' is not followed by whitespace.",
filepath + ":41:32: ')' is not preceeded with whitespace.",
filepath + ":41:34: '{' is not preceeded with whitespace.",
filepath + ":58:11: 'if' is not followed by whitespace.",
filepath + ":59:9: '{' should be on the previous line.",
filepath + ":63:9: '{' should be on the previous line.",
filepath + ":75:9: '{' should be on the previous line.",
filepath + ":76:19: 'return' is not followed by whitespace.",
filepath + ":76:20: '(' is not followed by whitespace.",
filepath + ":76:20: ')' is not preceeded with whitespace.",
filepath + ":79:9: '{' should be on the previous line.",
@ -239,37 +188,18 @@ public class CheckerTest
filepath + ":90:19: ')' is not preceeded with whitespace.",
filepath + ":97:22: '(' is not followed by whitespace.",
filepath + ":97:27: ')' is not preceeded with whitespace.",
filepath + ":97:29: '?' is not preceeded with whitespace.",
filepath + ":97:30: '?' is not followed by whitespace.",
filepath + ":97:34: ':' is not preceeded with whitespace.",
filepath + ":97:35: ':' is not followed by whitespace.",
filepath + ":98:14: '(' is not followed by whitespace.",
filepath + ":98:15: '==' is not preceeded with whitespace.",
filepath + ":98:17: '==' is not followed by whitespace.",
filepath + ":98:17: ')' is not preceeded with whitespace.",
filepath + ":104:20: '*' is not followed by whitespace.",
filepath + ":104:21: '*' is not preceeded with whitespace.",
filepath + ":111:22: '!' is followed by whitespace.",
filepath + ":112:23: '~' is followed by whitespace.",
filepath + ":119:18: '%' is not preceeded with whitespace.",
filepath + ":120:20: '%' is not followed by whitespace.",
filepath + ":121:18: '%' is not preceeded with whitespace.",
filepath + ":121:19: '%' is not followed by whitespace.",
filepath + ":123:18: '/' is not preceeded with whitespace.",
filepath + ":124:20: '/' is not followed by whitespace.",
filepath + ":125:18: '/' is not preceeded with whitespace.",
filepath + ":125:19: '/' is not followed by whitespace.",
filepath + ":129:17: '.' is preceeded with whitespace.",
filepath + ":129:24: '.' is followed by whitespace.",
filepath + ":136:10: '.' is preceeded with whitespace.",
filepath + ":136:12: '.' is followed by whitespace.",
filepath + ":150:28: '(' is not followed by whitespace.",
filepath + ":150:31: ')' is not preceeded with whitespace.",
filepath + ":153:15: 'assert' is not followed by whitespace.",
filepath + ":153:16: '(' is not followed by whitespace.",
filepath + ":153:19: ')' is not preceeded with whitespace.",
filepath + ":156:20: ':' is not preceeded with whitespace.",
filepath + ":156:21: ':' is not followed by whitespace.",
};
verify(c, filepath, expected);
}
@ -302,12 +232,10 @@ public class CheckerTest
final String[] expected = {
filepath + ":29: 'do' construct must use '{}'s.",
filepath + ":41: 'while' construct must use '{}'s.",
filepath + ":41:14: 'while' is not followed by whitespace.",
filepath + ":42: 'while' construct must use '{}'s.",
filepath + ":44: 'while' construct must use '{}'s.",
filepath + ":45: 'if' construct must use '{}'s.",
filepath + ":58: 'for' construct must use '{}'s.",
filepath + ":58:12: 'for' is not followed by whitespace.",
filepath + ":58:23: ';' is not followed by whitespace.",
filepath + ":58:29: ';' is not followed by whitespace.",
filepath + ":59: 'for' construct must use '{}'s.",
@ -332,8 +260,6 @@ public class CheckerTest
final Checker c = createChecker();
final String filepath = getPath("InputBraces.java");
final String[] expected = {
filepath + ":41:14: 'while' is not followed by whitespace.",
filepath + ":58:12: 'for' is not followed by whitespace.",
filepath + ":58:23: ';' is not followed by whitespace.",
filepath + ":58:29: ';' is not followed by whitespace.",
};
@ -497,12 +423,6 @@ public class CheckerTest
filepath + ":132:20: Name 'InnerBlockVariable' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
filepath + ":142:30: Name 'BAD__NAME' must match pattern '^[A-Z](_?[A-Z0-9]+)*$'.",
filepath + ":145: Line is longer than 80 characters.",
filepath + ":153:27: '=' is not followed by whitespace.",
filepath + ":154:27: '=' is not followed by whitespace.",
filepath + ":155:27: '=' is not followed by whitespace.",
filepath + ":156:27: '=' is not followed by whitespace.",
filepath + ":157:27: '=' is not followed by whitespace.",
filepath + ":158:27: '=' is not followed by whitespace.",
filepath + ":161: Comment matches to-do format 'FIXME:'.",
filepath + ":162: Comment matches to-do format 'FIXME:'.",
filepath + ":163: Comment matches to-do format 'FIXME:'.",

View File

@ -23,7 +23,7 @@ public class FileLengthCheckTest
public void testAlarm() throws Exception
{
final String[] expected = {
"1:1: File length is 198 lines (max allowed is 20)."
"1: File length is 198 lines (max allowed is 20)."
};
runIt("20", expected);
}

View File

@ -19,7 +19,7 @@ public class HeaderCheckTest extends BaseCheckTestCase
final Checker c = createChecker(checkConfig);
final String fname = getPath("inputHeader.java");
final String[] expected = {
"1:1: Missing a header - not enough lines in file."
"1: Missing a header - not enough lines in file."
};
verify(c, fname, expected);
}
@ -34,7 +34,7 @@ public class HeaderCheckTest extends BaseCheckTestCase
final Checker c = createChecker(checkConfig);
final String fname = getPath("InputScopeAnonInner.java");
final String[] expected = {
"3:1: Line does not match expected header line of '// Created: 2002'."
"3: Line does not match expected header line of '// Created: 2002'."
};
verify(c, fname, expected);
}

View File

@ -21,7 +21,7 @@ public class TreeWalkerTest
public void testCreate()
{
new TreeWalker(new LocalizedMessages(0));
new TreeWalker(new LocalizedMessages(0), 8);
assertTrue(true);
}
}

View File

@ -3,7 +3,7 @@ package com.puppycrawl.tools.checkstyle;
import com.puppycrawl.tools.checkstyle.checks.WhitespaceAroundCheck;
public class WhitespaceAroundTest
extends BaseCheckTestCase
extends BaseCheckTestCase
{
public WhitespaceAroundTest(String aName)
{
@ -29,11 +29,15 @@ public class WhitespaceAroundTest
"29:13: '-=' is not followed by whitespace.",
"37:21: 'synchronized' is not followed by whitespace.",
"39:12: 'try' is not followed by whitespace.",
//"39:12: '{' is not preceeded with whitespace.",
"41:14: 'catch' is not followed by whitespace.",
//filepath + ":41:34: '{' is not preceeded with whitespace.",
"58:11: 'if' is not followed by whitespace.",
"76:19: 'return' is not followed by whitespace.",
"97:29: '?' is not preceeded with whitespace.",
"97:30: '?' is not followed by whitespace.",
"97:34: ':' is not preceeded with whitespace.",
"97:35: ':' is not followed by whitespace.",
"98:15: '==' is not preceeded with whitespace.",
"98:17: '==' is not followed by whitespace.",
"104:20: '*' is not followed by whitespace.",
@ -47,6 +51,42 @@ public class WhitespaceAroundTest
"125:18: '/' is not preceeded with whitespace.",
"125:19: '/' is not followed by whitespace.",
"153:15: 'assert' is not followed by whitespace.",
"156:20: ':' is not preceeded with whitespace.",
"156:21: ':' is not followed by whitespace.",
};
verify(c, fname, expected);
}
public void testIt2()
throws Exception
{
final CheckConfiguration checkConfig = new CheckConfiguration();
checkConfig.setClassname(WhitespaceAroundCheck.class.getName());
final Checker c = createChecker(checkConfig);
final String fname = getPath("InputSimple.java");
final String[] expected = {
"153:27: '=' is not followed by whitespace.",
"154:27: '=' is not followed by whitespace.",
"155:27: '=' is not followed by whitespace.",
"156:27: '=' is not followed by whitespace.",
"157:27: '=' is not followed by whitespace.",
"158:27: '=' is not followed by whitespace.",
};
verify(c, fname, expected);
}
public void testIt3()
throws Exception
{
final CheckConfiguration checkConfig = new CheckConfiguration();
checkConfig.setClassname(WhitespaceAroundCheck.class.getName());
final Checker c = createChecker(checkConfig);
final String fname = getPath("InputBraces.java");
final String[] expected = {
"41:14: 'while' is not followed by whitespace.",
"58:12: 'for' is not followed by whitespace.",
// + ":58:23: ';' is not followed by whitespace.",
// + ":58:29: ';' is not followed by whitespace.",
};
verify(c, fname, expected);
}