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

@@ -384,8 +384,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}).
+Warnings in code with a relevant \<@AnnotatedFor> annotation can still be suppressed by
+an in-scope \<@SuppressWarnings> annotation.
+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}).
 \end{sloppypar}
 
 The \refqualclass{framework/qual}{AnnotatedFor} annotation, written on a

docs/manual/warnings.tex

@@ -694,8 +694,11 @@
 
 \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, except for warnings suppressed by another
+mechanism such as \code{@SuppressWarnings}.
 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

@@ -2773,6 +2773,12 @@ public boolean shouldSuppressWarnings(Tree tree, String errKey) {
      *     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;
@@ -2781,49 +2787,46 @@ public boolean shouldSuppressWarnings(TreePath path, String errKey) {
 
             if (decl instanceof VariableTree) {
                 Element elt = TreeUtils.elementFromDeclaration((VariableTree) decl);
-                if (shouldSuppressWarnings(elt, errKey)) {
+                if (shouldSuppressWarningsOnElement(elt, errKey)) {
                     return true;
                 }
             } else if (decl instanceof MethodTree) {
                 Element elt = TreeUtils.elementFromDeclaration((MethodTree) decl);
-                if (shouldSuppressWarnings(elt, errKey)) {
+                if (shouldSuppressWarningsOnElement(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 (shouldSuppressWarningsOnElement(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 (shouldSuppressWarningsOnElement(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;
@@ -2882,20 +2885,30 @@ public boolean shouldSuppressWarnings(Element elt, String errKey) {
         }
 
         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 (shouldSuppressWarningsOnElement(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;
+        }
+        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 an corresponding {@code @SuppressWarnings} annotation
+     */
+    private boolean shouldSuppressWarningsOnElement(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/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;
+        }
+    }
 }