[java] Fix incompatible class bounds not checked during incorporation

docs/pages/release_notes.md

@@ -16,6 +16,8 @@ This is a {{ site.pmd.release_type }} release.
 
 ### 🐛 Fixed Issues
 
+* java
+ * [#4980](https://github.com/pmd/pmd/issues/4980): \[java] Bad intersection, unrelated class types java.lang.Object\[] and java.lang.Number
 * java-bestpractices
  * [#4278](https://github.com/pmd/pmd/issues/4278): \[java] UnusedPrivateMethod FP with Junit 5 @MethodSource and default factory method name
  * [#4975](https://github.com/pmd/pmd/issues/4975): \[java] UnusedPrivateMethod false positive when using @MethodSource on a @Nested test

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/IncorporationAction.java

@@ -9,6 +9,8 @@
 import java.util.ArrayList;
 import java.util.Set;
 
+import net.sourceforge.pmd.lang.java.symbols.JClassSymbol;
+import net.sourceforge.pmd.lang.java.symbols.JTypeDeclSymbol;
 import net.sourceforge.pmd.lang.java.types.InternalApiBridge;
 import net.sourceforge.pmd.lang.java.types.JTypeMirror;
 import net.sourceforge.pmd.lang.java.types.TypeOps.Convertibility;
@@ -36,6 +38,66 @@ abstract class IncorporationAction {
 
  abstract void apply(InferenceContext ctx);
 
+ /**
+ * Check that an upper bound with a class (not interface) or array
+ * is compatible with other upper bounds of class or array type.
+ * This is necessary to guarantee the existence of a glb for these,
+ * for {@link ReductionStep#UPPER}.
+ *
+ * <p>If the bound is {@code alpha <: T}, then we must check
+ * that {@code S <: T} or {@code T <: S} holds for all bounds
+ * {@code alpha <: S}, where S is a class or array type. Otherwise,
+ * the GLB does not exist.
+ */
+ static class CheckClassUpperBound extends IncorporationAction {
+
+ private final JTypeMirror myBound;
+
+ CheckClassUpperBound(InferenceVar ivar, JTypeMirror bound) {
+ super(ivar);
+ this.myBound = bound;
+ }
+
+ public static boolean needsCheck(BoundKind kind, JTypeMirror bound) {
+ if (kind == BoundKind.UPPER) {
+ JTypeDeclSymbol symbol = bound.getSymbol();
+ return symbol instanceof JClassSymbol && !symbol.isInterface();
+ }
+ return false;
+ }
+
+
+ @Override
+ public void apply(InferenceContext ctx) {
+ for (BoundKind k : BoundKind.EQ_UPPER) {
+ for (JTypeMirror b : ivar.getBounds(k)) {
+ if (!checkBound(b, ctx)) {
+ throw ResolutionFailedException.incompatibleBound(ctx.logger, ivar, BoundKind.UPPER, myBound, k, b);
+ }
+ }
+ }
+ }
+
+ private boolean checkBound(JTypeMirror otherBound, InferenceContext ctx) {
+
+ JTypeDeclSymbol sym = otherBound.getSymbol();
+ // either the bound is not a concrete class or array type
+ return !(sym instanceof JClassSymbol) || sym.isInterface()
+ // or both bounds are related in some way
+ || CheckBound.checkBound(false, otherBound, myBound, ctx)
+ || CheckBound.checkBound(false, myBound, otherBound, ctx);
+
+ }
+
+
+ @Override
+ public String toString() {
+ return "Check class bound " + BoundKind.UPPER.format(ivar, myBound);
+ }
+
+
+ }
+
  /**
  * Check that a bound is compatible with the other current bounds
  * of an ivar.
@@ -97,13 +159,13 @@ private boolean checkBound(JTypeMirror otherBound, BoundKind otherKind, Inferenc
  /**
  * If 'eq', checks that {@code T = S}, else checks that {@code T <: S}.
  */
- boolean checkBound(boolean eq, JTypeMirror t, JTypeMirror s, InferenceContext ctx) {
+ static 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 ? InternalApiBridge.isSameTypeInInference(t, s)
  : checkSubtype(t, s, ctx);
  }
 
- private boolean checkSubtype(JTypeMirror t, JTypeMirror s, InferenceContext ctx) {
+ private static boolean checkSubtype(JTypeMirror t, JTypeMirror s, InferenceContext ctx) {
  if (ctx.getSupertypeCheckCache().isCertainlyASubtype(t, s)) {
  return true; // supertype was already cached
  }

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/InferenceContext.java

@@ -33,6 +33,7 @@
 import net.sourceforge.pmd.lang.java.types.TypeSystem;
 import net.sourceforge.pmd.lang.java.types.internal.infer.ExprMirror.InvocationMirror.MethodCtDecl;
 import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.CheckBound;
+import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.CheckClassUpperBound;
 import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.PropagateAllBounds;
 import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.PropagateBounds;
 import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.SubstituteInst;
@@ -388,6 +389,9 @@ void onBoundAdded(InferenceVar ivar, BoundKind kind, JTypeMirror bound, boolean
 
  incorporationActions.add(new CheckBound(ivar, kind, bound));
  incorporationActions.add(new PropagateBounds(ivar, kind, bound));
+ if (CheckClassUpperBound.needsCheck(kind, bound)) {
+ incorporationActions.add(new CheckClassUpperBound(ivar, bound));
+ }
  }
  }
 

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/InferenceVar.java

@@ -304,7 +304,7 @@ public Set<BoundKind> complementSet(boolean eqIsAll) {
  // These sets are shared because otherwise *literal millions* of enumsets are created, with the same constants
  static final Set<BoundKind> ALL = EnumSet.allOf(BoundKind.class);
  static final Set<BoundKind> EQ_LOWER = EnumSet.of(EQ, LOWER);
- private static final Set<BoundKind> EQ_UPPER = EnumSet.of(EQ, UPPER);
+ static final Set<BoundKind> EQ_UPPER = EnumSet.of(EQ, UPPER);
  private static final Set<BoundKind> JUST_EQ = Collections.singleton(EQ);
 
  private final String sym;

pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/TypeGenerationUtil.kt

@@ -145,6 +145,7 @@ class RefTypeConstants(override val ts: TypeSystem) : TypeDslMixin {
  val `t_Enum{E}`: JClassType get() = java.lang.Enum::class.decl
 
  val t_Stream: JClassType get() = java.util.stream.Stream::class.raw
+ val `t_Stream{Number}`: JClassType get() = java.util.stream.Stream::class[t_Number]
  val t_Function: JClassType get() = java.util.function.Function::class.raw
  val t_Map: JClassType get() = java.util.Map::class.raw
  val t_MapEntry: JClassType get() = java.util.Map.Entry::class.raw

pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/PolyResolutionTest.kt

@@ -5,23 +5,19 @@
 
 package net.sourceforge.pmd.lang.java.types.internal.infer
 
-import net.sourceforge.pmd.lang.test.ast.shouldBe
-import net.sourceforge.pmd.lang.test.ast.shouldMatchN
 import net.sourceforge.pmd.lang.java.ast.*
 import net.sourceforge.pmd.lang.java.types.*
+import net.sourceforge.pmd.lang.test.ast.shouldBe
+import net.sourceforge.pmd.lang.test.ast.shouldMatchN
 import java.io.BufferedOutputStream
 import java.io.DataOutputStream
 import java.io.OutputStream
 
 /**
  * Expensive test cases for the overload resolution phase.
- *
- * Edit: So those used to be very expensive (think minutes of execution),
- * but optimisations made them very fast.
  */
 class PolyResolutionTest : ProcessorTestSpec({
 
-
  parserTest("Test context passing overcomes null lower bound") {
 
  val (acu, spy) = parser.parseWithTypeInferenceSpy("""
@@ -49,7 +45,7 @@ class PolyResolutionTest : ProcessorTestSpec({
 
  static <T> T id(T t) { return t; }
 
- {
+ void foo(int x) {
  id(x > 0 ? Double.POSITIVE_INFINITY
  : x < 0 ? Double.NEGATIVE_INFINITY
  : Double.NaN);
@@ -373,4 +369,27 @@ class Scratch {
  lambda3 shouldHaveType Runnable::class.decl
  }
  }
+
+ parserTest("Test inference with varargs - bug #4980") {
+
+ val (acu, spy) = parser.parseWithTypeInferenceSpy("""
+ import java.util.stream.Stream;
+
+ class Foo {
+ public <T extends Number> T foo() {
+ return null;
+ }
+
+ public Stream<Number> bar() {
+ return Stream.of(foo());
+ }
+ }
+ """)
+
+ val call = acu.firstMethodCall()
+
+ spy.shouldBeOk {
+ call shouldHaveType gen.`t_Stream{Number}`
+ }
+ }
 })