Fix "unneeded.suppression" warning

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

dataflow/src/main/java/org/checkerframework/dataflow/analysis/AbstractAnalysis.java

@@ -497,7 +497,6 @@ protected TransferResult<V, S> callTransferFunction(
     }
     transferInput.node = node;
     setCurrentNode(node);
-    @SuppressWarnings("nullness") // CF bug: "INFERENCE FAILED"
     TransferResult<V, S> transferResult = node.accept(transferFunction, transferInput);
     setCurrentNode(null);
     if (node instanceof AssignmentNode assignment) {

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<Block> blocks = cfg.getAllBlocks();
     Set<Block> removedBlocks = new HashSet<>();

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
 <ul>
   <li>Is it possible to build a tool that enforces good style in using Optional?
   </li>
-  <li>How much effort is is for programmers to use such a tool?
+  <li>How much effort is it for programmers to use such a tool?
   </li>
   <li>Does real-world code obey the rules about use of Optional, or not?
   </li>

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 \<unneeded.suppression> 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.

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 \<unneeded.suppression>
-warning.
+purity-related messages when using \<-AcheckPurityAnnotations>.
+The Checker Framework never issues an \<unneeded.suppression> warning
+about a \<@SuppressWarnings> whose checkername is \<"allcheckers">.
 
 The special messagekey ``\<all>'' 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 \<assert> message}{assumeassertion}

framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java

@@ -449,15 +449,9 @@ 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";
 
-  /** 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";
 
@@ -2472,52 +2466,66 @@ protected void warnUnneededSuppressions() {
     this.elementsWithSuppressedWarnings.clear();
 
     Set<String> prefixes = new HashSet<>(getSuppressWarningsPrefixes());
-    Set<String> 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();
   }
 
   /**
-   * 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
-   * @param prefixes the SuppressWarnings prefixes that suppress all warnings from this checker
-   * @param allErrorKeys all error keys that can be issued by this checker
+   * @param checkerPrefixes the SuppressWarnings prefixes associated with this checker
    */
   protected void warnUnneededSuppressions(
-      Set<Element> elementsSuppress, Set<String> prefixes, Set<String> allErrorKeys) {
+      Set<Element> elementsSuppress, Set<String> checkerPrefixes) {
     for (Tree tree : getVisitor().treesWithSuppressWarnings) {
       Element elt = TreeUtils.elementFromTree(tree);
       // TODO: This test is too coarse.  The fact that this @SuppressWarnings suppressed
       // *some* warning doesn't mean that every value in it did so.
       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) {
         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 (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 (!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()
+                      && suppressWarningsString.endsWith("unneeded.suppression");
+              if (!isUnneededSuppression) {
+                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.
           }
         }
       }
@@ -2535,7 +2543,7 @@ private void reportUnneededSuppression(Tree tree, String suppressWarningsString)
     report(
         swTree,
         Diagnostic.Kind.MANDATORY_WARNING,
-        SourceChecker.UNNEEDED_SUPPRESSION_KEY,
+        "unneeded.suppression",
         "\"" + suppressWarningsString + "\"",
         getClass().getSimpleName());
   }
@@ -2605,7 +2613,6 @@ private boolean shouldSuppressWarnings(@Nullable Object src, String errKey) {
    *     otherwise
    */
   public boolean shouldSuppressWarnings(Tree tree, String errKey) {
-
     Collection<String> prefixes = getSuppressWarningsPrefixes();
     if (prefixes.isEmpty() || (prefixes.contains(SUPPRESS_ALL_PREFIX) && prefixes.size() == 1)) {
       throw new BugInCF(
@@ -2777,8 +2784,8 @@ public boolean shouldSuppressWarnings(@Nullable Element elt, String errKey) {
    *       with a message key that contains "generic.argument".
    * </ol>
    *
-   * {@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.
    *
    * <p>If the {@code -ArequirePrefixInWarningSuppressions} command-line argument was supplied, then
    * {@code "partial-message-key"} has no effect; {@code "prefix"} and {@code
@@ -2813,43 +2820,44 @@ 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_KEY);
+          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)) {
-          // Prefixes aren't required and the SuppressWarnings string is "all".
+        } else if (currentSuppressWarningsInEffect.equals("all")
+            || currentSuppressWarningsInEffect.equals("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 = !currentSuppressWarningsInEffect.equals(UNNEEDED_SUPPRESSION_KEY);
+          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 =
             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.
+          // The prefix of this SuppressWarnings string is not a prefix supported
+          // by 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;
       }
     }
@@ -2864,16 +2872,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 + ".");
   }
 
   /**

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();
+  }
+}

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<? extends Element> getRecordComponents(TypeElement element) {
     return element.getRecordComponents();
   }