Fix false-negative when overriding method type-parameter bounds

checker/jtreg/nullness/issue824/Class1.astub → checker/jtreg/nullness/issue824/Issue824_1.astub

@@ -1,7 +1,7 @@
 import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
- class Class1<Q extends @NonNull Object> {
+ class Issue824_1<Q extends @NonNull Object> {
     public <T extends @NonNull Object> T methodTypeParam(T t);
     public void classTypeParam(Q e);
 

checker/jtreg/nullness/issue824/Class2.java → checker/jtreg/nullness/issue824/Issue824_2.java

@@ -4,15 +4,15 @@
  * The defaults for type variable upper bounds with type Object changed since
  * the issue was filed.  So, this test case has been changed so that
  * annotations on type variable bounds in stub files is still tested.
- * @compile -Xlint:unchecked ../issue824lib/Class1.java
- * @compile/fail/ref=Class2.out -XDrawDiagnostics -Xlint:unchecked -processor org.checkerframework.checker.nullness.NullnessChecker -Anomsgtext Class2.java -Astubs=Class1.astub
- * @compile -Xlint:unchecked -processor org.checkerframework.checker.nullness.NullnessChecker -Anomsgtext Class2.java
+ * @compile -Xlint:unchecked ../issue824lib/Issue824_1.java
+ * @compile/fail/ref=Issue824_2.out -XDrawDiagnostics -Xlint:unchecked -processor org.checkerframework.checker.nullness.NullnessChecker -Anomsgtext Issue824_2.java -Astubs=Issue824_1.astub
+ * @compile -Xlint:unchecked -processor org.checkerframework.checker.nullness.NullnessChecker -Anomsgtext Issue824_2.java
  */
 
 import org.checkerframework.checker.nullness.qual.Nullable;
 
-public class Class2<X> extends Class1<X> {
-  void call(Class1<@Nullable X> class1, Gen<@Nullable X> gen) {
+public class Issue824_2<X> extends Issue824_1<X> {
+  void call(Issue824_1<@Nullable X> class1, Gen<@Nullable X> gen) {
     class1.methodTypeParam(null);
     class1.classTypeParam(null);
 

checker/jtreg/nullness/issue824/Issue824_2.out

@@ -0,0 +1,9 @@
+Issue824_2.java:14:39: compiler.err.proc.messager: [type.argument]
+Issue824_2.java:15:20: compiler.err.proc.messager: [type.argument]
+Issue824_2.java:15:45: compiler.err.proc.messager: [type.argument]
+Issue824_2.java:16:27: compiler.err.proc.messager: [type.arguments.not.inferred]
+Issue824_2.java:19:27: compiler.err.proc.messager: [type.arguments.not.inferred]
+Issue824_2.java:20:26: compiler.err.proc.messager: [argument]
+Issue824_2.java:24:14: compiler.err.proc.messager: [override.return]
+Issue824_2.java:25:33: compiler.err.proc.messager: [type.arguments.not.inferred]
+8 errors

checker/jtreg/nullness/issue824lib/Class1.java → checker/jtreg/nullness/issue824lib/Issue824_1.java

@@ -1,4 +1,4 @@
-public class Class1<Q> {
+public class Issue824_1<Q> {
   class Gen<S> {}
 
   public <T> T methodTypeParam(T t) {

checker/tests/nullness/Issue6864.java

@@ -0,0 +1,25 @@
+import java.util.LinkedList;
+import java.util.List;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+class Issue6864_A {
+  <T extends List<Integer>> T m(T x) {
+    return x;
+  }
+}
+
+class Issue6864_B extends Issue6864_A {
+  // :: error: [override.typeparam]
+  <T extends List<@Nullable Integer>> T m(T x) {
+    x.add(null);
+    return x;
+  }
+}
+
+public class Issue6864 {
+  public static void main(String[] args) {
+    Issue6864_A x = new Issue6864_B();
+    List<Integer> y = new LinkedList<>();
+    x.m(y).get(0).toString();
+  }
+}

docs/manual/contributors.tex

@@ -141,6 +141,7 @@
 Steph Dietzel,
 Stephan Schroevers,
 Stuart Pernsteiner,
+Suvrat Acharya,
 Suzanne Millstein,
 Thomas Schweizer,
 Thomas Wei\ss schuh,

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

@@ -4140,7 +4140,13 @@ public boolean checkOverride() {
         return true;
       }
 
-      boolean result = checkReturn();
+      // First, ensure that method type parameter bounds are compatible. This must be
+      // checked before return/parameter checks because those checks may depend on
+      // the meaning of the type parameters.
+      boolean result = checkTypeParameters();
+
+      // Then check return type, parameters, receiver, and other properties.
+      result &= checkReturn();
       result &= checkParameters();
       if (isMethodReference) {
         result &= checkMemberReferenceReceivers();
@@ -4153,6 +4159,47 @@ public boolean checkOverride() {
       return result;
     }
 
+    /**
+     * Check that corresponding method type parameters have compatible bounds.
+     *
+     * @return true if type parameter bounds are compatible
+     */
+    private boolean checkTypeParameters() {
+      List<AnnotatedTypeVariable> subTypeVars = overrider.getTypeVariables();
+      List<AnnotatedTypeVariable> superTypeVars = overridden.getTypeVariables();
+
+      if (subTypeVars == null || superTypeVars == null) {
+        return true;
+      }
+      if (subTypeVars.size() != superTypeVars.size()) {
+        // Arity mismatch between the two methods' type parameters; Java will report an error.
+        return true;
+      }
+
+      boolean result = true;
+      for (int i = 0; i < subTypeVars.size(); i++) {
+        AnnotatedTypeVariable subVar = subTypeVars.get(i);
+        AnnotatedTypeVariable superVar = superTypeVars.get(i);
+
+        boolean ok = testTypevarContainment(subVar, superVar);
+        if (!ok) {
+          FoundRequired pair = FoundRequired.of(subVar, superVar);
+          checker.reportError(
+              overriderTree,
+              "override.typeparam",
+              subVar.getUnderlyingType().asElement().getSimpleName().toString(),
+              pair.found,
+              pair.required,
+              overriderType,
+              overrider,
+              overriddenType,
+              overridden);
+        }
+        result &= ok;
+      }
+      return result;
+    }
+
     /** Check that an override respects purity. */
     private void checkPurity() {
       String msgKey = isMethodReference ? "purity.methodref" : "purity.overriding";

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

@@ -37,6 +37,7 @@ explicit.annotation.ignored=The qualifier %s is ignored in favor of %s. Either d
 override.return=Incompatible return type.%nfound   : %s%nrequired: %s%nConsequence: method in %s%n  %s%ncannot override method in %s%n  %s
 override.param=Incompatible parameter type for %s.%nfound   : %s%nrequired: %s%nConsequence: method in %s%n  %s%ncannot override method in %s%n  %s
 override.receiver=Incompatible receiver type%nfound   : %s%nrequired: %s%nConsequence: method in %s%n  %s%ncannot override method in %s%n  %s
+override.typeparam=Incompatible type parameter bounds for type parameter %s.%nfound   : %s%nrequired: %s%nConsequence: method in %s%n  %s%ncannot override method in %s%n  %s
 methodref.return=Incompatible return type%nfound   : %s%nrequired: %s%nConsequence: method in %s%n  %s%nis not a valid method reference for method in %s%n  %s
 methodref.param=Incompatible parameter type for %s%nfound   : %s%nrequired: %s%nConsequence: method in %s%n  %s%nis not a valid method reference for method in %s%n  %s
 methodref.receiver=Incompatible receiver type%nfound   : %s%nrequired: %s%nConsequence: method in %s%n  %s%nis not a valid method reference for method in %s%n  %s