From 41bc5ac309007045508eacf3ab7faa29fe97d7ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 29 Nov 2022 17:19:15 +0100 Subject: [PATCH] Fix test with capture --- .../lang/java/internal/JavaAstProcessor.java | 2 +- .../java/symbols/internal/FakeSymAnnot.java | 10 +++ .../pmd/lang/java/types/TypeOps.java | 65 ++++++++++++++----- .../internal/infer/IncorporationAction.java | 4 +- .../internal/infer/PhaseOverloadSet.java | 2 +- .../lang/java/types/TestUtilitiesForTypes.kt | 16 ++++- .../pmd/lang/java/types/TypeCreationDsl.kt | 6 +- .../infer/TypeAnnotationsInferenceTest.kt | 36 +++++++++- .../pmd/lang/java/types/IteratorUtilCopy.txt | 14 ++-- 9 files changed, 118 insertions(+), 37 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaAstProcessor.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaAstProcessor.java index 05c71238de..486b4c78a3 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaAstProcessor.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaAstProcessor.java @@ -58,7 +58,7 @@ public final class JavaAstProcessor { static { Level level; try { - level = Level.valueOf(System.getenv("PMD_DEBUG_LEVEL").toLowerCase(Locale.ROOT)); + level = Level.valueOf(System.getenv("PMD_DEBUG_LEVEL").toUpperCase(Locale.ROOT)); } catch (IllegalArgumentException | NullPointerException ignored) { level = null; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/FakeSymAnnot.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/FakeSymAnnot.java index 7723f31d1b..ad36afed44 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/FakeSymAnnot.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/FakeSymAnnot.java @@ -60,4 +60,14 @@ public class FakeSymAnnot implements SymAnnot { public String toString() { return "@" + annotationClass.getCanonicalName(); } + + @Override + public boolean equals(Object o) { + return SymbolEquality.ANNOTATION.equals(this, o); + } + + @Override + public int hashCode() { + return SymbolEquality.ANNOTATION.hash(this); + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeOps.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeOps.java index ada9c6a2e6..2bcdadd9e1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeOps.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeOps.java @@ -67,7 +67,11 @@ public final class TypeOps { */ public static boolean isSameType(JTypeMirror t, JTypeMirror s) { - return isSameType(t, s, false); + return isSameType(t, s, false, false); + } + + public static boolean isSameTypeWithSameAnnotations(JTypeMirror t, JTypeMirror s) { + return isSameType(t, s, false, true); } /** @@ -75,7 +79,16 @@ public final class TypeOps { * true, then encountering inference variables produces side effects * on them, adding bounds. */ - public static boolean isSameType(JTypeMirror t, JTypeMirror s, boolean inInference) { + public static boolean isSameTypeInInference(JTypeMirror t, JTypeMirror s) { + return isSameType(t, s, true, false); + } + + /** + * Returns true if t and s are the same type. If 'inInference' is + * true, then encountering inference variables produces side effects + * on them, adding bounds. + */ + private static boolean isSameType(JTypeMirror t, JTypeMirror s, boolean inInference, boolean considerAnnotations) { if (t == s) { // also returns true if both t and s are null return true; @@ -86,7 +99,14 @@ public final class TypeOps { } if (!inInference) { - return t.acceptVisitor(SameTypeVisitor.PURE, s); + if (considerAnnotations) { + if (!t.getTypeAnnotations().equals(s.getTypeAnnotations())) { + return false; + } + return t.acceptVisitor(SameTypeVisitor.PURE_WITH_ANNOTATIONS, s); + } else { + return t.acceptVisitor(SameTypeVisitor.PURE, s); + } } // reorder @@ -98,34 +118,42 @@ public final class TypeOps { } public static boolean areSameTypes(List ts, List ss, boolean inInference) { - return areSameTypes(ts, ss, EMPTY, inInference); + return areSameTypes(ts, ss, EMPTY, inInference, false); + } + + public static boolean areSameTypes(List ts, List ss, boolean inInference, boolean considerAnnotations) { + return areSameTypes(ts, ss, EMPTY, inInference, considerAnnotations); } public static boolean areSameTypes(List ts, List ss, Substitution subst) { - return areSameTypes(ts, ss, subst, false); + return areSameTypes(ts, ss, subst, false, false); } - public static boolean areSameTypes(List ts, List ss, Substitution subst, boolean inInference) { + public static boolean areSameTypes(List ts, List ss, Substitution subst, boolean inInference, boolean considerAnnotations) { if (ts.size() != ss.size()) { return false; } for (int i = 0; i < ts.size(); i++) { - if (!isSameType(ts.get(i), ss.get(i).subst(subst), inInference)) { + if (!isSameType(ts.get(i), ss.get(i).subst(subst), inInference, considerAnnotations)) { return false; } } return true; } + // note that this does not take type annotations into account private static final class SameTypeVisitor implements JTypeVisitor { - static final SameTypeVisitor INFERENCE = new SameTypeVisitor(true); - static final SameTypeVisitor PURE = new SameTypeVisitor(false); + static final SameTypeVisitor INFERENCE = new SameTypeVisitor(true, false); + static final SameTypeVisitor PURE = new SameTypeVisitor(false, false); + static final SameTypeVisitor PURE_WITH_ANNOTATIONS = new SameTypeVisitor(false, true); private final boolean inInference; + private final boolean considerAnnotations; - private SameTypeVisitor(boolean inInference) { + private SameTypeVisitor(boolean inInference, boolean considerAnnotations) { this.inInference = inInference; + this.considerAnnotations = considerAnnotations; } @Override @@ -145,8 +173,8 @@ public final class TypeOps { JClassType s2 = (JClassType) s; return t.getSymbol().equals(s2.getSymbol()) // maybe compare the type system as well. && t.hasErasedSuperTypes() == s2.hasErasedSuperTypes() - && isSameType(t.getEnclosingType(), s2.getEnclosingType(), inInference) - && areSameTypes(t.getTypeArgs(), s2.getTypeArgs(), inInference); + && isSameType(t.getEnclosingType(), s2.getEnclosingType(), inInference, considerAnnotations) + && areSameTypes(t.getTypeArgs(), s2.getTypeArgs(), inInference, considerAnnotations); } return false; } @@ -162,7 +190,7 @@ public final class TypeOps { return false; } JWildcardType s2 = (JWildcardType) s; - return s2.isUpperBound() == t.isUpperBound() && isSameType(t.getBound(), s2.getBound(), inInference); + return s2.isUpperBound() == t.isUpperBound() && isSameType(t.getBound(), s2.getBound(), inInference, considerAnnotations); } @Override @@ -204,7 +232,7 @@ public final class TypeOps { return false; } - if (!isSameType(t.getPrimaryBound(), s2.getPrimaryBound(), inInference)) { + if (!isSameType(t.getPrimaryBound(), s2.getPrimaryBound(), inInference, considerAnnotations)) { return false; } @@ -213,7 +241,7 @@ public final class TypeOps { boolean found = false; for (JTypeMirror si : sComps) { // todo won't this behaves weirdly during inference? test it - if (isSameType(ti, si, inInference)) { + if (isSameType(ti, si, inInference, considerAnnotations)) { found = true; break; } @@ -228,7 +256,7 @@ public final class TypeOps { @Override public Boolean visitArray(JArrayType t, JTypeMirror s) { return s instanceof JArrayType - && isSameType(t.getComponentType(), ((JArrayType) s).getComponentType(), inInference); + && isSameType(t.getComponentType(), ((JArrayType) s).getComponentType(), inInference, considerAnnotations); } } @@ -641,7 +669,7 @@ public final class TypeOps { // ⊥ -------U(T)-----U(S)------> Object (L(T) = L(S) = ⊥) // ⊥ -------L(S)-----L(T)------> Object (U(T) = U(S) = Object) - if (isSameType(s, t, true)) { + if (isSameTypeInInference(s, t)) { // S <= S return Convertibility.SUBTYPING; } @@ -727,6 +755,9 @@ public final class TypeOps { @Override public Convertibility visitTypeVar(JTypeVar t, JTypeMirror s) { + if (s instanceof JTypeVar && t.getSymbol() != null && Objects.equals(t.getSymbol(), s.getSymbol())) { + return Convertibility.SUBTYPING; + } if (isTypeRange(s)) { return isConvertible(t, lowerBoundRec(s)); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/IncorporationAction.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/IncorporationAction.java index db3c1bea15..e4cc4e5f7e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/IncorporationAction.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/IncorporationAction.java @@ -5,7 +5,7 @@ package net.sourceforge.pmd.lang.java.types.internal.infer; import static net.sourceforge.pmd.lang.java.types.TypeOps.isConvertible; -import static net.sourceforge.pmd.lang.java.types.TypeOps.isSameType; +import static net.sourceforge.pmd.lang.java.types.TypeOps.isSameTypeInInference; import java.util.ArrayList; import java.util.Set; @@ -99,7 +99,7 @@ abstract class IncorporationAction { */ boolean checkBound(boolean eq, JTypeMirror t, JTypeMirror s, InferenceContext ctx) { // eq bounds are so rare we shouldn't care if they're cached - return eq ? isSameType(t, s, true) + return eq ? isSameTypeInInference(t, s) : checkSubtype(t, s, ctx); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/PhaseOverloadSet.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/PhaseOverloadSet.java index d8876cbbb9..a71882e53f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/PhaseOverloadSet.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/PhaseOverloadSet.java @@ -235,7 +235,7 @@ final class PhaseOverloadSet extends OverloadSet { if (TypeOps.mentionsAny(x, sfun.getTypeParameters()) && !ctx.isGround(y)) { return false; } else { - TypeOps.isSameType(x, y.subst(tToS), true); // adds an equality constraint + TypeOps.isSameTypeInInference(x, y.subst(tToS)); // adds an equality constraint } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/TestUtilitiesForTypes.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/TestUtilitiesForTypes.kt index 0b6178a9e0..6e79dd688e 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/TestUtilitiesForTypes.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/TestUtilitiesForTypes.kt @@ -7,8 +7,9 @@ package net.sourceforge.pmd.lang.java.types -import io.kotest.assertions.fail -import io.kotest.assertions.withClue +import io.kotest.assertions.* +import io.kotest.assertions.print.Printed +import io.kotest.assertions.print.print import io.kotest.matchers.shouldBe import net.sourceforge.pmd.lang.ast.test.shouldBe import net.sourceforge.pmd.lang.ast.test.shouldBeA @@ -52,7 +53,16 @@ fun JTypeMirror.shouldBeUnresolvedClass(canonicalName: String) = infix fun TypeNode.shouldHaveType(expected: JTypeMirror) { withClue(this) { - this.typeMirror shouldBe expected + val actual = this.typeMirror + // test considering annotations + if (!isSameTypeWithSameAnnotations(actual, expected)) + errorCollector.collectOrThrow( + failure( + Expected(actual.print()), + Actual(expected.print()), + ) + ) + } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/TypeCreationDsl.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/TypeCreationDsl.kt index 8674dd6e7b..3ec8501692 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/TypeCreationDsl.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/TypeCreationDsl.kt @@ -9,10 +9,8 @@ package net.sourceforge.pmd.lang.java.types import net.sourceforge.pmd.lang.java.ast.JavaNode import net.sourceforge.pmd.lang.java.ast.ParserTestSpec import net.sourceforge.pmd.lang.java.symbols.JClassSymbol -import net.sourceforge.pmd.lang.java.symbols.SymbolicValue import net.sourceforge.pmd.lang.java.symbols.SymbolicValue.SymAnnot import net.sourceforge.pmd.lang.java.symbols.internal.FakeSymAnnot -import net.sourceforge.pmd.lang.java.symbols.internal.TypeAnnotTestUtil import net.sourceforge.pmd.lang.java.symbols.testdata.ClassWithTypeAnnotationsInside import net.sourceforge.pmd.lang.java.types.TypeOps.isSameType import kotlin.reflect.KClass @@ -72,9 +70,9 @@ interface TypeDslMixin { */ val `@`: (JClassSymbol) -> SymAnnotDsl get() = { SymAnnotDsl(FakeSymAnnot(it)) } - class SymAnnotDsl(delegate: SymAnnot) : SymAnnot by delegate { + class SymAnnotDsl(private val annot: SymAnnot) { /** An infix fun to be able to write the annotation first. */ - infix fun on(t: JTypeMirror): JTypeMirror = t.addAnnotation(this) + infix fun on(t: JTypeMirror): JTypeMirror = t.addAnnotation(annot) } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/TypeAnnotationsInferenceTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/TypeAnnotationsInferenceTest.kt index 4534795c66..b43cc4be6f 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/TypeAnnotationsInferenceTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/TypeAnnotationsInferenceTest.kt @@ -8,6 +8,7 @@ package net.sourceforge.pmd.lang.java.types.internal.infer import net.sourceforge.pmd.lang.java.ast.* import net.sourceforge.pmd.lang.java.types.* +import net.sourceforge.pmd.lang.java.types.internal.infer.TypeInferenceLogger.VerboseLogger import java.util.* /** @@ -21,11 +22,11 @@ import java.lang.annotation.*; class Foo { @Target(ElementType.TYPE_USE) @interface A {} - + T genericMethod(T t) { return t; } - + void someMethod(@A int i) { var i2 = genericMethod(i); } @@ -51,5 +52,36 @@ class Foo { } } + parserTest("Test type annotations do not break wildcard capture") { + val (acu, spy) = parser.parseWithTypeInferenceSpy( + """ +import java.lang.annotation.*; +class Foo { + @Target(ElementType.TYPE_USE) + @interface A {} + + interface Predicate { boolean test(E e); } + + boolean someMethod(T t, Predicate predicate) { + return predicate.test(t); + } +} + + """.trimIndent() + ) + + val (_, A, t_Predicate) = acu.typeDeclarations().toList { it.symbol } + + + spy.shouldBeOk { + val `@A` = `@`(A) + acu.firstMethodCall() shouldHaveType boolean + acu.firstMethodCall().methodType.shouldMatchMethod( + named = "test", + declaredIn = t_Predicate[captureMatcher( `?` `super` (`@A` on acu.typeVar("T")))] + ) + } + } + }) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/IteratorUtilCopy.txt b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/IteratorUtilCopy.txt index 2c68c5249e..2eea3d34ae 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/IteratorUtilCopy.txt +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/IteratorUtilCopy.txt @@ -153,20 +153,20 @@ | | | | +- WildcardType[@TypeMirror = "? extends T"] | | | | +- ClassOrInterfaceType[@TypeMirror = "T"] | | | +- VariableDeclaratorId[@Name = "iter", @TypeMirror = "java.util.Iterator"] - | | +- FormalParameter[@TypeMirror = "java.util.function.Function>"] + | | +- FormalParameter[@TypeMirror = "java.util.function.Function>"] | | +- ModifierList[] - | | +- ClassOrInterfaceType[@TypeMirror = "java.util.function.Function>"] + | | +- ClassOrInterfaceType[@TypeMirror = "java.util.function.Function>"] | | | +- TypeArguments[] | | | +- WildcardType[@TypeMirror = "? super T"] | | | | +- ClassOrInterfaceType[@TypeMirror = "T"] - | | | +- WildcardType[@TypeMirror = "? extends java.util.Iterator"] - | | | +- ClassOrInterfaceType[@TypeMirror = "java.util.Iterator"] + | | | +- WildcardType[@TypeMirror = "? extends @Nullable java.util.Iterator"] + | | | +- ClassOrInterfaceType[@TypeMirror = "@Nullable java.util.Iterator"] | | | +- Annotation[@TypeMirror = "*Nullable"] | | | | +- ClassOrInterfaceType[@TypeMirror = "*Nullable"] | | | +- TypeArguments[] | | | +- WildcardType[@TypeMirror = "? extends R"] | | | +- ClassOrInterfaceType[@TypeMirror = "R"] - | | +- VariableDeclaratorId[@Name = "f", @TypeMirror = "java.util.function.Function>"] + | | +- VariableDeclaratorId[@Name = "f", @TypeMirror = "java.util.function.Function>"] | +- Block[] | +- ReturnStatement[] | +- ConstructorCall[@Failed = false, @Function = "net.sourceforge.pmd.internal.util.IteratorUtilCopy$AbstractIterator.new() -> net.sourceforge.pmd.internal.util.IteratorUtilCopy$AbstractIterator", @MethodName = "new", @TypeMirror = "net.sourceforge.pmd.internal.util.IteratorUtilCopy$AbstractIterator", @Unchecked = false, @VarargsCall = false] @@ -222,8 +222,8 @@ | | | | +- ClassOrInterfaceType[@TypeMirror = "R"] | | | +- VariableDeclarator[@TypeMirror = "void"] | | | +- VariableDeclaratorId[@Name = "next", @TypeMirror = "java.util.Iterator"] - | | | +- MethodCall[@Failed = false, @Function = "java.util.function.Function>.apply(capture#... of ? super T) -> capture#... of ? extends java.util.Iterator", @MethodName = "apply", @TypeMirror = "capture#... of ? extends java.util.Iterator", @Unchecked = false, @VarargsCall = false] - | | | +- VariableAccess[@Name = "f", @TypeMirror = "java.util.function.Function>"] + | | | +- MethodCall[@Failed = true, @Function = "(*unknown*).(*unknown method*)() -> (*unknown*)", @MethodName = "apply", @TypeMirror = "(*unknown*)", @Unchecked = false, @VarargsCall = false] + | | | +- VariableAccess[@Name = "f", @TypeMirror = "java.util.function.Function>"] | | | +- ArgumentList[] | | | +- MethodCall[@Failed = false, @Function = "java.util.Iterator.next() -> capture#... of ? extends T", @MethodName = "next", @TypeMirror = "capture#... of ? extends T", @Unchecked = false, @VarargsCall = false] | | | +- VariableAccess[@Name = "iter", @TypeMirror = "java.util.Iterator"]