From c022d38b8cb69b34c8fa6bd9e05d1d7b0f0fc4a9 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 12:02:22 -0700 Subject: [PATCH 01/20] Use local variables --- .../common/basetype/BaseTypeValidator.java | 31 +++++++------------ .../common/basetype/BaseTypeVisitor.java | 11 +++---- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeValidator.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeValidator.java index 3f1f14515492..c0d009105ee6 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeValidator.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeValidator.java @@ -31,6 +31,7 @@ import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedWildcardType; import org.checkerframework.framework.type.AnnotatedTypeParameterBounds; import org.checkerframework.framework.type.QualifierHierarchy; +import org.checkerframework.framework.type.TypeHierarchy; import org.checkerframework.framework.type.visitor.AnnotatedTypeScanner; import org.checkerframework.framework.type.visitor.SimpleAnnotatedTypeScanner; import org.checkerframework.framework.util.AnnotatedTypes; @@ -580,30 +581,30 @@ protected Void visitParameterizedType(AnnotatedDeclaredType type, ParameterizedT } } + TypeHierarchy hierarchy = atypeFactory.getTypeHierarchy(); for (int i = 0; i < numTypeArgs; i++) { AnnotatedTypeMirror captureTypeArg = capturedType.getTypeArguments().get(i); if (type.getTypeArguments().get(i).getKind() == TypeKind.WILDCARD) { AnnotatedWildcardType wildcard = (AnnotatedWildcardType) type.getTypeArguments().get(i); + AnnotatedTypeMirror extendsBound = wildcard.getExtendsBound(); if (TypesUtils.isCapturedTypeVariable(captureTypeArg.getUnderlyingType())) { AnnotatedTypeVariable capturedTypeVar = (AnnotatedTypeVariable) captureTypeArg; // Substitute the captured type variables with their wildcards. Without // this, the isSubtype check crashes because wildcards aren't comparable // with type variables. + AnnotatedTypeMirror upperBound = capturedTypeVar.getUpperBound(); AnnotatedTypeMirror captureTypeVarUB = atypeFactory .getTypeVarSubstitutor() - .substituteWithoutCopyingTypeArguments( - typeVarToWildcard, capturedTypeVar.getUpperBound()); - if (!atypeFactory - .getTypeHierarchy() - .isSubtype(captureTypeVarUB, wildcard.getExtendsBound())) { + .substituteWithoutCopyingTypeArguments(typeVarToWildcard, upperBound); + if (!hierarchy.isSubtype(captureTypeVarUB, extendsBound)) { checker.reportError( tree.getTypeArguments().get(i), "type.argument", element.getTypeParameters().get(i), element.getSimpleName(), - wildcard.getExtendsBound(), - capturedTypeVar.getUpperBound()); + extendsBound, + upperBound); } } else if (AnnotatedTypes.hasExplicitSuperBound(wildcard)) { // If the super bound of the wildcard is the same as the upper bound of the @@ -615,19 +616,11 @@ protected Void visitParameterizedType(AnnotatedDeclaredType type, ParameterizedT // For example, Set<@1 ? super @2 Object> will collapse into Set<@2 Object>. // So, issue a warning if the annotations on the extends bound are not the // same as the annotations on the super bound. - - if (!(atypeFactory - .getTypeHierarchy() - .isSubtypeShallowEffective(wildcard.getSuperBound(), wildcard.getExtendsBound()) - && atypeFactory - .getTypeHierarchy() - .isSubtypeShallowEffective( - wildcard.getExtendsBound(), wildcard.getSuperBound()))) { + AnnotatedTypeMirror superBound = wildcard.getSuperBound(); + if (!(hierarchy.isSubtypeShallowEffective(superBound, extendsBound) + && hierarchy.isSubtypeShallowEffective(extendsBound, superBound))) { checker.reportError( - tree.getTypeArguments().get(i), - "super.wildcard", - wildcard.getExtendsBound(), - wildcard.getSuperBound()); + tree.getTypeArguments().get(i), "super.wildcard", extendsBound, superBound); } } } 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 cefeb158817d..76ca30ee77b8 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -3712,21 +3712,20 @@ protected void checkConstructorInvocation( if (explicitAnnos.isEmpty()) { return; } + AnnotatedTypeMirror retType = constructor.getReturnType(); for (AnnotationMirror explicit : explicitAnnos) { // The return type of the constructor (resultAnnos) must be comparable to the // annotations on the constructor invocation (explicitAnnos). boolean resultIsSubtypeOfExplicit = - typeHierarchy.isSubtypeShallowEffective(constructor.getReturnType(), explicit); - if (!(typeHierarchy.isSubtypeShallowEffective(explicit, constructor.getReturnType()) + typeHierarchy.isSubtypeShallowEffective(retType, explicit); + if (!(typeHierarchy.isSubtypeShallowEffective(explicit, retType) || resultIsSubtypeOfExplicit)) { - AnnotationMirror resultAnno = - constructor.getReturnType().getPrimaryAnnotationInHierarchy(explicit); + AnnotationMirror resultAnno = retType.getPrimaryAnnotationInHierarchy(explicit); checker.reportError( newClassTree, "constructor.invocation", constructor.toString(), explicit, resultAnno); return; } else if (!resultIsSubtypeOfExplicit) { - AnnotationMirror resultAnno = - constructor.getReturnType().getPrimaryAnnotationInHierarchy(explicit); + AnnotationMirror resultAnno = retType.getPrimaryAnnotationInHierarchy(explicit); // Issue a warning if the annotations on the constructor invocation is a subtype of // the constructor result type. This is equivalent to down-casting. checker.reportWarning( From 3d91ebd02ce1ebb3344a24b9650c010ca3f66bf1 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 12:10:09 -0700 Subject: [PATCH 02/20] Simplify logic --- .../common/basetype/BaseTypeVisitor.java | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 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 76ca30ee77b8..8cb20bdf01eb 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -3718,19 +3718,20 @@ protected void checkConstructorInvocation( // annotations on the constructor invocation (explicitAnnos). boolean resultIsSubtypeOfExplicit = typeHierarchy.isSubtypeShallowEffective(retType, explicit); - if (!(typeHierarchy.isSubtypeShallowEffective(explicit, retType) - || resultIsSubtypeOfExplicit)) { - AnnotationMirror resultAnno = retType.getPrimaryAnnotationInHierarchy(explicit); - checker.reportError( - newClassTree, "constructor.invocation", constructor.toString(), explicit, resultAnno); - return; - } else if (!resultIsSubtypeOfExplicit) { - AnnotationMirror resultAnno = retType.getPrimaryAnnotationInHierarchy(explicit); - // Issue a warning if the annotations on the constructor invocation is a subtype of - // the constructor result type. This is equivalent to down-casting. - checker.reportWarning( - newClassTree, "cast.unsafe.constructor.invocation", resultAnno, explicit); - return; + if (!resultIsSubtypeOfExplicit) { + if (!typeHierarchy.isSubtypeShallowEffective(explicit, retType)) { + AnnotationMirror resultAnno = retType.getPrimaryAnnotationInHierarchy(explicit); + checker.reportError( + newClassTree, "constructor.invocation", constructor.toString(), explicit, resultAnno); + return; + } else { + AnnotationMirror resultAnno = retType.getPrimaryAnnotationInHierarchy(explicit); + // Issue a warning if the annotations on the constructor invocation is a subtype of + // the constructor result type. This is equivalent to down-casting. + checker.reportWarning( + newClassTree, "cast.unsafe.constructor.invocation", resultAnno, explicit); + return; + } } } From 30144ce87ef62a4c2db1b591c0f0441f03ca35f8 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 15:12:18 -0700 Subject: [PATCH 03/20] Warn about unneeded suppressions in tests --- .../framework/test/CheckerFrameworkPerDirectoryTest.java | 1 + .../framework/test/CheckerFrameworkPerFileTest.java | 4 ++-- .../framework/test/CheckerFrameworkRootedTest.java | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerDirectoryTest.java b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerDirectoryTest.java index bd0028dc91ff..e340b555e2c9 100644 --- a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerDirectoryTest.java +++ b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerDirectoryTest.java @@ -142,6 +142,7 @@ protected CheckerFrameworkPerDirectoryTest( this.checkerOptions = new ArrayList<>(Arrays.asList(checkerOptions)); this.checkerOptions.add("-AajavaChecks"); this.checkerOptions.add("-AconvertTypeArgInferenceCrashToWarning=false"); + this.checkerOptions.add("-AwarnUnneededSuppressions"); } /** Run the tests. */ diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java index 102e5f08380b..2708daee8947 100644 --- a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java +++ b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java @@ -2,7 +2,6 @@ import java.io.File; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import javax.annotation.processing.AbstractProcessor; @@ -82,7 +81,8 @@ protected CheckerFrameworkPerFileTest( this.testFile = testFile; this.checker = checker; this.testDir = testDir; - this.checkerOptions = new ArrayList<>(Arrays.asList(checkerOptions)); + this.checkerOptions = new ArrayList<>(checkerOptions); + this.checkerOptions.add("-AwarnUnneededSuppressions"); } @Test diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java index 2197d495529d..b09fc0094146 100644 --- a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java +++ b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java @@ -5,7 +5,7 @@ /** Encapsulates the directory root to search within for test files to compile. */ abstract class CheckerFrameworkRootedTest { - /** Constructs a test that will assert that can resolve its tests root directory. */ + /** Constructs a test that uses {@code @TestRootDirectory} to find its test directory. */ public CheckerFrameworkRootedTest() {} /** From 5f7a2302f66036cf4d3613b65e31357f41017425 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 15:28:27 -0700 Subject: [PATCH 04/20] Make type argument explicit --- .../framework/test/CheckerFrameworkPerFileTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java index 2708daee8947..a69408a5cf37 100644 --- a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java +++ b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java @@ -81,7 +81,7 @@ protected CheckerFrameworkPerFileTest( this.testFile = testFile; this.checker = checker; this.testDir = testDir; - this.checkerOptions = new ArrayList<>(checkerOptions); + this.checkerOptions = new ArrayList(checkerOptions); this.checkerOptions.add("-AwarnUnneededSuppressions"); } From 26cb9a893e3678fb34874729caab25814952b562 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 15:37:40 -0700 Subject: [PATCH 05/20] Fix type mismatch --- .../framework/test/CheckerFrameworkPerFileTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java index a69408a5cf37..b90ef0176ae5 100644 --- a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java +++ b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java @@ -81,7 +81,7 @@ protected CheckerFrameworkPerFileTest( this.testFile = testFile; this.checker = checker; this.testDir = testDir; - this.checkerOptions = new ArrayList(checkerOptions); + this.checkerOptions = new ArrayList(Arrays.asList(checkerOptions)); this.checkerOptions.add("-AwarnUnneededSuppressions"); } From 1cd5d461ff3fd5903ae785d2e5294ac62cea3f30 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 15:48:24 -0700 Subject: [PATCH 06/20] Add import --- .../framework/test/CheckerFrameworkPerFileTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java index b90ef0176ae5..48ad90ee1428 100644 --- a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java +++ b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java @@ -2,6 +2,7 @@ import java.io.File; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import javax.annotation.processing.AbstractProcessor; From 73de83d9dddcd4ac8f3c017ed0fb03ad9ed0fe60 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 17:02:42 -0700 Subject: [PATCH 07/20] Improve efficiency of "unneeded.suppression" warning --- .../framework/source/SourceChecker.java | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index 96feb1d6a233..9062fbec4c87 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -455,9 +455,6 @@ public abstract class SourceChecker extends AbstractTypeProcessor implements Opt /** The SuppressWarnings prefix that will suppress warnings for all checkers. */ public static final String SUPPRESS_ALL_PREFIX = "allcheckers"; - /** The message key emitted when an unused warning suppression is found. */ - public static final @CompilerMessageKey String UNNEEDED_SUPPRESSION_KEY = "unneeded.suppression"; - /** File name of the localized messages. */ protected static final String MSGS_FILE = "messages.properties"; @@ -2492,11 +2489,11 @@ protected void warnUnneededSuppressions() { * * @param elementsSuppress elements with a {@code @SuppressWarnings} that actually suppressed a * warning - * @param prefixes the SuppressWarnings prefixes that suppress all warnings from this checker + * @param checkerPrefixes the SuppressWarnings prefixes associated with this checker * @param allErrorKeys all error keys that can be issued by this checker */ protected void warnUnneededSuppressions( - Set elementsSuppress, Set prefixes, Set allErrorKeys) { + Set elementsSuppress, Set checkerPrefixes, Set allErrorKeys) { for (Tree tree : getVisitor().treesWithSuppressWarnings) { Element elt = TreeUtils.elementFromTree(tree); // TODO: This test is too coarse. The fact that this @SuppressWarnings suppressed @@ -2504,7 +2501,7 @@ protected void warnUnneededSuppressions( if (elementsSuppress.contains(elt)) { continue; } - // tree has a @SuppressWarnings annotation that didn't suppress any warnings. + // `tree` has a @SuppressWarnings annotation that didn't suppress any warnings. SuppressWarnings suppressAnno = elt.getAnnotation(SuppressWarnings.class); String[] suppressWarningsStrings = suppressAnno.value(); for (String suppressWarningsString : suppressWarningsStrings) { @@ -2512,12 +2509,17 @@ protected void warnUnneededSuppressions( && warnUnneededSuppressionsExceptions.matcher(suppressWarningsString).find(0)) { continue; } - for (String prefix : prefixes) { - if (suppressWarningsString.equals(prefix) - || (suppressWarningsString.startsWith(prefix + ":") - && !suppressWarningsString.equals(prefix + ":" + UNNEEDED_SUPPRESSION_KEY))) { - reportUnneededSuppression(tree, suppressWarningsString); - break; // Don't report the same warning string more than once. + if (checkerPrefixes.contains(suppressWarningsString)) { + reportUnneededSuppression(tree, suppressWarningsString); + } else { + int colonPos = suppressWarningsString.indexOf(":"); + if (colonPos != -1) { + String warningPrefix = suppressWarningsString.substring(0, colonPos); + if (checkerPrefixes.contains(warningPrefix) + && !suppressWarningsString.regionMatches( + colonPos, ":unneeded.suppression", 0, ":unneeded.suppression".length())) { + reportUnneededSuppression(tree, suppressWarningsString); + } } } } @@ -2535,7 +2537,7 @@ private void reportUnneededSuppression(Tree tree, String suppressWarningsString) report( swTree, Diagnostic.Kind.MANDATORY_WARNING, - SourceChecker.UNNEEDED_SUPPRESSION_KEY, + "unneeded.suppression", "\"" + suppressWarningsString + "\"", getClass().getSimpleName()); } @@ -2820,7 +2822,7 @@ private boolean shouldSuppress( if (prefixes.contains(currentSuppressWarningsInEffect)) { // The value in the @SuppressWarnings is exactly a prefix. // Suppress the warning unless its message key is "unneeded.suppression". - boolean result = !currentSuppressWarningsInEffect.equals(UNNEEDED_SUPPRESSION_KEY); + boolean result = !currentSuppressWarningsInEffect.equals("unneeded.suppression"); return result; } else if (requirePrefixInWarningSuppressions) { // A prefix is required, but this SuppressWarnings string does not have a @@ -2829,7 +2831,7 @@ private boolean shouldSuppress( } else if (currentSuppressWarningsInEffect.equals(SUPPRESS_ALL_MESSAGE_KEY)) { // Prefixes aren't required and the SuppressWarnings string is "all". // Suppress the warning unless its message key is "unneeded.suppression". - boolean result = !currentSuppressWarningsInEffect.equals(UNNEEDED_SUPPRESSION_KEY); + boolean result = !currentSuppressWarningsInEffect.equals("unneeded.suppression"); return result; } // The currentSuppressWarningsInEffect is not a prefix or a prefix:message-key, so From 990b6a6b82fc652d7409bf8aa401c3f115c1ae40 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 17:06:14 -0700 Subject: [PATCH 08/20] Tweak documentation --- .../checkerframework/framework/source/SourceChecker.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index 9062fbec4c87..5582b5f61727 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -2483,9 +2483,9 @@ protected void warnUnneededSuppressions() { } /** - * Issues a warning about any {@code @SuppressWarnings} string that didn't suppress a warning, but - * starts with one of the given prefixes (checker names). Does nothing if the string doesn't start - * with a checker name. + * Issues a warning about any {@code @SuppressWarnings} string that didn't suppress a warning. Has + * an effect only if the string is one of the given prefixes (checker names) or starts with one + * followed by a colon. * * @param elementsSuppress elements with a {@code @SuppressWarnings} that actually suppressed a * warning From 7cedddc817210dcba131241afba1c8d7e5a2d786 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 17:42:58 -0700 Subject: [PATCH 09/20] Code review changes --- .../framework/source/SourceChecker.java | 58 +++++++++---------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index 5582b5f61727..cd5c1598cdc7 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -449,9 +449,6 @@ public abstract class SourceChecker extends AbstractTypeProcessor implements Opt // TODO A checker should export itself through a separate interface, and maybe have an interface // for all the methods for which it's safe to override. - /** The message key that will suppress all warnings (it matches any message key). */ - public static final String SUPPRESS_ALL_MESSAGE_KEY = "all"; - /** The SuppressWarnings prefix that will suppress warnings for all checkers. */ public static final String SUPPRESS_ALL_PREFIX = "allcheckers"; @@ -2469,15 +2466,13 @@ protected void warnUnneededSuppressions() { this.elementsWithSuppressedWarnings.clear(); Set prefixes = new HashSet<>(getSuppressWarningsPrefixes()); - Set errorKeys = new HashSet<>(messagesProperties.stringPropertyNames()); for (SourceChecker subChecker : subcheckers) { allElementsWithSuppressedWarnings.addAll(subChecker.elementsWithSuppressedWarnings); subChecker.elementsWithSuppressedWarnings.clear(); prefixes.addAll(subChecker.getSuppressWarningsPrefixes()); - errorKeys.addAll(subChecker.messagesProperties.stringPropertyNames()); subChecker.getVisitor().treesWithSuppressWarnings.clear(); } - warnUnneededSuppressions(allElementsWithSuppressedWarnings, prefixes, errorKeys); + warnUnneededSuppressions(allElementsWithSuppressedWarnings, prefixes); getVisitor().treesWithSuppressWarnings.clear(); } @@ -2490,10 +2485,9 @@ protected void warnUnneededSuppressions() { * @param elementsSuppress elements with a {@code @SuppressWarnings} that actually suppressed a * warning * @param checkerPrefixes the SuppressWarnings prefixes associated with this checker - * @param allErrorKeys all error keys that can be issued by this checker */ protected void warnUnneededSuppressions( - Set elementsSuppress, Set checkerPrefixes, Set allErrorKeys) { + Set elementsSuppress, Set checkerPrefixes) { for (Tree tree : getVisitor().treesWithSuppressWarnings) { Element elt = TreeUtils.elementFromTree(tree); // TODO: This test is too coarse. The fact that this @SuppressWarnings suppressed @@ -2515,10 +2509,14 @@ protected void warnUnneededSuppressions( int colonPos = suppressWarningsString.indexOf(":"); if (colonPos != -1) { String warningPrefix = suppressWarningsString.substring(0, colonPos); - if (checkerPrefixes.contains(warningPrefix) - && !suppressWarningsString.regionMatches( - colonPos, ":unneeded.suppression", 0, ":unneeded.suppression".length())) { - reportUnneededSuppression(tree, suppressWarningsString); + if (checkerPrefixes.contains(warningPrefix)) { + // Test whether the error key is "unneeded.suppression", without creating a String. + boolean isUnneededSuppression = + suppressWarningsString.length() == colonPos + 1 + "unneeded.suppression".length() + && suppressWarningsString.endsWith("unneeded.suppression"); + if (!isUnneededSuppression) { + reportUnneededSuppression(tree, suppressWarningsString); + } } } } @@ -2815,28 +2813,28 @@ private boolean shouldSuppress( for (String currentSuppressWarningsInEffect : suppressWarningsInEffect) { int colonPos = currentSuppressWarningsInEffect.indexOf(':'); - String messageKeyInSuppressWarningsString; + String messageKeyInEffect; if (colonPos == -1) { - // The SuppressWarnings string has no colon, so it is not of the form + // `currentSuppressWarningsInEffect` has no colon, so it is not of the form // prefix:partial-message-key. if (prefixes.contains(currentSuppressWarningsInEffect)) { // The value in the @SuppressWarnings is exactly a prefix. // Suppress the warning unless its message key is "unneeded.suppression". - boolean result = !currentSuppressWarningsInEffect.equals("unneeded.suppression"); + boolean result = !messageKey.equals("unneeded.suppression"); return result; } else if (requirePrefixInWarningSuppressions) { // A prefix is required, but this SuppressWarnings string does not have a - // prefix; check the next SuppressWarnings string. + // prefix. continue; - } else if (currentSuppressWarningsInEffect.equals(SUPPRESS_ALL_MESSAGE_KEY)) { + } else if (currentSuppressWarningsInEffect.equals("all")) { // Prefixes aren't required and the SuppressWarnings string is "all". // Suppress the warning unless its message key is "unneeded.suppression". - boolean result = !currentSuppressWarningsInEffect.equals("unneeded.suppression"); + boolean result = !messageKey.equals("unneeded.suppression"); return result; } - // The currentSuppressWarningsInEffect is not a prefix or a prefix:message-key, so + // The currentSuppressWarningsInEffect is not a checker name, so // it might be a message key. - messageKeyInSuppressWarningsString = currentSuppressWarningsInEffect; + messageKeyInEffect = currentSuppressWarningsInEffect; } else { // The SuppressWarnings string has a colon; that is, it has a prefix. String currentSuppressWarningsPrefix = @@ -2846,12 +2844,11 @@ private boolean shouldSuppress( // this checker. Proceed to the next SuppressWarnings string. continue; } - messageKeyInSuppressWarningsString = - currentSuppressWarningsInEffect.substring(colonPos + 1); + messageKeyInEffect = currentSuppressWarningsInEffect.substring(colonPos + 1); } // Check if the message key in the warning suppression is part of the message key that // the checker is emitting. - if (messageKeyMatches(messageKey, messageKeyInSuppressWarningsString)) { + if (messageKeyMatches(messageKey, messageKeyInEffect)) { return true; } } @@ -2866,16 +2863,15 @@ private boolean shouldSuppress( * * @param messageKey the message key of the error that is being emitted, without any "checker:" * prefix - * @param messageKeyInSuppressWarningsString the message key in a {@code @SuppressWarnings} - * annotation + * @param messageKeyInEffect the message key in a {@code @SuppressWarnings} annotation that is + * currently in effect * @return true if the arguments match */ - protected boolean messageKeyMatches( - String messageKey, String messageKeyInSuppressWarningsString) { - return messageKey.equals(messageKeyInSuppressWarningsString) - || messageKey.startsWith(messageKeyInSuppressWarningsString + ".") - || messageKey.endsWith("." + messageKeyInSuppressWarningsString) - || messageKey.contains("." + messageKeyInSuppressWarningsString + "."); + protected boolean messageKeyMatches(String messageKey, String messageKeyInEffect) { + return messageKey.equals(messageKeyInEffect) + || messageKey.startsWith(messageKeyInEffect + ".") + || messageKey.endsWith("." + messageKeyInEffect) + || messageKey.contains("." + messageKeyInEffect + "."); } /** From 3c2c8b12bf0f7ea9851c06ba0ad35e7043b8c277 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 17:44:16 -0700 Subject: [PATCH 10/20] Warn about unneeded suppressions in tests --- checker/tests/index/ArrayIntro.java | 1 - checker/tests/index/UpperBoundRefinement.java | 1 - checker/tests/mustcall/PolyTests.java | 2 -- .../Issue1315.java | 4 +-- .../nullness/SuppressWarningsPartialKeys.java | 1 + checker/tests/regex/GroupCounts.java | 2 -- .../framework/source/SourceChecker.java | 35 +++++++++++-------- framework/tests/all-systems/ForEach.java | 3 -- .../tests/all-systems/GenericTest11full.java | 3 -- framework/tests/all-systems/GetClassTest.java | 7 ---- .../tests/all-systems/InferTypeArgs.java | 2 +- framework/tests/all-systems/Issue1708.java | 1 - framework/tests/all-systems/Issue1809.java | 2 +- framework/tests/all-systems/Issue457.java | 1 - framework/tests/all-systems/Issue759.java | 5 ++- .../all-systems/MissingBoundAnnotations.java | 1 - framework/tests/all-systems/SetOfSet.java | 5 +-- ...{WildCardCrash.java => WildcardCrash.java} | 4 +-- .../tests/all-systems/WildcardSuper2.java | 2 +- .../java8inference/CollectorsToList.java | 1 - .../java8inference/MemRefInfere.java | 1 - framework/tests/flow/Values.java | 3 -- 22 files changed, 30 insertions(+), 57 deletions(-) rename framework/tests/all-systems/{WildCardCrash.java => WildcardCrash.java} (93%) diff --git a/checker/tests/index/ArrayIntro.java b/checker/tests/index/ArrayIntro.java index a5ab41647a51..5451ce161a9a 100644 --- a/checker/tests/index/ArrayIntro.java +++ b/checker/tests/index/ArrayIntro.java @@ -1,6 +1,5 @@ import org.checkerframework.common.value.qual.MinLen; -@SuppressWarnings("lowerbound") public class ArrayIntro { void test() { int @MinLen(5) [] arr = new int[5]; diff --git a/checker/tests/index/UpperBoundRefinement.java b/checker/tests/index/UpperBoundRefinement.java index 9209b5f25547..419c450c856c 100644 --- a/checker/tests/index/UpperBoundRefinement.java +++ b/checker/tests/index/UpperBoundRefinement.java @@ -1,6 +1,5 @@ import org.checkerframework.checker.index.qual.LTLengthOf; -@SuppressWarnings("lowerbound") public class UpperBoundRefinement { // If expression i has type @LTLengthOf(value = "f2", offset = "f1.length") int and expression // j is less than or equal to the length of f1, then the type of i + j is @LTLengthOf("f2") diff --git a/checker/tests/mustcall/PolyTests.java b/checker/tests/mustcall/PolyTests.java index 2389dc4ddc01..4b080822cea4 100644 --- a/checker/tests/mustcall/PolyTests.java +++ b/checker/tests/mustcall/PolyTests.java @@ -19,8 +19,6 @@ static void test2(@Owning @MustCall({}) Object o) { @MustCall({}) Object o2 = id(o); } - // These sort of constructors will always appear in stub files and are unverifiable for now. - @SuppressWarnings("mustcall:annotations.on.use") @PolyMustCall PolyTests(@PolyMustCall Object obj) {} static void test3(@Owning @MustCall({"close"}) Object o) { diff --git a/checker/tests/nullness-checkcastelementtype/Issue1315.java b/checker/tests/nullness-checkcastelementtype/Issue1315.java index edcbefa2182a..5617a4177bde 100644 --- a/checker/tests/nullness-checkcastelementtype/Issue1315.java +++ b/checker/tests/nullness-checkcastelementtype/Issue1315.java @@ -17,9 +17,7 @@ T test1(@Nullable Object p) { return (T) p; } - // The Nullness Checker should not issue a cast.unsafe warning, - // but the KeyFor Checker does, so suppress that warning. - @SuppressWarnings({"unchecked", "keyfor:cast.unsafe"}) + @SuppressWarnings("unchecked") T test2(Object p) { return (T) p; } diff --git a/checker/tests/nullness/SuppressWarningsPartialKeys.java b/checker/tests/nullness/SuppressWarningsPartialKeys.java index d7a6c424c2e7..968ade610af5 100644 --- a/checker/tests/nullness/SuppressWarningsPartialKeys.java +++ b/checker/tests/nullness/SuppressWarningsPartialKeys.java @@ -1,5 +1,6 @@ import org.checkerframework.checker.nullness.qual.NonNull; +@SuppressWarnings("unneeded.suppression") public class SuppressWarningsPartialKeys { @SuppressWarnings("return") diff --git a/checker/tests/regex/GroupCounts.java b/checker/tests/regex/GroupCounts.java index dc9861b039bf..fbf385911148 100644 --- a/checker/tests/regex/GroupCounts.java +++ b/checker/tests/regex/GroupCounts.java @@ -43,9 +43,7 @@ void testPatternCompileGroupCount(@Regex String r, @Regex(3) String r3, @Regex(5 // Make sure Pattern.compile still works when passed an @UnknownRegex String // that's actually a regex, with the warning suppressed. - @SuppressWarnings("regex:argument") Pattern p6 = Pattern.compile("(" + r + ")"); - @SuppressWarnings("regex:argument") Pattern p6a = Pattern.compile("(" + r + ")", 0); } diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index 96feb1d6a233..3d78a45faacf 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -455,9 +455,6 @@ public abstract class SourceChecker extends AbstractTypeProcessor implements Opt /** The SuppressWarnings prefix that will suppress warnings for all checkers. */ public static final String SUPPRESS_ALL_PREFIX = "allcheckers"; - /** The message key emitted when an unused warning suppression is found. */ - public static final @CompilerMessageKey String UNNEEDED_SUPPRESSION_KEY = "unneeded.suppression"; - /** File name of the localized messages. */ protected static final String MSGS_FILE = "messages.properties"; @@ -2492,11 +2489,11 @@ protected void warnUnneededSuppressions() { * * @param elementsSuppress elements with a {@code @SuppressWarnings} that actually suppressed a * warning - * @param prefixes the SuppressWarnings prefixes that suppress all warnings from this checker + * @param checkerPrefixes the SuppressWarnings prefixes associated with this checker * @param allErrorKeys all error keys that can be issued by this checker */ protected void warnUnneededSuppressions( - Set elementsSuppress, Set prefixes, Set allErrorKeys) { + Set elementsSuppress, Set checkerPrefixes, Set allErrorKeys) { for (Tree tree : getVisitor().treesWithSuppressWarnings) { Element elt = TreeUtils.elementFromTree(tree); // TODO: This test is too coarse. The fact that this @SuppressWarnings suppressed @@ -2504,20 +2501,28 @@ protected void warnUnneededSuppressions( if (elementsSuppress.contains(elt)) { continue; } - // tree has a @SuppressWarnings annotation that didn't suppress any warnings. + // `tree` has a @SuppressWarnings annotation that didn't suppress any warnings. SuppressWarnings suppressAnno = elt.getAnnotation(SuppressWarnings.class); String[] suppressWarningsStrings = suppressAnno.value(); + System.out.printf( + "checkerPrefixes = %s%nsuppressWarningsStrings = %s%n", + checkerPrefixes, Arrays.toString(suppressWarningsStrings)); for (String suppressWarningsString : suppressWarningsStrings) { if (warnUnneededSuppressionsExceptions != null && warnUnneededSuppressionsExceptions.matcher(suppressWarningsString).find(0)) { continue; } - for (String prefix : prefixes) { - if (suppressWarningsString.equals(prefix) - || (suppressWarningsString.startsWith(prefix + ":") - && !suppressWarningsString.equals(prefix + ":" + UNNEEDED_SUPPRESSION_KEY))) { - reportUnneededSuppression(tree, suppressWarningsString); - break; // Don't report the same warning string more than once. + if (checkerPrefixes.contains(suppressWarningsString)) { + reportUnneededSuppression(tree, suppressWarningsString); + } else { + int colonPos = suppressWarningsString.indexOf(":"); + if (colonPos != -1) { + String warningPrefix = suppressWarningsString.substring(0, colonPos); + if (checkerPrefixes.contains(warningPrefix) + && !suppressWarningsString.regionMatches( + colonPos, ":unneeded.suppression", 0, ":unneeded.suppression".length())) { + reportUnneededSuppression(tree, suppressWarningsString); + } } } } @@ -2535,7 +2540,7 @@ private void reportUnneededSuppression(Tree tree, String suppressWarningsString) report( swTree, Diagnostic.Kind.MANDATORY_WARNING, - SourceChecker.UNNEEDED_SUPPRESSION_KEY, + "unneeded.suppression", "\"" + suppressWarningsString + "\"", getClass().getSimpleName()); } @@ -2820,7 +2825,7 @@ private boolean shouldSuppress( if (prefixes.contains(currentSuppressWarningsInEffect)) { // The value in the @SuppressWarnings is exactly a prefix. // Suppress the warning unless its message key is "unneeded.suppression". - boolean result = !currentSuppressWarningsInEffect.equals(UNNEEDED_SUPPRESSION_KEY); + boolean result = !currentSuppressWarningsInEffect.equals("unneeded.suppression"); return result; } else if (requirePrefixInWarningSuppressions) { // A prefix is required, but this SuppressWarnings string does not have a @@ -2829,7 +2834,7 @@ private boolean shouldSuppress( } else if (currentSuppressWarningsInEffect.equals(SUPPRESS_ALL_MESSAGE_KEY)) { // Prefixes aren't required and the SuppressWarnings string is "all". // Suppress the warning unless its message key is "unneeded.suppression". - boolean result = !currentSuppressWarningsInEffect.equals(UNNEEDED_SUPPRESSION_KEY); + boolean result = !currentSuppressWarningsInEffect.equals("unneeded.suppression"); return result; } // The currentSuppressWarningsInEffect is not a prefix or a prefix:message-key, so diff --git a/framework/tests/all-systems/ForEach.java b/framework/tests/all-systems/ForEach.java index 07c2aabf7b28..96d4e6e1063b 100644 --- a/framework/tests/all-systems/ForEach.java +++ b/framework/tests/all-systems/ForEach.java @@ -32,9 +32,6 @@ void m4(T p) { } } - @SuppressWarnings( - "nonempty") // TODO: the Non-Empty Checker requires the Side Effects Only checker to - // eliminate the false positive here. public static List removeDuplicates(List l) { // There are shorter solutions that do not maintain order. HashSet hs = new HashSet<>(l.size()); diff --git a/framework/tests/all-systems/GenericTest11full.java b/framework/tests/all-systems/GenericTest11full.java index b25edeb85743..7e6744a84628 100644 --- a/framework/tests/all-systems/GenericTest11full.java +++ b/framework/tests/all-systems/GenericTest11full.java @@ -1,7 +1,4 @@ public class GenericTest11full { - @SuppressWarnings( - "nonempty:method.invocation") // Cannot determine statically whether the getBeans(...) - // call below will return a non-empty container. public void m(BeanManager beanManager) { Bean bean = beanManager.getBeans(GenericTest11full.class).iterator().next(); CreationalContext context = beanManager.createCreationalContext(bean); diff --git a/framework/tests/all-systems/GetClassTest.java b/framework/tests/all-systems/GetClassTest.java index 37a15b627409..9e7e244b78e0 100644 --- a/framework/tests/all-systems/GetClassTest.java +++ b/framework/tests/all-systems/GetClassTest.java @@ -9,13 +9,7 @@ void context() { Integer i = 4; i.getClass(); Class a = i.getClass(); - // Type arguments don't match - @SuppressWarnings("fenum:assignment") Class b = i.getClass(); - @SuppressWarnings({ - "fenum:assignment", // Type arguments don't match - "signedness:assignment" // Type arguments don't match - }) Class c = i.getClass(); Class d = i.getClass(); @@ -24,7 +18,6 @@ void context() { } void m(Date d) { - @SuppressWarnings("fenum:assignment") Class c = d.getClass(); } } diff --git a/framework/tests/all-systems/InferTypeArgs.java b/framework/tests/all-systems/InferTypeArgs.java index 9d8923466bf8..84b857130131 100644 --- a/framework/tests/all-systems/InferTypeArgs.java +++ b/framework/tests/all-systems/InferTypeArgs.java @@ -75,7 +75,7 @@ protected FlowAnalysis createFlowAnalysis() { return result; } - @SuppressWarnings({"nullness:return", "lock:return", "immutabilitysub:type.argument"}) + @SuppressWarnings({"nullness:return", "immutabilitysub:type.argument"}) public static T invokeConstructorFor() { return null; } diff --git a/framework/tests/all-systems/Issue1708.java b/framework/tests/all-systems/Issue1708.java index eea4466b5117..9eda8ea33fc9 100644 --- a/framework/tests/all-systems/Issue1708.java +++ b/framework/tests/all-systems/Issue1708.java @@ -8,7 +8,6 @@ @SuppressWarnings({ "unchecked", "ainfertest", - "value" }) // Don't issue warnings during ainfer tests, because more than one round of inference is required // for the value checker public class Issue1708 { diff --git a/framework/tests/all-systems/Issue1809.java b/framework/tests/all-systems/Issue1809.java index fc3f89d74030..46635b6f8c4e 100644 --- a/framework/tests/all-systems/Issue1809.java +++ b/framework/tests/all-systems/Issue1809.java @@ -30,7 +30,7 @@ interface S {} // The Checker Framework does not refine the type of Stream#filter based on post conditions of // the passed function. - @SuppressWarnings({"nullness", "optional"}) + // TODO: use more specific suppressions. @SuppressWarnings({"nullness", "optional"}) private Stream xrefsFor(B b) { return concat(b.g().stream().flatMap(a -> a.h().stream().map(c -> f()))) .filter(Optional::isPresent) diff --git a/framework/tests/all-systems/Issue457.java b/framework/tests/all-systems/Issue457.java index f20634ece0e2..1fa2e02a57ed 100644 --- a/framework/tests/all-systems/Issue457.java +++ b/framework/tests/all-systems/Issue457.java @@ -6,7 +6,6 @@ public class Issue457 { public void f(T t) { final T obj = t; - @SuppressWarnings("signedness:assignment") // cast Float objFloat = (obj instanceof Float) ? (Float) obj : null; // An error will be emitted on this line before the fix for Issue457 diff --git a/framework/tests/all-systems/Issue759.java b/framework/tests/all-systems/Issue759.java index b6d23961e5b2..b3a315a80f56 100644 --- a/framework/tests/all-systems/Issue759.java +++ b/framework/tests/all-systems/Issue759.java @@ -3,8 +3,7 @@ @SuppressWarnings({ "nullness", "unchecked", - "ainfertest", - "value" + "ainfertest" }) // See checker/test/nullness/Issue759.java; ainfertest and value are suppressed because WPI // errors shouldn't be issued here, just checked for crashes public class Issue759 { @@ -28,7 +27,7 @@ T[] getConstants() { } } -@SuppressWarnings("nullness") +// @SuppressWarnings("nullness") class IncompatibleTypes { void possibleValues(final Gen genType) { lowercase(genType.getConstants()); diff --git a/framework/tests/all-systems/MissingBoundAnnotations.java b/framework/tests/all-systems/MissingBoundAnnotations.java index 2c15fde814c7..4be65187097a 100644 --- a/framework/tests/all-systems/MissingBoundAnnotations.java +++ b/framework/tests/all-systems/MissingBoundAnnotations.java @@ -4,7 +4,6 @@ import java.util.Map; public final class MissingBoundAnnotations { - @SuppressWarnings("nullness:type.argument") public static , V> Collection sortedKeySet(Map m) { ArrayList theKeys = new ArrayList<>(m.keySet()); Collections.sort(theKeys); diff --git a/framework/tests/all-systems/SetOfSet.java b/framework/tests/all-systems/SetOfSet.java index a67789071488..35df3f5e6b0c 100644 --- a/framework/tests/all-systems/SetOfSet.java +++ b/framework/tests/all-systems/SetOfSet.java @@ -1,9 +1,6 @@ package wildcards; -@SuppressWarnings({ - "initialization", - "initializedfields:contracts.postcondition" -}) // field isn't set +@SuppressWarnings("initialization") public class SetOfSet { public void addAll(SetOfSet s) { diff --git a/framework/tests/all-systems/WildCardCrash.java b/framework/tests/all-systems/WildcardCrash.java similarity index 93% rename from framework/tests/all-systems/WildCardCrash.java rename to framework/tests/all-systems/WildcardCrash.java index c9408adab447..fbeac11faedc 100644 --- a/framework/tests/all-systems/WildCardCrash.java +++ b/framework/tests/all-systems/WildcardCrash.java @@ -1,4 +1,4 @@ -public class WildCardCrash {} +public class WildcardCrash {} abstract class AbstractTransfer123< IndexStore extends CFAbstractStore123, @@ -16,7 +16,7 @@ public SomeGen(CFAbstractAnalysis123 analysis) {} class CFValue123 extends CFAbstractValue123 {} -@SuppressWarnings({"initialization", "initializedfields:contracts.postcondition"}) +@SuppressWarnings("initialization") class CFAbstractTransfer123< V extends CFAbstractValue123, S extends CFAbstractStore123, diff --git a/framework/tests/all-systems/WildcardSuper2.java b/framework/tests/all-systems/WildcardSuper2.java index 2278ba1b5d9c..c03dfac2d542 100644 --- a/framework/tests/all-systems/WildcardSuper2.java +++ b/framework/tests/all-systems/WildcardSuper2.java @@ -8,7 +8,7 @@ interface AInterface { class B implements AInterface { // This shouldn't work for nullness as the function won't take possibly nullable values. - @SuppressWarnings({"nullness", "fenum:override.param", "aliasing"}) + // @SuppressWarnings({"nullness", "aliasing"}) @Override public int transform(List function) { return 0; diff --git a/framework/tests/all-systems/java8inference/CollectorsToList.java b/framework/tests/all-systems/java8inference/CollectorsToList.java index a65794c91ed5..723773c2ae21 100644 --- a/framework/tests/all-systems/java8inference/CollectorsToList.java +++ b/framework/tests/all-systems/java8inference/CollectorsToList.java @@ -22,7 +22,6 @@ void m(List strings) { // This works: List<@Nullable String> collectedStrings2 = s.collect(Collectors.toList()); // This works: - @SuppressWarnings("nullness") List collectedStrings3 = s.collect(Collectors.toList()); // This assignment issues a warning due to incompatible types: diff --git a/framework/tests/all-systems/java8inference/MemRefInfere.java b/framework/tests/all-systems/java8inference/MemRefInfere.java index 0ddd3bd9060e..225c70ae7ef2 100644 --- a/framework/tests/all-systems/java8inference/MemRefInfere.java +++ b/framework/tests/all-systems/java8inference/MemRefInfere.java @@ -13,7 +13,6 @@ public static MemRefInfere copyOf(Map Collector> toImmutableMap( Function keyFunction, Function valueFunction, diff --git a/framework/tests/flow/Values.java b/framework/tests/flow/Values.java index 3dd5ff46cdec..ad6a1828d682 100644 --- a/framework/tests/flow/Values.java +++ b/framework/tests/flow/Values.java @@ -52,17 +52,14 @@ void foo1(@ValueTypeAnno(1) Object o) {} void foo2(@ValueTypeAnno(2) Object o) {} - @SuppressWarnings("flowtest:return") @ValueTypeAnno Object get() { return null; } - @SuppressWarnings("flowtest:return") @ValueTypeAnno(1) Object get1() { return null; } - @SuppressWarnings("flowtest:return") @ValueTypeAnno(2) Object get2() { return null; } From ee084d77c0ac313b64be1a03b0c83677c8c20990 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 18:22:27 -0700 Subject: [PATCH 11/20] Remove unnecessary `@SuppressWarnings` --- .../checkerframework/dataflow/analysis/AbstractAnalysis.java | 1 - .../dataflow/cfg/builder/CFGTranslationPhaseThree.java | 1 - .../main/java/org/checkerframework/javacutil/ElementUtils.java | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/analysis/AbstractAnalysis.java b/dataflow/src/main/java/org/checkerframework/dataflow/analysis/AbstractAnalysis.java index 4743a3962a23..1cbb5a206d2e 100644 --- a/dataflow/src/main/java/org/checkerframework/dataflow/analysis/AbstractAnalysis.java +++ b/dataflow/src/main/java/org/checkerframework/dataflow/analysis/AbstractAnalysis.java @@ -497,7 +497,6 @@ protected TransferResult callTransferFunction( } transferInput.node = node; setCurrentNode(node); - @SuppressWarnings("nullness") // CF bug: "INFERENCE FAILED" TransferResult transferResult = node.accept(transferFunction, transferInput); setCurrentNode(null); if (node instanceof AssignmentNode assignment) { diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/CFGTranslationPhaseThree.java b/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/CFGTranslationPhaseThree.java index 1ebc14092052..1d49d92d057c 100644 --- a/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/CFGTranslationPhaseThree.java +++ b/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/CFGTranslationPhaseThree.java @@ -65,7 +65,6 @@ protected static interface PredecessorHolder { * not allowed to read or modify {@code cfg} after the call to {@code process} any more. * @return the resulting control flow graph */ - @SuppressWarnings("nullness") // TODO: successors public static ControlFlowGraph process(ControlFlowGraph cfg) { Set blocks = cfg.getAllBlocks(); Set removedBlocks = new HashSet<>(); diff --git a/javacutil/src/main/java/org/checkerframework/javacutil/ElementUtils.java b/javacutil/src/main/java/org/checkerframework/javacutil/ElementUtils.java index 48f40fa0a79d..8790e65735fb 100644 --- a/javacutil/src/main/java/org/checkerframework/javacutil/ElementUtils.java +++ b/javacutil/src/main/java/org/checkerframework/javacutil/ElementUtils.java @@ -1021,7 +1021,7 @@ public static ElementKind getKindRecordAsClass(Element elt) { * @deprecated use {@link TypeElement#getRecordComponents} */ @Deprecated(forRemoval = true, since = "4.0.0") - @SuppressWarnings({"unchecked", "nullness"}) // because of cast from reflection + @SuppressWarnings("unchecked") public static List getRecordComponents(TypeElement element) { return element.getRecordComponents(); } From c6bc342a5052bae087378fef0b16ccd7d75fdf84 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 19:24:01 -0700 Subject: [PATCH 12/20] Handle "allcheckers" as well as "all" --- .../framework/source/SourceChecker.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index cd5c1598cdc7..074fcfa0ef33 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -2826,8 +2826,9 @@ private boolean shouldSuppress( // A prefix is required, but this SuppressWarnings string does not have a // prefix. continue; - } else if (currentSuppressWarningsInEffect.equals("all")) { - // Prefixes aren't required and the SuppressWarnings string is "all". + } else if (currentSuppressWarningsInEffect.equals("all") + || currentSuppressWarningsInEffect.equals("allcheckers")) { + // Prefixes aren't required and the SuppressWarnings string is "all" or "allcheckers". // Suppress the warning unless its message key is "unneeded.suppression". boolean result = !messageKey.equals("unneeded.suppression"); return result; @@ -2839,9 +2840,10 @@ private boolean shouldSuppress( // The SuppressWarnings string has a colon; that is, it has a prefix. String currentSuppressWarningsPrefix = currentSuppressWarningsInEffect.substring(0, colonPos); - if (!prefixes.contains(currentSuppressWarningsPrefix)) { - // The prefix of this SuppressWarnings string is a not a prefix supported by - // this checker. Proceed to the next SuppressWarnings string. + if (currentSuppressWarningsPrefix.equals("allcheckers") + || !prefixes.contains(currentSuppressWarningsPrefix)) { + // The prefix of this SuppressWarnings string is "allcheckers" or is not a prefix + // supported by this checker. Proceed to the next SuppressWarnings string. continue; } messageKeyInEffect = currentSuppressWarningsInEffect.substring(colonPos + 1); From d370f37658cb70e506cf653f3cece3c4b9809e1f Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 19:33:19 -0700 Subject: [PATCH 13/20] Reduce warning suppressions --- .../calledmethods/EnsuresCalledMethodsIfTest.java | 2 -- checker/tests/nullness/FullyQualifiedAnnotation.java | 2 -- checker/tests/regex/MyMatchResult.java | 3 --- checker/tests/tainting/Issue2330.java | 4 ---- .../framework/source/SourceChecker.java | 12 +++++++----- framework/tests/all-systems/Enums.java | 1 - framework/tests/all-systems/GenericNull.java | 2 +- framework/tests/all-systems/InferAndWildcards.java | 1 - framework/tests/all-systems/java17/Figure.java | 4 ---- framework/tests/all-systems/java17/Issue6100.java | 1 - .../tests/all-systems/java8inference/Issue1397.java | 1 - .../tests/all-systems/java8inference/Issue1407.java | 1 - .../tests/all-systems/java8inference/Issue1416.java | 1 - .../tests/all-systems/java8inference/Issue6046.java | 2 +- 14 files changed, 9 insertions(+), 28 deletions(-) diff --git a/checker/tests/calledmethods/EnsuresCalledMethodsIfTest.java b/checker/tests/calledmethods/EnsuresCalledMethodsIfTest.java index 50e6b2f5efcb..8651cc684551 100644 --- a/checker/tests/calledmethods/EnsuresCalledMethodsIfTest.java +++ b/checker/tests/calledmethods/EnsuresCalledMethodsIfTest.java @@ -49,8 +49,6 @@ public static void closeSockOK2(EnsuresCalledMethodsIfTest sock) throws Exceptio void close() throws IOException {} - @SuppressWarnings( - "calledmethods") // like the JDK's isOpen methods; makes this test case self-contained @EnsuresCalledMethodsIf( expression = "this", result = false, diff --git a/checker/tests/nullness/FullyQualifiedAnnotation.java b/checker/tests/nullness/FullyQualifiedAnnotation.java index fda3122451d5..be0c2d02b794 100644 --- a/checker/tests/nullness/FullyQualifiedAnnotation.java +++ b/checker/tests/nullness/FullyQualifiedAnnotation.java @@ -15,13 +15,11 @@ void client2(Iterator i) { } void client3(Iterator i) { - @SuppressWarnings("nullness") @org.checkerframework.checker.nullness.qual.NonNull Object handle2 = i.next(); handle2.toString(); } void client4(Iterator i) { - @SuppressWarnings("nullness") @org.checkerframework.checker.nullness.qual.NonNull Object handle2 = i.next(); handle2.toString(); } diff --git a/checker/tests/regex/MyMatchResult.java b/checker/tests/regex/MyMatchResult.java index e08362735b19..5bc17d4bbfa5 100644 --- a/checker/tests/regex/MyMatchResult.java +++ b/checker/tests/regex/MyMatchResult.java @@ -1,8 +1,5 @@ import java.util.regex.MatchResult; -// Outside of scope of the Regex Checker to verify an implementation of MatchResult, -// so just check for crashes. -@SuppressWarnings("regex") public class MyMatchResult implements MatchResult { @Override diff --git a/checker/tests/tainting/Issue2330.java b/checker/tests/tainting/Issue2330.java index c796c81998ec..2415b3dab958 100644 --- a/checker/tests/tainting/Issue2330.java +++ b/checker/tests/tainting/Issue2330.java @@ -3,12 +3,8 @@ import org.checkerframework.checker.tainting.qual.Untainted; public class Issue2330 { - // Checker can't verify that this creates an untainted Issue2330 - @SuppressWarnings("tainting") public @Untainted Issue2330(@PolyTainted int i) {} - // Checker can't verify that this creates an untainted Issue2330 - @SuppressWarnings("tainting") public @Untainted Issue2330() {} public static void f(@PolyTainted int i) { diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index e3992a7f7a17..01400f9a9789 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -2829,8 +2829,9 @@ private boolean shouldSuppress( // A prefix is required, but this SuppressWarnings string does not have a // prefix. continue; - } else if (currentSuppressWarningsInEffect.equals("all")) { - // Prefixes aren't required and the SuppressWarnings string is "all". + } else if (currentSuppressWarningsInEffect.equals("all") + || currentSuppressWarningsInEffect.equals("allcheckers")) { + // Prefixes aren't required and the SuppressWarnings string is "all" or "allcheckers". // Suppress the warning unless its message key is "unneeded.suppression". boolean result = !messageKey.equals("unneeded.suppression"); return result; @@ -2842,9 +2843,10 @@ private boolean shouldSuppress( // The SuppressWarnings string has a colon; that is, it has a prefix. String currentSuppressWarningsPrefix = currentSuppressWarningsInEffect.substring(0, colonPos); - if (!prefixes.contains(currentSuppressWarningsPrefix)) { - // The prefix of this SuppressWarnings string is a not a prefix supported by - // this checker. Proceed to the next SuppressWarnings string. + if (currentSuppressWarningsPrefix.equals("allcheckers") + || !prefixes.contains(currentSuppressWarningsPrefix)) { + // The prefix of this SuppressWarnings string is "allcheckers" or is not a prefix + // supported by this checker. Proceed to the next SuppressWarnings string. continue; } messageKeyInEffect = currentSuppressWarningsInEffect.substring(colonPos + 1); diff --git a/framework/tests/all-systems/Enums.java b/framework/tests/all-systems/Enums.java index 7f8bd397607c..135027ce5854 100644 --- a/framework/tests/all-systems/Enums.java +++ b/framework/tests/all-systems/Enums.java @@ -32,7 +32,6 @@ void m(Class p) { } public SSS firstNonNull(SSS first, SSS second) { - @SuppressWarnings("nullness:nulltest.redundant") SSS res = first != null ? first : checkNotNull(second); return res; } diff --git a/framework/tests/all-systems/GenericNull.java b/framework/tests/all-systems/GenericNull.java index 14d9547e544e..13c8d1569636 100644 --- a/framework/tests/all-systems/GenericNull.java +++ b/framework/tests/all-systems/GenericNull.java @@ -25,7 +25,7 @@ public class GenericNull { * this test. For the Lock Checker, null's type is bottom for the @GuardedByUnknown hierarchy but * not for the @LockPossiblyHeld hierarchy. */ - @SuppressWarnings({"nullness", "lock"}) + @SuppressWarnings("nullness") T f() { return null; } diff --git a/framework/tests/all-systems/InferAndWildcards.java b/framework/tests/all-systems/InferAndWildcards.java index fcb3f4b50663..bdcf75ac7d32 100644 --- a/framework/tests/all-systems/InferAndWildcards.java +++ b/framework/tests/all-systems/InferAndWildcards.java @@ -1,4 +1,3 @@ -@SuppressWarnings("interning") public class InferAndWildcards { Class b(Class clazz) { return clazz; diff --git a/framework/tests/all-systems/java17/Figure.java b/framework/tests/all-systems/java17/Figure.java index 7db6ea5d0923..473142d41b00 100644 --- a/framework/tests/all-systems/java17/Figure.java +++ b/framework/tests/all-systems/java17/Figure.java @@ -8,22 +8,18 @@ public sealed class Figure // defined in the same file. {} -@SuppressWarnings("initializedfields:contracts.postcondition") final class Circle extends Figure { float radius; } -@SuppressWarnings("initializedfields:contracts.postcondition") non-sealed class Square extends Figure { float side; } -@SuppressWarnings("initializedfields:contracts.postcondition") sealed class Rectangle extends Figure { float length, width; } -@SuppressWarnings("initializedfields:contracts.postcondition") final class FilledRectangle extends Rectangle { int red, green, blue, sealed; } diff --git a/framework/tests/all-systems/java17/Issue6100.java b/framework/tests/all-systems/java17/Issue6100.java index de8151f74590..1612b9b655ac 100644 --- a/framework/tests/all-systems/java17/Issue6100.java +++ b/framework/tests/all-systems/java17/Issue6100.java @@ -3,7 +3,6 @@ // @infer-jaifs-skip-test The AFU's JAIF reading/writing libraries don't support records. // @below-java17-jdk-skip-test -@SuppressWarnings("signedness") public record Issue6100(List<@NonNegative Integer> bar) { public Issue6100 { diff --git a/framework/tests/all-systems/java8inference/Issue1397.java b/framework/tests/all-systems/java8inference/Issue1397.java index 8ad4572e0cb5..bca8ab1b123c 100644 --- a/framework/tests/all-systems/java8inference/Issue1397.java +++ b/framework/tests/all-systems/java8inference/Issue1397.java @@ -10,7 +10,6 @@ abstract class CrashCompound { abstract T unbox(Box p); - @SuppressWarnings("units") void foo(Box bb) { boolean res = false; res |= chk(unbox(bb)); diff --git a/framework/tests/all-systems/java8inference/Issue1407.java b/framework/tests/all-systems/java8inference/Issue1407.java index 375845d9bf39..e03207fe4b08 100644 --- a/framework/tests/all-systems/java8inference/Issue1407.java +++ b/framework/tests/all-systems/java8inference/Issue1407.java @@ -6,7 +6,6 @@ abstract class Issue1407 { abstract T bar(int p1, T p2); - @SuppressWarnings({"interning", "signedness"}) int demo() { return foo(bar(5, 3), 3); } diff --git a/framework/tests/all-systems/java8inference/Issue1416.java b/framework/tests/all-systems/java8inference/Issue1416.java index c7cae68ff39a..4b408bbdaf41 100644 --- a/framework/tests/all-systems/java8inference/Issue1416.java +++ b/framework/tests/all-systems/java8inference/Issue1416.java @@ -5,7 +5,6 @@ import java.util.stream.Stream; public class Issue1416 { - @SuppressWarnings("signedness") long order(Stream sl) { return sl.max(Comparator.naturalOrder()).orElse(0L); } diff --git a/framework/tests/all-systems/java8inference/Issue6046.java b/framework/tests/all-systems/java8inference/Issue6046.java index 55481d3e6983..38f97585f719 100644 --- a/framework/tests/all-systems/java8inference/Issue6046.java +++ b/framework/tests/all-systems/java8inference/Issue6046.java @@ -13,7 +13,7 @@ public interface Record extends Comparable, Formattable {} public interface Result extends List, Formattable {} - @SuppressWarnings({"unchecked", "nullness:new.array", "index:array.access.unsafe.high.constant"}) + @SuppressWarnings({"unchecked", "index:array.access.unsafe.high.constant"}) public static Collector>> intoResultGroups( Function keyMapper) { From 83391f6a00a36045e0ab5e65b18a9873096c4a07 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 20:19:46 -0700 Subject: [PATCH 14/20] Update jtreg tests --- checker/jtreg/index/UnneededSuppressionsTest.out | 3 +-- checker/jtreg/nullness/issue2318/RequireCheckerPrefix.1.out | 5 ++++- checker/jtreg/nullness/issue2318/RequireCheckerPrefix.2.out | 5 ++++- .../checkerframework/framework/source/SourceChecker.java | 6 ++++-- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/checker/jtreg/index/UnneededSuppressionsTest.out b/checker/jtreg/index/UnneededSuppressionsTest.out index a26e0d3e5475..4b432deba062 100644 --- a/checker/jtreg/index/UnneededSuppressionsTest.out +++ b/checker/jtreg/index/UnneededSuppressionsTest.out @@ -1,5 +1,4 @@ UnneededSuppressionsTest.java:22:3: compiler.warn.proc.messager: [unneeded.suppression] warning suppression "lowerbound" is not used by IndexChecker UnneededSuppressionsTest.java:24:5: compiler.warn.proc.messager: [unneeded.suppression] warning suppression "upperbound:assignment" is not used by IndexChecker UnneededSuppressionsTest.java:39:3: compiler.warn.proc.messager: [unneeded.suppression] warning suppression "index:foo.bar.baz" is not used by IndexChecker -UnneededSuppressionsTest.java:42:3: compiler.warn.proc.messager: [unneeded.suppression] warning suppression "allcheckers:purity.not.deterministic.call" is not used by IndexChecker -4 warnings +3 warnings diff --git a/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.1.out b/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.1.out index 03a059588cb5..87e2ac2ce3c7 100644 --- a/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.1.out +++ b/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.1.out @@ -1,10 +1,13 @@ RequireCheckerPrefix.java:19:25: compiler.err.proc.messager: [nullness:assignment] incompatible types in assignment. found : @Initialized @Nullable Object required: @UnknownInitialization @NonNull Object +RequireCheckerPrefix.java:21:25: compiler.err.proc.messager: [nullness:assignment] incompatible types in assignment. +found : @Initialized @Nullable Object +required: @UnknownInitialization @NonNull Object RequireCheckerPrefix.java:24:25: compiler.err.proc.messager: [nullness:assignment] incompatible types in assignment. found : @Initialized @Nullable Object required: @UnknownInitialization @NonNull Object RequireCheckerPrefix.java:27:25: compiler.err.proc.messager: [nullness:assignment] incompatible types in assignment. found : @Initialized @Nullable Object required: @UnknownInitialization @NonNull Object -3 errors +4 errors diff --git a/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.2.out b/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.2.out index f72ddfe2a3ee..eab01bacf1a8 100644 --- a/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.2.out +++ b/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.2.out @@ -1,4 +1,7 @@ RequireCheckerPrefix.java:19:25: compiler.err.proc.messager: [assignment] incompatible types in assignment. found : @Initialized @Nullable Object required: @UnknownInitialization @NonNull Object -1 error +RequireCheckerPrefix.java:21:25: compiler.err.proc.messager: [assignment] incompatible types in assignment. +found : @Initialized @Nullable Object +required: @UnknownInitialization @NonNull Object +2 errors diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index 074fcfa0ef33..b57d4f3ee1e3 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -2503,13 +2503,15 @@ protected void warnUnneededSuppressions( && warnUnneededSuppressionsExceptions.matcher(suppressWarningsString).find(0)) { continue; } - if (checkerPrefixes.contains(suppressWarningsString)) { + if (suppressWarningsString.equals("allcheckers")) { + // Do nothing. + } else if (checkerPrefixes.contains(suppressWarningsString)) { reportUnneededSuppression(tree, suppressWarningsString); } else { int colonPos = suppressWarningsString.indexOf(":"); if (colonPos != -1) { String warningPrefix = suppressWarningsString.substring(0, colonPos); - if (checkerPrefixes.contains(warningPrefix)) { + if (!warningPrefix.equals("allcheckers") && checkerPrefixes.contains(warningPrefix)) { // Test whether the error key is "unneeded.suppression", without creating a String. boolean isUnneededSuppression = suppressWarningsString.length() == colonPos + 1 + "unneeded.suppression".length() From b545b1d431c12849f69914158da116d17e5f4310 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 22:19:33 -0700 Subject: [PATCH 15/20] Undo undesirable change --- .../framework/source/SourceChecker.java | 8 +++----- framework/tests/all-systems/PuritySubstring.java | 13 +++++++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) create mode 100644 framework/tests/all-systems/PuritySubstring.java diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index b57d4f3ee1e3..0c4c62b5aa6b 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -2607,7 +2607,6 @@ private boolean shouldSuppressWarnings(@Nullable Object src, String errKey) { * otherwise */ public boolean shouldSuppressWarnings(Tree tree, String errKey) { - Collection prefixes = getSuppressWarningsPrefixes(); if (prefixes.isEmpty() || (prefixes.contains(SUPPRESS_ALL_PREFIX) && prefixes.size() == 1)) { throw new BugInCF( @@ -2842,10 +2841,9 @@ private boolean shouldSuppress( // The SuppressWarnings string has a colon; that is, it has a prefix. String currentSuppressWarningsPrefix = currentSuppressWarningsInEffect.substring(0, colonPos); - if (currentSuppressWarningsPrefix.equals("allcheckers") - || !prefixes.contains(currentSuppressWarningsPrefix)) { - // The prefix of this SuppressWarnings string is "allcheckers" or is not a prefix - // supported by this checker. Proceed to the next SuppressWarnings string. + if (!prefixes.contains(currentSuppressWarningsPrefix)) { + // The prefix of this SuppressWarnings string is is not a prefix supported + // by this checker. Proceed to the next SuppressWarnings string. continue; } messageKeyInEffect = currentSuppressWarningsInEffect.substring(colonPos + 1); diff --git a/framework/tests/all-systems/PuritySubstring.java b/framework/tests/all-systems/PuritySubstring.java new file mode 100644 index 000000000000..7e5237339a90 --- /dev/null +++ b/framework/tests/all-systems/PuritySubstring.java @@ -0,0 +1,13 @@ +import org.checkerframework.dataflow.qual.SideEffectFree; + +public class PuritySubstring { + + @SuppressWarnings({"allcheckers:purity", "lock"}) // local StringBuilder + @SideEffectFree + @Override + public String toString() { + StringBuilder sb = new StringBuilder(); + sb.append("hello"); + return sb.toString(); + } +} From 27b40dfc033668f10d13cdc05e472dad7e72eaca Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Wed, 27 May 2026 23:05:26 -0700 Subject: [PATCH 16/20] Adjust jtreg goal --- checker/jtreg/nullness/issue2318/RequireCheckerPrefix.1.out | 5 +---- checker/jtreg/nullness/issue2318/RequireCheckerPrefix.2.out | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.1.out b/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.1.out index 87e2ac2ce3c7..03a059588cb5 100644 --- a/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.1.out +++ b/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.1.out @@ -1,13 +1,10 @@ RequireCheckerPrefix.java:19:25: compiler.err.proc.messager: [nullness:assignment] incompatible types in assignment. found : @Initialized @Nullable Object required: @UnknownInitialization @NonNull Object -RequireCheckerPrefix.java:21:25: compiler.err.proc.messager: [nullness:assignment] incompatible types in assignment. -found : @Initialized @Nullable Object -required: @UnknownInitialization @NonNull Object RequireCheckerPrefix.java:24:25: compiler.err.proc.messager: [nullness:assignment] incompatible types in assignment. found : @Initialized @Nullable Object required: @UnknownInitialization @NonNull Object RequireCheckerPrefix.java:27:25: compiler.err.proc.messager: [nullness:assignment] incompatible types in assignment. found : @Initialized @Nullable Object required: @UnknownInitialization @NonNull Object -4 errors +3 errors diff --git a/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.2.out b/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.2.out index eab01bacf1a8..f72ddfe2a3ee 100644 --- a/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.2.out +++ b/checker/jtreg/nullness/issue2318/RequireCheckerPrefix.2.out @@ -1,7 +1,4 @@ RequireCheckerPrefix.java:19:25: compiler.err.proc.messager: [assignment] incompatible types in assignment. found : @Initialized @Nullable Object required: @UnknownInitialization @NonNull Object -RequireCheckerPrefix.java:21:25: compiler.err.proc.messager: [assignment] incompatible types in assignment. -found : @Initialized @Nullable Object -required: @UnknownInitialization @NonNull Object -2 errors +1 error From b7e2316e35f978631e86b1b838e01414c9b90c36 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Thu, 28 May 2026 08:15:08 -0700 Subject: [PATCH 17/20] Grammar --- docs/developer/new-contributor-projects.html-old | 2 +- .../org/checkerframework/framework/source/SourceChecker.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/developer/new-contributor-projects.html-old b/docs/developer/new-contributor-projects.html-old index 97e49338d671..392212247b05 100644 --- a/docs/developer/new-contributor-projects.html-old +++ b/docs/developer/new-contributor-projects.html-old @@ -671,7 +671,7 @@ The goal of this project is to build a tool to check uses of Optional and run it
  • Is it possible to build a tool that enforces good style in using Optional?
    • -
  • How much effort is is for programmers to use such a tool? +
  • How much effort is it for programmers to use such a tool?
  • Does real-world code obey the rules about use of Optional, or not?
    • diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index 0c4c62b5aa6b..0c916c813f30 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -2842,7 +2842,7 @@ private boolean shouldSuppress( String currentSuppressWarningsPrefix = currentSuppressWarningsInEffect.substring(0, colonPos); if (!prefixes.contains(currentSuppressWarningsPrefix)) { - // The prefix of this SuppressWarnings string is is not a prefix supported + // The prefix of this SuppressWarnings string is not a prefix supported // by this checker. Proceed to the next SuppressWarnings string. continue; } From 1bc7cb58a123079eb37f9a91010fbebdc02cf3da Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Thu, 28 May 2026 10:18:29 -0700 Subject: [PATCH 18/20] Add `@SuppressWarnings` --- framework/tests/all-systems/Enums.java | 1 + framework/tests/all-systems/ForEach.java | 4 ++++ framework/tests/all-systems/GenericTest11full.java | 3 +++ framework/tests/all-systems/Issue5042.java | 1 - 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/framework/tests/all-systems/Enums.java b/framework/tests/all-systems/Enums.java index 135027ce5854..7f8bd397607c 100644 --- a/framework/tests/all-systems/Enums.java +++ b/framework/tests/all-systems/Enums.java @@ -32,6 +32,7 @@ void m(Class p) { } public SSS firstNonNull(SSS first, SSS second) { + @SuppressWarnings("nullness:nulltest.redundant") SSS res = first != null ? first : checkNotNull(second); return res; } diff --git a/framework/tests/all-systems/ForEach.java b/framework/tests/all-systems/ForEach.java index 96d4e6e1063b..c95fa54d7c17 100644 --- a/framework/tests/all-systems/ForEach.java +++ b/framework/tests/all-systems/ForEach.java @@ -32,6 +32,10 @@ void m4(T p) { } } + @SuppressWarnings({ + "nonempty:argument", + "nonempty:method.invocation" + }) // generic type inference bug? public static List removeDuplicates(List l) { // There are shorter solutions that do not maintain order. HashSet hs = new HashSet<>(l.size()); diff --git a/framework/tests/all-systems/GenericTest11full.java b/framework/tests/all-systems/GenericTest11full.java index 7e6744a84628..b25edeb85743 100644 --- a/framework/tests/all-systems/GenericTest11full.java +++ b/framework/tests/all-systems/GenericTest11full.java @@ -1,4 +1,7 @@ public class GenericTest11full { + @SuppressWarnings( + "nonempty:method.invocation") // Cannot determine statically whether the getBeans(...) + // call below will return a non-empty container. public void m(BeanManager beanManager) { Bean bean = beanManager.getBeans(GenericTest11full.class).iterator().next(); CreationalContext context = beanManager.createCreationalContext(bean); diff --git a/framework/tests/all-systems/Issue5042.java b/framework/tests/all-systems/Issue5042.java index e4d3a29b6215..0bc0e99d0a86 100644 --- a/framework/tests/all-systems/Issue5042.java +++ b/framework/tests/all-systems/Issue5042.java @@ -1,7 +1,6 @@ import java.util.function.Function; import org.checkerframework.checker.nullness.qual.Nullable; -@SuppressWarnings("initializedfields") // The fields are initialized. public class Issue5042 { interface PromptViewModel { boolean isPending(); From 661030d9ed69cc386aff8babc2eff408f8ed30fd Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Thu, 28 May 2026 10:31:36 -0700 Subject: [PATCH 19/20] Remove diagnostic output --- .../org/checkerframework/framework/source/SourceChecker.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index 2f49e5de08e2..0c916c813f30 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -2498,9 +2498,6 @@ protected void warnUnneededSuppressions( // `tree` has a @SuppressWarnings annotation that didn't suppress any warnings. SuppressWarnings suppressAnno = elt.getAnnotation(SuppressWarnings.class); String[] suppressWarningsStrings = suppressAnno.value(); - System.out.printf( - "checkerPrefixes = %s%nsuppressWarningsStrings = %s%n", - checkerPrefixes, Arrays.toString(suppressWarningsStrings)); for (String suppressWarningsString : suppressWarningsStrings) { if (warnUnneededSuppressionsExceptions != null && warnUnneededSuppressionsExceptions.matcher(suppressWarningsString).find(0)) { From 17a068138cedc5cc2ec776bb3037f0e7d811d289 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Thu, 28 May 2026 11:24:32 -0700 Subject: [PATCH 20/20] Documentation --- docs/manual/introduction.tex | 4 ++- docs/manual/warnings.tex | 28 ++++++++++--------- .../framework/source/SourceChecker.java | 13 +++++++-- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/docs/manual/introduction.tex b/docs/manual/introduction.tex index 026b395ea95a..926f15d87438 100644 --- a/docs/manual/introduction.tex +++ b/docs/manual/introduction.tex @@ -707,9 +707,11 @@ \item \<-AshowSuppressWarningsStrings> With each warning, show all possible strings to suppress that warning. \item \<-AwarnUnneededSuppressions> - Issue a warning if a \<@SuppressWarnings> did not suppress a warning + Issue a \ warning for each \<@SuppressWarnings> + that did not suppress a warning issued by the checker. This only warns about \<@SuppressWarnings> strings that contain a checker name + other than \<"allcheckers"> (for syntax, Section~\ref{suppresswarnings-annotation-syntax}). The \<-ArequirePrefixInWarningSuppressions> command-line argument ensures that all \<@SuppressWarnings> strings contain a checker name. diff --git a/docs/manual/warnings.tex b/docs/manual/warnings.tex index 5fda87f0a5fd..d1d4a4208bb0 100644 --- a/docs/manual/warnings.tex +++ b/docs/manual/warnings.tex @@ -73,8 +73,9 @@ \noindent The rest of this chapter explains these mechanisms in turn. -You can use the \code{-AwarnUnneededSuppressions} command-line option to issue a -warning if a \code{@SuppressWarnings} did not suppress any warnings issued by the current checker. +You can use the \code{-AwarnUnneededSuppressions} command-line option to +issue a warning for each \code{@SuppressWarnings} that does not suppress +any warnings issued by the current checker. \sectionAndLabel{\code{@SuppressWarnings} annotation}{suppresswarnings-annotation} @@ -150,11 +151,9 @@ The checkername \<"allcheckers"> means all checkers. Using this is not recommended, except for messages common to all checkers such as -purity-related messages when using \<-AcheckPurityAnnotations>. If you use -\<"allcheckers">, you run some checker that does not issue any warnings, -and you supply the \<-AwarnUnneededSuppressions> command-line argument, then -the Checker Framework will issue an \ -warning. +purity-related messages when using \<-AcheckPurityAnnotations>. +The Checker Framework never issues an \ warning +about a \<@SuppressWarnings> whose checkername is \<"allcheckers">. The special messagekey ``\'' means to suppress all warnings. @@ -351,12 +350,15 @@ \end{Verbatim} \noindent -Please report false positive warnings, then reference them in your warning suppressions. -This permits the Checker Framework maintainers to know about the -problem, it helps them with prioritization (by knowing how often in your -codebase a particular issue arises), and it enables you to know when an -issue has been fixed (though the \<-AwarnUnneededSuppressions> command-line -option also serves the latter purpose). +When you encounter a false positive warning, please upvote the +corresponding issue on the +\href{https://github.com/typetools/checker-framework/issues}{issue +tracker}, or create a new issue. This helps the Checker Framework +maintainers to prioritize their work. +If you reference the issue URL in your warning suppression, then you can +later follow the URL to check whether an issue has been fixed (though the +\<-AwarnUnneededSuppressions> command-line option also serves this +purpose). \sectionAndLabel{\code{@AssumeAssertion} string in an \ message}{assumeassertion} diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index 0c916c813f30..77d538edda3c 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -2520,6 +2520,12 @@ protected void warnUnneededSuppressions( reportUnneededSuppression(tree, suppressWarningsString); } } + } else { + // The annotation might be of the form `@SuppressWarnings("generic.argument")`. + // However, per the manual, "This only warns about @SuppressWarnings strings that + // contain a checker name". Thus, `@SuppressWarnings("generic.argument")` is treated + // the same as `@SuppressWarnings("allcheckers:generic.argument")` in that no warning is + // ever issued about it. } } } @@ -2778,8 +2784,8 @@ public boolean shouldSuppressWarnings(@Nullable Element elt, String errKey) { * with a message key that contains "generic.argument". * * - * {@code "allcheckers"} is a prefix that suppresses a warning from any checker. {@code "all"} is - * a partial-message-key that suppresses a warning with any message key. + * {@code "allcheckers"} is a prefix/checkername that suppresses a warning from any checker. + * {@code "all"} is a partial-message-key that suppresses a warning with any message key. * *

    If the {@code -ArequirePrefixInWarningSuppressions} command-line argument was supplied, then * {@code "partial-message-key"} has no effect; {@code "prefix"} and {@code @@ -2829,7 +2835,8 @@ private boolean shouldSuppress( continue; } else if (currentSuppressWarningsInEffect.equals("all") || currentSuppressWarningsInEffect.equals("allcheckers")) { - // Prefixes aren't required and the SuppressWarnings string is "all" or "allcheckers". + // Prefixes aren't required and the whole SuppressWarnings string is + // the messagekey "all" or the checkername "allcheckers". // Suppress the warning unless its message key is "unneeded.suppression". boolean result = !messageKey.equals("unneeded.suppression"); return result;