Issue an error when super bounded wildcards are collapsed

checker/tests/lock/GuardSatisfiedTest.java

@@ -247,7 +247,7 @@ void testGuardSatisfiedOnWildCardExtendsBound(
 
   void testGuardSatisfiedOnWildCardSuperBound(
       // :: error: (guardsatisfied.location.disallowed)
-      MyParameterizedClass1<? super @GuardSatisfied Object> l) {}
+      MyParameterizedClass1<? super @GuardSatisfied String> l) {}
 
   @GuardSatisfied(1) Object testGuardSatisfiedOnParameters(
       @GuardSatisfied GuardSatisfiedTest this,

checker/tests/lock/LockExpressionIsFinal.java

@@ -281,9 +281,9 @@ void testGuardedByExpressionIsFinal() {
 
   class MyParameterizedClass1<T extends @GuardedByUnknown Object> {}
 
-  MyParameterizedClass1<? super @GuardedBy("finalField") Object> m1;
+  MyParameterizedClass1<? super @GuardedBy("finalField") String> m1;
   // :: error: (lock.expression.not.final)
-  MyParameterizedClass1<? super @GuardedBy("nonFinalField") Object> m2;
+  MyParameterizedClass1<? super @GuardedBy("nonFinalField") String> m2;
 
   MyParameterizedClass1<? extends @GuardedBy("finalField") Object> m3;
   // :: error: (lock.expression.not.final)

checker/tests/nullness/generics/Issue5006.java

@@ -0,0 +1,18 @@
+public class Issue5006 {
+
+  static class C<T> {
+    T get() {
+      throw new RuntimeException("");
+    }
+  }
+
+  interface X {
+    C<? extends Object> get();
+  }
+
+  interface Y extends X {
+    @Override
+    // :: error: (super.wildcard)
+    C<? super Object> get();
+  }
+}

checker/tests/nullness/generics/WildcardAnnos.java

@@ -8,13 +8,18 @@ public class WildcardAnnos {
 
   // The implicit upper bound is Nullable, because the annotation
   // on the wildcard is propagated. Therefore this type is:
-  // @Nullable List<? super @NonNull Object extends @Nullable Object> l3 = null;
-  @Nullable List<@Nullable ? super @NonNull Object> l3 = null;
+  // @Nullable List<? super @NonNull String extends @Nullable Object> l3 = null;
+  @Nullable List<@Nullable ? super @NonNull String> l3 = null;
+
+  // The bounds need to have the same annotations because capture conversion,
+  // converts the type argument to just Object.
+  // :: error: (super.wildcard)
+  @Nullable List<@Nullable ? super @NonNull Object> l3b = null;
 
   // :: error: (bound)
-  @Nullable List<@NonNull ? super @Nullable Object> l4 = null;
+  @Nullable List<@NonNull ? super @Nullable String> l4 = null;
 
-  @Nullable List<? super @Nullable Object> l5 = null;
+  @Nullable List<? super @Nullable String> l5 = null;
 
   @Nullable List<? extends @Nullable Object> inReturn() {
     return null;
@@ -25,9 +30,9 @@ void asParam(List<? extends @Nullable Object> p) {}
   // :: error: (conflicting.annos)
   @Nullable List<@Nullable @NonNull ? extends @Nullable Object> l6 = null;
   // :: error: (conflicting.annos)
-  @Nullable List<@Nullable @NonNull ? super @NonNull Object> l7 = null;
+  @Nullable List<@Nullable @NonNull ? super @NonNull String> l7 = null;
   // :: error: (conflicting.annos)
   @Nullable List<? extends @Nullable @NonNull Object> l8 = null;
   // :: error: (conflicting.annos)
-  @Nullable List<? super @Nullable @NonNull Object> l9 = null;
+  @Nullable List<? super @Nullable @NonNull String> l9 = null;
 }

checker/tests/tainting/SameTypeBounds.java

@@ -7,9 +7,9 @@ public class SameTypeBounds {
   static class MyGen<T> {}
 
   void test1(MyGen<Object> p) {
-    // TODO: error: invalid type
     // The upper and lower bound must have the same annotation because the bounds are collasped
     // during capture conversion.
+    // :: error: (super.wildcard)
     MyGen<? super Object> o = p;
     // :: error: (assignment)
     p = o;
@@ -35,8 +35,9 @@ static class MySubClass extends MyClass {}
 
   class Gen<E extends MyClass> {}
 
+  // :: error: (super.wildcard)
   void test3(Gen<MyClass> p, Gen<? super MyClass> p2) {
-    // TODO: error: invalid type
+    // :: error: (super.wildcard)
     Gen<? super MyClass> o = p;
     o = p2;
     // :: error: (assignment)

framework/src/main/java/org/checkerframework/common/basetype/BaseTypeValidator.java

@@ -580,27 +580,52 @@ protected Void visitParameterizedType(AnnotatedDeclaredType type, ParameterizedT
 
       for (int i = 0; i < numTypeArgs; i++) {
         AnnotatedTypeMirror captureTypeArg = capturedType.getTypeArguments().get(i);
-        if (TypesUtils.isCapturedTypeVariable(captureTypeArg.getUnderlyingType())
-            && type.getTypeArguments().get(i).getKind() == TypeKind.WILDCARD) {
-          AnnotatedTypeVariable capturedTypeVar = (AnnotatedTypeVariable) captureTypeArg;
+        if (type.getTypeArguments().get(i).getKind() == TypeKind.WILDCARD) {
           AnnotatedWildcardType wildcard = (AnnotatedWildcardType) type.getTypeArguments().get(i);
-          // Substitute the captured type variables with their wildcards. Without this, the
-          // isSubtype check crashes because wildcards aren't comparable with type variables.
-          AnnotatedTypeMirror catpureTypeVarUB =
-              atypeFactory
-                  .getTypeVarSubstitutor()
-                  .substituteWithoutCopyingTypeArguments(
-                      typeVarToWildcard, capturedTypeVar.getUpperBound());
-          if (!atypeFactory
-              .getTypeHierarchy()
-              .isSubtype(catpureTypeVarUB, wildcard.getExtendsBound())) {
-            checker.reportError(
-                tree.getTypeArguments().get(i),
-                "type.argument",
-                element.getTypeParameters().get(i),
-                element.getSimpleName(),
-                wildcard.getExtendsBound(),
-                capturedTypeVar.getUpperBound());
+          if (TypesUtils.isCapturedTypeVariable(captureTypeArg.getUnderlyingType())) {
+            AnnotatedTypeVariable capturedTypeVar = (AnnotatedTypeVariable) captureTypeArg;
+            // Substitute the captured type variables with their wildcards. Without this, the
+            // isSubtype check crashes because wildcards aren't comparable with type variables.
+            AnnotatedTypeMirror catpureTypeVarUB =
+                atypeFactory
+                    .getTypeVarSubstitutor()
+                    .substituteWithoutCopyingTypeArguments(
+                        typeVarToWildcard, capturedTypeVar.getUpperBound());
+            if (!atypeFactory
+                .getTypeHierarchy()
+                .isSubtype(catpureTypeVarUB, wildcard.getExtendsBound())) {
+              checker.reportError(
+                  tree.getTypeArguments().get(i),
+                  "type.argument",
+                  element.getTypeParameters().get(i),
+                  element.getSimpleName(),
+                  wildcard.getExtendsBound(),
+                  capturedTypeVar.getUpperBound());
+            }
+          } else if (AnnotatedTypes.isExplicitlySuperBounded(wildcard)) {
+            // If the super bound of the wildcard is the same as the upper bound of the type
+            // parameter, then javac uses the bound rather than creating a fresh type variable.
+            // (See https://bugs.openjdk.java.net/browse/JDK-8054309.)
+            // In this case, the Checker Framework uses the annotations on the super bound of the
+            // wildcard and ignores the annotations on the extends bound. So, issue a warning if
+            // the annotations on the extends bound are not the same as the annotations on the super
+            // bound.
+            if (!(atypeFactory
+                    .getQualifierHierarchy()
+                    .isSubtype(
+                        wildcard.getSuperBound().getEffectiveAnnotations(),
+                        wildcard.getExtendsBound().getAnnotations())
+                && atypeFactory
+                    .getQualifierHierarchy()
+                    .isSubtype(
+                        wildcard.getExtendsBound().getAnnotations(),
+                        wildcard.getSuperBound().getEffectiveAnnotations()))) {
+              checker.reportError(
+                  tree.getTypeArguments().get(i),
+                  "super.wildcard",
+                  wildcard.getExtendsBound(),
+                  wildcard.getSuperBound());
+            }
           }
         }
       }

framework/src/main/java/org/checkerframework/common/basetype/messages.properties

@@ -15,6 +15,7 @@ annotation=incompatible types in annotation.%nfound   : %s%nrequired: %s
 conditional=incompatible types in conditional expression.%nfound   : %s%nrequired: %s
 switch.expression=incompatible types in switch expression.%nfound   : %s%nrequired: %s
 type.argument=incompatible type argument for type parameter %s of %s.%nfound   : %s%nrequired: %s
+super.wildcard=bounds must have the same annotations.%nsuper bound  : %s%nextends bound: %s
 argument=incompatible argument for parameter %s of %s.%nfound   : %s%nrequired: %s
 varargs=incompatible types in varargs.%nfound   : %s%nrequired: %s
 type.incompatible=incompatible types.%nfound   : %s%nrequired: %s

framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java

@@ -4823,7 +4823,7 @@ public AnnotatedTypeMirror applyCaptureConversion(
           // Javac used a declared type instead of a captured type variable.  This seems to happen
           // when the bounds of the captured type variable would have been identical. This seems to
           // be a violation of the JLS, but javac does this, so the Checker Framework must handle
-          // that case.
+          // that case. (See https://bugs.openjdk.java.net/browse/JDK-8054309.)
           replaceAnnotations(
               ((AnnotatedWildcardType) uncapturedTypeArg).getSuperBound(), capturedTypeArg);
         }

framework/tests/defaulting/lowerbound/LowerBoundDefaulting.java

@@ -5,9 +5,9 @@
 
 import org.checkerframework.framework.testchecker.defaulting.LowerBoundQual.*;
 
-class MyArrayList<MAL extends String> {}
+class MyArrayList<MAL extends CharSequence> {}
 
-class MyExplicitArray<MEA extends String> {}
+class MyExplicitArray<MEA extends CharSequence> {}
 
 public class LowerBoundDefaulting {
 
@@ -31,12 +31,13 @@ public void implicitsWildcard(MyArrayList<?> myArrayList) {
 
     // should fail because @LbImplicit is below @LbTop
     // :: error: (assignment)
-    @LbTop MyArrayList<@LbTop ? extends @LbTop String> iwLowerBoundIncompatible = myArrayList;
+    @LbTop MyArrayList<@LbTop ? extends @LbTop CharSequence> iwLowerBoundIncompatible = myArrayList;
 
     // :: error: (assignment)
-    @LbTop MyArrayList<@LbExplicit ? extends @LbTop String> iwLowerBoundCompatible = myArrayList;
+    @LbTop MyArrayList<@LbExplicit ? extends @LbTop CharSequence> iwLowerBoundCompatible = myArrayList;
 
-    @LbTop MyArrayList<@LbImplicit ? extends @LbTop String> iwLowerBoundStillCompatible = myArrayList;
+    @LbTop MyArrayList<@LbImplicit ? extends @LbTop CharSequence> iwLowerBoundStillCompatible =
+        myArrayList;
   }
 
   public void implicitExtendBoundedWildcard(MyArrayList<? extends String> iebList) {
@@ -51,16 +52,16 @@ public void implicitExtendBoundedWildcard(MyArrayList<? extends String> iebList)
     @LbTop MyArrayList<@LbImplicit ? extends @LbTop String> iebLowerBoundCompatible = iebList;
   }
 
-  // :: error: (type.argument)
+  // MyArrayList<@LbTop MAL extends @LbTop CharSequence>
+  // elbLsit is MyArrayList<@LbTop ? super @LbExplicit String>
+  // capture is MyArrayList<cap#1 @LbTop ? super @LbTop String>
+  // The super bound is the LUB of @LbTop and @LbExplicit.
   public void explicitLowerBoundedWildcard(MyArrayList<? super String> elbList) {
-    // should fail because @LbExplicit is below @LbTop
     // :: error: (assignment)
-    @LbTop MyArrayList<@LbTop ? super @LbTop String> iebLowerBoundIncompatible = elbList;
+    @LbTop MyArrayList<@LbBottom ? super @LbBottom String> iebLowerBoundIncompatible = elbList;
 
-    // :: error: (type.argument)
     @LbTop MyArrayList<@LbTop ? super @LbExplicit String> iebLowerBoundStillIncompatible = elbList;
 
-    // :: error: (assignment)
     @LbTop MyArrayList<@LbTop ? super @LbImplicit String> iebLowerBoundCompatible = elbList;
   }
 }

framework/tests/defaulting/upperbound/UpperBoundDefaulting.java

@@ -6,10 +6,10 @@
 import org.checkerframework.framework.testchecker.defaulting.UpperBoundQual.*;
 
 // Upper bound: @UbExplicit, Lower bound: @UbBottom.
-class MyArrayList<MAL extends String> {}
+class MyArrayList<MAL extends CharSequence> {}
 
 // Upper bound: @UbExplicit, Lower bound: @UbBottom.
-class MyExplicitArray<MEA extends String> {}
+class MyExplicitArray<MEA extends CharSequence> {}
 
 public class UpperBoundDefaulting {
 
@@ -28,11 +28,11 @@ public <UAL extends String> void explicitUpperBoundTypeVar() {
 
   public void implicitsWildcard(MyArrayList<?> myArrayList) {
 
-    @UbTop MyArrayList<@UbBottom ? extends @UbTop String> iwLowerBoundIncompatible = myArrayList;
+    @UbTop MyArrayList<@UbBottom ? extends @UbTop CharSequence> iwLowerBoundIncompatible = myArrayList;
 
-    @UbTop MyArrayList<@UbBottom ? extends @UbExplicit String> iwLowerBoundCompatible = myArrayList;
+    @UbTop MyArrayList<@UbBottom ? extends @UbExplicit CharSequence> iwLowerBoundCompatible = myArrayList;
 
-    @UbTop MyArrayList<@UbBottom ? extends @UbImplicit String> iwLowerBoundStillCompatible =
+    @UbTop MyArrayList<@UbBottom ? extends @UbImplicit CharSequence> iwLowerBoundStillCompatible =
         // :: error: (assignment)
         myArrayList;
   }
@@ -51,6 +51,7 @@ public void explicitLowerBoundedWildcard(MyArrayList<? super String> elbList) {
     @UbTop MyArrayList<@UbTop ? super @UbBottom String> iebLowerBoundIncompatible = elbList;
 
     // Upper bound: GLB(@UbExplicit, @UbImplicit), Lower bound: @UbBottom.
+    // :: error: (assignment)
     @UbTop MyArrayList<@UbImplicit ? super @UbBottom String> iebLowerBoundStillIncompatible = elbList;
 
     @UbTop MyArrayList<@UbExplicit ? super @UbBottom String> iebLowerBoundCompatible = elbList;