[java] Fix incompatible class bounds not checked during incorporation

docs/pages/release_notes.md

@@ -16,13 +16,8 @@ This is a {{ site.pmd.release_type }} release.
 
 ### 🐛 Fixed Issues
 
-* core
- * [#4978](https://github.com/pmd/pmd/issues/4978): \[core] Referenced Rulesets do not emit details on validation errors
- * [#4983](https://github.com/pmd/pmd/pull/4983): \[cpd] Fix CPD crashes about unicode escapes
 * java
- * [#4912](https://github.com/pmd/pmd/issues/4912): \[java] Unable to parse some Java9+ resource references
- * [#4973](https://github.com/pmd/pmd/pull/4973): \[java] Stop parsing Java for CPD
- * [#4988](https://github.com/pmd/pmd/pull/4988): \[java] Fix impl of ASTVariableId::isResourceDeclaration / VariableId/@<!-- -->ResourceDeclaration
+ * [#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
  * [#4852](https://github.com/pmd/pmd/issues/4852): \[java] ReplaceVectorWithList false-positive (neither Vector nor List usage) 

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeOps.java

@@ -398,11 +398,17 @@ public Void visitPrimitive(JPrimitiveType t, Set<JTypeMirror> result) {
 
 
  public static Convertibility isConvertible(@NonNull JTypeMirror t, @NonNull JTypeMirror s) {
- return isConvertible(t, s, true);
+ return isConvertible(t, s, true, false);
  }
 
+ @Deprecated // unused
  public static Convertibility isConvertibleNoCapture(@NonNull JTypeMirror t, @NonNull JTypeMirror s) {
- return isConvertible(t, s, false);
+ return isConvertible(t, s, false, false);
+ }
+
+ // does not perform side effects on inference vars
+ public static Convertibility isConvertiblePure(JTypeMirror t, JTypeMirror s) {
+ return isConvertible(t, s, true, true);
  }
 
  /**
@@ -414,7 +420,7 @@ public static Convertibility isConvertibleNoCapture(@NonNull JTypeMirror t, @Non
  * @param t A type T
  * @param s A type S
  */
- public static Convertibility isConvertible(@NonNull JTypeMirror t, @NonNull JTypeMirror s, boolean capture) {
+ public static Convertibility isConvertible(@NonNull JTypeMirror t, @NonNull JTypeMirror s, boolean capture, boolean pure) {
  // This is commented out as it makes JTypeMirror#isSubtypeOf partial,
  // which is not nice for the API... But this assert caught a bug and
  // should probably be enabled.
@@ -428,15 +434,17 @@ public static Convertibility isConvertible(@NonNull JTypeMirror t, @NonNull JTyp
  } else if (s.isVoid() || t.isVoid()) { // t != s
  return Convertibility.NEVER;
  } else if (s instanceof InferenceVar) {
- // it's possible to add a bound to UNKNOWN or ERROR
- ((InferenceVar) s).addBound(BoundKind.LOWER, t);
+ if (!pure) {
+ // it's possible to add a bound to UNKNOWN or ERROR
+ ((InferenceVar) s).addBound(BoundKind.LOWER, t);
+ }
  return Convertibility.SUBTYPING;
  } else if (isTypeRange(s)) {
  // If s is a type range L..U,
  // then showing t <: s is the same thing as t <: L
  JTypeMirror lower = lowerBoundRec(s);
  if (!lower.isBottom()) {
- return isConvertible(t, lower, capture);
+ return isConvertible(t, lower, capture, pure);
  }
  // otherwise fallthrough
  } else if (isSpecialUnresolved(t)) {
@@ -452,32 +460,22 @@ public static Convertibility isConvertible(@NonNull JTypeMirror t, @NonNull JTyp
  // subtypes of (nearly) anything. This allows them to
  // pass bound checks on type variables.
  if (Objects.equals(t.getSymbol(), s.getSymbol())) {
- return typeArgsAreContained((JClassType) t, (JClassType) s);
+ return typeArgsAreContained((JClassType) t, (JClassType) s, pure);
  } else {
  return Convertibility.subtypeIf(s instanceof JClassType); // excludes array or so
  }
  } else if (s instanceof JIntersectionType) { // TODO test intersection with tvars & arrays
  // If S is an intersection, then T must conform to *all* bounds of S
  // Symmetrically, if T is an intersection, T <: S requires only that
  // at least one bound of T is a subtype of S.
- return Convertibility.subtypesAll(t, asList(s));
+ return Convertibility.subtypesAll(t, asList(s), pure);
  }
 
  if (capture) {
  t = capture(t);
  }
- return t.acceptVisitor(SubtypeVisitor.INSTANCE, s);
- }
-
- // does not perform side effects on inference vars
- private static Convertibility isSubtypePure(JTypeMirror t, JTypeMirror s) {
- if (t instanceof InferenceVar) {
- return Convertibility.subtypeIf(((InferenceVar) t).isSubtypeNoSideEffect(s));
- } else if (s instanceof InferenceVar) {
- return Convertibility.subtypeIf(((InferenceVar) s).isSupertypeNoSideEffect(t));
- }
-
- return isConvertible(t, s);
+ SubtypeVisitor visitor = pure ? SubtypeVisitor.PURE : SubtypeVisitor.INSTANCE;
+ return t.acceptVisitor(visitor, s);
  }
 
  public static boolean allArgsAreUnboundedWildcards(List<JTypeMirror> sargs) {
@@ -606,10 +604,10 @@ static Convertibility subtypeIf(boolean b) {
  return b ? SUBTYPING : NEVER;
  }
 
- static Convertibility subtypesAll(JTypeMirror t, Iterable<? extends JTypeMirror> supers) {
+ static Convertibility subtypesAll(JTypeMirror t, Iterable<? extends JTypeMirror> supers, boolean pure) {
  Convertibility result = SUBTYPING;
  for (JTypeMirror ui : supers) {
- Convertibility sub = isConvertible(t, ui);
+ Convertibility sub = isConvertible(t, ui, true, pure);
  if (sub == NEVER) {
  return NEVER;
  }
@@ -618,10 +616,10 @@ static Convertibility subtypesAll(JTypeMirror t, Iterable<? extends JTypeMirror>
  return result;
  }
 
- static Convertibility anySubTypesAny(Iterable<? extends JTypeMirror> us, Iterable<? extends JTypeMirror> vs) {
+ static Convertibility anySubTypesAny(Iterable<? extends JTypeMirror> us, Iterable<? extends JTypeMirror> vs, boolean pure) {
  for (JTypeMirror ui : us) {
  for (JTypeMirror vi : vs) {
- Convertibility sub = isConvertible(ui, vi);
+ Convertibility sub = isConvertible(ui, vi, true, pure);
  if (sub != NEVER) {
  return sub.and(SUBTYPING); // never return identity here
  }
@@ -685,7 +683,7 @@ private static boolean isCvar(JTypeMirror s) {
  *
  * <p>Defined in JLS§4.5.1 (Type Arguments of Parameterized Types)
  */
- static Convertibility typeArgContains(JTypeMirror s, JTypeMirror t) {
+ static Convertibility typeArgContains(JTypeMirror s, JTypeMirror t, boolean pure) {
  // the contains relation can be understood intuitively if we
  // represent types as ranges on a line:
 
@@ -703,7 +701,7 @@ static Convertibility typeArgContains(JTypeMirror s, JTypeMirror t) {
  // ⊥ -------U(T)-----U(S)------> Object (L(T) = L(S) = ⊥)
  // ⊥ -------L(S)-----L(T)------> Object (U(T) = U(S) = Object)
 
- if (isSameTypeInInference(s, t)) {
+ if (isSameType(s, t, !pure, false)) {
  // S <= S
  return Convertibility.SUBTYPING;
  }
@@ -719,10 +717,10 @@ static Convertibility typeArgContains(JTypeMirror s, JTypeMirror t) {
 
  if (sw.isUpperBound()) {
  // Test U(T) <: U(S), we already know L(S) <: L(T), because L(S) is bottom
- return isConvertible(wildUpperBound(t), sw.asUpperBound());
+ return isConvertible(wildUpperBound(t), sw.asUpperBound(), pure, false);
  } else {
  // Test L(S) <: L(T), we already know U(T) <: U(S), because U(S) is top
- return isConvertible(sw.asLowerBound(), wildLowerBound(t));
+ return isConvertible(sw.asLowerBound(), wildLowerBound(t), pure, false);
  }
  }
 
@@ -733,7 +731,7 @@ static Convertibility typeArgContains(JTypeMirror s, JTypeMirror t) {
  /**
  * Generalises containment to check if for each i, {@code Ti <= Si}.
  */
- static Convertibility typeArgsAreContained(JClassType t, JClassType s) {
+ static Convertibility typeArgsAreContained(JClassType t, JClassType s, boolean pure) {
  List<JTypeMirror> targs = t.getTypeArgs();
  List<JTypeMirror> sargs = s.getTypeArgs();
 
@@ -768,7 +766,7 @@ static Convertibility typeArgsAreContained(JClassType t, JClassType s) {
 
  Convertibility result = Convertibility.SUBTYPING;
  for (int i = 0; i < targs.size(); i++) {
- Convertibility sub = typeArgContains(sargs.get(i), targs.get(i));
+ Convertibility sub = typeArgContains(sargs.get(i), targs.get(i), pure);
  if (sub == Convertibility.NEVER) {
  return Convertibility.NEVER;
  }
@@ -780,7 +778,13 @@ static Convertibility typeArgsAreContained(JClassType t, JClassType s) {
 
  private static final class SubtypeVisitor implements JTypeVisitor<Convertibility, JTypeMirror> {
 
- static final SubtypeVisitor INSTANCE = new SubtypeVisitor();
+ static final SubtypeVisitor INSTANCE = new SubtypeVisitor(false);
+ static final SubtypeVisitor PURE = new SubtypeVisitor(true);
+ private final boolean pure;
+
+ private SubtypeVisitor(boolean pure) {
+ this.pure = pure;
+ }
 
  @Override
  public Convertibility visit(JTypeMirror t, JTypeMirror s) {
@@ -793,9 +797,9 @@ public Convertibility visitTypeVar(JTypeVar t, JTypeMirror s) {
  return Convertibility.SUBTYPING;
  }
  if (isTypeRange(s)) {
- return isConvertible(t, lowerBoundRec(s));
+ return isConvertible(t, lowerBoundRec(s), true, pure);
  }
- return isConvertible(t.getUpperBound(), s);
+ return isConvertible(t.getUpperBound(), s, true, pure);
  }
 
  @Override
@@ -815,8 +819,10 @@ public Convertibility visitInferenceVar(InferenceVar t, JTypeMirror s) {
  if (s == t.getTypeSystem().NULL_TYPE || s instanceof JPrimitiveType) {
  return Convertibility.NEVER;
  }
- // here we add a constraint on the variable
- t.addBound(BoundKind.UPPER, s);
+ if (!pure) {
+ // here we add a constraint on the variable
+ t.addBound(BoundKind.UPPER, s);
+ }
  return Convertibility.SUBTYPING;
  }
 
@@ -845,7 +851,7 @@ public Convertibility visitClass(JClassType t, JTypeMirror s) {
  // a raw type C is a supertype for all the family of parameterized type generated by C<F1, .., Fn>
  return Convertibility.SUBTYPING;
  } else {
- return typeArgsAreContained(superDecl, cs);
+ return typeArgsAreContained(superDecl, cs, pure);
  }
  }
 
@@ -869,7 +875,7 @@ public Convertibility visitIntersection(JIntersectionType t, JTypeMirror s) {
  // what we mean is, if S is an intersection, then
  // "any component of T subtypes any component of S"
 
- return Convertibility.anySubTypesAny(t.getComponents(), asList(s));
+ return Convertibility.anySubTypesAny(t.getComponents(), asList(s), pure);
  }
 
  @Override
@@ -890,7 +896,7 @@ public Convertibility visitArray(JArrayType t, JTypeMirror s) {
  // arrays of primitive types have no sub-/ supertype
  return Convertibility.subtypeIf(cs.getComponentType() == t.getComponentType());
  } else {
- return isConvertible(t.getComponentType(), cs.getComponentType());
+ return isConvertible(t.getComponentType(), cs.getComponentType(), true, pure);
  }
  }
 
@@ -1744,7 +1750,7 @@ public static Set<JTypeMirror> mostSpecific(Collection<? extends JTypeMirror> se
  for (JTypeMirror v : set) {
  for (JTypeMirror w : set) {
  if (!w.equals(v) && !hasUnresolvedSymbol(w)) {
- Convertibility isConvertible = isSubtypePure(w, v);
+ Convertibility isConvertible = isConvertiblePure(w, v);
  if (isConvertible.bySubtyping()
  // This last case covers unchecked conversion. It is made antisymmetric by the
  // test for a symbol. eg |G| <~> G<?> so it would fail.

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

@@ -9,8 +9,11 @@
 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;
 import net.sourceforge.pmd.lang.java.types.TypeOps.Convertibility;
 import net.sourceforge.pmd.lang.java.types.internal.infer.InferenceVar.BoundKind;
 
@@ -51,6 +54,11 @@ static class CheckBound extends IncorporationAction {
  this.myBound = bound;
  }
 
+ public static boolean isClassType(JTypeMirror bound) {
+ JTypeDeclSymbol symbol = bound.getSymbol();
+ return symbol instanceof JClassSymbol && !symbol.isInterface();
+ }
+
  /**
  * The list of bound kinds to be checked. If the new bound is
  * equality, then all other bounds need to be checked. Otherwise,
@@ -70,8 +78,26 @@ public void apply(InferenceContext ctx) {
  }
  }
  }
+
+ if (myKind == BoundKind.UPPER && isClassType(myBound)) {
+ // Check that other upper bounds that are class types are related to this bound.
+ // Otherwise, GLB does not exist and its construction would fail during ReductionStep#UPPER.
+ for (JTypeMirror otherBound : ivar.getBounds(BoundKind.UPPER)) {
+ if (otherBound != myBound && isClassType(otherBound)) {
+ // Since we are testing both directions we cannot let those tests add bounds on the ivars,
+ // because they could be contradictory.
+ boolean areRelated = TypeOps.isConvertiblePure(myBound, otherBound).somehow()
+ || TypeOps.isConvertiblePure(otherBound, myBound).somehow();
+
+ if (!areRelated) {
+ throw ResolutionFailedException.incompatibleBound(ctx.logger, ivar, myKind, myBound, BoundKind.UPPER, otherBound);
+ }
+ }
+ }
+ }
  }
 
+
  /**
  * Check compatibility between this bound and another.
  */
@@ -97,13 +123,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/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/TestUtilitiesForTypes.kt

@@ -168,7 +168,7 @@ fun assertSubtypeOrdering(vararg ts: JTypeMirror) {
 fun JClassType.parameterize(m1: JTypeMirror, vararg mirror: JTypeMirror): JClassType = withTypeArguments(listOf(m1, *mirror))
 
 fun assertSubtype(t: JTypeMirror, s: JTypeMirror, capture: Boolean = true, passes: Convertibility.() -> Boolean) {
- val res = isConvertible(t, s, capture)
+ val res = isConvertible(t, s, capture, true)
  assertTrue("$t \n\t\t<: $s") {
  res.passes()
  }

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