Allow for multiple method parameters to be @MustCallAlias in Resource Leak Checker; fixes #4785

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

@@ -238,19 +238,21 @@ private void handleInvocation(Set<Obligation> obligations, Node node) {
 
   /**
    * If node is an invocation of a this or super constructor that has a MustCallAlias return type
-   * and a MustCallAlias parameter, check if any variable in the current set of obligations is being
-   * passed to the other constructor. If so, remove it from the obligations.
+   * and some MustCallAlias parameter(s), check if any variable in the current set of obligations is
+   * being passed to the other constructor. If so, remove it from the obligations.
    *
    * @param obligations current obligations
    * @param node a super or this constructor invocation
    */
   private void handleThisOrSuperConstructorMustCallAlias(Set<Obligation> obligations, Node node) {
-    Node mustCallAliasArgument = getMustCallAliasArgumentNode(node);
+    List<Node> mustCallAliasArguments = getMustCallAliasArgumentNodes(node);
     // If the MustCallAlias argument is also in the set of obligations, then remove it -- its
     // obligation has been fulfilled by being passed on to the MustCallAlias constructor (because a
     // this/super constructor call can only occur in the body of another constructor).
-    if (mustCallAliasArgument instanceof LocalVariableNode) {
-      removeObligationsContainingVar(obligations, (LocalVariableNode) mustCallAliasArgument);
+    for (Node mustCallAliasArgument : mustCallAliasArguments) {
+      if (mustCallAliasArgument instanceof LocalVariableNode) {
+        removeObligationsContainingVar(obligations, (LocalVariableNode) mustCallAliasArgument);
+      }
     }
   }
 
@@ -406,8 +408,8 @@ private boolean representSame(JavaExpression target, @Nullable JavaExpression en
   /**
    * Given a node representing a method or constructor call, checks that if the result of the call
    * has a non-empty {@code @MustCall} type, then the result is pseudo-assigned to some location
-   * that can take ownership of the result. Searches for the set of same resources in {@code
-   * obligations} and adds the new resource alias to it if one exists. Otherwise creates a new set.
+   * that can take ownership of the result. Searches for the set(s) of same resources in {@code
+   * obligations} and adds the new resource alias to it if any exists. Otherwise creates a new set.
    *
    * @param obligations the currently-tracked obligations. This is always side-effected: an
    *     obligation is either modified (by removing it from the obligation set and adding a new one)
@@ -426,39 +428,39 @@ private void trackInvocationResult(Set<Obligation> obligations, Node node) {
       return;
     }
 
-    // `mustCallAlias` is the argument passed in the MustCallAlias position, if any exists,
-    // otherwise null.
-    Node mustCallAlias = getMustCallAliasArgumentNode(node);
-    // If mustCallAlias is null and call returns @This, set mustCallAlias to the receiver.
-    if (mustCallAlias == null
-        && node instanceof MethodInvocationNode
+    // mustCallAliases is a (possibly-empty) list of arguments passed in a MustCallAlias position
+    List<Node> mustCallAliases = getMustCallAliasArgumentNodes(node);
+    // If call returns @This, add the receiver to mustCallAliases.
+    if (node instanceof MethodInvocationNode
         && typeFactory.returnsThis((MethodInvocationTree) tree)) {
-      mustCallAlias =
-          removeCastsAndGetTmpVarIfPresent(((MethodInvocationNode) node).getTarget().getReceiver());
+      mustCallAliases.add(
+          removeCastsAndGetTmpVarIfPresent(
+              ((MethodInvocationNode) node).getTarget().getReceiver()));
     }
 
     ResourceAlias tmpVarAsResourceAlias = new ResourceAlias(new LocalVariable(tmpVar), tree);
-    if (mustCallAlias instanceof FieldAccessNode) {
-      // Do not track the call result if the MustCallAlias argument is a field.  Handling of
-      // @Owning fields is a completely separate check, and there is never a need to track an alias
-      // of
-      // a non-@Owning field, as by definition such a field does not have obligations!
-    } else if (mustCallAlias instanceof LocalVariableNode) {
-      Obligation obligationContainingMustCallAlias =
-          getObligationForVar(obligations, (LocalVariableNode) mustCallAlias);
-      // If mustCallAlias is a local variable already being tracked, add tmpVarAsResourceAlias
-      // to the set containing mustCallAlias.
-      if (obligationContainingMustCallAlias != null) {
-        Set<ResourceAlias> newResourceAliasSet =
-            FluentIterable.from(obligationContainingMustCallAlias.resourceAliases)
-                .append(tmpVarAsResourceAlias)
-                .toSet();
-        obligations.remove(obligationContainingMustCallAlias);
-        obligations.add(new Obligation(newResourceAliasSet));
+    for (Node mustCallAlias : mustCallAliases) {
+      if (mustCallAlias instanceof FieldAccessNode) {
+        // Do not track the call result if the MustCallAlias argument is a field.  Handling of
+        // @Owning fields is a completely separate check, and there is never a need to track an
+        // alias of a non-@Owning field, as by definition such a field does not have obligations!
+      } else if (mustCallAlias instanceof LocalVariableNode) {
+        Obligation obligationContainingMustCallAlias =
+            getObligationForVar(obligations, (LocalVariableNode) mustCallAlias);
+        // If mustCallAlias is a local variable already being tracked, add tmpVarAsResourceAlias
+        // to the set containing mustCallAlias.
+        if (obligationContainingMustCallAlias != null) {
+          Set<ResourceAlias> newResourceAliasSet =
+              FluentIterable.from(obligationContainingMustCallAlias.resourceAliases)
+                  .append(tmpVarAsResourceAlias)
+                  .toSet();
+          obligations.remove(obligationContainingMustCallAlias);
+          obligations.add(new Obligation(newResourceAliasSet));
+        }
       }
-    } else {
-      // If mustCallAlias is neither a field nor a local already in the set of obligations,
-      // add it to a new set.
+    }
+    if (mustCallAliases.isEmpty()) {
+      // If mustCallAliases is an empty List, add tmpVarAsResourceAlias to a new set.
       obligations.add(new Obligation(ImmutableSet.of(tmpVarAsResourceAlias)));
     }
   }
@@ -513,9 +515,13 @@ private boolean shouldTrackInvocationResult(Set<Obligation> obligations, Node no
    *     field or a definitely non-owning pointer
    */
   private boolean returnTypeIsMustCallAliasWithUntrackable(MethodInvocationNode node) {
-    Node mustCallAliasArgument = getMustCallAliasArgumentNode(node);
-    return mustCallAliasArgument instanceof FieldAccessNode
-        || mustCallAliasArgument instanceof ThisNode;
+    List<Node> mustCallAliasArguments = getMustCallAliasArgumentNodes(node);
+    return !mustCallAliasArguments.isEmpty()
+        && mustCallAliasArguments.stream()
+            .allMatch(
+                mustCallAliasArgument ->
+                    mustCallAliasArgument instanceof FieldAccessNode
+                        || mustCallAliasArgument instanceof ThisNode);
   }
 
   /**
@@ -986,38 +992,38 @@ private String receiverAsString(FieldAccessNode fieldAccessNode) {
   }
 
   /**
-   * Finds the argument passed in the {@code @MustCallAlias} position for a call.
+   * Finds the arguments passed in the {@code @MustCallAlias} positions for a call.
    *
    * @param callNode callNode representing the call; must be {@link MethodInvocationNode} or {@link
    *     ObjectCreationNode}
    * @return if {@code callNode} invokes a method with a {@code @MustCallAlias} annotation on some
-   *     formal parameter (or the receiver), returns the result of calling {@link
-   *     #removeCastsAndGetTmpVarIfPresent(Node)} on the argument passed in that position.
-   *     Otherwise, returns {@code null}.
+   *     formal parameter(s) (or the receiver), returns the result of calling {@link
+   *     #removeCastsAndGetTmpVarIfPresent(Node)} on the argument(s) passed in corresponding
+   *     position(s). Otherwise, returns an empty List.
    */
-  private @Nullable Node getMustCallAliasArgumentNode(Node callNode) {
+  private List<Node> getMustCallAliasArgumentNodes(Node callNode) {
     Preconditions.checkArgument(
         callNode instanceof MethodInvocationNode || callNode instanceof ObjectCreationNode);
+    List<Node> result = new ArrayList<>();
     if (!typeFactory.hasMustCallAlias(callNode.getTree())) {
-      return null;
+      return result;
     }
 
-    Node result = null;
     List<Node> args = getArgumentsOfInvocation(callNode);
     List<? extends VariableElement> parameters = getParametersOfInvocation(callNode);
     for (int i = 0; i < args.size(); i++) {
       if (typeFactory.hasMustCallAlias(parameters.get(i))) {
-        result = args.get(i);
-        break;
+        result.add(removeCastsAndGetTmpVarIfPresent(args.get(i)));
       }
     }
 
     // If none of the parameters were @MustCallAlias, it must be the receiver
-    if (result == null && callNode instanceof MethodInvocationNode) {
-      result = ((MethodInvocationNode) callNode).getTarget().getReceiver();
+    if (result.isEmpty() && callNode instanceof MethodInvocationNode) {
+      result.add(
+          removeCastsAndGetTmpVarIfPresent(
+              ((MethodInvocationNode) callNode).getTarget().getReceiver()));
     }
 
-    result = removeCastsAndGetTmpVarIfPresent(result);
     return result;
   }
 

checker/tests/resourceleak/MultipleMethodParamsMustCallAliasTest.java

@@ -0,0 +1,62 @@
+import java.io.*;
+import java.net.*;
+import org.checkerframework.checker.calledmethods.qual.*;
+import org.checkerframework.checker.mustcall.qual.*;
+import org.checkerframework.common.returnsreceiver.qual.*;
+
+class MultipleMethodParamsMustCallAliasTest {
+
+  void testMultiMethodParamsCorrect(@Owning InputStream in1, @Owning InputStream in2)
+      throws IOException {
+
+    ReplicaInputStreams r = new ReplicaInputStreams(in1, in2);
+
+    r.close();
+  }
+
+  // :: error: required.method.not.called
+  void testMultiMethodParamsWrong1(@Owning InputStream in1, @Owning InputStream in2)
+      throws IOException {
+
+    ReplicaInputStreams r = new ReplicaInputStreams(in1, in2);
+
+    in1.close();
+  }
+
+  // :: error: required.method.not.called
+  void testMultiMethodParamsWrong2(@Owning InputStream in1, @Owning InputStream in2)
+      throws IOException {
+
+    ReplicaInputStreams r = new ReplicaInputStreams(in1, in2);
+
+    in2.close();
+  }
+
+  // :: error: required.method.not.called
+  void testMultiMethodParamsWrong3(@Owning InputStream in1) throws IOException {
+    // :: error: required.method.not.called
+    Socket socket = new Socket("address", 12);
+    ReplicaInputStreams r = new ReplicaInputStreams(in1, socket.getInputStream());
+  }
+
+  class ReplicaInputStreams implements Closeable {
+
+    private final @Owning InputStream in1;
+    private final @Owning InputStream in2;
+
+    public @MustCallAlias ReplicaInputStreams(
+        @MustCallAlias InputStream i1, @MustCallAlias InputStream i2) {
+      this.in1 = i1;
+      this.in2 = i2;
+    }
+
+    @Override
+    @EnsuresCalledMethods(
+        value = {"this.in1", "this.in2"},
+        methods = {"close"})
+    public void close() throws IOException {
+      in1.close();
+      in2.close();
+    }
+  }
+}