diff --git a/docs/pages/next_major_development.md b/docs/pages/next_major_development.md
index 2586d1d946..6963c8e4bb 100644
--- a/docs/pages/next_major_development.md
+++ b/docs/pages/next_major_development.md
@@ -15,6 +15,29 @@ TODO
{% include note.html content="Current plans are listed [here](https://github.com/pmd/pmd/labels/in%3Aast) and in particular [here](https://github.com/pmd/pmd/issues/1019)" %}
+### Type grammar changes
+
+{% jdoc_nspace :jast java::lang.java.ast %}
+
+* {% jdoc jast::ASTType %} and {% jdoc jast::ASTReferenceType %} have been turned into
+interfaces, implemented by {% jdoc jast::ASTPrimitiveType %}, {% jdoc jast::ASTClassOrInterfaceType %},
+and the new node {% jdoc jast::ASTArrayType %}. This reduces the depth of the relevant
+subtrees, and allows to explore them more easily and consistently.
+
+* {% jdoc jast::ASTClassOrInterfaceType %} appears to be left recursive now.
+TODO document that when we're done discussing the semantic rewrite phase.
+
+* **Migrating**:
+ * `Type/ReferenceType/ClassOrInterfaceType` -> `ClassOrInterfaceType`
+ * `Type/PrimitiveType` -> `PrimitiveType`.
+ * `Type/ReferenceType[@ArrayDepth>1]/ClassOrInterfaceType` -> `ArrayType/ClassOrInterfaceType`.
+ * `Type/ReferenceType/PrimitiveType` -> `ArrayType/PrimitiveType`.
+ * Note that in most cases you should check the type of a variable with
+ `VariableDeclaratorId[pmd-java:typeIs("java.lang.String[]")]` because it
+ considers the additional dimensions on declarations like `String foo[];`.
+ The Java equivalent is `TypeHelper.isA(id, String[].class);`
+
+
## New API support guidelines
diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt
index a4350ba67b..84f1aae9c7 100644
--- a/pmd-java/etc/grammar/Java.jjt
+++ b/pmd-java/etc/grammar/Java.jjt
@@ -1,4 +1,8 @@
/**
+ * Change parsing of types to flatten their AST and preserve their structure.
+ * Refs #1150 [java] ClassOrInterfaceType AST improvements
+ * Clément Fournier 02/2019
+ *====================================================================
* Add support for Java 12 switch expressions and switch rules.
* Andreas Dangel, Clément Fournier 03/2019
*====================================================================
@@ -1962,8 +1966,8 @@ void Type() #void:
void Dims() #ArrayTypeDims:
{}
{
- // the list of dimensions is flat which is not cool for annotation
- // placement, but realistically that's a corner case.
+ // the list of dimensions is flat, but annotations are
+ // preserved within each specific dim.
(ArrayTypeDim())+
}
@@ -2104,14 +2108,16 @@ FloatingPointType:
void PrimitiveType() :
{}
{
- "boolean" {jjtThis.setImage("boolean");}
-| "char" {jjtThis.setImage("char");}
-| "byte" {jjtThis.setImage("byte");}
-| "short" {jjtThis.setImage("short");}
-| "int" {jjtThis.setImage("int");}
-| "long" {jjtThis.setImage("long");}
-| "float" {jjtThis.setImage("float");}
-| "double" {jjtThis.setImage("double");}
+ ( "boolean"
+ | "char"
+ | "byte"
+ | "short"
+ | "int"
+ | "long"
+ | "float"
+ | "double"
+ )
+ {jjtThis.setImage(getToken(0).getImage());}
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java
index f858976922..3c11580d3f 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java
@@ -6,7 +6,34 @@
package net.sourceforge.pmd.lang.java.ast;
/**
- * TODO should we merge with {@link ASTArrayTypeDims}?
+ * TODO get rid of Dimensionable here. Ideally reuse {@link ASTArrayTypeDim},
+ * or use a special node for array creation, bc {@link ASTAllocationExpression}
+ * is too broad.
+ *
+ * Current grammar:
+ *
+ *
+ * ArrayDimsAndInit ::= TypeAnnotation* "[" Expression "]" ( "[" "]" )*
+ * | ( "[" "]" )+ ArrayInitializer()
+ *
+ *
+ *
+ * Actual JLS (https://docs.oracle.com/javase/specs/jls/se9/html/jls-15.html#jls-DimExprs):
+ *
+ *
+ * (: This production doesn't exist in the JLS, they have a special production for Array creation expressions :)
+ * (: Ideally we'd do the same. :)
+ *
+ * ArrayDimsAndInit ::= TypeAnnotation* DimExpr* Dim*
+ * | Dim* {@link ASTArrayInitializer ArrayInitializer}
+ *
+ * (: Notice that annotations are allowed before any Dim or DimExpr :)
+ *
+ * DimExpr ::= TypeAnnotation* [ Expression ]
+ * Dim ::= {@link ASTArrayTypeDim ArrayTypeDim}
+ *
+ *
+ *
*/
public class ASTArrayDimsAndInits extends AbstractJavaNode implements Dimensionable {
private int arrayDepth;
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java
index ef4bb804a0..bee539e2e2 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java
@@ -15,8 +15,11 @@ import net.sourceforge.pmd.annotation.Experimental;
* Represents a class or interface type, possibly parameterised with type arguments.
* This node comes in two productions (see below):
*
- * The first is a left-recursive variant, allowing to parse references to type members
- * unambiguously. The resulting node's {@linkplain #getLeftHandSide() left-hand-side type}
+ *
The first is a simple, flat variant. The image of the node corresponds to the
+ * chain of identifiers. May have type arguments.
+ *
+ *
The second is a left-recursive variant, allowing to preserve the position of
+ * type parameters and annotations. The resulting node's {@linkplain #getLeftHandSide() left-hand-side type}
* addresses the type parent of the type. The position of type arguments and annotations are
* preserved.
*
@@ -26,13 +29,15 @@ import net.sourceforge.pmd.annotation.Experimental;
* qualified type name (e.g. {@code java.util.List}, where the full sequence refers to a unique
* type, but individual segments don't).
*
- *
TODO We could uniformise those with an AST visit provided we have a classloader
- * Could be preliminary to type resolution
+ *
TODO I expect to fix that before 7.0.0
+ * We can add an analysis phase that rewrites nodes using semantic info from the symbol table.
+ * That could be useful for many things (eg disambiguate obscured expression names in the
+ * expression grammar).
*
*
*
- * ClassOrInterfaceType ::= ClassOrInterfaceType ( "." {@link ASTAnnotation Annotation}* <IDENTIFIER> {@link ASTTypeArguments TypeArguments}? )+
- * | <IDENTIFIER> ( "." <IDENTIFIER> ) * {@link ASTTypeArguments TypeArguments}?
+ * ClassOrInterfaceType ::= <IDENTIFIER> ( "." <IDENTIFIER> ) * {@link ASTTypeArguments TypeArguments}?
+ * | ClassOrInterfaceType ( "." {@link ASTAnnotation Annotation}* <IDENTIFIER> {@link ASTTypeArguments TypeArguments}? )+
*
*
*/
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrimitiveType.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrimitiveType.java
index 548b2d96ae..0c20836379 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrimitiveType.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrimitiveType.java
@@ -6,8 +6,14 @@
package net.sourceforge.pmd.lang.java.ast;
import java.util.Arrays;
+import java.util.Collections;
import java.util.Locale;
+import java.util.Map;
import java.util.Optional;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+import net.sourceforge.pmd.annotation.Experimental;
/**
@@ -63,9 +69,17 @@ public class ASTPrimitiveType extends AbstractJavaTypeNode implements ASTType {
}
+ public PrimitiveType getModelConstant() {
+ return PrimitiveType.fromToken(getImage()).get();
+ }
+
+
/**
- * Model constants to work with when the node's position is not important.
+ * Constants to symbolise a primitive type when the tree context is
+ * not important. I expect this may be fleshed out to be used by type
+ * resolution or something.
*/
+ @Experimental
public enum PrimitiveType {
BOOLEAN,
CHAR,
@@ -76,6 +90,14 @@ public class ASTPrimitiveType extends AbstractJavaTypeNode implements ASTType {
DOUBLE,
FLOAT;
+ private static final Map LOOKUP =
+ Collections.unmodifiableMap(
+ Arrays.stream(values()).collect(Collectors.toMap(
+ PrimitiveType::getToken,
+ Function.identity()
+ ))
+ );
+
/**
* Returns the token used to represent the type in source,
@@ -95,9 +117,8 @@ public class ASTPrimitiveType extends AbstractJavaTypeNode implements ASTType {
* @return A constant, or the empty optional if the token doesn't correspond to a constant
*/
public static Optional fromToken(String token) {
- return Arrays.stream(values()).filter(v -> v.getToken().equals(token)).findFirst();
+ return Optional.ofNullable(LOOKUP.get(token));
}
-
}
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java
index 2e7bfde05a..8e8737a026 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java
@@ -31,6 +31,10 @@ public final class TypeHelper {
return isA(n, clazz);
}
+ // FIXME checking against the image is for the most part meaningless.
+ // Many type nodes don't have an image or have one that has no relation
+ // to their type. The TypeNode interface should have a method getTypeImage,
+ // or better, we could use symbols instead.
return clazzName.equals(n.getImage()) || clazzName.endsWith("." + n.getImage());
}
diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceTypeTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTTypeTest.kt
similarity index 95%
rename from pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceTypeTest.kt
rename to pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTTypeTest.kt
index 8b94450d8d..8d09e1c7ec 100644
--- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceTypeTest.kt
+++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTTypeTest.kt
@@ -7,9 +7,9 @@ import io.kotlintest.specs.FunSpec
* @author Clément Fournier
* @since 7.0.0
*/
-class ASTClassOrInterfaceTypeTest : FunSpec({
+class ASTTypeTest : FunSpec({
- testGroup("Test non-recursive COITs") {
+ testGroup("Test non-recursive ClassOrInterfaceTypes") {
"java.util.List" should matchType {
it.typeImage shouldBe "java.util.List"
@@ -31,7 +31,7 @@ class ASTClassOrInterfaceTypeTest : FunSpec({
}
}
- testGroup("Test recursive COITs") {
+ testGroup("Test recursive ClassOrInterfaceTypes") {
"java.util.Map.@Foo Entry" should matchType {
it.typeImage shouldBe "java.util.Map.Entry"