hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Improve try-with-resources grammar

Browse Source
This commit is contained in:
Clément Fournier
2019-07-01 12:36:29 +02:00
parent 46fcccb7f7
commit 968c7d019a
11 changed files with 198 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
pmd-java/etc/grammar/Java.jjt
View File

@@ -1,4 +1,8 @@
/**
* Make Resources #void, rename ResourceSpecification to ResourceList,
* put a LocalVariableDeclaration node inside Resource.
* Clément Fournier 07/2019
*====================================================================
* Change expression, type and annotation grammar to remove unnecessary nodes,
* eliminate some inconsistencies, and most importantly have an expressive tree
* for primary expressions. Expressions and types now appear to be left-recursive.
@@ -3071,27 +3075,27 @@ void TryStatement() :
[ FinallyStatement() ]
}
void ResourceSpecification() :
void ResourceSpecification() #ResourceList:
{}
{
{checkForBadTryWithResourcesUsage();}
"("
Resources()
(LOOKAHEAD(2) ";")?
(LOOKAHEAD(2) ";" {jjtThis.setTrailingSemi();})?
")"
}
void Resources() :
void Resources() #void:
{}
{
Resource() (LOOKAHEAD(2) ";" Resource())*
}
void Resource() :
void Resource():
{}
{
LOOKAHEAD(("final" | Annotation())* LocalVariableType() VariableDeclaratorId() "=" )
( ( "final" {jjtThis.setFinal(true);} | Annotation() )* LocalVariableType() VariableDeclaratorId() "=" Expression() )
( ( "final" {jjtThis.setFinal(true);} | Annotation() )* LocalVariableType() VariableDeclarator() ) #LocalVariableDeclaration
|
PrimaryExpression() {checkForBadConciseTryWithResourcesUsage(); } {}
}

73
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTResource.java
View File

@@ -4,7 +4,26 @@
package net.sourceforge.pmd.lang.java.ast;
public final class ASTResource extends ASTFormalParameter {
import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.lang.ast.Node;
/**
* A resource of a {@linkplain try-with-resources}. This contains another
* node that represents the resource, according to the grammar below.
*
* <p>In the case of concise try-with resources, the subexpressions are
* required to be only field accesses or variable references to compile.
*
* <pre class="grammar">
*
* Resource ::= {@link ASTLocalVariableDeclaration LocalVariableDeclaration}
* | {@link ASTPrimaryExpression PrimaryExpression}
*
* </pre>
*/
public final class ASTResource extends AbstractJavaNode {
ASTResource(int id) {
super(id);
@@ -25,6 +44,56 @@ public final class ASTResource extends ASTFormalParameter {
visitor.visit(this, data);
}
// TODO Should we deprecate all methods from ASTFormalParameter?
/**
* Returns true if this appears as an expression, and not as a
* local variable declaration.
*/
public boolean isConciseResource() {
return jjtGetChild(0) instanceof ASTExpression;
}
/**
* Gets the name with which the resource can be accessed in the
* body of the try statement. If this is a {@linkplain #isConciseResource() concise resource},
* then returns the sequence of names that identifies the expression.
* If this has a local variable declaration, then returns the name
* of the variable.
*/
public String getStableName() {
if (isConciseResource()) {
ASTExpression expr = getInitializer();
StringBuilder builder = new StringBuilder();
while (expr instanceof ASTFieldAccess) {
ASTFieldAccess fa = (ASTFieldAccess) expr;
builder.insert(0, "." + fa.getFieldName());
expr = fa.getLhsExpression();
}
builder.insert(0, ((ASTVariableReference) expr).getVariableName());
return builder.toString();
} else {
return asLocalVariableDeclaration().iterator().next().getVariableName();
}
}
@Nullable
public ASTLocalVariableDeclaration asLocalVariableDeclaration() {
return AstImplUtil.getChildAs(this, 0, ASTLocalVariableDeclaration.class);
}
/**
* Returns the initializer of the expression.
* If this is a concise resource, then returns that expression.
* If this is a local variable declaration, returns the initializer of
* the variable.
*/
public ASTExpression getInitializer() {
Node c = jjtGetChild(0);
if (c instanceof ASTExpression) {
return (ASTExpression) c;
} else {
return ((ASTLocalVariableDeclaration) c).iterator().next().getInitializer();
}
}
}

57
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTResourceList.java Normal file
View File

@@ -0,0 +1,57 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.ast;
import java.util.Iterator;
/**
* A list of resources in a {@linkplain try-with-resources}.
*
* <pre class="grammar">
*
* ResourceList ::= "(" {@link ASTResource Resource} ( ";" {@link ASTResource Resource} )* ";"? ")"
*
* </pre>
*/
public final class ASTResourceList extends AbstractJavaNode implements Iterable<ASTResource> {
private boolean trailingSemi;
ASTResourceList(int id) {
super(id);
}
ASTResourceList(JavaParser p, int id) {
super(p, id);
}
@Override
public Object jjtAccept(JavaParserVisitor visitor, Object data) {
return visitor.visit(this, data);
}
@Override
public <T> void jjtAccept(SideEffectingVisitor<T> visitor, T data) {
visitor.visit(this, data);
}
void setTrailingSemi() {
this.trailingSemi = true;
}
/**
* Returns true if this resource list has a trailing semicolon, eg
* in {@code try (InputStream is = getInputStream();) { ... }}.
*/
public boolean hasTrailingSemiColon() {
return trailingSemi;
}
@Override
public Iterator<ASTResource> iterator() {
return new NodeChildrenIterator<>(this, ASTResource.class);
}
}

27
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTResourceSpecification.java
View File

@@ -1,27 +0,0 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.ast;
public final class ASTResourceSpecification extends AbstractJavaNode {
ASTResourceSpecification(int id) {
super(id);
}
ASTResourceSpecification(JavaParser p, int id) {
super(p, id);
}
@Override
public Object jjtAccept(JavaParserVisitor visitor, Object data) {
return visitor.visit(this, data);
}
@Override
public <T> void jjtAccept(SideEffectingVisitor<T> visitor, T data) {
visitor.visit(this, data);
}
}

1
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTResources.java
View File

@@ -4,6 +4,7 @@
package net.sourceforge.pmd.lang.java.ast;
@Deprecated
public final class ASTResources extends AbstractJavaNode {
ASTResources(int id) {

9
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTryStatement.java
View File

@@ -9,9 +9,14 @@ import java.util.List;
/**
* Try statement node.
*
*
* <pre class="grammar">
*
* TryStatement ::= "try" ( ResourceSpecification )? Block ( CatchStatement )* [ FinallyStatement ]
* TryStatement ::= "try" {@link ASTResourceList ResourceSpecification}?
* {@link ASTBlock Block}
* {@link ASTCatchStatement CatchStatement}*
* {@link ASTFinallyStatement FinallyStatement}?
*
* </pre>
*/
@@ -43,7 +48,7 @@ public final class ASTTryStatement extends AbstractJavaNode {
* has a ResourceSpecification child node.
*/
public boolean isTryWithResources() {
return getFirstChildOfType(ASTResourceSpecification.class) != null;
return jjtGetChild(0) instanceof ASTResourceList;
}

5
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryModifierRule.java
View File

@@ -22,6 +22,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTEnumBody;
import net.sourceforge.pmd.lang.java.ast.ASTEnumConstant;
import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTResource;
@@ -76,7 +77,7 @@ public class UnnecessaryModifierRule extends AbstractJavaRule {
} else if (node instanceof ASTFieldDeclaration) {
return ((ASTFieldDeclaration) node).getVariableName();
} else if (node instanceof ASTResource) {
return ((ASTResource) node).getVariableDeclaratorId().getImage();
return ((ASTResource) node).getStableName();
} else {
return node.getImage();
}
@@ -215,7 +216,7 @@ public class UnnecessaryModifierRule extends AbstractJavaRule {
@Override
public Object visit(final ASTResource node, final Object data) {
if (node.isFinal()) {
if (!node.isConciseResource() && node.asLocalVariableDeclaration().isFinal()) {
reportUnnecessaryModifiers(data, node, Modifier.FINAL, "resource specifications are implicitly final");
}

14
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/OccurrenceFinder.java
View File

@@ -24,20 +24,6 @@ public class OccurrenceFinder extends JavaParserVisitorAdapter {
private final Set<NameDeclaration> additionalDeclarations = new HashSet<>();
@Override
public Object visit(ASTResource node, Object data) {
// is this a concise resource reference?
if (node.jjtGetNumChildren() == 1) {
ASTExpression nameNode = (ASTExpression) node.jjtGetChild(0);
for (StringTokenizer st = new StringTokenizer(nameNode.getImage(), "."); st.hasMoreTokens();) {
JavaNameOccurrence occ = new JavaNameOccurrence(nameNode, st.nextToken());
new Search(occ).execute();
}
}
return super.visit(node, data);
}
@Override
public Object visit(ASTPrimaryExpression node, Object data) {
NameFinder nameFinder = new NameFinder(node);

16
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java
View File

@@ -71,7 +71,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType;
import net.sourceforge.pmd.lang.java.ast.ASTReferenceType;
import net.sourceforge.pmd.lang.java.ast.ASTRelationalExpression;
import net.sourceforge.pmd.lang.java.ast.ASTResource;
import net.sourceforge.pmd.lang.java.ast.ASTShiftExpression;
import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression;
import net.sourceforge.pmd.lang.java.ast.ASTSwitchExpression;
@@ -705,21 +704,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
return data;
}
@Override
public Object visit(ASTResource node, Object data) {
super.visit(node, data);
// resolve "var" types: the type of the initializer expression
ASTType type = node.getTypeNode();
if (type == null) {
// no type node -> type is inferred
ASTExpression initializer = node.getFirstChildOfType(ASTExpression.class);
if (node.getVariableDeclaratorId() != null) {
setTypeDefinition(node.getVariableDeclaratorId(), initializer.getTypeDefinition());
}
}
return data;
}
@Override
public Object visit(ASTPrimitiveType node, Object data) {

4
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/Java10Test.java
View File

@@ -144,8 +144,8 @@ public class Java10Test {
List<ASTResource> resources = compilationUnit.findDescendantsOfType(ASTResource.class);
assertEquals(1, resources.size());
assertNull(resources.get(0).getTypeNode());
ASTVariableDeclaratorId varId = resources.get(0).getVariableDeclaratorId();
assertNull(resources.get(0).asLocalVariableDeclaration().getTypeNode());
ASTVariableDeclaratorId varId = resources.get(0).asLocalVariableDeclaration().iterator().next();
assertSame("type should be FileInputStream", FileInputStream.class, varId.getType());
}

78
pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTTryStatementTest.kt
View File

@@ -23,32 +23,40 @@ class ASTTryStatementTest : ParserTestSpec({
"try (Foo a = 2){}" should matchStmt<ASTTryStatement> {
child<ASTResourceSpecification> {
child<ASTResources> {
child<ASTResource> {
child<ASTClassOrInterfaceType> {}
child<ASTVariableDeclaratorId> {}
child<ASTNumericLiteral> {}
child<ASTResourceList> {
child<ASTResource> {
it::isConciseResource shouldBe false
it::getInitializer shouldBe fromChild<ASTLocalVariableDeclaration, ASTExpression> {
it::isFinal shouldBe false
classType("Foo")
fromChild<ASTVariableDeclarator, ASTExpression> {
variableId("a")
int(2)
}
}
}
}
child<ASTBlock> {}
block()
}
"try (final Foo a = 2){}" should matchStmt<ASTTryStatement> {
child<ASTResourceSpecification> {
child<ASTResources> {
child<ASTResource> {
child<ASTResourceList> {
child<ASTResource> {
it::isConciseResource shouldBe false
it::getInitializer shouldBe fromChild<ASTLocalVariableDeclaration, ASTExpression> {
it::isFinal shouldBe true
child<ASTClassOrInterfaceType> {}
child<ASTVariableDeclaratorId> {}
child<ASTNumericLiteral> {}
classType("Foo")
fromChild<ASTVariableDeclarator, ASTExpression> {
variableId("a")
int(2)
}
}
}
}
child<ASTBlock> {}
block()
}
}
@@ -57,35 +65,45 @@ class ASTTryStatementTest : ParserTestSpec({
"try (a){}" should matchStmt<ASTTryStatement> {
child<ASTResourceSpecification> {
child<ASTResources> {
child<ASTResource> {
child<ASTVariableReference> {}
}
child<ASTResourceList> {
child<ASTResource> {
it::isConciseResource shouldBe true
it::getInitializer shouldBe variableRef("a")
}
it::hasTrailingSemiColon shouldBe false
}
block()
}
child<ASTBlock> {}
"try (a;){}" should matchStmt<ASTTryStatement> {
child<ASTResourceList> {
child<ASTResource> {
it::isConciseResource shouldBe true
it::getInitializer shouldBe variableRef("a")
}
it::hasTrailingSemiColon shouldBe true
}
block()
}
"try (a.b){}" should matchStmt<ASTTryStatement> {
child<ASTResourceSpecification> {
child<ASTResources> {
child<ASTResource> {
child<ASTFieldAccess> {
child<ASTAmbiguousName> {
}
}
child<ASTResourceList> {
child<ASTResource> {
it::isConciseResource shouldBe true
it::getInitializer shouldBe fieldAccess("b") {
ambiguousName("a")
}
}
}
child<ASTBlock> {}
block()
}
@@ -94,4 +112,4 @@ class ASTTryStatementTest : ParserTestSpec({
}
})
})