Navigation Menu

Skip to content

Commit

Permalink
No Whitespace After Check, fixed NPE, fixed false-positives at multid…
Browse files Browse the repository at this point in the history 
…imensional arrays, issue #542
  • Loading branch information
@alexkravin @romani
alexkravin authored and romani committed Dec 30, 2014
1 parent a34fe7f commit e6638c3
Show file tree
Hide file tree
Showing 8 changed files with 301 additions and 24 deletions.
1 change: 1 addition & 0 deletions checkstyle_checks.xml
View file
Expand Up @@ -130,6 +130,7 @@
<property name="tokens" value="LNOT"/>
<property name="tokens" value="UNARY_MINUS"/>
<property name="tokens" value="UNARY_PLUS"/>
<property name="tokens" value="ARRAY_DECLARATOR"/>
</module>

<module name="NoWhitespaceBefore"/>
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheck.java
View file
Expand Up @@ -59,7 +59,7 @@ public class UnnecessaryParenthesesCheck extends Check
private static final int MAX_QUOTED_LENGTH = 25;

/** Token types for literals. */
private static final int [] LITERALS = {
private static final int[] LITERALS = {
TokenTypes.NUM_DOUBLE,
TokenTypes.NUM_FLOAT,
TokenTypes.NUM_INT,
Expand All @@ -71,7 +71,7 @@ public class UnnecessaryParenthesesCheck extends Check
};

/** Token types for assignment operations. */
private static final int [] ASSIGNMENTS = {
private static final int[] ASSIGNMENTS = {
TokenTypes.ASSIGN,
TokenTypes.BAND_ASSIGN,
TokenTypes.BOR_ASSIGN,
Expand All @@ -97,7 +97,7 @@ public class UnnecessaryParenthesesCheck extends Check
@Override
public int[] getDefaultTokens()
{
return new int [] {
return new int[] {
TokenTypes.EXPR,
TokenTypes.IDENT,
TokenTypes.NUM_DOUBLE,
Expand Down Expand Up @@ -261,7 +261,7 @@ private boolean exprSurrounded(DetailAST aAST)
* @return <code>true</code> if <code>aType</code> was found in <code>
* aTokens</code>.
*/
private boolean inTokenList(int aType, int [] aTokens)
private boolean inTokenList(int aType, int[] aTokens)
{
// NOTE: Given the small size of the two arrays searched, I'm not sure
// it's worth bothering with doing a binary search or using a
Expand Down
212 changes: 198 additions & 14 deletions src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/NoWhitespaceAfterCheck.java
View file
Expand Up @@ -39,6 +39,7 @@
* {@link TokenTypes#INC INC},
* {@link TokenTypes#LNOT LNOT},
* {@link TokenTypes#UNARY_MINUS UNARY_MINUS},
* {@link TokenTypes#ARRAY_DECLARATOR ARRAY_DECLARATOR},
* {@link TokenTypes#UNARY_PLUS UNARY_PLUS}. It also supports the operator
* {@link TokenTypes#TYPECAST TYPECAST}.
* </p>
Expand All @@ -59,6 +60,7 @@
* </pre>
* @author Rick Giles
* @author lkuehne
* @author <a href="mailto:nesterenko-aleksey@list.ru">Aleksey Nesterenko</a>
* @version 1.0
*/
public class NoWhitespaceAfterCheck extends Check
Expand All @@ -78,6 +80,7 @@ public int[] getDefaultTokens()
TokenTypes.BNOT,
TokenTypes.LNOT,
TokenTypes.DOT,
TokenTypes.ARRAY_DECLARATOR,
};
}

Expand All @@ -94,34 +97,150 @@ public int[] getAcceptableTokens()
TokenTypes.LNOT,
TokenTypes.DOT,
TokenTypes.TYPECAST,
TokenTypes.ARRAY_DECLARATOR,
};
}

@Override
public void visitToken(DetailAST aAST)
{
DetailAST targetAST = aAST;
if (targetAST.getType() == TokenTypes.TYPECAST) {
targetAST = targetAST.findFirstToken(TokenTypes.RPAREN);
DetailAST ast = aAST;
if (aAST.getType() == TokenTypes.ARRAY_DECLARATOR
|| aAST.getType() == TokenTypes.TYPECAST)
{
ast = getPreceded(aAST);
}

final String line = getLine(aAST.getLineNo() - 1);
final int after =
targetAST.getColumnNo() + targetAST.getText().length();
final int after = getPositionAfter(ast);

if ((after >= line.length() || Character.isWhitespace(line.charAt(after)))
&& hasRedundantWhitespace(line, after))
{
log(ast.getLineNo(), after,
"ws.followed", ast.getText());
}
}

/**
* Gets possible place where redundant whitespace could be.
* @param aArrayOrTypeCast {@link TokenTypes#ARRAY_DECLARATOR ARRAY_DECLARATOR}
* or {@link TokenTypes#TYPECAST TYPECAST}.
* @return possible place of redundant whitespace.
*/
private static DetailAST getPreceded(DetailAST aArrayOrTypeCast)
{
DetailAST preceded = aArrayOrTypeCast;
switch (aArrayOrTypeCast.getType()) {
case TokenTypes.TYPECAST:
preceded = aArrayOrTypeCast.findFirstToken(TokenTypes.RPAREN);
break;
case TokenTypes.ARRAY_DECLARATOR:
preceded = getArrayTypeOrIdentifier(aArrayOrTypeCast);
break;
default:
throw new IllegalStateException(aArrayOrTypeCast.toString());
}
return preceded;
}

if ((after >= line.length())
|| Character.isWhitespace(line.charAt(after)))
/**
* Gets position after token (place of possible redundant whitespace).
* @param aAST Node representing token.
* @return position after token.
*/
private static int getPositionAfter(DetailAST aAST)
{
int after;
//If target of possible redundant whitespace is in method definition
if (aAST.getType() == TokenTypes.IDENT
&& aAST.getNextSibling() != null
&& aAST.getNextSibling().getType() == TokenTypes.LPAREN)
{
boolean flag = !mAllowLineBreaks;
for (int i = after + 1; !flag && (i < line.length()); i++) {
if (!Character.isWhitespace(line.charAt(i))) {
flag = true;
final DetailAST methodDef = aAST.getParent();
final DetailAST endOfParams = methodDef.findFirstToken(TokenTypes.RPAREN);
after = endOfParams.getColumnNo() + 1;
}
else {
after = aAST.getColumnNo() + aAST.getText().length();
}
return after;
}

/**
* Gets target place of possible redundant whitespace (array's type or identifier)
* after which {@link TokenTypes#ARRAY_DECLARATOR ARRAY_DECLARATOR} is set.
* @param aArrayDeclarator {@link TokenTypes#ARRAY_DECLARATOR ARRAY_DECLARATOR}
* @return target place before possible redundant whitespace.
*/
private static DetailAST getArrayTypeOrIdentifier(DetailAST aArrayDeclarator)
{
DetailAST typeOrIdent = aArrayDeclarator;
if (isArrayInstantiation(aArrayDeclarator)) {
typeOrIdent = aArrayDeclarator.getParent().getFirstChild();
}
else if (isMultiDimensionalArray(aArrayDeclarator)) {
if (isCstyleMultiDimensionalArrayDeclaration(aArrayDeclarator)) {
if (aArrayDeclarator.getParent().getType() != TokenTypes.ARRAY_DECLARATOR) {
typeOrIdent = getArrayIdentifier(aArrayDeclarator);
}
}
if (flag) {
log(targetAST.getLineNo(), after,
"ws.followed", targetAST.getText());
else {
DetailAST arrayIdentifier = aArrayDeclarator.getFirstChild();
while (arrayIdentifier != null) {
typeOrIdent = arrayIdentifier;
arrayIdentifier = arrayIdentifier.getFirstChild();
}
}
}
else {
if (isCstyleArrayDeclaration(aArrayDeclarator)) {
typeOrIdent = getArrayIdentifier(aArrayDeclarator);
}
else {
typeOrIdent = aArrayDeclarator.getFirstChild();
}
}
return typeOrIdent;
}

/**
* Gets array identifier, e.g.:
* <p>
* <code>
* int[] someArray;
* <code>
* </p>
* <p>
* someArray is identifier.
* </p>
* @param aArrayDeclarator {@link TokenTypes#ARRAY_DECLARATOR ARRAY_DECLARATOR}
* @return array identifier.
*/
private static DetailAST getArrayIdentifier(DetailAST aArrayDeclarator)
{
return aArrayDeclarator.getParent().getNextSibling();
}

/**
* Checks if current array is multidimensional.
* @param aArrayDeclaration {@link TokenTypes#ARRAY_DECLARATOR ARRAY_DECLARATOR}
* @return true if current array is multidimensional.
*/
private static boolean isMultiDimensionalArray(DetailAST aArrayDeclaration)
{
return aArrayDeclaration.getParent().getType() == TokenTypes.ARRAY_DECLARATOR
|| aArrayDeclaration.getFirstChild().getType() == TokenTypes.ARRAY_DECLARATOR;
}

/**
* Checks if current array declaration is part of array instantiation.
* @param aArrayDeclaration {@link TokenTypes#ARRAY_DECLARATOR ARRAY_DECLARATOR}
* @return true if current array declaration is part of array instantiation.
*/
private static boolean isArrayInstantiation(DetailAST aArrayDeclaration)
{
return aArrayDeclaration.getParent().getType() == TokenTypes.LITERAL_NEW;
}

/**
Expand All @@ -133,4 +252,69 @@ public void setAllowLineBreaks(boolean aAllowLineBreaks)
{
mAllowLineBreaks = aAllowLineBreaks;
}

/**
* Checks if current array is declared in C style, e.g.:
* <p>
* <code>
* int array[] = { ... }; //C style
* </code>
* </p>
* <p>
* <code>
* int[] array = { ... }; //Java style
* </code>
* </p>
* @param aArrayDeclaration {@link TokenTypes#ARRAY_DECLARATOR ARRAY_DECLARATOR}
* @return true if array is declared in C style
*/
private static boolean isCstyleArrayDeclaration(DetailAST aArrayDeclaration)
{
boolean result = false;
final DetailAST identifier = getArrayIdentifier(aArrayDeclaration);
if (identifier != null) {
final int arrayDeclarationStart = aArrayDeclaration.getColumnNo();
final int identifierEnd = identifier.getColumnNo() + identifier.getText().length();
result = arrayDeclarationStart == identifierEnd
|| arrayDeclarationStart > identifierEnd;
}
return result;
}

/**
* Works with multidimensional arrays.
* @param aArrayDeclaration {@link TokenTypes#ARRAY_DECLARATOR ARRAY_DECLARATOR}
* @return true if multidimensional array is declared in C style.
*/
private static boolean isCstyleMultiDimensionalArrayDeclaration(DetailAST aArrayDeclaration)
{
boolean result = false;
DetailAST parentArrayDeclaration = aArrayDeclaration;
while (parentArrayDeclaration != null) {
if (parentArrayDeclaration.getParent() != null
&& parentArrayDeclaration.getParent().getType() == TokenTypes.TYPE)
{
result = isCstyleArrayDeclaration(parentArrayDeclaration);
}
parentArrayDeclaration = parentArrayDeclaration.getParent();
}
return result;
}

/**
* Checks if current line has redundant whitespace after specified index.
* @param aLine line of java source.
* @param aAfter specified index.
* @return true if line contains redundant whitespace.
*/
private boolean hasRedundantWhitespace(String aLine, int aAfter)
{
boolean result = !mAllowLineBreaks;
for (int i = aAfter + 1; !result && (i < aLine.length()); i++) {
if (!Character.isWhitespace(aLine.charAt(i))) {
result = true;
}
}
return result;
}
}
12 changes: 6 additions & 6 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocUtilsTest.java
View file
Expand Up @@ -38,7 +38,7 @@ public class JavadocUtilsTest
@Test
public void testTags()
{
final String [] text = {
final String[] text = {
"/** @see elsewhere ",
" * {@link List }, {@link List link text }",
" {@link List#add(Object) link text}",
Expand All @@ -53,7 +53,7 @@ public void testTags()
@Test
public void testTagType()
{
final String [] text = {
final String[] text = {
"/** @see block",
" * {@link List inline}, {@link List#add(Object)}",
};
Expand All @@ -69,7 +69,7 @@ public void testTagType()
@Test
public void testInlineTagLinkText()
{
final String [] text = {
final String[] text = {
"/** {@link List link text }",
};
final Comment comment = new Comment(text, 1, 1, text[0].length());
Expand All @@ -81,7 +81,7 @@ public void testInlineTagLinkText()
@Test
public void testInlineTagMethodRef()
{
final String [] text = {
final String[] text = {
"/** {@link List#add(Object)}",
};
final Comment comment = new Comment(text, 1, 1, text[0].length());
Expand All @@ -93,7 +93,7 @@ public void testInlineTagMethodRef()
@Test
public void testTagPositions()
{
final String [] text = {
final String[] text = {
"/** @see elsewhere",
" also {@link Name value} */",
};
Expand Down Expand Up @@ -121,7 +121,7 @@ else if (JavadocTagInfo.LINK.getName().equals(tag.getTagName())) {
@Test
public void testInvalidTags()
{
final String [] text = {
final String[] text = {
"/** @fake block",
" * {@bogus inline}",
" * {@link List valid}",
Expand Down
34 changes: 34 additions & 0 deletions ...st/java/com/puppycrawl/tools/checkstyle/checks/whitespace/NoWhitespaceAfterCheckTest.java
View file
Expand Up @@ -78,4 +78,38 @@ public void testTypecast() throws Exception
verify(checkConfig, getPath("InputWhitespace.java"), expected);
}

@Test
public void testArrayDeclarations() throws Exception
{
checkConfig.addAttribute("tokens", "ARRAY_DECLARATOR");
final String[] expected = {
"6:11: 'Object' is followed by whitespace.",
"8:22: 'someStuff3' is followed by whitespace.",
"9:8: 'int' is followed by whitespace.",
"10:13: 's' is followed by whitespace.",
"11:13: 'd' is followed by whitespace.",
"16:14: 'get' is followed by whitespace.",
"18:8: 'int' is followed by whitespace.",
"19:34: 'get1' is followed by whitespace.",
"28:8: 'int' is followed by whitespace.",
"29:12: 'cba' is followed by whitespace.",
"31:26: 'String' is followed by whitespace.",
"32:27: 'String' is followed by whitespace.",
"39:11: 'ar' is followed by whitespace.",
"39:24: 'int' is followed by whitespace.",
"40:16: 'int' is followed by whitespace.",
"43:63: 'getLongMultArray' is followed by whitespace.",
};
verify(checkConfig, getPath("whitespace/InputNoWhitespaceAfterArrayDeclarations.java"), expected);
}

@Test
public void testNpe() throws Exception
{
final String[] expected = {

};
verify(checkConfig, getPath("whitespace/InputNoWhiteSpaceAfterCheckFormerNpe.java"),
expected);
}
}

0 comments on commit e6638c3

Please sign in to comment.