diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 3e6f53a932..45ac709fa9 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -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 * @@ -1550,7 +1555,7 @@ void ReferenceType(): void ClassOrInterfaceType(): { - StringBuffer s = new StringBuffer(); + StringBuilder s = new StringBuilder(); Token t; } { @@ -1783,9 +1788,7 @@ 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() +| LOOKAHEAD(CastExpression()) CastExpression() | PostfixExpression() } @@ -1795,12 +1798,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();})* PrimitiveType() ")" UnaryExpression() +| "(" (Annotation() {checkForBadTypeAnnotations();})* Type() ( "&" {checkForBadIntersectionTypesInCasts(); jjtThis.setIntersectionTypes(true);} ReferenceType() )* ")" UnaryExpressionNotPlusMinus() } void PrimaryExpression() : @@ -2008,12 +2012,9 @@ void StatementExpression() : | PreDecrementExpression() | - LOOKAHEAD( PrimaryExpression() ("++" | "--") ) PostfixExpression() + LOOKAHEAD( PrimaryExpression() AssignmentOperator() ) PrimaryExpression() AssignmentOperator() Expression() | - PrimaryExpression() - [ - AssignmentOperator() Expression() - ] + PostfixExpression() } void SwitchStatement() : diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strings/InsufficientStringBufferDeclarationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strings/InsufficientStringBufferDeclarationRule.java index 6859c48c1f..2c85d965b8 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strings/InsufficientStringBufferDeclarationRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strings/InsufficientStringBufferDeclarationRule.java @@ -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(); + } } } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/unusedcode/UnusedLocalVariableRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/unusedcode/UnusedLocalVariableRule.java index 960e8f240a..c8a791f2e2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/unusedcode/UnusedLocalVariableRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/unusedcode/UnusedLocalVariableRule.java @@ -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 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 usages) { + for (NameOccurrence occ : usages) { + JavaNameOccurrence jocc = (JavaNameOccurrence) occ; + if (!jocc.isOnLeftHandSide()) { + return true; + } + } + return false; + } + +} diff --git a/pmd-java/src/main/resources/rulesets/java/controversial.xml b/pmd-java/src/main/resources/rulesets/java/controversial.xml index 1e3393209e..1640a842ca 100644 --- a/pmd-java/src/main/resources/rulesets/java/controversial.xml +++ b/pmd-java/src/main/resources/rulesets/java/controversial.xml @@ -379,7 +379,7 @@ adverse impacts on performance. diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java index 9a0b20d366..809b95357e 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java @@ -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 diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/unusedcode/xml/UnusedLocalVariable.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/unusedcode/xml/UnusedLocalVariable.xml index 9f0d702ec6..6281eb38cf 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/unusedcode/xml/UnusedLocalVariable.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/unusedcode/xml/UnusedLocalVariable.xml @@ -368,6 +368,21 @@ public class Test { System.out.println(list.size() + " (" + (notEmpty) + " not empty)"); } +} + ]]> + + + + #257 UnusedLocalVariable - false positive - parenthesis + 0 +