From 303b9628e45339e356f871c0a53de7895e209601 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 26 Jun 2019 18:14:30 +0200 Subject: [PATCH] Remove name node in import & package decl Make ImportDeclaration not a TypeNode --- pmd-java/etc/grammar/Java.jjt | 31 ++++++++++++--- .../lang/java/ast/ASTImportDeclaration.java | 39 ++----------------- .../java/rule/AbstractLombokAwareRule.java | 3 +- .../rule/codestyle/DuplicateImportsRule.java | 6 +-- .../typeresolution/ClassTypeResolver.java | 17 -------- .../typeresolution/ClassTypeResolverTest.java | 2 - 6 files changed, 32 insertions(+), 66 deletions(-) diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 793937caff..ffac80588c 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -1709,15 +1709,17 @@ private void ModuleDeclLahead() #void: void PackageDeclaration() : -{} +{String image;} { - ( Annotation() )* "package" Name() ";" + ( Annotation() )* "package" image=VoidName() { jjtThis.setImage(image); } ";" } void ImportDeclaration() : -{} +{String image;} { - "import" [ "static" {checkForBadStaticImportUsage();jjtThis.setStatic();} ] Name() [ "." "*" {jjtThis.setImportOnDemand();} ] ";" + "import" [ "static" {checkForBadStaticImportUsage();jjtThis.setStatic();} ] + image=ImportName() { jjtThis.setImage(image); } + [ "." "*" {jjtThis.setImportOnDemand();} ] ";" } /* @@ -3148,10 +3150,10 @@ void Annotation() #void: {} { ( - LOOKAHEAD( "@" Name() "(" ( "=" | ")" )) + LOOKAHEAD( "@" VoidName() "(" ( "=" | ")" )) NormalAnnotation() | - LOOKAHEAD( "@" Name() "(" ) + LOOKAHEAD( "@" VoidName() "(" ) SingleMemberAnnotation() | MarkerAnnotation() @@ -3339,6 +3341,23 @@ void AmbiguousName(): String VoidName() #void: +/* This is identical to ImportName except we only need 1 token lookahead. */ +{ + StringBuilder s = new StringBuilder(); + Token t; +} +{ + t= + { + s.append(t.image); + } + ( "." t= + {s.append('.').append(t.image);} + )* + {return s.toString();} +} + +String ImportName() #void: /* * A lookahead of 2 is required below since "Name" can be followed * by a ".*" when used in the context of an "ImportDeclaration". diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTImportDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTImportDeclaration.java index 6514c64815..bd1cd00c12 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTImportDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTImportDeclaration.java @@ -4,31 +4,21 @@ package net.sourceforge.pmd.lang.java.ast; -import net.sourceforge.pmd.annotation.InternalApi; - /** * Represents an import declaration in a Java file. * * 
  *
- * ImportDeclaration ::= "import" "static"? {@linkplain ASTName Name} ( "." "*" )? ";"
+ * ImportDeclaration ::= "import" "static"? Name ( "." "*" )? ";"
  *
  *
* * @see JLS 7.5 */ -// TODO should this really be a type node? -// E.g. for on-demand imports, what's the type of this node? There's no type name, just a package name -// for on-demand static imports? -// for static imports of a field? the type of the field or the type of the enclosing type? -// for static imports of a method? -// I don't think we can work out a spec without surprising corner cases, and #1207 will abstract -// things away anyway, so I think we should make it a regular node -public final class ASTImportDeclaration extends AbstractJavaTypeNode { +public final class ASTImportDeclaration extends AbstractJavaNode { private boolean isImportOnDemand; private boolean isStatic; - private Package pkg; ASTImportDeclaration(int id) { super(id); @@ -75,19 +65,13 @@ public final class ASTImportDeclaration extends AbstractJavaTypeNode { return isStatic; } - // TODO - this should go away, but the DuplicateImports rule still uses it - // (in a clunky way) - public ASTName getImportedNameNode() { - return (ASTName) jjtGetChild(0); - } - /** * Returns the full name of the import. For on-demand imports, this is the name without * the final dot and asterisk. */ public String getImportedName() { - return jjtGetChild(0).getImage(); + return getImage(); } @@ -132,22 +116,5 @@ public final class ASTImportDeclaration extends AbstractJavaTypeNode { visitor.visit(this, data); } - @InternalApi - @Deprecated - public void setPackage(Package packge) { - this.pkg = packge; - } - - /** - * Returns the {@link Package} instance representing the package of the - * type or method imported by this declaration. This may be null if the - * auxclasspath is not correctly set, as this method depends on correct - * type resolution. - */ - // TODO deprecate? This is only used in a test. I don't think it's really - // useful and it gives work to ClassTypeResolver. - public Package getPackage() { - return this.pkg; - } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractLombokAwareRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractLombokAwareRule.java index e0f8fd5345..8a0adeb0ba 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractLombokAwareRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractLombokAwareRule.java @@ -56,8 +56,7 @@ public class AbstractLombokAwareRule extends AbstractIgnoredAnnotationRule { @Override public Object visit(ASTImportDeclaration node, Object data) { - ASTName name = node.getFirstChildOfType(ASTName.class); - if (!lombokImported && name != null && name.getImage() != null & name.getImage().startsWith(LOMBOK_PACKAGE)) { + if (!lombokImported && node.getImage() != null & node.getImage().startsWith(LOMBOK_PACKAGE)) { lombokImported = true; } return super.visit(node, data); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/DuplicateImportsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/DuplicateImportsRule.java index 28963d7557..655cd8c600 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/DuplicateImportsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/DuplicateImportsRule.java @@ -87,19 +87,19 @@ public class DuplicateImportsRule extends AbstractJavaRule { @Override public Object visit(ASTImportDeclaration node, Object data) { ImportWrapper wrapper = new ImportWrapper(node.getImportedName(), node.getImportedName(), - node.getImportedNameNode(), node.isStatic() && node.isImportOnDemand()); + node, node.isStatic() && node.isImportOnDemand()); // blahhhh... this really wants to be ASTImportDeclaration to be // polymorphic... if (node.isImportOnDemand()) { if (importOnDemandImports.contains(wrapper)) { - addViolation(data, node.getImportedNameNode(), node.getImportedNameNode().getImage()); + addViolation(data, node, node.getImportedName()); } else { importOnDemandImports.add(wrapper); } } else { if (singleTypeImports.contains(wrapper)) { - addViolation(data, node.getImportedNameNode(), node.getImportedNameNode().getImage()); + addViolation(data, node, node.getImportedName()); } else { singleTypeImports.add(wrapper); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 776a322479..887ae8cca1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -220,23 +220,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return data; } - @Override - public Object visit(ASTImportDeclaration node, Object data) { - ASTName importedType = (ASTName) node.jjtGetChild(0); - - if (importedType.getType() != null) { - setTypeDefinition(node, JavaTypeDefinition.forClass(importedType.getType())); - } else { - populateType(node, importedType.getImage()); - } - - if (node.getType() != null) { - node.setPackage(node.getType().getPackage()); - } - - // no need to visit children, the only child, ASTName, will have no type - return data; - } @Override public Object visit(ASTTypeDeclaration node, Object data) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index 7b009ae74a..1b6a69da8f 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -158,8 +158,6 @@ public class ClassTypeResolverTest { assertEquals(ArrayListFound.class, acu.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class).getType()); ASTImportDeclaration id = acu.getFirstDescendantOfType(ASTImportDeclaration.class); - assertEquals("java.util", id.getPackage().getName()); - assertEquals(ArrayList.class, id.getType()); assertEquals(ArrayList.class, acu.getFirstDescendantOfType(ASTClassOrInterfaceType.class).getType()); assertEquals(ArrayList.class, acu.getFirstDescendantOfType(ASTReferenceType.class).getType()); assertEquals(ArrayList.class, acu.getFirstDescendantOfType(ASTType.class).getType());