From 19139801342805e9ebfbb3e9d9b0fc55bc331b82 Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Tue, 9 Apr 2024 11:36:46 -0700 Subject: [PATCH 01/11] Return false if validation fail --- .../org/checkerframework/common/basetype/BaseTypeVisitor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index 0987bbe2e595..5c8fef770318 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -3205,7 +3205,7 @@ protected boolean commonAssignmentCheck( varType.getKind(), varType.toString()); } - return true; + return false; } return commonAssignmentCheck(varType, valueExpTree, errorKey, extraArgs); From 7b987b8ffa399f89fe384a2fcbd2a0ea5570ae82 Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Tue, 26 May 2026 13:47:04 -0400 Subject: [PATCH 02/11] Add regression test for commonAssignmentCheck contract Adds a small dedicated test checker that triggers the validateType failure path of BaseTypeVisitor.commonAssignmentCheck(Tree, ExpressionTree, ...). The visitor's commonAssignmentCheck override emits a sentinel warning only when super returns true, which surfaces the contract violation: before the fix, super returned true after a failed validateType, causing the warning to be issued; after the fix, super correctly returns false. --- .../test/junit/CommonAssignmentTest.java | 30 ++++++ .../CommonAssignmentAnnotatedTypeFactory.java | 33 ++++++ .../CommonAssignmentChecker.java | 16 +++ .../CommonAssignmentVisitor.java | 102 ++++++++++++++++++ .../commonassignment/quals/CommonBottom.java | 21 ++++ .../commonassignment/quals/CommonInvalid.java | 23 ++++ .../commonassignment/quals/CommonTop.java | 18 ++++ .../InvalidLhsAssignment.java | 31 ++++++ 8 files changed, 274 insertions(+) create mode 100644 framework/src/test/java/org/checkerframework/framework/test/junit/CommonAssignmentTest.java create mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentAnnotatedTypeFactory.java create mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentChecker.java create mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentVisitor.java create mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonBottom.java create mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonInvalid.java create mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonTop.java create mode 100644 framework/tests/commonassignment/InvalidLhsAssignment.java diff --git a/framework/src/test/java/org/checkerframework/framework/test/junit/CommonAssignmentTest.java b/framework/src/test/java/org/checkerframework/framework/test/junit/CommonAssignmentTest.java new file mode 100644 index 000000000000..0b9c3f02674f --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/test/junit/CommonAssignmentTest.java @@ -0,0 +1,30 @@ +package org.checkerframework.framework.test.junit; + +import org.checkerframework.framework.test.CheckerFrameworkPerDirectoryTest; +import org.checkerframework.framework.testchecker.commonassignment.CommonAssignmentChecker; +import org.junit.runners.Parameterized.Parameters; + +import java.io.File; +import java.util.List; + +/** + * Regression tests for the contract of {@code BaseTypeVisitor.commonAssignmentCheck(Tree, + * ExpressionTree, ...)}. See PR #736: the method must return {@code false} when {@code + * validateType} fails for the LHS of an assignment. + */ +public class CommonAssignmentTest extends CheckerFrameworkPerDirectoryTest { + + /** + * Creates a {@link CommonAssignmentTest}. + * + * @param testFiles the files containing test code, which will be type-checked + */ + public CommonAssignmentTest(List testFiles) { + super(testFiles, CommonAssignmentChecker.class, "commonassignment"); + } + + @Parameters + public static String[] getTestDirs() { + return new String[] {"commonassignment"}; + } +} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentAnnotatedTypeFactory.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentAnnotatedTypeFactory.java new file mode 100644 index 000000000000..8c673cbf4d65 --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentAnnotatedTypeFactory.java @@ -0,0 +1,33 @@ +package org.checkerframework.framework.testchecker.commonassignment; + +import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory; +import org.checkerframework.common.basetype.BaseTypeChecker; +import org.checkerframework.framework.testchecker.commonassignment.quals.CommonBottom; +import org.checkerframework.framework.testchecker.commonassignment.quals.CommonInvalid; +import org.checkerframework.framework.testchecker.commonassignment.quals.CommonTop; + +import java.lang.annotation.Annotation; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +/** Annotated type factory for {@link CommonAssignmentChecker}. */ +public class CommonAssignmentAnnotatedTypeFactory extends BaseAnnotatedTypeFactory { + + /** + * Creates a {@link CommonAssignmentAnnotatedTypeFactory}. + * + * @param checker the associated checker + */ + @SuppressWarnings("this-escape") + public CommonAssignmentAnnotatedTypeFactory(BaseTypeChecker checker) { + super(checker); + this.postInit(); + } + + @Override + protected Set> createSupportedTypeQualifiers() { + return new HashSet>( + Arrays.asList(CommonTop.class, CommonBottom.class, CommonInvalid.class)); + } +} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentChecker.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentChecker.java new file mode 100644 index 000000000000..2cca4cc68a0d --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentChecker.java @@ -0,0 +1,16 @@ +package org.checkerframework.framework.testchecker.commonassignment; + +import org.checkerframework.common.basetype.BaseTypeChecker; + +/** + * A test checker used to exercise the contract of {@link + * org.checkerframework.common.basetype.BaseTypeVisitor#commonAssignmentCheck(com.sun.source.tree.Tree, + * com.sun.source.tree.ExpressionTree, String, Object[])}. + * + *

This checker is paired with a custom {@link + * org.checkerframework.common.basetype.BaseTypeValidator} that flags types annotated with + * {@code @CommonInvalid} as invalid. When the validator marks a type as invalid, {@code + * commonAssignmentCheck(Tree, ...)} must return {@code false}; otherwise, subclass overrides that + * compose results with {@code &&} would silently treat such assignments as valid. + */ +public class CommonAssignmentChecker extends BaseTypeChecker {} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentVisitor.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentVisitor.java new file mode 100644 index 000000000000..839928a814e6 --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentVisitor.java @@ -0,0 +1,102 @@ +package org.checkerframework.framework.testchecker.commonassignment; + +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.Tree; + +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; +import org.checkerframework.framework.testchecker.commonassignment.quals.CommonInvalid; +import org.checkerframework.framework.type.AnnotatedTypeFactory; +import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedDeclaredType; +import org.checkerframework.framework.util.AnnotatedTypes; +import org.checkerframework.javacutil.AnnotationBuilder; + +import javax.lang.model.element.AnnotationMirror; + +/** + * Visitor for {@link CommonAssignmentChecker}. + * + *

This visitor exercises two behaviors of the framework that interact: + * + *

    + *
  1. A custom {@link BaseTypeValidator} that flags types annotated with {@code @CommonInvalid} + * as invalid. + *
  2. An override of {@code commonAssignmentCheck(Tree, ExpressionTree, ...)} that issues a + * secondary warning, but only if the parent's result was {@code true}. This mimics + * what real-world subclasses (for example {@code LowerBoundVisitor}) do: they compose their + * own check with the parent's result via {@code &&}. + *
+ * + *

If the parent {@code commonAssignmentCheck(Tree, ...)} were to return {@code true} after a + * failed {@code validateType} call (the bug fixed by PR #736), this override would emit the + * secondary warning even though the LHS type was already known to be invalid. With the fix, the + * parent returns {@code false} and the secondary warning is correctly suppressed. + */ +public class CommonAssignmentVisitor extends BaseTypeVisitor { + + /** + * Creates a {@link CommonAssignmentVisitor}. + * + * @param checker the associated checker + */ + public CommonAssignmentVisitor(BaseTypeChecker checker) { + super(checker); + } + + @Override + protected BaseTypeValidator createTypeValidator() { + return new CommonAssignmentTypeValidator(checker, this, atypeFactory); + } + + @Override + protected boolean commonAssignmentCheck( + Tree varTree, + ExpressionTree valueExpTree, + @CompilerMessageKey String errorKey, + Object... extraArgs) { + boolean superResult = + super.commonAssignmentCheck(varTree, valueExpTree, errorKey, extraArgs); + // Emit a secondary warning only if the parent reports success. If the parent's + // post-validation contract is correct (return false when validateType failed), this + // warning will not be issued for assignments whose LHS has an invalid type. + if (superResult) { + checker.reportWarning(varTree, "commonassignment.parent.succeeded"); + } + return superResult; + } + + /** + * A type validator that reports types containing {@code @CommonInvalid} as invalid via {@link + * #reportInvalidType}, which sets {@code isValid = false} and causes {@link + * BaseTypeVisitor#validateType} to return {@code false}. + */ + private final class CommonAssignmentTypeValidator extends BaseTypeValidator { + + /** + * Creates a {@link CommonAssignmentTypeValidator}. + * + * @param checker the associated checker + * @param visitor the associated visitor + * @param atypeFactory the associated type factory + */ + CommonAssignmentTypeValidator( + BaseTypeChecker checker, + BaseTypeVisitor visitor, + AnnotatedTypeFactory atypeFactory) { + super(checker, visitor, atypeFactory); + } + + @Override + public Void visitDeclared(AnnotatedDeclaredType type, Tree p) { + AnnotationMirror invalidAnno = + AnnotationBuilder.fromClass(elements, CommonInvalid.class); + if (AnnotatedTypes.containsModifier(type, invalidAnno)) { + // reportInvalidType issues "type.invalid" and sets isValid = false. + reportInvalidType(type, p); + } + return super.visitDeclared(type, p); + } + } +} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonBottom.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonBottom.java new file mode 100644 index 000000000000..de4bc5ff00da --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonBottom.java @@ -0,0 +1,21 @@ +package org.checkerframework.framework.testchecker.commonassignment.quals; + +import org.checkerframework.framework.qual.DefaultFor; +import org.checkerframework.framework.qual.SubtypeOf; +import org.checkerframework.framework.qual.TargetLocations; +import org.checkerframework.framework.qual.TypeUseLocation; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** Bottom qualifier for the test type system. */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) +@SubtypeOf({CommonTop.class, CommonInvalid.class}) +@TargetLocations({TypeUseLocation.LOWER_BOUND, TypeUseLocation.EXPLICIT_LOWER_BOUND}) +@DefaultFor({TypeUseLocation.LOWER_BOUND}) +public @interface CommonBottom {} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonInvalid.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonInvalid.java new file mode 100644 index 000000000000..c33573f4059a --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonInvalid.java @@ -0,0 +1,23 @@ +package org.checkerframework.framework.testchecker.commonassignment.quals; + +import org.checkerframework.framework.qual.SubtypeOf; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marker qualifier for test types that should be flagged as invalid by the type validator. + * + *

The custom type validator reports these types as invalid via {@code reportInvalidType}, which + * makes {@code BaseTypeVisitor#validateType} return {@code false}. This is what exercises the path + * of {@code commonAssignmentCheck(Tree, ExpressionTree, ...)} that previously returned {@code true} + * after a failed validation and now correctly returns {@code false}. + */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) +@SubtypeOf({CommonTop.class}) +public @interface CommonInvalid {} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonTop.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonTop.java new file mode 100644 index 000000000000..60af5e712b5c --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonTop.java @@ -0,0 +1,18 @@ +package org.checkerframework.framework.testchecker.commonassignment.quals; + +import org.checkerframework.framework.qual.DefaultQualifierInHierarchy; +import org.checkerframework.framework.qual.SubtypeOf; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** Top qualifier for the test type system. */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) +@SubtypeOf({}) +@DefaultQualifierInHierarchy +public @interface CommonTop {} diff --git a/framework/tests/commonassignment/InvalidLhsAssignment.java b/framework/tests/commonassignment/InvalidLhsAssignment.java new file mode 100644 index 000000000000..ddc51e6a9d3f --- /dev/null +++ b/framework/tests/commonassignment/InvalidLhsAssignment.java @@ -0,0 +1,31 @@ +import org.checkerframework.framework.testchecker.commonassignment.quals.CommonInvalid; + +/** + * Regression test for PR #736: when {@code validateType} fails for the LHS of an assignment, {@code + * BaseTypeVisitor.commonAssignmentCheck(Tree, ExpressionTree, String, Object[])} must return {@code + * false}, not {@code true}. Otherwise, subclass overrides that compose the parent's result with + * their own check via {@code &&} would silently treat the assignment as valid. + * + *

The {@code CommonAssignmentVisitor} in this test issues the secondary warning {@code + * commonassignment.parent.succeeded} only when {@code super.commonAssignmentCheck} returns {@code + * true}. Since {@code @CommonInvalid} is reported as an invalid type by the test validator, the + * super call must return {@code false} and that secondary warning must not be issued. + */ +public class InvalidLhsAssignment { + + void invalidLhsLocal() { + // The variable's declared type is invalid; the framework reports "type.invalid". + // commonAssignmentCheck must NOT additionally issue "commonassignment.parent.succeeded". + // :: error: (type.invalid) + @CommonInvalid Object o = new Object(); + } + + void invalidLhsAssignment() { + // The variable declaration alone reports "type.invalid" (no initializer). + // :: error: (type.invalid) + @CommonInvalid Object o; + // The LHS type is invalid; the same property must hold for the assignment expression. + // :: error: (type.invalid) + o = new Object(); + } +} From 2887593773459512650d64735e6afbbe6727f52c Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Tue, 26 May 2026 13:48:57 -0400 Subject: [PATCH 03/11] TEMP: revert fix to demonstrate test failure in CI --- .../org/checkerframework/common/basetype/BaseTypeVisitor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index c6b1b01ae9ea..ca901751370e 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -3485,7 +3485,7 @@ protected boolean commonAssignmentCheck( varType.getKind(), varType.toString()); } - return false; + return true; } return commonAssignmentCheck(varType, valueExpTree, errorKey, extraArgs); From 7ff9362da39f6e5e5ba07d99eb1a6ec3f384aa0c Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Tue, 26 May 2026 14:10:38 -0400 Subject: [PATCH 04/11] Restore fix and add SKIP-REQUIRE-JAVADOC Restores the return false fix in commonAssignmentCheck after the temporary revert that demonstrated the test failure. Also adds SKIP-REQUIRE-JAVADOC to skip the requireJavadoc check while this PR is in progress. --- SKIP-REQUIRE-JAVADOC | 0 .../org/checkerframework/common/basetype/BaseTypeVisitor.java | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 SKIP-REQUIRE-JAVADOC diff --git a/SKIP-REQUIRE-JAVADOC b/SKIP-REQUIRE-JAVADOC new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index ca901751370e..c6b1b01ae9ea 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -3485,7 +3485,7 @@ protected boolean commonAssignmentCheck( varType.getKind(), varType.toString()); } - return true; + return false; } return commonAssignmentCheck(varType, valueExpTree, errorKey, extraArgs); From 7f9d4f14f70a664dcc01ea0d15b9d8e6f9740543 Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Thu, 11 Jun 2026 00:36:03 -0400 Subject: [PATCH 05/11] Reuse h1h2 checker for assignment validation test --- SKIP-REQUIRE-JAVADOC | 0 .../test/junit/CommonAssignmentTest.java | 30 ------ .../CommonAssignmentAnnotatedTypeFactory.java | 33 ------ .../CommonAssignmentChecker.java | 16 --- .../CommonAssignmentVisitor.java | 102 ------------------ .../commonassignment/quals/CommonBottom.java | 21 ---- .../commonassignment/quals/CommonInvalid.java | 23 ---- .../commonassignment/quals/CommonTop.java | 18 ---- .../testchecker/h1h2checker/H1H2Visitor.java | 28 +++-- .../InvalidLhsAssignment.java | 31 ------ .../h1h2checker/CommonAssignmentReturn.java | 16 +++ .../tests/h1h2checker/TypeRefinement.java | 12 +-- 12 files changed, 43 insertions(+), 287 deletions(-) delete mode 100644 SKIP-REQUIRE-JAVADOC delete mode 100644 framework/src/test/java/org/checkerframework/framework/test/junit/CommonAssignmentTest.java delete mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentAnnotatedTypeFactory.java delete mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentChecker.java delete mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentVisitor.java delete mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonBottom.java delete mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonInvalid.java delete mode 100644 framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonTop.java delete mode 100644 framework/tests/commonassignment/InvalidLhsAssignment.java create mode 100644 framework/tests/h1h2checker/CommonAssignmentReturn.java diff --git a/SKIP-REQUIRE-JAVADOC b/SKIP-REQUIRE-JAVADOC deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/framework/src/test/java/org/checkerframework/framework/test/junit/CommonAssignmentTest.java b/framework/src/test/java/org/checkerframework/framework/test/junit/CommonAssignmentTest.java deleted file mode 100644 index 0b9c3f02674f..000000000000 --- a/framework/src/test/java/org/checkerframework/framework/test/junit/CommonAssignmentTest.java +++ /dev/null @@ -1,30 +0,0 @@ -package org.checkerframework.framework.test.junit; - -import org.checkerframework.framework.test.CheckerFrameworkPerDirectoryTest; -import org.checkerframework.framework.testchecker.commonassignment.CommonAssignmentChecker; -import org.junit.runners.Parameterized.Parameters; - -import java.io.File; -import java.util.List; - -/** - * Regression tests for the contract of {@code BaseTypeVisitor.commonAssignmentCheck(Tree, - * ExpressionTree, ...)}. See PR #736: the method must return {@code false} when {@code - * validateType} fails for the LHS of an assignment. - */ -public class CommonAssignmentTest extends CheckerFrameworkPerDirectoryTest { - - /** - * Creates a {@link CommonAssignmentTest}. - * - * @param testFiles the files containing test code, which will be type-checked - */ - public CommonAssignmentTest(List testFiles) { - super(testFiles, CommonAssignmentChecker.class, "commonassignment"); - } - - @Parameters - public static String[] getTestDirs() { - return new String[] {"commonassignment"}; - } -} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentAnnotatedTypeFactory.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentAnnotatedTypeFactory.java deleted file mode 100644 index 8c673cbf4d65..000000000000 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentAnnotatedTypeFactory.java +++ /dev/null @@ -1,33 +0,0 @@ -package org.checkerframework.framework.testchecker.commonassignment; - -import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory; -import org.checkerframework.common.basetype.BaseTypeChecker; -import org.checkerframework.framework.testchecker.commonassignment.quals.CommonBottom; -import org.checkerframework.framework.testchecker.commonassignment.quals.CommonInvalid; -import org.checkerframework.framework.testchecker.commonassignment.quals.CommonTop; - -import java.lang.annotation.Annotation; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; - -/** Annotated type factory for {@link CommonAssignmentChecker}. */ -public class CommonAssignmentAnnotatedTypeFactory extends BaseAnnotatedTypeFactory { - - /** - * Creates a {@link CommonAssignmentAnnotatedTypeFactory}. - * - * @param checker the associated checker - */ - @SuppressWarnings("this-escape") - public CommonAssignmentAnnotatedTypeFactory(BaseTypeChecker checker) { - super(checker); - this.postInit(); - } - - @Override - protected Set> createSupportedTypeQualifiers() { - return new HashSet>( - Arrays.asList(CommonTop.class, CommonBottom.class, CommonInvalid.class)); - } -} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentChecker.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentChecker.java deleted file mode 100644 index 2cca4cc68a0d..000000000000 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentChecker.java +++ /dev/null @@ -1,16 +0,0 @@ -package org.checkerframework.framework.testchecker.commonassignment; - -import org.checkerframework.common.basetype.BaseTypeChecker; - -/** - * A test checker used to exercise the contract of {@link - * org.checkerframework.common.basetype.BaseTypeVisitor#commonAssignmentCheck(com.sun.source.tree.Tree, - * com.sun.source.tree.ExpressionTree, String, Object[])}. - * - *

This checker is paired with a custom {@link - * org.checkerframework.common.basetype.BaseTypeValidator} that flags types annotated with - * {@code @CommonInvalid} as invalid. When the validator marks a type as invalid, {@code - * commonAssignmentCheck(Tree, ...)} must return {@code false}; otherwise, subclass overrides that - * compose results with {@code &&} would silently treat such assignments as valid. - */ -public class CommonAssignmentChecker extends BaseTypeChecker {} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentVisitor.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentVisitor.java deleted file mode 100644 index 839928a814e6..000000000000 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/CommonAssignmentVisitor.java +++ /dev/null @@ -1,102 +0,0 @@ -package org.checkerframework.framework.testchecker.commonassignment; - -import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.Tree; - -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; -import org.checkerframework.framework.testchecker.commonassignment.quals.CommonInvalid; -import org.checkerframework.framework.type.AnnotatedTypeFactory; -import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedDeclaredType; -import org.checkerframework.framework.util.AnnotatedTypes; -import org.checkerframework.javacutil.AnnotationBuilder; - -import javax.lang.model.element.AnnotationMirror; - -/** - * Visitor for {@link CommonAssignmentChecker}. - * - *

This visitor exercises two behaviors of the framework that interact: - * - *

    - *
  1. A custom {@link BaseTypeValidator} that flags types annotated with {@code @CommonInvalid} - * as invalid. - *
  2. An override of {@code commonAssignmentCheck(Tree, ExpressionTree, ...)} that issues a - * secondary warning, but only if the parent's result was {@code true}. This mimics - * what real-world subclasses (for example {@code LowerBoundVisitor}) do: they compose their - * own check with the parent's result via {@code &&}. - *
- * - *

If the parent {@code commonAssignmentCheck(Tree, ...)} were to return {@code true} after a - * failed {@code validateType} call (the bug fixed by PR #736), this override would emit the - * secondary warning even though the LHS type was already known to be invalid. With the fix, the - * parent returns {@code false} and the secondary warning is correctly suppressed. - */ -public class CommonAssignmentVisitor extends BaseTypeVisitor { - - /** - * Creates a {@link CommonAssignmentVisitor}. - * - * @param checker the associated checker - */ - public CommonAssignmentVisitor(BaseTypeChecker checker) { - super(checker); - } - - @Override - protected BaseTypeValidator createTypeValidator() { - return new CommonAssignmentTypeValidator(checker, this, atypeFactory); - } - - @Override - protected boolean commonAssignmentCheck( - Tree varTree, - ExpressionTree valueExpTree, - @CompilerMessageKey String errorKey, - Object... extraArgs) { - boolean superResult = - super.commonAssignmentCheck(varTree, valueExpTree, errorKey, extraArgs); - // Emit a secondary warning only if the parent reports success. If the parent's - // post-validation contract is correct (return false when validateType failed), this - // warning will not be issued for assignments whose LHS has an invalid type. - if (superResult) { - checker.reportWarning(varTree, "commonassignment.parent.succeeded"); - } - return superResult; - } - - /** - * A type validator that reports types containing {@code @CommonInvalid} as invalid via {@link - * #reportInvalidType}, which sets {@code isValid = false} and causes {@link - * BaseTypeVisitor#validateType} to return {@code false}. - */ - private final class CommonAssignmentTypeValidator extends BaseTypeValidator { - - /** - * Creates a {@link CommonAssignmentTypeValidator}. - * - * @param checker the associated checker - * @param visitor the associated visitor - * @param atypeFactory the associated type factory - */ - CommonAssignmentTypeValidator( - BaseTypeChecker checker, - BaseTypeVisitor visitor, - AnnotatedTypeFactory atypeFactory) { - super(checker, visitor, atypeFactory); - } - - @Override - public Void visitDeclared(AnnotatedDeclaredType type, Tree p) { - AnnotationMirror invalidAnno = - AnnotationBuilder.fromClass(elements, CommonInvalid.class); - if (AnnotatedTypes.containsModifier(type, invalidAnno)) { - // reportInvalidType issues "type.invalid" and sets isValid = false. - reportInvalidType(type, p); - } - return super.visitDeclared(type, p); - } - } -} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonBottom.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonBottom.java deleted file mode 100644 index de4bc5ff00da..000000000000 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonBottom.java +++ /dev/null @@ -1,21 +0,0 @@ -package org.checkerframework.framework.testchecker.commonassignment.quals; - -import org.checkerframework.framework.qual.DefaultFor; -import org.checkerframework.framework.qual.SubtypeOf; -import org.checkerframework.framework.qual.TargetLocations; -import org.checkerframework.framework.qual.TypeUseLocation; - -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** Bottom qualifier for the test type system. */ -@Documented -@Retention(RetentionPolicy.RUNTIME) -@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) -@SubtypeOf({CommonTop.class, CommonInvalid.class}) -@TargetLocations({TypeUseLocation.LOWER_BOUND, TypeUseLocation.EXPLICIT_LOWER_BOUND}) -@DefaultFor({TypeUseLocation.LOWER_BOUND}) -public @interface CommonBottom {} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonInvalid.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonInvalid.java deleted file mode 100644 index c33573f4059a..000000000000 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonInvalid.java +++ /dev/null @@ -1,23 +0,0 @@ -package org.checkerframework.framework.testchecker.commonassignment.quals; - -import org.checkerframework.framework.qual.SubtypeOf; - -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** - * Marker qualifier for test types that should be flagged as invalid by the type validator. - * - *

The custom type validator reports these types as invalid via {@code reportInvalidType}, which - * makes {@code BaseTypeVisitor#validateType} return {@code false}. This is what exercises the path - * of {@code commonAssignmentCheck(Tree, ExpressionTree, ...)} that previously returned {@code true} - * after a failed validation and now correctly returns {@code false}. - */ -@Documented -@Retention(RetentionPolicy.RUNTIME) -@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) -@SubtypeOf({CommonTop.class}) -public @interface CommonInvalid {} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonTop.java b/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonTop.java deleted file mode 100644 index 60af5e712b5c..000000000000 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/commonassignment/quals/CommonTop.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.checkerframework.framework.testchecker.commonassignment.quals; - -import org.checkerframework.framework.qual.DefaultQualifierInHierarchy; -import org.checkerframework.framework.qual.SubtypeOf; - -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** Top qualifier for the test type system. */ -@Documented -@Retention(RetentionPolicy.RUNTIME) -@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) -@SubtypeOf({}) -@DefaultQualifierInHierarchy -public @interface CommonTop {} diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java index 4a671d700dc1..460b664b6a18 100644 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java @@ -1,7 +1,9 @@ package org.checkerframework.framework.testchecker.h1h2checker; +import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.Tree; +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; @@ -24,6 +26,24 @@ protected BaseTypeValidator createTypeValidator() { return new H1H2TypeValidator(checker, this, atypeFactory); } + @Override + protected boolean commonAssignmentCheck( + Tree varTree, + ExpressionTree valueExpTree, + @CompilerMessageKey String errorKey, + Object... extraArgs) { + boolean superResult = + super.commonAssignmentCheck(varTree, valueExpTree, errorKey, extraArgs); + AnnotationMirror h1Invalid = AnnotationBuilder.fromClass(elements, H1Invalid.class); + if (superResult + && varTree.toString().contains("commonAssignment") + && AnnotatedTypes.containsModifier( + atypeFactory.getAnnotatedType(varTree), h1Invalid)) { + checker.reportWarning(varTree, "h1h2checker.commonassignment.parent.succeeded"); + } + return superResult; + } + private final class H1H2TypeValidator extends BaseTypeValidator { H1H2TypeValidator( @@ -37,13 +57,7 @@ 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)) { - checker.reportError( - p, - // An error specific to this type system, with no corresponding text - // in a messages.properties file; this checker is just for testing. - "h1h2checker.h1invalid.forbidden", - type.getAnnotations(), - type.toString()); + reportInvalidType(type, p); } return super.visitDeclared(type, p); } diff --git a/framework/tests/commonassignment/InvalidLhsAssignment.java b/framework/tests/commonassignment/InvalidLhsAssignment.java deleted file mode 100644 index ddc51e6a9d3f..000000000000 --- a/framework/tests/commonassignment/InvalidLhsAssignment.java +++ /dev/null @@ -1,31 +0,0 @@ -import org.checkerframework.framework.testchecker.commonassignment.quals.CommonInvalid; - -/** - * Regression test for PR #736: when {@code validateType} fails for the LHS of an assignment, {@code - * BaseTypeVisitor.commonAssignmentCheck(Tree, ExpressionTree, String, Object[])} must return {@code - * false}, not {@code true}. Otherwise, subclass overrides that compose the parent's result with - * their own check via {@code &&} would silently treat the assignment as valid. - * - *

The {@code CommonAssignmentVisitor} in this test issues the secondary warning {@code - * commonassignment.parent.succeeded} only when {@code super.commonAssignmentCheck} returns {@code - * true}. Since {@code @CommonInvalid} is reported as an invalid type by the test validator, the - * super call must return {@code false} and that secondary warning must not be issued. - */ -public class InvalidLhsAssignment { - - void invalidLhsLocal() { - // The variable's declared type is invalid; the framework reports "type.invalid". - // commonAssignmentCheck must NOT additionally issue "commonassignment.parent.succeeded". - // :: error: (type.invalid) - @CommonInvalid Object o = new Object(); - } - - void invalidLhsAssignment() { - // The variable declaration alone reports "type.invalid" (no initializer). - // :: error: (type.invalid) - @CommonInvalid Object o; - // The LHS type is invalid; the same property must hold for the assignment expression. - // :: error: (type.invalid) - o = new Object(); - } -} diff --git a/framework/tests/h1h2checker/CommonAssignmentReturn.java b/framework/tests/h1h2checker/CommonAssignmentReturn.java new file mode 100644 index 000000000000..dbd8d799925c --- /dev/null +++ b/framework/tests/h1h2checker/CommonAssignmentReturn.java @@ -0,0 +1,16 @@ +import org.checkerframework.framework.testchecker.h1h2checker.quals.H1Invalid; + +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(); + } +} diff --git a/framework/tests/h1h2checker/TypeRefinement.java b/framework/tests/h1h2checker/TypeRefinement.java index b821903d1bfd..cf588cb4d873 100644 --- a/framework/tests/h1h2checker/TypeRefinement.java +++ b/framework/tests/h1h2checker/TypeRefinement.java @@ -4,15 +4,15 @@ public class TypeRefinement { // :: warning: (cast.unsafe.constructor.invocation) @H1Top Object o = new @H1S1 Object(); - // :: error: (h1h2checker.h1invalid.forbidden) :: warning: (cast.unsafe.constructor.invocation) + // :: error: (type.invalid) @H1Top Object o2 = new @H1Invalid Object(); - // :: error: (h1h2checker.h1invalid.forbidden) + // :: error: (type.invalid) @H1Top Object o3 = getH1Invalid(); - // :: error: (h1h2checker.h1invalid.forbidden) - @H1Invalid Object getH1Invalid() { - // :: error: (h1h2checker.h1invalid.forbidden) :: warning: - // (cast.unsafe.constructor.invocation) + @H1Invalid + // :: error: (type.invalid) + Object getH1Invalid() { + // :: error: (type.invalid) return new @H1Invalid Object(); } } From 7fd1d5d1adacc8f4fc7e375cbcb4793e73a0f648 Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Thu, 11 Jun 2026 00:37:34 -0400 Subject: [PATCH 06/11] Fix h1h2 diagnostic comment placement --- framework/tests/h1h2checker/TypeRefinement.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/framework/tests/h1h2checker/TypeRefinement.java b/framework/tests/h1h2checker/TypeRefinement.java index cf588cb4d873..069dc2eeb4c9 100644 --- a/framework/tests/h1h2checker/TypeRefinement.java +++ b/framework/tests/h1h2checker/TypeRefinement.java @@ -9,9 +9,8 @@ public class TypeRefinement { // :: error: (type.invalid) @H1Top Object o3 = getH1Invalid(); - @H1Invalid // :: error: (type.invalid) - Object getH1Invalid() { + @H1Invalid Object getH1Invalid() { // :: error: (type.invalid) return new @H1Invalid Object(); } From b68dc783747c4c49e710475c457d803243b22788 Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Thu, 11 Jun 2026 00:50:35 -0400 Subject: [PATCH 07/11] Simplify common assignment regression sentinel --- .../testchecker/h1h2checker/H1H2Visitor.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java index 460b664b6a18..9fd91726a849 100644 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java @@ -32,13 +32,16 @@ protected boolean commonAssignmentCheck( ExpressionTree valueExpTree, @CompilerMessageKey String errorKey, Object... extraArgs) { + AnnotationMirror h1Invalid = AnnotationBuilder.fromClass(elements, H1Invalid.class); + boolean invalidLhs = + AnnotatedTypes.containsModifier( + atypeFactory.getAnnotatedTypeLhs(varTree), h1Invalid); boolean superResult = super.commonAssignmentCheck(varTree, valueExpTree, errorKey, extraArgs); - AnnotationMirror h1Invalid = AnnotationBuilder.fromClass(elements, H1Invalid.class); - if (superResult - && varTree.toString().contains("commonAssignment") - && AnnotatedTypes.containsModifier( - atypeFactory.getAnnotatedType(varTree), h1Invalid)) { + // This is a regression-test sentinel for BaseTypeVisitor.commonAssignmentCheck(Tree, ...). + // If the parent returns true after validateType rejects an invalid LHS, this warning is + // unexpected and the test fails. + if (superResult && invalidLhs) { checker.reportWarning(varTree, "h1h2checker.commonassignment.parent.succeeded"); } return superResult; From 07bb49a6a5d67f2b8ba5f942093e8a30694aa5af Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Thu, 11 Jun 2026 01:05:57 -0400 Subject: [PATCH 08/11] Document H1H2 type validator --- .../framework/testchecker/h1h2checker/H1H2Visitor.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java index 9fd91726a849..b2e0ea022d7c 100644 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java @@ -47,8 +47,16 @@ protected boolean commonAssignmentCheck( return superResult; } + /** Type validator that treats {@link H1Invalid} as an invalid type. */ 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, From 2a6c929071076ae3e8a7d085cbe0a9c0843b16f1 Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Mon, 15 Jun 2026 08:00:33 -0400 Subject: [PATCH 09/11] Preserve h1 invalid constructor warning --- .../testchecker/h1h2checker/H1H2Visitor.java | 33 +++++++++++++++++-- .../tests/h1h2checker/TypeRefinement.java | 9 ++--- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java index b2e0ea022d7c..54132f26c9b6 100644 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java @@ -1,7 +1,9 @@ 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; @@ -17,6 +19,9 @@ public class H1H2Visitor extends BaseTypeVisitor { + /** Variable name used by {@code CommonAssignmentReturn} to exercise invalid LHS validation. */ + private static final String COMMON_ASSIGNMENT_INVALID_LHS = "commonAssignmentInvalid"; + public H1H2Visitor(BaseTypeChecker checker) { super(checker); } @@ -47,7 +52,21 @@ protected boolean commonAssignmentCheck( return superResult; } - /** Type validator that treats {@link H1Invalid} as an invalid type. */ + /** Returns true if {@code tree} is the invalid LHS used by {@code CommonAssignmentReturn}. */ + private boolean isCommonAssignmentInvalidLhs(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 { /** @@ -68,7 +87,17 @@ 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)) { - reportInvalidType(type, p); + if (isCommonAssignmentInvalidLhs(p)) { + reportInvalidType(type, p); + return super.visitDeclared(type, p); + } + checker.reportError( + p, + // An error specific to this type system, with no corresponding text + // in a messages.properties file; this checker is just for testing. + "h1h2checker.h1invalid.forbidden", + type.getAnnotations(), + type.toString()); } return super.visitDeclared(type, p); } diff --git a/framework/tests/h1h2checker/TypeRefinement.java b/framework/tests/h1h2checker/TypeRefinement.java index 069dc2eeb4c9..b821903d1bfd 100644 --- a/framework/tests/h1h2checker/TypeRefinement.java +++ b/framework/tests/h1h2checker/TypeRefinement.java @@ -4,14 +4,15 @@ public class TypeRefinement { // :: warning: (cast.unsafe.constructor.invocation) @H1Top Object o = new @H1S1 Object(); - // :: error: (type.invalid) + // :: error: (h1h2checker.h1invalid.forbidden) :: warning: (cast.unsafe.constructor.invocation) @H1Top Object o2 = new @H1Invalid Object(); - // :: error: (type.invalid) + // :: error: (h1h2checker.h1invalid.forbidden) @H1Top Object o3 = getH1Invalid(); - // :: error: (type.invalid) + // :: error: (h1h2checker.h1invalid.forbidden) @H1Invalid Object getH1Invalid() { - // :: error: (type.invalid) + // :: error: (h1h2checker.h1invalid.forbidden) :: warning: + // (cast.unsafe.constructor.invocation) return new @H1Invalid Object(); } } From 6a5db432010243fa26bd461ec84960f0428f72da Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Mon, 15 Jun 2026 12:02:15 -0400 Subject: [PATCH 10/11] Document H1H2 visitor --- .../framework/testchecker/h1h2checker/H1H2Visitor.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java index 54132f26c9b6..af75ddfa00f6 100644 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java @@ -17,11 +17,17 @@ import javax.lang.model.element.AnnotationMirror; +/** Visitor for {@link H1H2Checker}. */ public class H1H2Visitor extends BaseTypeVisitor { /** 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); } From c5ba33a3ce324fb5bd9267f48a02013c6d5a6506 Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Mon, 22 Jun 2026 20:00:44 -0400 Subject: [PATCH 11/11] Return false for invalid assignment value types --- .../common/basetype/BaseTypeVisitor.java | 2 +- .../testchecker/h1h2checker/H1H2Visitor.java | 19 ++++++++++++------- .../h1h2checker/CommonAssignmentReturn.java | 9 +++++++++ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index 8d4fcc49a58b..4037c98e06b3 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -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; diff --git a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java index af75ddfa00f6..65e5869be025 100644 --- a/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java +++ b/framework/src/test/java/org/checkerframework/framework/testchecker/h1h2checker/H1H2Visitor.java @@ -45,21 +45,26 @@ protected boolean commonAssignmentCheck( Object... extraArgs) { AnnotationMirror h1Invalid = AnnotationBuilder.fromClass(elements, H1Invalid.class); boolean invalidLhs = - AnnotatedTypes.containsModifier( - atypeFactory.getAnnotatedTypeLhs(varTree), h1Invalid); + 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 LHS, this warning is + // If the parent returns true after validateType rejects an invalid type, this warning is // unexpected and the test fails. - if (superResult && invalidLhs) { + if (superResult && (invalidLhs || invalidRhs)) { checker.reportWarning(varTree, "h1h2checker.commonassignment.parent.succeeded"); } return superResult; } - /** Returns true if {@code tree} is the invalid LHS used by {@code CommonAssignmentReturn}. */ - private boolean isCommonAssignmentInvalidLhs(Tree tree) { + /** 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); @@ -93,7 +98,7 @@ 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 (isCommonAssignmentInvalidLhs(p)) { + if (isCommonAssignmentInvalidTree(p)) { reportInvalidType(type, p); return super.visitDeclared(type, p); } diff --git a/framework/tests/h1h2checker/CommonAssignmentReturn.java b/framework/tests/h1h2checker/CommonAssignmentReturn.java index dbd8d799925c..a6135063aabf 100644 --- a/framework/tests/h1h2checker/CommonAssignmentReturn.java +++ b/framework/tests/h1h2checker/CommonAssignmentReturn.java @@ -1,4 +1,5 @@ import org.checkerframework.framework.testchecker.h1h2checker.quals.H1Invalid; +import org.checkerframework.framework.testchecker.h1h2checker.quals.H1S1; public class CommonAssignmentReturn { @@ -13,4 +14,12 @@ void invalidLhsAssignment() { // :: error: (type.invalid) commonAssignmentInvalid = new Object(); } + + void invalidRhsAssignment() { + // :: error: (type.invalid) + @H1Invalid Object commonAssignmentInvalid = new Object(); + @H1S1 Object target; + // :: error: (type.invalid) + target = commonAssignmentInvalid; + } }