Fix #5472 (#5476)

The underlying problem is that `@NotOwning` and `@Owning` are owned by the Must Call Checker, not the Resource Leak Checker. So, only the Must Call Checker has access to such annotations if (and only if) they were written in stub files. I fixed both the problem described in #5472 and all other uses of `getDeclAnnotation(elt)` involving `Owning` or `NotOwning` in the consistency analyzer.

checker/build.gradle

@@ -62,6 +62,8 @@ dependencies {
     testImplementation 'com.amazonaws:aws-java-sdk-kms'
     // The AWS SDK is used for testing the Called Methods Checker.
     testImplementation platform('com.amazonaws:aws-java-sdk-bom:1.12.293')
+    // For the Resource Leak Checker's support for JavaEE.
+    testImplementation 'javax.servlet:javax.servlet-api:3.1.0'
 
     testImplementation group: 'junit', name: 'junit', version: '4.13.2'
     testImplementation project(':framework-test')

checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java

@@ -640,14 +640,14 @@ private boolean isValidCreatesMustCallForExpression(
     if (expression instanceof FieldAccess) {
       Element elt = ((FieldAccess) expression).getField();
       if (!checker.hasOption(MustCallChecker.NO_LIGHTWEIGHT_OWNERSHIP)
-          && typeFactory.getDeclAnnotation(elt, Owning.class) != null) {
+          && typeFactory.hasOwning(elt)) {
         // The expression is an Owning field.  This satisfies case 1.
         return true;
       }
     } else if (expression instanceof LocalVariable) {
       Element elt = ((LocalVariable) expression).getElement();
       if (!checker.hasOption(MustCallChecker.NO_LIGHTWEIGHT_OWNERSHIP)
-          && typeFactory.getDeclAnnotation(elt, Owning.class) != null) {
+          && typeFactory.hasOwning(elt)) {
         // The expression is an Owning formal parameter. Note that this cannot actually
         // be a local variable (despite expressions's type being LocalVariable) because
         // the @Owning annotation can only be written on methods, parameters, and fields;
@@ -956,19 +956,14 @@ private void removeObligationsAtOwnershipTransferToParameters(
 
           // check if parameter has an @Owning annotation
           VariableElement parameter = parameters.get(i);
-          Set<AnnotationMirror> annotationMirrors = typeFactory.getDeclAnnotations(parameter);
-          for (AnnotationMirror anno : annotationMirrors) {
-            if (AnnotationUtils.areSameByName(
-                anno, "org.checkerframework.checker.mustcall.qual.Owning")) {
-              Obligation localObligation = getObligationForVar(obligations, local);
-              // Passing to an owning parameter is not sufficient to resolve the
-              // obligation created from a MustCallAlias parameter, because the containing
-              // method must actually return the value.
-              if (!localObligation.derivedFromMustCallAlias()) {
-                // Transfer ownership!
-                obligations.remove(localObligation);
-                break;
-              }
+          if (typeFactory.hasOwning(parameter)) {
+            Obligation localObligation = getObligationForVar(obligations, local);
+            // Passing to an owning parameter is not sufficient to resolve the
+            // obligation created from a MustCallAlias parameter, because the containing
+            // method must actually return the value.
+            if (!localObligation.derivedFromMustCallAlias()) {
+              // Transfer ownership!
+              obligations.remove(localObligation);
             }
           }
         }
@@ -1032,7 +1027,7 @@ private boolean isTransferOwnershipAtReturn(ControlFlowGraph cfg) {
       //  not be transferred.
       MethodTree method = ((UnderlyingAST.CFGMethod) underlyingAST).getMethod();
       ExecutableElement executableElement = TreeUtils.elementFromDeclaration(method);
-      return typeFactory.getDeclAnnotation(executableElement, NotOwning.class) == null;
+      return !typeFactory.hasNotOwning(executableElement);
     }
     return false;
   }
@@ -1061,7 +1056,7 @@ private void updateObligationsForAssignment(
     if (lhsElement.getKind() == ElementKind.FIELD) {
       boolean isOwningField =
           !checker.hasOption(MustCallChecker.NO_LIGHTWEIGHT_OWNERSHIP)
-              && typeFactory.getDeclAnnotation(lhsElement, Owning.class) != null;
+              && typeFactory.hasOwning(lhsElement);
       // Check that the must-call obligations of the lhs have been satisfied, if the field is
       // non-final and owning.
       if (isOwningField
@@ -1116,7 +1111,7 @@ private boolean hasAtMostOneOwningField(TypeElement element) {
     // Has an owning field already been encountered?
     boolean hasOwningField = false;
     for (VariableElement field : fields) {
-      if (typeFactory.getDeclAnnotation(field, Owning.class) != null) {
+      if (typeFactory.hasOwning(field)) {
         if (hasOwningField) {
           return false;
         } else {
@@ -1635,7 +1630,7 @@ private boolean shouldTrackReturnType(MethodInvocationNode node) {
       return false;
     }
     // check for absence of @NotOwning annotation
-    return (typeFactory.getDeclAnnotation(executableElement, NotOwning.class) == null);
+    return !typeFactory.hasNotOwning(executableElement);
   }
 
   /**

checker/src/main/java/org/checkerframework/checker/resourceleak/ResourceLeakAnnotatedTypeFactory.java

@@ -23,6 +23,8 @@
 import org.checkerframework.checker.mustcall.qual.CreatesMustCallFor;
 import org.checkerframework.checker.mustcall.qual.MustCall;
 import org.checkerframework.checker.mustcall.qual.MustCallAlias;
+import org.checkerframework.checker.mustcall.qual.NotOwning;
+import org.checkerframework.checker.mustcall.qual.Owning;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.checkerframework.common.basetype.BaseTypeChecker;
 import org.checkerframework.dataflow.cfg.ControlFlowGraph;
@@ -355,4 +357,34 @@ public ExecutableElement getCreatesMustCallForValueElement() {
   public ExecutableElement getCreatesMustCallForListValueElement() {
     return createsMustCallForListValueElement;
   }
+
+  /**
+   * Does the given element have an {@code @NotOwning} annotation (including in stub files)?
+   *
+   * <p>Prefer this method to calling {@link #getDeclAnnotation(Element, Class)} on the type factory
+   * directly, which won't find this annotation in stub files (it only considers stub files loaded
+   * by this checker, not subcheckers).
+   *
+   * @param elt an element
+   * @return whether there is a NotOwning annotation on the given element
+   */
+  public boolean hasNotOwning(Element elt) {
+    MustCallAnnotatedTypeFactory mcatf = getTypeFactoryOfSubchecker(MustCallChecker.class);
+    return mcatf.getDeclAnnotation(elt, NotOwning.class) != null;
+  }
+
+  /**
+   * Does the given element have an {@code @Owning} annotation (including in stub files)?
+   *
+   * <p>Prefer this method to calling {@link #getDeclAnnotation(Element, Class)} on the type factory
+   * directly, which won't find this annotation in stub files (it only considers stub files loaded
+   * by this checker, not subcheckers).
+   *
+   * @param elt an element
+   * @return whether there is an Owning annotation on the given element
+   */
+  public boolean hasOwning(Element elt) {
+    MustCallAnnotatedTypeFactory mcatf = getTypeFactoryOfSubchecker(MustCallChecker.class);
+    return mcatf.getDeclAnnotation(elt, Owning.class) != null;
+  }
 }

checker/tests/resourceleak/JavaEETest.java

@@ -0,0 +1,7 @@
+// Test for https://github.com/typetools/checker-framework/issues/5472
+
+class JavaEETest {
+  static void foo(javax.servlet.ServletResponse s) throws java.io.IOException {
+    s.getWriter();
+  }
+}