Method call compiles with javac but not ecj with upper bound of type variable (eclipse-jdt#4116)

Fix CaptureBinding.initializeBounds() by more closely adhering to JLS
+ document correspondence to JLS

Fix a bug in glb concerning T#RAW vs T<X> or T<?>
+ this fix includes making RawTypeBinding.isSubtypeOf() stricter

Fixes eclipse-jdt#2523

Co-authored-by: coehlrich <coehlrich@users.noreply.github.com>

Author: stephan-herrmann

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/CaptureBinding.java

@@ -22,6 +22,7 @@
  *******************************************************************************/
 package org.eclipse.jdt.internal.compiler.lookup;
 
+import java.util.Arrays;
 import org.eclipse.jdt.core.compiler.CharOperation;
 import org.eclipse.jdt.internal.compiler.ast.ASTNode;
 import org.eclipse.jdt.internal.compiler.ast.Wildcard;
@@ -203,6 +204,12 @@ public void initializeBounds(Scope scope, ParameterizedTypeBinding capturedParam
 			}
 			return;
 		}
+		// JLS 5.1.10
+		//   Let G name a generic type declaration (§8.1.2, §9.1.2) with n type parameters A1,...,An with corresponding bounds U1,...,Un.
+		// our equivalents:
+		//   G = capturedParameterizedType.type
+		//   Ai = wildcardVariable
+		//   Ui = wildcardVariable.superclass & wildcardVariable.superInterfaces()
 		ReferenceBinding originalVariableSuperclass = wildcardVariable.superclass;
 		ReferenceBinding substitutedVariableSuperclass = (ReferenceBinding) Scope.substitute(capturedParameterizedType, originalVariableSuperclass);
 		// prevent cyclic capture: given X<T>, capture(X<? extends T> could yield a circular type
@@ -217,40 +224,62 @@ public void initializeBounds(Scope scope, ParameterizedTypeBinding capturedParam
 			}
 		}
 		// no substitution for wildcard bound (only formal bounds from type variables are to be substituted: 104082)
-		TypeBinding originalWildcardBound = this.wildcard.bound;
+		TypeBinding wildcardBound = this.wildcard.bound;
 
 		switch (this.wildcard.boundKind) {
 			case Wildcard.EXTENDS :
-				// still need to capture bound supertype as well so as not to expose wildcards to the outside (111208)
-				TypeBinding capturedWildcardBound = originalWildcardBound; // as spec'd
-				if (originalWildcardBound.isInterface()) {
-					this.setSuperClass(substitutedVariableSuperclass);
-					// merge wildcard bound into variable superinterfaces using glb
-					if (substitutedVariableInterfaces == Binding.NO_SUPERINTERFACES) {
-						this.setSuperInterfaces(new ReferenceBinding[] { (ReferenceBinding) capturedWildcardBound });
-					} else {
-						int length = substitutedVariableInterfaces.length;
-						System.arraycopy(substitutedVariableInterfaces, 0, substitutedVariableInterfaces = new ReferenceBinding[length+1], 1, length);
-						substitutedVariableInterfaces[0] =  (ReferenceBinding) capturedWildcardBound;
-						this.setSuperInterfaces(Scope.greaterLowerBound(substitutedVariableInterfaces));
-					}
-				} else {
-					// the wildcard bound should be a subtype of variable superclass
-					// it may occur that the bound is less specific, then consider glb (202404)
-					if (capturedWildcardBound.isArrayType() || TypeBinding.equalsEquals(capturedWildcardBound, this)) {
-						this.setSuperClass(substitutedVariableSuperclass);
-					} else {
-						this.setSuperClass((ReferenceBinding) capturedWildcardBound);
-						if (this.superclass.isSuperclassOf(substitutedVariableSuperclass)) {
-							this.setSuperClass(substitutedVariableSuperclass);
+				// JLS:
+				//   If Ti is a wildcard type argument of the form ? extends Bi, then
+				//     Si is a fresh type variable
+				//     - whose upper bound is glb(Bi, Ui[A1:=S1,...,An:=Sn])
+				//     - and whose lower bound is the null type.
+				// our equivalents:
+				//   Ti = this.wildcard
+				//   Si = this
+				//   Bi = wildcardBound
+				//   substitution [A1:=S1,...,An:=Sn] = substitute(capturedParameterizedType, X)
+				//   upper bound = Bi & subst(wildcardVariable.superclass) & subst(wildcardVariable.superInterfaces)
+				//               = wildcardBound & substitutedVariableSuperclass & substitutedVariableInteraces
+				// minimize this intersection using glb and distribute it into this.superclass & this.superInterfaces:
+				TypeBinding[] allBounds;
+				int ifcLen = substitutedVariableInterfaces.length;
+				if (wildcardBound instanceof ReferenceBinding) {
+					allBounds = Arrays.copyOf(substitutedVariableInterfaces, ifcLen + 2);
+					allBounds[ifcLen+1] = wildcardBound;
+				} else { // ignoring non-ReferenceBinding wildcardBound (ArrayBinding?), will be set as firstBound only
+					allBounds = Arrays.copyOf(substitutedVariableInterfaces, ifcLen + 1);
+				}
+				allBounds[ifcLen] = substitutedVariableSuperclass;
+				TypeBinding[] fullGlb = Scope.greaterLowerBound(allBounds, scope, scope.environment());
+				if (fullGlb != null) {
+					// optimistically split the fullGlb into one superClass plus n interfaces
+					ReferenceBinding superClass = scope.getJavaLangObject();
+					ReferenceBinding[] interfaces = new ReferenceBinding[fullGlb.length];
+					int j=0;
+					for (int i=0; i < fullGlb.length; i++) {
+						if (fullGlb[i] instanceof ReferenceBinding ref) {
+							if (ref.isInterface())
+								interfaces[j++] = ref;
+							else
+								superClass = ref;
 						}
-						// TODO: there are cases were we need to compute glb(capturedWildcardBound, substitutedVariableSuperclass)
-						//       but then when glb (perhaps triggered inside setFirstBound()) fails, how to report the error??
 					}
-					this.setSuperInterfaces(substitutedVariableInterfaces);
+					setSuperClass(superClass);
+					if (j != fullGlb.length)
+						interfaces = Arrays.copyOf(interfaces, j);
+					setSuperInterfaces(interfaces);
+				} else {
+					// if glb is null, this capture type is uninhabitable.
+					// opportunistically use weaker check to determine the stronger bound,
+					// just so we give more interesting follow-up errors, if any.
+					if (wildcardBound.original().isCompatibleWith(substitutedVariableSuperclass.original()))
+						setSuperClass((ReferenceBinding) wildcardBound);
+					else
+						setSuperClass(substitutedVariableSuperclass);
+					setSuperInterfaces(substitutedVariableInterfaces);
 				}
-				this.setFirstBound(capturedWildcardBound);
-				if ((capturedWildcardBound.tagBits & TagBits.HasTypeVariable) == 0)
+				this.setFirstBound(wildcardBound);
+				if ((wildcardBound.tagBits & TagBits.HasTypeVariable) == 0)
 					this.tagBits &= ~TagBits.HasTypeVariable;
 				break;
 			case Wildcard.UNBOUND :
@@ -266,19 +295,20 @@ public void initializeBounds(Scope scope, ParameterizedTypeBinding capturedParam
 				break;
 			case Wildcard.SUPER :
 				this.setSuperClass(substitutedVariableSuperclass);
-				if (TypeBinding.equalsEquals(wildcardVariable.firstBound, substitutedVariableSuperclass) || TypeBinding.equalsEquals(originalWildcardBound, substitutedVariableSuperclass)) {
+				if (TypeBinding.equalsEquals(wildcardVariable.firstBound, substitutedVariableSuperclass) || TypeBinding.equalsEquals(wildcardBound, substitutedVariableSuperclass)) {
 					this.setFirstBound(substitutedVariableSuperclass);
 				}
 				this.setSuperInterfaces(substitutedVariableInterfaces);
-				this.lowerBound = originalWildcardBound;
-				if ((originalWildcardBound.tagBits & TagBits.HasTypeVariable) == 0)
+				this.lowerBound = wildcardBound;
+				if ((wildcardBound.tagBits & TagBits.HasTypeVariable) == 0)
 					this.tagBits &= ~TagBits.HasTypeVariable;
 				break;
 		}
 		if(scope.environment().usesNullTypeAnnotations()) {
 			evaluateNullAnnotations(scope, null);
 		}
 	}
+
 	@Override
 	public TypeBinding upwardsProjection(Scope scope, TypeBinding[] mentionedTypeVariables) {
 		if (enterRecursiveProjectionFunction()) {

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/RawTypeBinding.java

@@ -218,6 +218,8 @@ public boolean isProvablyDistinct(TypeBinding otherType) {
 	public boolean isSubtypeOf(TypeBinding right, boolean simulatingBugJDK8026527) {
     	if (simulatingBugJDK8026527) {
     		right = this.environment.convertToRawType(right.erasure(), false);
+    	} else if (!right.isRawType() && TypeBinding.equalsEquals(this.type, right.erasure())) {
+			return false; // outside the scope of JDK-8026527 T#RAW should not be seen as subtype of T<X> nor T<?>
     	}
     	return super.isSubtypeOf(right, simulatingBugJDK8026527);
     }

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java

@@ -422,7 +422,7 @@ public static TypeBinding[] greaterLowerBound(TypeBinding[] types, /*@Nullable*/
 				if (isMalformedPair(iType, jType, scope)) {
 					return null;
 				}
-				if (iType.isCompatibleWith(jType, scope)) { // if Vi <: Vj, Vj is removed
+				if (iType.isSubtypeOf(jType, false)) { // if Vi <: Vj, Vj is removed
 					if (result == types) { // defensive copy
 						System.arraycopy(result, 0, result = new TypeBinding[length], 0, length);
 					}

org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/GenericsRegressionTest.java

@@ -5393,6 +5393,11 @@ public void testBug456459a() {
 		"	EnumSet<? extends T> set = EnumSet.allOf(enumType);\n" +
 		"	                                         ^^^^^^^^\n" +
 		"Type safety: The expression of type Class needs unchecked conversion to conform to Class<Enum<Enum<E>>>\n" +
+		"----------\n" +
+		"6. ERROR in EnumTest.java (at line 10)\n" +
+		"	return set.iterator().next();\n" +
+		"	       ^^^^^^^^^^^^^^^^^^^^^\n" +
+		"Type mismatch: cannot convert from capture#1-of ? extends T to T\n" +
 		"----------\n");
 }
 // simple conflict introduced by additional wildcard bound
@@ -6737,5 +6742,29 @@ public static void main(String[] args) {
 			"----------\n"
 		);
 }
+// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2523
+// Compilation error on ecj but not javac due to usage of incorrect super interface
+public void testIssue2523() {
+        this.runConformTest(
+            new String[] {
+                "X.java",
+                """
+                public class X {
+                    public void test() {
+                        C<? extends D> c = null;
+                        optional(c, null);
+                    }
+
+                    private <I extends F, T extends E<I>> void optional(C<T> l, I i) {}
+
+                    public static interface C<T extends E<?>> {}
+                    public static class D implements E<F> {}
+                    public static interface E<I extends F> {}
+                    public static class F {}
+                }
+                """
+            }
+        );
+}
 }