Add extraArgs argument to commonAssignmentCheck

typetools · Jul 20, 2020 · 6c2cf99 · 6c2cf99
1 parent 9b1cbbd
commit 6c2cf99
Show file tree

Hide file tree

Showing 15 changed files with 116 additions and 49 deletions.
diff --git a/changelog.txt b/changelog.txt
@@ -1,11 +1,16 @@
-Version 3.?.?, August 3, 2020
+Version 3.6.0, August 3, 2020
 
 The Interning Checker supports method annotations @EqualsMethod and
 @CompareToMethod.  Place them on methods like equals(), compareTo(), and
 compare() to permit certain uses of == on non-interned values.
 
 Added an overloaded version of NullnessUtil.castNonNull that takes an error message.
 
+Implementation details:
+
+commonAssignmentCheck() now takes an additional argument.  Type system
+authors must update their overriding implementations.
+
 ---------------------------------------------------------------------------
 
 Version 3.5.0, July 1, 2020

diff --git a/checker/src/main/java/org/checkerframework/checker/formatter/FormatterVisitor.java b/checker/src/main/java/org/checkerframework/checker/formatter/FormatterVisitor.java
@@ -176,8 +176,9 @@ protected void commonAssignmentCheck(
             AnnotatedTypeMirror varType,
             AnnotatedTypeMirror valueType,
             Tree valueTree,
-            @CompilerMessageKey String errorKey) {
-        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey);
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
+        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey, extraArgs);
 
         AnnotationMirror rhs = valueType.getAnnotationInHierarchy(atypeFactory.UNKNOWNFORMAT);
         AnnotationMirror lhs = varType.getAnnotationInHierarchy(atypeFactory.UNKNOWNFORMAT);

diff --git a/checker/src/main/java/org/checkerframework/checker/guieffect/GuiEffectVisitor.java b/checker/src/main/java/org/checkerframework/checker/guieffect/GuiEffectVisitor.java
@@ -455,11 +455,13 @@ public Void visitNewClass(NewClassTree node, Void p) {
 
     /**
      * This method is called to traverse the path back up from any anonymous inner class or lambda
-     * which has been inferred to be UI affecting and re-run {@link #commonAssignmentCheck(Tree,
-     * ExpressionTree, String)} as needed on places where the class declaration or lambda expression
-     * are being assigned to a variable, passed as a parameter or returned from a method. This is
-     * necessary because the normal visitor traversal only checks assignments on the way down the
-     * AST, before inference has had a chance to run.
+     * which has been inferred to be UI affecting and re-run {@code commonAssignmentCheck} as needed
+     * on places where the class declaration or lambda expression are being assigned to a variable,
+     * passed as a parameter or returned from a method. This is necessary because the normal visitor
+     * traversal only checks assignments on the way down the AST, before inference has had a chance
+     * to run.
+     *
+     * @param path the path to traverse up from a UI-affecting class
      */
     private void scanUp(TreePath path) {
         Tree tree = path.getLeaf();

diff --git a/checker/src/main/java/org/checkerframework/checker/i18nformatter/I18nFormatterVisitor.java b/checker/src/main/java/org/checkerframework/checker/i18nformatter/I18nFormatterVisitor.java
@@ -120,7 +120,8 @@ protected void commonAssignmentCheck(
             AnnotatedTypeMirror varType,
             AnnotatedTypeMirror valueType,
             Tree valueTree,
-            @CompilerMessageKey String errorKey) {
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
         AnnotationMirror rhs = valueType.getAnnotationInHierarchy(atypeFactory.I18NUNKNOWNFORMAT);
         AnnotationMirror lhs = varType.getAnnotationInHierarchy(atypeFactory.I18NUNKNOWNFORMAT);
 
@@ -161,6 +162,6 @@ protected void commonAssignmentCheck(
         // issued for a given line of code will take precedence over the
         // assignment.type.incompatible
         // issued by super.commonAssignmentCheck.
-        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey);
+        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey, extraArgs);
     }
 }
diff --git a/checker/src/main/java/org/checkerframework/checker/index/inequality/LessThanVisitor.java b/checker/src/main/java/org/checkerframework/checker/index/inequality/LessThanVisitor.java
@@ -24,7 +24,10 @@ public LessThanVisitor(BaseTypeChecker checker) {
 
     @Override
     protected void commonAssignmentCheck(
-            Tree varTree, ExpressionTree valueTree, @CompilerMessageKey String errorKey) {
+            Tree varTree,
+            ExpressionTree valueTree,
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
 
         // check that when an assignment to a variable declared as @HasSubsequence(a, from, to)
         // occurs, from <= to.
@@ -53,15 +56,16 @@ protected void commonAssignmentCheck(
             }
         }
 
-        super.commonAssignmentCheck(varTree, valueTree, errorKey);
+        super.commonAssignmentCheck(varTree, valueTree, errorKey, extraArgs);
     }
 
     @Override
     protected void commonAssignmentCheck(
             AnnotatedTypeMirror varType,
             AnnotatedTypeMirror valueType,
             Tree valueTree,
-            @CompilerMessageKey String errorKey) {
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
         // If value is less than all expressions in the annotation in varType,
         // using the Value Checker, then skip the common assignment check.
         // Also skip the check if the only expression is "a + 1" and the valueTree
@@ -98,7 +102,7 @@ protected void commonAssignmentCheck(
                 return;
             }
         }
-        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey);
+        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey, extraArgs);
     }
 
     @Override

diff --git a/checker/src/main/java/org/checkerframework/checker/index/lowerbound/LowerBoundVisitor.java b/checker/src/main/java/org/checkerframework/checker/index/lowerbound/LowerBoundVisitor.java
@@ -62,7 +62,10 @@ public Void visitNewArray(NewArrayTree tree, Void type) {
 
     @Override
     protected void commonAssignmentCheck(
-            Tree varTree, ExpressionTree valueTree, @CompilerMessageKey String errorKey) {
+            Tree varTree,
+            ExpressionTree valueTree,
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
 
         // check that when an assignment to a variable declared as @HasSubsequence(a, from, to)
         // occurs, from is non-negative.
@@ -88,6 +91,6 @@ protected void commonAssignmentCheck(
             }
         }
 
-        super.commonAssignmentCheck(varTree, valueTree, errorKey);
+        super.commonAssignmentCheck(varTree, valueTree, errorKey, extraArgs);
     }
 }
diff --git a/checker/src/main/java/org/checkerframework/checker/index/samelen/SameLenVisitor.java b/checker/src/main/java/org/checkerframework/checker/index/samelen/SameLenVisitor.java
@@ -33,7 +33,8 @@ protected void commonAssignmentCheck(
             AnnotatedTypeMirror varType,
             AnnotatedTypeMirror valueType,
             Tree valueTree,
-            @CompilerMessageKey String errorKey) {
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
         if (IndexUtil.isSequenceType(valueType.getUnderlyingType())
                 && TreeUtils.isExpressionTree(valueTree)
                 // if both annotations are @PolySameLen, there is nothing to do
@@ -57,6 +58,6 @@ protected void commonAssignmentCheck(
                 valueType.replaceAnnotation(newSameLen);
             }
         }
-        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey);
+        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey, extraArgs);
     }
 }
diff --git a/checker/src/main/java/org/checkerframework/checker/index/upperbound/UpperBoundVisitor.java b/checker/src/main/java/org/checkerframework/checker/index/upperbound/UpperBoundVisitor.java
@@ -190,7 +190,10 @@ private void visitAccess(ExpressionTree indexTree, ExpressionTree arrTree) {
 
     @Override
     protected void commonAssignmentCheck(
-            Tree varTree, ExpressionTree valueTree, @CompilerMessageKey String errorKey) {
+            Tree varTree,
+            ExpressionTree valueTree,
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
 
         // check that when an assignment to a variable b declared as @HasSubsequence(a, from, to)
         // occurs, to <= a.length, i.e. to is @LTEqLengthOf(a).
@@ -237,14 +240,15 @@ protected void commonAssignmentCheck(
             }
         }
 
-        super.commonAssignmentCheck(varTree, valueTree, errorKey);
+        super.commonAssignmentCheck(varTree, valueTree, errorKey, extraArgs);
     }
 
     @Override
     protected void commonAssignmentCheck(
             AnnotatedTypeMirror varType,
             ExpressionTree valueTree,
-            @CompilerMessageKey String errorKey) {
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
         AnnotatedTypeMirror valueType = atypeFactory.getAnnotatedType(valueTree);
         commonAssignmentCheckStartDiagnostic(varType, valueType, valueTree);
         if (!relaxedCommonAssignment(varType, valueTree)) {
@@ -253,7 +257,7 @@ protected void commonAssignmentCheck(
                     varType,
                     valueType,
                     valueTree);
-            super.commonAssignmentCheck(varType, valueTree, errorKey);
+            super.commonAssignmentCheck(varType, valueTree, errorKey, extraArgs);
         } else if (checker.hasOption("showchecks")) {
             commonAssignmentCheckEndDiagnostic(
                     true, "relaxedCommonAssignment", varType, valueType, valueTree);

diff --git a/checker/src/main/java/org/checkerframework/checker/initialization/InitializationVisitor.java b/checker/src/main/java/org/checkerframework/checker/initialization/InitializationVisitor.java
@@ -110,7 +110,10 @@ protected void checkThisOrSuperConstructorCall(
 
     @Override
     protected void commonAssignmentCheck(
-            Tree varTree, ExpressionTree valueExp, @CompilerMessageKey String errorKey) {
+            Tree varTree,
+            ExpressionTree valueExp,
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
         // field write of the form x.f = y
         if (TreeUtils.isFieldAccess(varTree)) {
             // cast is safe: a field access can only be an IdentifierTree or
@@ -141,7 +144,7 @@ protected void commonAssignmentCheck(
                 }
             }
         }
-        super.commonAssignmentCheck(varTree, valueExp, errorKey);
+        super.commonAssignmentCheck(varTree, valueExp, errorKey, extraArgs);
     }
 
     @Override

diff --git a/checker/src/main/java/org/checkerframework/checker/lock/LockVisitor.java b/checker/src/main/java/org/checkerframework/checker/lock/LockVisitor.java
@@ -343,7 +343,8 @@ protected void commonAssignmentCheck(
             AnnotatedTypeMirror varType,
             AnnotatedTypeMirror valueType,
             Tree valueTree,
-            @CompilerMessageKey String errorKey) {
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
 
         Kind valueTreeKind = valueTree.getKind();
 
@@ -430,7 +431,7 @@ protected void commonAssignmentCheck(
             }
         }
 
-        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey);
+        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey, extraArgs);
     }
 
     @Override

diff --git a/checker/src/main/java/org/checkerframework/checker/nullness/NullnessVisitor.java b/checker/src/main/java/org/checkerframework/checker/nullness/NullnessVisitor.java
@@ -175,7 +175,10 @@ private boolean containsSameByName(
 
     @Override
     protected void commonAssignmentCheck(
-            Tree varTree, ExpressionTree valueExp, @CompilerMessageKey String errorKey) {
+            Tree varTree,
+            ExpressionTree valueExp,
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
 
         // allow MonotonicNonNull to be initialized to null at declaration
         if (varTree.getKind() == Tree.Kind.VARIABLE) {
@@ -187,28 +190,30 @@ protected void commonAssignmentCheck(
                 return;
             }
         }
-        super.commonAssignmentCheck(varTree, valueExp, errorKey);
+        super.commonAssignmentCheck(varTree, valueExp, errorKey, extraArgs);
     }
 
     @Override
     protected void commonAssignmentCheck(
             AnnotatedTypeMirror varType,
             ExpressionTree valueExp,
-            @CompilerMessageKey String errorKey) {
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
         // Use the valueExp as the context because data flow will have a value for that tree.
         // It might not have a value for the var tree.  This is sound because
         // if data flow has determined @PolyNull is @Nullable at the RHS, then
         // it is also @Nullable for the LHS.
         atypeFactory.replacePolyQualifier(varType, valueExp);
-        super.commonAssignmentCheck(varType, valueExp, errorKey);
+        super.commonAssignmentCheck(varType, valueExp, errorKey, extraArgs);
     }
 
     @Override
     protected void commonAssignmentCheck(
             AnnotatedTypeMirror varType,
             AnnotatedTypeMirror valueType,
             Tree valueTree,
-            @CompilerMessageKey String errorKey) {
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
         if (TypesUtils.isPrimitive(varType.getUnderlyingType())
                 && !TypesUtils.isPrimitive(valueType.getUnderlyingType())) {
             boolean succeed = checkForNullability(valueType, valueTree, UNBOXING_OF_NULLABLE);
@@ -217,7 +222,7 @@ protected void commonAssignmentCheck(
                 return;
             }
         }
-        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey);
+        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey, extraArgs);
     }
 
     /** Case 1: Check for null dereferencing. */

diff --git a/framework/src/main/java/org/checkerframework/common/aliasing/AliasingVisitor.java b/framework/src/main/java/org/checkerframework/common/aliasing/AliasingVisitor.java
@@ -150,8 +150,11 @@ private void isUniqueCheck(
     // this isn't called for pseudo-assignments.
     @Override
     protected void commonAssignmentCheck(
-            Tree varTree, ExpressionTree valueExp, @CompilerMessageKey String errorKey) {
-        super.commonAssignmentCheck(varTree, valueExp, errorKey);
+            Tree varTree,
+            ExpressionTree valueExp,
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
+        super.commonAssignmentCheck(varTree, valueExp, errorKey, extraArgs);
         if (isInUniqueConstructor() && TreeUtils.isExplicitThisDereference(valueExp)) {
             // If an assignment occurs inside a constructor with
             // result type @Unique, it will invalidate the @Unique property
@@ -167,8 +170,9 @@ protected void commonAssignmentCheck(
             AnnotatedTypeMirror varType,
             AnnotatedTypeMirror valueType,
             Tree valueTree,
-            @CompilerMessageKey String errorKey) {
-        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey);
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
+        super.commonAssignmentCheck(varType, valueType, valueTree, errorKey, extraArgs);
 
         // If we are visiting a pseudo-assignment, visitorLeafKind is either
         // Kind.NEW_CLASS or Kind.METHOD_INVOCATION.

diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java
@@ -117,6 +117,7 @@
 import org.checkerframework.javacutil.BugInCF;
 import org.checkerframework.javacutil.ElementUtils;
 import org.checkerframework.javacutil.Pair;
+import org.checkerframework.javacutil.SystemUtil;
 import org.checkerframework.javacutil.TreeUtils;
 import org.checkerframework.javacutil.TypesUtils;
 
@@ -1280,7 +1281,7 @@ private boolean isTypeAnnotation(AnnotationTree anno) {
 
     /**
      * Performs two checks: subtyping and assignability checks, using {@link
-     * #commonAssignmentCheck(Tree, ExpressionTree, String)}.
+     * #commonAssignmentCheck(Tree, ExpressionTree, String, Object[])}.
      *
      * <p>If the subtype check fails, it issues a "assignment.type.incompatible" error.
      */
@@ -2345,17 +2346,21 @@ protected Set<? extends AnnotationMirror> getThrowUpperBoundAnnotations() {
      * @param varTree the AST node for the lvalue (usually a variable)
      * @param valueExp the AST node for the rvalue (the new value)
      * @param errorKey the error message key to use if the check fails
+     * @param extraArgs arguments to the error message key, before "found" and "expected" types
      */
     protected void commonAssignmentCheck(
-            Tree varTree, ExpressionTree valueExp, @CompilerMessageKey String errorKey) {
+            Tree varTree,
+            ExpressionTree valueExp,
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
         AnnotatedTypeMirror var = atypeFactory.getAnnotatedTypeLhs(varTree);
         assert var != null : "no variable found for tree: " + varTree;
 
         if (!validateType(varTree, var)) {
             return;
         }
 
-        commonAssignmentCheck(var, valueExp, errorKey);
+        commonAssignmentCheck(var, valueExp, errorKey, extraArgs);
     }
 
     /**
@@ -2365,11 +2370,13 @@ protected void commonAssignmentCheck(
      * @param varType the annotated type of the lvalue (usually a variable)
      * @param valueExp the AST node for the rvalue (the new value)
      * @param errorKey the error message key to use if the check fails
+     * @param extraArgs arguments to the error message key, before "found" and "expected" types
      */
     protected void commonAssignmentCheck(
             AnnotatedTypeMirror varType,
             ExpressionTree valueExp,
-            @CompilerMessageKey String errorKey) {
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
         if (shouldSkipUses(valueExp)) {
             return;
         }
@@ -2393,7 +2400,7 @@ protected void commonAssignmentCheck(
         }
         AnnotatedTypeMirror valueType = atypeFactory.getAnnotatedType(valueExp);
         assert valueType != null : "null type for expression: " + valueExp;
-        commonAssignmentCheck(varType, valueType, valueExp, errorKey);
+        commonAssignmentCheck(varType, valueType, valueExp, errorKey, extraArgs);
     }
 
     /**
@@ -2488,12 +2495,14 @@ protected final void commonAssignmentCheckEndDiagnostic(
      * @param valueType the annotated type of the value
      * @param valueTree the location to use when reporting the error message
      * @param errorKey the error message key to use if the check fails
+     * @param extraArgs arguments to the error message key, before "found" and "expected" types
      */
     protected void commonAssignmentCheck(
             AnnotatedTypeMirror varType,
             AnnotatedTypeMirror valueType,
             Tree valueTree,
-            @CompilerMessageKey String errorKey) {
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
 
         commonAssignmentCheckStartDiagnostic(varType, valueType, valueTree);
 
@@ -2522,7 +2531,10 @@ protected void commonAssignmentCheck(
             FoundRequired pair = FoundRequired.of(valueType, varType);
             String valueTypeString = pair.found;
             String varTypeString = pair.required;
-            checker.reportError(valueTree, errorKey, valueTypeString, varTypeString);
+            checker.reportError(
+                    valueTree,
+                    errorKey,
+                    SystemUtil.concatenate(extraArgs, valueTypeString, varTypeString));
         }
     }