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/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/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/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 96feb1d6a233..77d538edda3c 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/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,31 +2466,28 @@ 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(); } /** - * 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 elementsSuppress, Set prefixes, 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 @@ -2504,7 +2495,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 +2503,29 @@ 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 (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 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". * * - * {@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 @@ -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 + "."); } /** 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(); + } +} 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(); }