Return false if validation fails in commonAssignmentCheck

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

@@ -3568,7 +3568,7 @@ protected boolean commonAssignmentCheck(
                         varType.getKind(),
                         varType.toString());
             }
-            return true;
+            return false;
         }
 
         return commonAssignmentCheck(varType, valueExpTree, errorKey, extraArgs);
@@ -3645,7 +3645,7 @@ protected boolean commonAssignmentCheck(
                         varType.getKind(),
                         varType.toString());
             }
-            return result;
+            return false;
         }
         AnnotatedTypeMirror valueType = atypeFactory.getAnnotatedType(valueExpTree);
         assert valueType != null : "null type for expression: " + valueExpTree;

framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java

@@ -1,7 +1,11 @@
 package org.checkerframework.framework.testchecker.h1h2checker;
 
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.IdentifierTree;
 import com.sun.source.tree.Tree;
+import com.sun.source.tree.VariableTree;
 
+import org.checkerframework.checker.compilermsgs.qual.CompilerMessageKey;
 import org.checkerframework.common.basetype.BaseTypeChecker;
 import org.checkerframework.common.basetype.BaseTypeValidator;
 import org.checkerframework.common.basetype.BaseTypeVisitor;
@@ -13,8 +17,17 @@
 
 import javax.lang.model.element.AnnotationMirror;
 
+/** Visitor for {@link H1H2Checker}. */
 public class H1H2Visitor extends BaseTypeVisitor<H1H2AnnotatedTypeFactory> {
 
+    /** Variable name used by {@code CommonAssignmentReturn} to exercise invalid LHS validation. */
+    private static final String COMMON_ASSIGNMENT_INVALID_LHS = "commonAssignmentInvalid";
+
+    /**
+     * Creates an {@link H1H2Visitor}.
+     *
+     * @param checker the associated checker
+     */
     public H1H2Visitor(BaseTypeChecker checker) {
         super(checker);
     }
@@ -24,8 +37,56 @@ protected BaseTypeValidator createTypeValidator() {
         return new H1H2TypeValidator(checker, this, atypeFactory);
     }
 
+    @Override
+    protected boolean commonAssignmentCheck(
+            Tree varTree,
+            ExpressionTree valueExpTree,
+            @CompilerMessageKey String errorKey,
+            Object... extraArgs) {
+        AnnotationMirror h1Invalid = AnnotationBuilder.fromClass(elements, H1Invalid.class);
+        boolean invalidLhs =
+                isCommonAssignmentInvalidTree(varTree)
+                        && AnnotatedTypes.containsModifier(
+                                atypeFactory.getAnnotatedTypeLhs(varTree), h1Invalid);
+        boolean invalidRhs =
+                isCommonAssignmentInvalidTree(valueExpTree)
+                        && AnnotatedTypes.containsModifier(
+                                atypeFactory.getAnnotatedType(valueExpTree), h1Invalid);
+        boolean superResult =
+                super.commonAssignmentCheck(varTree, valueExpTree, errorKey, extraArgs);
+        // This is a regression-test sentinel for BaseTypeVisitor.commonAssignmentCheck(Tree, ...).
+        // If the parent returns true after validateType rejects an invalid type, this warning is
+        // unexpected and the test fails.
+        if (superResult && (invalidLhs || invalidRhs)) {
+            checker.reportWarning(varTree, "h1h2checker.commonassignment.parent.succeeded");
+        }
+        return superResult;
+    }
+
+    /** Returns true if {@code tree} is the invalid type used by {@code CommonAssignmentReturn}. */
+    private boolean isCommonAssignmentInvalidTree(Tree tree) {
+        switch (tree.getKind()) {
+            case VARIABLE:
+                return ((VariableTree) tree).getName().contentEquals(COMMON_ASSIGNMENT_INVALID_LHS);
+            case IDENTIFIER:
+                return ((IdentifierTree) tree)
+                        .getName()
+                        .contentEquals(COMMON_ASSIGNMENT_INVALID_LHS);
+            default:
+                return false;
+        }
+    }
+
+    /** Type validator that handles the invalid-type qualifiers in the H1 hierarchy. */
     private final class H1H2TypeValidator extends BaseTypeValidator {
 
+        /**
+         * Creates an {@link H1H2TypeValidator}.
+         *
+         * @param checker the associated checker
+         * @param visitor the associated visitor
+         * @param atypeFactory the associated type factory
+         */
         H1H2TypeValidator(
                 BaseTypeChecker checker,
                 BaseTypeVisitor<?> visitor,
@@ -37,6 +98,10 @@ private final class H1H2TypeValidator extends BaseTypeValidator {
         public Void visitDeclared(AnnotatedDeclaredType type, Tree p) {
             AnnotationMirror h1Invalid = AnnotationBuilder.fromClass(elements, H1Invalid.class);
             if (AnnotatedTypes.containsModifier(type, h1Invalid)) {
+                if (isCommonAssignmentInvalidTree(p)) {
+                    reportInvalidType(type, p);
+                    return super.visitDeclared(type, p);
+                }
                 checker.reportError(
                         p,
                         // An error specific to this type system, with no corresponding text

framework/tests/h1h2checker/CommonAssignmentReturn.java

@@ -0,0 +1,25 @@
+import org.checkerframework.framework.testchecker.h1h2checker.quals.H1Invalid;
+import org.checkerframework.framework.testchecker.h1h2checker.quals.H1S1;
+
+public class CommonAssignmentReturn {
+
+    void invalidLhsLocal() {
+        // :: error: (type.invalid)
+        @H1Invalid Object commonAssignmentInvalid = new Object();
+    }
+
+    void invalidLhsAssignment() {
+        // :: error: (type.invalid)
+        @H1Invalid Object commonAssignmentInvalid;
+        // :: error: (type.invalid)
+        commonAssignmentInvalid = new Object();
+    }
+
+    void invalidRhsAssignment() {
+        // :: error: (type.invalid)
+        @H1Invalid Object commonAssignmentInvalid = new Object();
+        @H1S1 Object target;
+        // :: error: (type.invalid)
+        target = commonAssignmentInvalid;
+    }
+}