Add test for @SuppressWarnings and @AnnotatedFor interaction and refine SourceChecker logic

checker/tests/nulless-conservative-defaults/annotatedfornullness/AnnotatedForNullness.java

@@ -73,4 +73,14 @@ void bar() {
         annotatedForNullness(initializedField);
         annotatedForNullnessAndInitialization(initializedField);
     }
+
+    // @SuppressWarnings("nullness") should suppress all nullness diagnostics within the annotated
+    // declaration's scope, including those in nested @AnnotatedFor("nullness") scopes.
+    @SuppressWarnings("nullness")
+    class SuppressWarningsClassWithAnnotatedForMethod {
+        @AnnotatedFor("nullness")
+        @NonNull Object m() {
+            return null;
+        }
+    }
 }

docs/manual/annotating-libraries.tex

@@ -409,8 +409,12 @@
 conservative defaults
 (see Section~\ref{defaults-classfile}) for any type use with no explicit
 user-written annotation, \emph{and} the checker issues no warnings.
-The command-line argument \code{-AonlyAnnotatedFor} can be used to  suppress errors and warnings outside of the scope of an \<@AnnotatedFor> annotation,
-but does not change the default qualifiers for source code (See Section~\ref{aonlyannotatedfor}).
+The command-line argument \code{-AonlyAnnotatedFor} can be used to suppress
+errors and warnings outside of the scope of an \<@AnnotatedFor> annotation,
+but does not change the default qualifiers for source code (see
+Section~\ref{aonlyannotatedfor}).
+In code with a relevant \<@AnnotatedFor> annotation, warnings can be suppressed
+by an explicit \<@SuppressWarnings> annotation.
 \end{sloppypar}
 
 The \refqualclass{framework/qual}{AnnotatedFor} annotation, written on a

docs/manual/warnings.tex

@@ -718,8 +718,13 @@
 
 \sectionAndLabel{\code{-AonlyAnnotatedFor} command-line option}{aonlyannotatedfor}
 
-You can suppress all errors and warnings for code outside of a corresponding \code{@AnnotatedFor} by applying this command-line option.
-Note that the \code{@AnnotatedFor} annotation must include the checker's name to enable warnings from that checker.
+You can suppress all errors and warnings for code outside of a corresponding
+\code{@AnnotatedFor} by applying this command-line option.
+Note that the \code{@AnnotatedFor} annotation must include the checker's name to
+enable warnings from that checker.
+An explicit \code{@SuppressWarnings} annotation can suppress warnings in
+\code{@AnnotatedFor} code and code outside the scope of a corresponding
+\code{@AnnotatedFor} annotation.
 For example, use \code{@AnnotatedFor("nullness")} for the Nullness Checker.
 This flag only suppresses warnings, compared to \code{-AuseConservativeDefaultsForUncheckedCode=source},
 which also applies conservative defaults for code outside the scope of an \code{@AnnotatedFor} annotation.

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

@@ -1193,6 +1193,15 @@ public void initChecker() {
             messageStore = new TreeSet<>();
         }
 
+        Collection<String> prefixes = getSuppressWarningsPrefixes();
+        if (prefixes.isEmpty()
+                || (prefixes.size() == 1 && prefixes.contains(SUPPRESS_ALL_PREFIX))) {
+            throw new BugInCF(
+                    "Checker must provide a SuppressWarnings prefix."
+                            + " SourceChecker#getSuppressWarningsPrefixes was not overridden"
+                            + " correctly.");
+        }
+
         // Validate the lint flags, if they haven't been used already.
         if (this.activeLints == null) {
             this.activeLints = createActiveLints(getOptions());
@@ -2785,21 +2794,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(
-                    "Checker must provide a SuppressWarnings prefix."
-                            + " SourceChecker#getSuppressWarningsPrefixes was not overridden"
-                            + " correctly.");
-        }
-
-        if (shouldSuppress(getSuppressWarningsStringsFromOption(), errKey)) {
-            // If the error key matches a warning string in the -AsuppressWarnings, then suppress
-            // the warning.
-            return true;
-        }
-
         assert this.currentRoot != null : "this.currentRoot == null";
         TreePath path = pathToTree(tree);
 
@@ -2830,13 +2824,26 @@ public boolean shouldSuppressWarnings(Tree tree, String errKey) {
      * Returns true if the path is within the scope of a @SuppressWarnings annotation, one of whose
      * values suppresses the checker's warning.
      *
+     * <p>This overload also accounts for source-position-based suppression from unchecked code: if
+     * no matching {@code @SuppressWarnings} is found, then warnings outside a relevant {@link
+     * AnnotatedFor} scope are suppressed when {@code
+     * -AuseConservativeDefaultsForUncheckedCode=source} or {@code -AonlyAnnotatedFor} is in effect.
+     *
      * @param path the TreePath that might be a source of, or related to, a warning
      * @param errKey the error key the checker is emitting
-     * @return true if no warning should be emitted for the given path because it is contained by a
-     *     declaration with an appropriately-valued {@code @SuppressWarnings} annotation; false
+     * @return true if no warning should be emitted for the given path, either because it is
+     *     contained by a declaration with an appropriately-valued {@code @SuppressWarnings}
+     *     annotation, because it is suppressed by command-line arguments, or because it is outside
+     *     an {@link AnnotatedFor} scope when source-based conservative defaults are enabled; false
      *     otherwise
      */
     public boolean shouldSuppressWarnings(TreePath path, String errKey) {
+        if (shouldSuppress(getSuppressWarningsStringsFromOption(), errKey)) {
+            return true;
+        }
+
+        boolean foundAnnotatedFor = false;
+
         // iterate through the path; continue until path contains no declarations
         for (TreePath declPath = TreePathUtil.enclosingDeclarationPath(path);
                 declPath != null;
@@ -2845,49 +2852,46 @@ public boolean shouldSuppressWarnings(TreePath path, String errKey) {
 
             if (decl instanceof VariableTree) {
                 Element elt = TreeUtils.elementFromDeclaration((VariableTree) decl);
-                if (shouldSuppressWarnings(elt, errKey)) {
+                if (hasSuppressWarningsAnnotationForErrorKey(elt, errKey)) {
                     return true;
                 }
             } else if (decl instanceof MethodTree) {
                 Element elt = TreeUtils.elementFromDeclaration((MethodTree) decl);
-                if (shouldSuppressWarnings(elt, errKey)) {
+                if (hasSuppressWarningsAnnotationForErrorKey(elt, errKey)) {
                     return true;
                 }
 
-                if (isAnnotatedForThisCheckerOrUpstreamChecker(elt)) {
-                    // Return false immediately. Do NOT check for AnnotatedFor in the enclosing
-                    // elements as the closest AnnotatedFor is already found.
-                    return false;
+                if (!foundAnnotatedFor && isAnnotatedForThisCheckerOrUpstreamChecker(elt)) {
+                    foundAnnotatedFor = true;
                 }
             } else if (TreeUtils.classTreeKinds().contains(decl.getKind())) {
                 // A class tree
                 Element elt = TreeUtils.elementFromDeclaration((ClassTree) decl);
-                if (shouldSuppressWarnings(elt, errKey)) {
+                if (hasSuppressWarningsAnnotationForErrorKey(elt, errKey)) {
                     return true;
                 }
 
-                if (isAnnotatedForThisCheckerOrUpstreamChecker(elt)) {
-                    // Return false immediately. Do NOT check for AnnotatedFor in the enclosing
-                    // elements as the closest AnnotatedFor is already found.
-                    return false;
+                if (!foundAnnotatedFor && isAnnotatedForThisCheckerOrUpstreamChecker(elt)) {
+                    foundAnnotatedFor = true;
                 }
                 Element packageElement = elt.getEnclosingElement();
                 if (packageElement != null && packageElement.getKind() == ElementKind.PACKAGE) {
-                    if (shouldSuppressWarnings(packageElement, errKey)) {
+                    if (hasSuppressWarningsAnnotationForErrorKey(packageElement, errKey)) {
                         return true;
                     }
-                    if (isAnnotatedForThisCheckerOrUpstreamChecker(packageElement)) {
-                        // Return false immediately. Do NOT check for AnnotatedFor in the enclosing
-                        // elements as the closest AnnotatedFor is already found.
-                        return false;
+                    if (!foundAnnotatedFor
+                            && isAnnotatedForThisCheckerOrUpstreamChecker(packageElement)) {
+                        foundAnnotatedFor = true;
                     }
                 }
             } else {
                 throw new BugInCF("Unexpected declaration kind: " + decl.getKind() + " " + decl);
             }
         }
 
-        if (useConservativeDefaultsSource || onlyAnnotatedFor) {
+        if (foundAnnotatedFor) {
+            return false;
+        } else if (useConservativeDefaultsSource || onlyAnnotatedFor) {
             // If we got this far without hitting an @AnnotatedFor and returning
             // false, we DO suppress the warning.
             return true;
@@ -2934,32 +2938,62 @@ public boolean useConservativeDefault(String kindOfCode) {
      * true if the element is within the scope of a @SuppressWarnings annotation, one of whose
      * values suppresses all the checker's warnings.
      *
+     * <p>This overload also accounts for source-position-based suppression from unchecked code: if
+     * no matching {@code @SuppressWarnings} is found, then warnings outside a relevant {@link
+     * AnnotatedFor} scope are suppressed when {@code
+     * -AuseConservativeDefaultsForUncheckedCode=source} or {@code -AonlyAnnotatedFor} is in effect.
+     *
      * @param elt the Element that might be a source of, or related to, a warning
      * @param errKey the error key the checker is emitting
-     * @return true if no warning should be emitted for the given Element because it is contained by
-     *     a declaration with an appropriately-valued {@code @SuppressWarnings} annotation; false
+     * @return true if no warning should be emitted for the given Element, either because it is
+     *     contained by a declaration with an appropriately-valued {@code @SuppressWarnings}
+     *     annotation, because it is suppressed by command-line arguments, or because it is outside
+     *     an {@link AnnotatedFor} scope when source-based conservative defaults are enabled; false
      *     otherwise
      */
     public boolean shouldSuppressWarnings(Element elt, String errKey) {
         if (shouldSuppress(getSuppressWarningsStringsFromOption(), errKey)) {
             return true;
         }
 
+        boolean foundAnnotatedFor = false;
         for (Element currElt = elt; currElt != null; currElt = currElt.getEnclosingElement()) {
-            SuppressWarnings suppressWarningsAnno = currElt.getAnnotation(SuppressWarnings.class);
-            if (suppressWarningsAnno != null) {
-                String[] suppressWarningsStrings = suppressWarningsAnno.value();
-                if (shouldSuppress(suppressWarningsStrings, errKey)) {
-                    if (warnUnneededSuppressions) {
-                        elementsWithSuppressedWarnings.add(currElt);
-                    }
-                    return true;
-                }
+            if (hasSuppressWarningsAnnotationForErrorKey(currElt, errKey)) {
+                return true;
             }
-            if (isAnnotatedForThisCheckerOrUpstreamChecker(elt)) {
-                // Return false immediately. Do NOT check for AnnotatedFor in the
-                // enclosing elements, because they may not have an @AnnotatedFor.
-                return false;
+            if (!foundAnnotatedFor && isAnnotatedForThisCheckerOrUpstreamChecker(currElt)) {
+                foundAnnotatedFor = true;
+            }
+        }
+
+        if (foundAnnotatedFor) {
+            return false;
+        } else if (useConservativeDefaultsSource || onlyAnnotatedFor) {
+            // If we got this far without hitting an @AnnotatedFor and returning
+            // false, we DO suppress the warning.
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * Returns true if the given element has a {@code @SuppressWarnings} annotation that suppresses
+     * the given error key.
+     *
+     * @param elt the element whose annotations to check
+     * @param errKey the error key the checker is emitting
+     * @return true if {@code elt} has a corresponding {@code @SuppressWarnings} annotation
+     */
+    private boolean hasSuppressWarningsAnnotationForErrorKey(Element elt, String errKey) {
+        SuppressWarnings suppressWarningsAnno = elt.getAnnotation(SuppressWarnings.class);
+        if (suppressWarningsAnno != null) {
+            String[] suppressWarningsStrings = suppressWarningsAnno.value();
+            if (shouldSuppress(suppressWarningsStrings, errKey)) {
+                if (warnUnneededSuppressions) {
+                    elementsWithSuppressedWarnings.add(elt);
+                }
+                return true;
             }
         }
         return false;

framework/src/test/java/org/checkerframework/framework/test/junit/ElementSuppressionTest.java

@@ -0,0 +1,31 @@
+package org.checkerframework.framework.test.junit;
+
+import org.checkerframework.framework.test.CheckerFrameworkPerDirectoryTest;
+import org.junit.runners.Parameterized.Parameters;
+
+import java.io.File;
+import java.util.List;
+
+/** Tests suppression of diagnostics reported on elements. */
+public class ElementSuppressionTest extends CheckerFrameworkPerDirectoryTest {
+
+    /**
+     * @param testFiles the files containing test code, which will be type-checked
+     */
+    public ElementSuppressionTest(List<File> testFiles) {
+        super(
+                testFiles,
+                org.checkerframework.framework.testchecker.elementsuppression
+                        .ElementSuppressionChecker.class,
+                "elementsuppression",
+                "-AuseConservativeDefaultsForUncheckedCode=source,bytecode");
+    }
+
+    /**
+     * @return the directories containing test files
+     */
+    @Parameters
+    public static String[] getTestDirs() {
+        return new String[] {"conservative-defaults/elementsuppression"};
+    }
+}

framework/src/test/java/org/checkerframework/framework/testchecker/elementsuppression/ElementSuppressionAnnotatedTypeFactory.java

@@ -0,0 +1,33 @@
+package org.checkerframework.framework.testchecker.elementsuppression;
+
+import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory;
+import org.checkerframework.common.basetype.BaseTypeChecker;
+import org.checkerframework.framework.testchecker.util.SubQual;
+import org.checkerframework.framework.testchecker.util.SuperQual;
+
+import java.lang.annotation.Annotation;
+import java.util.LinkedHashSet;
+import java.util.Set;
+
+/** The annotated type factory for {@link ElementSuppressionChecker}. */
+public class ElementSuppressionAnnotatedTypeFactory extends BaseAnnotatedTypeFactory {
+
+    /**
+     * Creates a new ElementSuppressionAnnotatedTypeFactory.
+     *
+     * @param checker the checker
+     */
+    @SuppressWarnings("this-escape")
+    public ElementSuppressionAnnotatedTypeFactory(BaseTypeChecker checker) {
+        super(checker);
+        this.postInit();
+    }
+
+    @Override
+    protected Set<Class<? extends Annotation>> createSupportedTypeQualifiers() {
+        Set<Class<? extends Annotation>> qualSet = new LinkedHashSet<>();
+        qualSet.add(SubQual.class);
+        qualSet.add(SuperQual.class);
+        return qualSet;
+    }
+}

framework/src/test/java/org/checkerframework/framework/testchecker/elementsuppression/ElementSuppressionChecker.java

@@ -0,0 +1,6 @@
+package org.checkerframework.framework.testchecker.elementsuppression;
+
+import org.checkerframework.common.basetype.BaseTypeChecker;
+
+/** A test checker for suppression of diagnostics reported on elements. */
+public class ElementSuppressionChecker extends BaseTypeChecker {}

framework/src/test/java/org/checkerframework/framework/testchecker/elementsuppression/ElementSuppressionVisitor.java

@@ -0,0 +1,34 @@
+package org.checkerframework.framework.testchecker.elementsuppression;
+
+import com.sun.source.tree.ClassTree;
+
+import org.checkerframework.common.basetype.BaseTypeChecker;
+import org.checkerframework.common.basetype.BaseTypeVisitor;
+import org.checkerframework.javacutil.TreeUtils;
+
+import javax.lang.model.element.Element;
+
+/** Reports a diagnostic on selected class elements to test element-based suppression. */
+public class ElementSuppressionVisitor
+        extends BaseTypeVisitor<org.checkerframework.common.basetype.BaseAnnotatedTypeFactory> {
+
+    /**
+     * Creates a new ElementSuppressionVisitor.
+     *
+     * @param checker the checker
+     */
+    public ElementSuppressionVisitor(BaseTypeChecker checker) {
+        super(checker);
+    }
+
+    @Override
+    public void processClassTree(ClassTree node) {
+        Element elt = TreeUtils.elementFromDeclaration(node);
+        if (elt != null && elt.getSimpleName().toString().startsWith("ReportOnMe")) {
+            // report a custom error on the Element itself!
+            // "type.invalid" is a built-in error key we can misuse for testing.
+            checker.reportError(elt, "type.invalid", "mock type", "mock message");
+        }
+        super.processClassTree(node);
+    }
+}

framework/tests/conservative-defaults/annotatedfor/AnnotatedForTest.java

@@ -247,4 +247,14 @@ static class staticAnnotatedAndWarningsSuppressedClass {
             so4 = staticUnannotatedMethod(so1);
         }
     }
+
+    // @SuppressWarnings("subtyping") should suppress all subtyping diagnostics within the annotated
+    // declaration's scope, including those in nested @AnnotatedFor("subtyping") scopes.
+    @SuppressWarnings("subtyping")
+    class SuppressWarningsClassWithAnnotatedForMethod {
+        @AnnotatedFor("subtyping")
+        @SubQual Object m(@SuperQual Object p) {
+            return p;
+        }
+    }
 }