phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[java] Improve grammar for CastExpression

Browse Source 
- Provide a definite fix for casts. This allowed to remove a bunch of hacks.
 - I had to change the grammar a bit... `(int)` is now a cast with a
    PrimitiveType as child, not a Type with a nested PrimitiveType. This is due
    to JavaCC completely ignoring whatever you place in the LOOKAHEAD in favor
    of just looking the follow-up expression completely.
 - Removing the hacks raised an issue with expression orders, caused also by JavaCC's
    disregard to LOOKAHEAD contents
This commit is contained in:
Juan Martín Sotuyo Dodero
2017-02-14 02:10:51 -03:00
committed by Andreas Dangel
parent 151f796043
commit 907972f0ca
6 changed files with 106 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
pmd-java/etc/grammar/Java.jjt
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
*
@ -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() :

28
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strings/InsufficientStringBufferDeclarationRule.java
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();
}
}
}
}

93
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/unusedcode/UnusedLocalVariableRule.java
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;
}
}

2
pmd-java/src/main/resources/rulesets/java/controversial.xml
View File

@ -379,7 +379,7 @@ adverse impacts on performance.
<property name="xpath">
<value>
<![CDATA[
//PrimitiveType[@Image = 'short'][name(../..) != 'CastExpression']
//PrimitiveType[@Image = 'short'][name(..) != 'CastExpression']
]]>
</value>
</property>

14
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java
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

15
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/unusedcode/xml/UnusedLocalVariable.xml
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>