Fix viewpoint-adapted varargs type

docs/CHANGELOG.md

@@ -175,7 +175,7 @@ Other improvements and bug fixes:
 
 **Closed issues:**
 
-eisop#433, eisop#792, eisop#863, eisop#1015, eisop#1074, eisop#1315,
+eisop#386, eisop#433, eisop#792, eisop#863, eisop#1015, eisop#1074, eisop#1315,
 eisop#1653, eisop#1801, eisop#1819.
 
 

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

@@ -195,7 +195,7 @@
  * @see TypeHierarchy#isSubtype
  * @see AnnotatedTypeFactory
  */
 public class BaseTypeVisitor<Factory extends GenericAnnotatedTypeFactory<?, ?, ?, ?>>
         extends SourceVisitor<Void, Void> {
 
     /** The {@link BaseTypeChecker} for error reporting. */
@@ -2555,6 +2555,10 @@
             return;
         }
 
+        if (!shouldCheckVarargs(tree)) {
+            return;
+        }
+
         // This is the varags type, an array.
         AnnotatedArrayType lastParamAnnotatedType = invokedMethod.getVarargType();
 
@@ -2580,6 +2584,23 @@
                 lastParamAnnotatedType, wrappedVarargsType, tree, "varargs.type.incompatible");
     }
 
+    /**
+     * Returns true if the varargs array created for the given varargs invocation should be checked
+     * against the formal varargs parameter type.
+     *
+     * <p>The default implementation returns false if zero varargs actual arguments were passed at
+     * the call site, because no user arguments were passed to populate the varargs array. Checkers
+     * that enforce container array annotations on implicit empty arrays (such as the Value Checker
+     * enforcing {@code @MinLen}) may override this method to return true even when zero varargs
+     * actual arguments were passed.
+     *
+     * @param tree the invocation tree
+     * @return true if the varargs array should be checked
+     */
+    protected boolean shouldCheckVarargs(Tree tree) {
+        return !TreeUtils.isCallToVarargsMethodWithZeroVarargsActuals(tree);
+    }
+
     /**
      * Checks that all the given {@code preconditions} hold true immediately prior to the method
      * invocation or variable access at {@code tree}.

framework/src/main/java/org/checkerframework/common/value/ValueVisitor.java

@@ -48,6 +48,13 @@ public ValueVisitor(BaseTypeChecker checker) {
         super(checker);
     }
 
+    @Override
+    protected boolean shouldCheckVarargs(Tree tree) {
+        // The Value Checker enforces container array annotations (such as @MinLen) on
+        // implicit empty arrays created for zero-argument varargs calls.
+        return true;
+    }
+
     /**
      * ValueVisitor overrides this method so that it does not have to check variables annotated with
      * the {@link IntRangeFromPositive} annotation, the {@link IntRangeFromNonNegative} annotation,

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

@@ -106,10 +106,11 @@ public void viewpointAdaptConstructor(
             AnnotatedTypeMirror receiverType,
             ExecutableElement constructorElt,
             AnnotatedExecutableType constructorType) {
-        // constructorType's typevar are not substituted when calling viewpointAdaptConstructor
+        // constructorType's type variables are not substituted when calling
+        // viewpointAdaptConstructor.
         AnnotatedExecutableType unsubstitutedConstructorType = constructorType.deepCopy();
 
-        // For constructors, we adapt parameter types, return type and type parameters
+        // For constructors, we adapt parameter types, return type, and type parameters.
         List<AnnotatedTypeMirror> parameterTypes = unsubstitutedConstructorType.getParameterTypes();
         List<AnnotatedTypeVariable> typeVariables = unsubstitutedConstructorType.getTypeVariables();
         AnnotatedTypeMirror constructorReturn = unsubstitutedConstructorType.getReturnType();
@@ -135,6 +136,8 @@ public void viewpointAdaptConstructor(
         constructorType.setParameterTypes(unsubstitutedConstructorType.getParameterTypes());
         constructorType.setTypeVariables(unsubstitutedConstructorType.getTypeVariables());
         constructorType.setReturnType(unsubstitutedConstructorType.getReturnType());
+        // Recompute the vararg type to ensure it points to the newly updated parameter list.
+        constructorType.computeVarargType();
     }
 
     @Override
@@ -146,10 +149,10 @@ public void viewpointAdaptMethod(
             return;
         }
 
-        // methodType's typevar are not substituted when calling viewpointAdaptMethod
+        // methodType's type variables are not substituted when calling viewpointAdaptMethod.
         AnnotatedExecutableType unsubstitutedMethodType = methodType.deepCopy();
 
-        // For methods, we additionally adapt method receiver compared to constructors
+        // For methods, we additionally adapt the method receiver (compared to constructors).
         List<AnnotatedTypeMirror> parameterTypes = unsubstitutedMethodType.getParameterTypes();
         List<AnnotatedTypeVariable> typeVariables = unsubstitutedMethodType.getTypeVariables();
         AnnotatedTypeMirror returnType = unsubstitutedMethodType.getReturnType();
@@ -183,12 +186,15 @@ public void viewpointAdaptMethod(
                         AnnotatedTypeCopierWithReplacement.replace(
                                 unsubstitutedMethodType, mappings);
 
-        // Because we can't viewpoint adapt asMemberOf result, we adapt the declared method first,
-        // and sets the corresponding parts to asMemberOf result
+        // Because we cannot viewpoint-adapt the result of asMemberOf, we adapt the declared method
+        // first, and set the corresponding parts on the method type before passing it to
+        // asMemberOf.
         methodType.setReturnType(unsubstitutedMethodType.getReturnType());
         methodType.setReceiverType(unsubstitutedMethodType.getReceiverType());
         methodType.setParameterTypes(unsubstitutedMethodType.getParameterTypes());
         methodType.setTypeVariables(unsubstitutedMethodType.getTypeVariables());
+        // Recompute the vararg type to ensure it points to the newly updated parameter list.
+        methodType.computeVarargType();
     }
 
     /**

framework/tests/viewpointtest/Issue386.java

@@ -0,0 +1,29 @@
+// Test case for EISOP issue #386:
+// https://github.com/eisop/checker-framework/issues/386
+import viewpointtest.quals.*;
+
+public class Issue386 {
+    public class Inner {
+        Inner() {}
+
+        Inner(@ReceiverDependentQual Object... args) {}
+    }
+
+    public class MethodReceiver {
+        void method(@ReceiverDependentQual Object... args) {}
+    }
+
+    @SuppressWarnings("cast.unsafe.constructor.invocation")
+    public void constructorTest(@A Object aObj, @A Object otherAObj, @B Object bObj) {
+        this.new @A Inner(aObj, otherAObj);
+        // :: error: (argument.type.incompatible)
+        this.new @A Inner(aObj, bObj);
+    }
+
+    public void methodTest(
+            @A MethodReceiver receiver, @A Object aObj, @A Object otherAObj, @B Object bObj) {
+        receiver.method(aObj, otherAObj);
+        // :: error: (argument.type.incompatible)
+        receiver.method(aObj, bObj);
+    }
+}

framework/tests/viewpointtest/VarargsConstructor.java

@@ -17,7 +17,8 @@ void foo() {
     void invokeConstructor(@A Object aObj, @B Object bObj, @Top Object topObj) {
         @A Object a = new @A VarargsConstructor(aObj);
         @B Object b = new @B VarargsConstructor(bObj);
-        // :: error: (argument.type.incompatible) :: error: (new.class.type.invalid)
+        // :: error: (argument.type.incompatible) :: error: (new.class.type.invalid) :: error:
+        // (varargs.type.incompatible)
         @Top Object top = new @Top VarargsConstructor(topObj);
         // :: error: (argument.type.incompatible)
         new @A VarargsConstructor(bObj);
@@ -42,7 +43,8 @@ void foo() {
         void invokeConstructor(@A Object aObj, @B Object bObj, @Top Object topObj) {
             @A Object a = new @A Inner(aObj);
             @B Object b = new @B Inner(bObj);
-            // :: error: (argument.type.incompatible) :: error: (new.class.type.invalid)
+            // :: error: (argument.type.incompatible) :: error: (new.class.type.invalid) :: error:
+            // (varargs.type.incompatible)
             @Top Object top = new @Top Inner(topObj);
             // :: error: (argument.type.incompatible)
             new @A Inner(bObj);

javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java

@@ -3018,24 +3018,56 @@ public static boolean isSignaturePolymorphic(final MethodInvocationTree invok) {
         return (symbol.flags() & Flags.SIGNATURE_POLYMORPHIC) != 0;
     }
 
+    /**
+     * Returns true if the given invocation tree is an invocation of a method or constructor with a
+     * vararg parameter, and the invocation has zero vararg actuals.
+     *
+     * @param tree a method invocation or constructor invocation tree
+     * @return true if the given invocation has zero vararg actuals
+     */
+    public static boolean isCallToVarargsMethodWithZeroVarargsActuals(Tree tree) {
+        switch (tree.getKind()) {
+            case METHOD_INVOCATION:
+                return isCallToVarargsMethodWithZeroVarargsActuals((MethodInvocationTree) tree);
+            case NEW_CLASS:
+                return isCallToVarargsMethodWithZeroVarargsActuals((NewClassTree) tree);
+            default:
+                return false;
+        }
+    }
+
     /**
      * Returns true if the given method invocation is an invocation of a method with a vararg
      * parameter, and the invocation has zero vararg actuals.
      *
      * @param invok the method invocation
      * @return true if the given method invocation is an invocation of a method with a vararg
-     *     parameter, and the invocation has with zero vararg actuals
+     *     parameter, and the invocation has zero vararg actuals
      */
     public static boolean isCallToVarargsMethodWithZeroVarargsActuals(MethodInvocationTree invok) {
-        if (!TreeUtils.isVarArgs(invok)) {
+        if (!isVarargsCall(invok)) {
             return false;
         }
         int numParams = elementFromUse(invok).getParameters().size();
-        // The comparison of the number of arguments to the number of formals (minus one) checks
-        // whether there are no varargs actuals.
         return invok.getArguments().size() == numParams - 1;
     }
 
+    /**
+     * Returns true if the given constructor invocation is an invocation of a constructor with a
+     * vararg parameter, and the invocation has zero vararg actuals.
+     *
+     * @param newClassTree the constructor invocation
+     * @return true if the given constructor invocation is an invocation of a constructor with a
+     *     vararg parameter, and the invocation has zero vararg actuals
+     */
+    public static boolean isCallToVarargsMethodWithZeroVarargsActuals(NewClassTree newClassTree) {
+        if (!isVarargsCall(newClassTree)) {
+            return false;
+        }
+        int numParams = elementFromUse(newClassTree).getParameters().size();
+        return newClassTree.getArguments().size() == numParams - 1;
+    }
+
     /**
      * Returns true if the given constructor invocation is a varargs invocation.
      *