Merge branch 'issue-257' into pmd/5.5.x

This commit is contained in:
Andreas Dangel
2017-02-20 16:06:26 +01:00
6 changed files with 112 additions and 72 deletions

View File

@ -1,4 +1,9 @@
/**
* Provide a better fix for CastExpression, getting rid of most hacks.
* Bug #257
*
* Juan Martin Sotuyo Dodero 02/2017
*====================================================================
* Allow local classes to carry more than one annotation.
* Bug #208
*
@ -1558,7 +1563,7 @@ void ReferenceType():
void ClassOrInterfaceType():
{
StringBuffer s = new StringBuffer();
StringBuilder s = new StringBuilder();
Token t;
}
{
@ -1791,9 +1796,13 @@ void UnaryExpressionNotPlusMinus() #UnaryExpressionNotPlusMinus((jjtn000.getImag
{}
{
( "~" {jjtThis.setImage("~");} | "!" {jjtThis.setImage("!");} ) UnaryExpression()
| LOOKAHEAD( { getToken(1).kind == LPAREN && getToken(2).kind == IDENTIFIER && getToken(3).kind == RPAREN && getToken(4).kind == PLUS } ) PostfixExpression()
| LOOKAHEAD( CastExpression() ) CastExpression()
| LOOKAHEAD("(" Type() ")" "(") CastExpression()
/*
* This is really ugly... we are repeting the CastExpression lookahead and full expression...
* If we don't the lookahead within CastExpression is ignored, and it simply looks for the expression,
* meaning we can't be explicit as to what can be casted depending on the cast type (primitive or otherwhise)
*/
| LOOKAHEAD("(" (Annotation())* PrimitiveType() ")") CastExpression()
| LOOKAHEAD("(" (Annotation())* Type() ( "&" ReferenceType() )* ")" UnaryExpressionNotPlusMinus()) CastExpression()
| PostfixExpression()
}
@ -1803,12 +1812,13 @@ void PostfixExpression() #PostfixExpression((jjtn000.getImage() != null)):
PrimaryExpression() [ "++" {jjtThis.setImage("++");} | "--" {jjtThis.setImage("--");} ]
}
void CastExpression() #CastExpression(>1):
void CastExpression() :
{}
{
LOOKAHEAD("(" (Annotation())* Type() ")") "(" (Annotation() {checkForBadTypeAnnotations();})* Type() ")" UnaryExpression()
| LOOKAHEAD("(" (Annotation())* Type() "&") "(" (Annotation() {checkForBadTypeAnnotations();})* Type() ( "&" {checkForBadIntersectionTypesInCasts(); jjtThis.setIntersectionTypes(true);} ReferenceType() )+ ")" UnaryExpressionNotPlusMinus()
| "(" (Annotation() {checkForBadTypeAnnotations();})* Type() ")" UnaryExpressionNotPlusMinus()
LOOKAHEAD(
"(" (Annotation())* PrimitiveType() ")"
) "(" (Annotation() {checkForBadTypeAnnotations();})* Type() ")" UnaryExpression()
| "(" (Annotation() {checkForBadTypeAnnotations();})* Type() ( "&" {checkForBadIntersectionTypesInCasts(); jjtThis.setIntersectionTypes(true);} ReferenceType() )* ")" UnaryExpressionNotPlusMinus()
}
void PrimaryExpression() :
@ -2012,12 +2022,9 @@ void StatementExpression() :
|
PreDecrementExpression()
|
LOOKAHEAD( PrimaryExpression() ("++" | "--") ) PostfixExpression()
LOOKAHEAD( PrimaryExpression() AssignmentOperator() ) PrimaryExpression() AssignmentOperator() Expression()
|
PrimaryExpression()
[
AssignmentOperator() Expression()
]
PostfixExpression()
}
void SwitchStatement() :

View File

@ -25,7 +25,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
import net.sourceforge.pmd.lang.java.ast.ASTSwitchLabel;
import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement;
import net.sourceforge.pmd.lang.java.ast.ASTType;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.ASTVariableInitializer;
@ -197,19 +196,20 @@ public class InsufficientStringBufferDeclarationRule extends AbstractJavaRule {
anticipatedLength += str.length() - 2;
} else if (literal.isCharLiteral()) {
anticipatedLength += 1;
} else if(literal.isIntLiteral() && str.startsWith("0x")){
// but only if we are not inside a cast expression
Node parentNode = literal.jjtGetParent().jjtGetParent().jjtGetParent();
if (parentNode instanceof ASTCastExpression
&& parentNode.getFirstChildOfType(ASTType.class).getType() == char.class) {
anticipatedLength += 1;
} else {
// e.g. 0xdeadbeef -> will be converted to a base 10 integer string: 3735928559
anticipatedLength += String.valueOf(Long.parseLong(str.substring(2), 16)).length();
}
} else {
anticipatedLength += str.length();
}
} else if (literal.isIntLiteral() && str.startsWith("0x")) {
// but only if we are not inside a cast expression
Node parentNode = literal.jjtGetParent().jjtGetParent().jjtGetParent();
if (parentNode instanceof ASTCastExpression
&& ((ASTCastExpression) parentNode).getType() == char.class) {
anticipatedLength += 1;
} else {
// e.g. 0xdeadbeef -> will be converted to a
// base 10 integer string: 3735928559
anticipatedLength += String.valueOf(Long.parseLong(str.substring(2), 16)).length();
}
} else {
anticipatedLength += str.length();
}
}
}
}

View File

@ -1,45 +1,48 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.unusedcode;
import java.util.List;
import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
public class UnusedLocalVariableRule extends AbstractJavaRule {
public Object visit(ASTLocalVariableDeclaration decl, Object data) {
for (int i = 0; i < decl.jjtGetNumChildren(); i++) {
if (!(decl.jjtGetChild(i) instanceof ASTVariableDeclarator)) {
continue;
}
ASTVariableDeclaratorId node = (ASTVariableDeclaratorId) decl.jjtGetChild(i).jjtGetChild(0);
// TODO this isArray() check misses some cases
// need to add DFAish code to determine if an array
// is initialized locally or gotten from somewhere else
if (!node.getNameDeclaration().isArray() && !actuallyUsed(node.getUsages())) {
addViolation(data, node, node.getNameDeclaration().getImage());
}
}
return data;
}
private boolean actuallyUsed(List<NameOccurrence> usages) {
for (NameOccurrence occ: usages) {
JavaNameOccurrence jocc = (JavaNameOccurrence)occ;
if (jocc.isOnLeftHandSide()) {
continue;
} else {
return true;
}
}
return false;
}
}
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.unusedcode;
import java.util.List;
import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
public class UnusedLocalVariableRule extends AbstractJavaRule {
public UnusedLocalVariableRule() {
addRuleChainVisit(ASTLocalVariableDeclaration.class);
}
public Object visit(ASTLocalVariableDeclaration decl, Object data) {
for (int i = 0; i < decl.jjtGetNumChildren(); i++) {
if (!(decl.jjtGetChild(i) instanceof ASTVariableDeclarator)) {
continue;
}
ASTVariableDeclaratorId node = (ASTVariableDeclaratorId) decl.jjtGetChild(i).jjtGetChild(0);
// TODO this isArray() check misses some cases
// need to add DFAish code to determine if an array
// is initialized locally or gotten from somewhere else
if (!node.getNameDeclaration().isArray() && !actuallyUsed(node.getUsages())) {
addViolation(data, node, node.getNameDeclaration().getImage());
}
}
return data;
}
private boolean actuallyUsed(List<NameOccurrence> usages) {
for (NameOccurrence occ : usages) {
JavaNameOccurrence jocc = (JavaNameOccurrence) occ;
if (!jocc.isOnLeftHandSide()) {
return true;
}
}
return false;
}
}

View File

@ -157,6 +157,20 @@ public class ParserCornersTest extends ParserTst {
String c = IOUtils.toString(this.getClass().getResourceAsStream("GitHubBug208.java"));
parseJava15(c);
}
@Test
public void testGitHubBug257NonExistingCast() throws Exception {
String code = "public class Test {" + PMD.EOL
+ " public static void main(String[] args) {" + PMD.EOL
+ " double a = 4.0;" + PMD.EOL
+ " double b = 2.0;" + PMD.EOL
+ " double result = Math.sqrt((a) - b);" + PMD.EOL
+ " System.out.println(result);" + PMD.EOL
+ " }" + PMD.EOL
+ "}";
ASTCompilationUnit compilationUnit = parseJava15(code);
assertEquals("A cast was found when none expected", 0, compilationUnit.findDescendantsOfType(ASTCastExpression.class).size());
}
/**
* This triggered bug #1484 UnusedLocalVariable - false positive - parenthesis

View File

@ -368,6 +368,21 @@ public class Test {
System.out.println(list.size() + " (" + (notEmpty) + " not empty)");
}
}
]]></code>
</test-code>
<test-code>
<description>#257 UnusedLocalVariable - false positive - parenthesis</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Test {
public static void main(String[] args) {
double a = 4.0;
double b = 2.0;
double result = Math.sqrt((a) - b);
System.out.println(result);
}
}
]]></code>
</test-code>

View File

@ -79,6 +79,7 @@ This new rule is part of the `java-design` ruleset.
* [#246](https://github.com/pmd/pmd/issues/246): \[java] UnusedModifier doesn't check annotations
* [#247](https://github.com/pmd/pmd/issues/247): \[java] UnusedModifier doesn't check annotations inner classes
* [#248](https://github.com/pmd/pmd/issues/248): \[java] UnusedModifier doesn't check static keyword on nested enum declaration
* [#257](https://github.com/pmd/pmd/issues/257): \[java] UnusedLocalVariable false positive
### API Changes