@CreatesMustCallFor create an obligation on exceptional successors

checker/src/main/java/org/checkerframework/checker/calledmethods/CalledMethodsTransfer.java

@@ -27,6 +27,7 @@
 import org.checkerframework.dataflow.cfg.node.Node;
 import org.checkerframework.dataflow.expression.JavaExpression;
 import org.checkerframework.framework.flow.CFAbstractStore;
+import org.checkerframework.framework.flow.CFAbstractValue;
 import org.checkerframework.framework.type.AnnotatedTypeMirror;
 import org.checkerframework.framework.util.JavaExpressionParseUtil;
 import org.checkerframework.framework.util.StringToJavaExpression;
@@ -185,21 +186,25 @@ public void accumulate(
    * allows propagation, along those paths, of the fact that the method being invoked in {@code
    * node} was definitely called.
    *
+   * @param <S> the type of store
+   * @param <V> the type of abstract values
    * @param node a method invocation
    * @param input the transfer input associated with the method invocation
    * @return a map from types to stores. The keys are the same keys used by {@link
    *     ExceptionBlock#getExceptionalSuccessors()}. The values are copies of the regular store from
    *     {@code input}.
    */
-  private Map<TypeMirror, AccumulationStore> makeExceptionalStores(
-      MethodInvocationNode node, TransferInput<AccumulationValue, AccumulationStore> input) {
+  public static <V extends CFAbstractValue<V>, S extends CFAbstractStore<V, S>>
+      Map<TypeMirror, S> makeExceptionalStores(
+          MethodInvocationNode node, TransferInput<V, S> input) {
     if (!(node.getBlock() instanceof ExceptionBlock)) {
       // This can happen in some weird (buggy?) cases:
       // see https://github.com/typetools/checker-framework/issues/3585
       return Collections.emptyMap();
     }
+
     ExceptionBlock block = (ExceptionBlock) node.getBlock();
-    Map<TypeMirror, AccumulationStore> result = new LinkedHashMap<>();
+    Map<TypeMirror, S> result = new LinkedHashMap<>();
     block
         .getExceptionalSuccessors()
         .forEach((tm, b) -> result.put(tm, input.getRegularStore().copy()));

checker/src/main/java/org/checkerframework/checker/mustcall/MustCallAnalysis.java

@@ -0,0 +1,46 @@
+package org.checkerframework.checker.mustcall;
+
+import com.google.common.collect.ImmutableSet;
+import com.sun.tools.javac.code.Type;
+import java.util.Set;
+import javax.lang.model.type.TypeMirror;
+import org.checkerframework.checker.signature.qual.CanonicalName;
+import org.checkerframework.framework.flow.CFAnalysis;
+
+/**
+ * The analysis for the Must Call Checker. The analysis is specialized to ignore certain exception
+ * types; see {@link #isIgnoredExceptionType(TypeMirror)}.
+ */
+public class MustCallAnalysis extends CFAnalysis {
+
+  /**
+   * Constructs an MustCallAnalysis.
+   *
+   * @param checker the checker
+   * @param factory the type factory
+   */
+  public MustCallAnalysis(MustCallChecker checker, MustCallAnnotatedTypeFactory factory) {
+    super(checker, factory);
+  }
+
+  /**
+   * The fully-qualified names of the exception types that are ignored by this checker when
+   * computing dataflow stores.
+   */
+  protected static final Set<@CanonicalName String> ignoredExceptionTypes =
+      ImmutableSet.of(
+          // Use the Nullness Checker instead.
+          NullPointerException.class.getCanonicalName(),
+          // Ignore run-time errors, which cannot be predicted statically. Doing
+          // so is unsound in the sense that they could still occur - e.g., the
+          // program could run out of memory - but if they did, the checker's
+          // results would be useless anyway.
+          Error.class.getCanonicalName(),
+          RuntimeException.class.getCanonicalName());
+
+  @Override
+  public boolean isIgnoredExceptionType(TypeMirror exceptionType) {
+    return ignoredExceptionTypes.contains(
+        ((Type) exceptionType).tsym.getQualifiedName().toString());
+  }
+}

checker/src/main/java/org/checkerframework/checker/mustcall/MustCallAnnotatedTypeFactory.java

@@ -23,6 +23,7 @@
 import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.TypeElement;
+import javax.lang.model.type.TypeKind;
 import javax.lang.model.type.TypeMirror;
 import org.checkerframework.checker.mustcall.qual.CreatesMustCallFor;
 import org.checkerframework.checker.mustcall.qual.InheritableMustCall;
@@ -164,6 +165,11 @@ protected Set<Class<? extends Annotation>> createSupportedTypeQualifiers() {
         Arrays.asList(MustCall.class, MustCallUnknown.class, PolyMustCall.class));
   }
 
+  @Override
+  protected MustCallAnalysis createFlowAnalysis() {
+    return new MustCallAnalysis((MustCallChecker) checker, this);
+  }
+
   @Override
   protected TreeAnnotator createTreeAnnotator() {
     return new ListTreeAnnotator(super.createTreeAnnotator(), new MustCallTreeAnnotator(this));
@@ -226,6 +232,16 @@ protected void constructorFromUsePreSubstitution(
     super.constructorFromUsePreSubstitution(tree, type, resolvePolyQuals);
   }
 
+  @Override
+  public boolean isIgnoredExceptionType(TypeMirror exceptionType) {
+    if (exceptionType.getKind() == TypeKind.DECLARED) {
+      return ((MustCallChecker) checker)
+          .getIgnoredExceptions()
+          .contains(analysis.getTypes(), exceptionType);
+    }
+    return false;
+  }
+
   /**
    * Class to implement the customized semantics of {@link MustCallAlias} (and {@link PolyMustCall})
    * annotations; see the {@link MustCallAlias} documentation for details.

checker/src/main/java/org/checkerframework/checker/mustcall/MustCallChecker.java

@@ -1,6 +1,17 @@
 package org.checkerframework.checker.mustcall;
 
+import com.google.common.collect.ImmutableSet;
+import java.io.UnsupportedEncodingException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import javax.lang.model.element.TypeElement;
+import javax.lang.model.type.TypeMirror;
+import javax.tools.Diagnostic;
 import org.checkerframework.checker.mustcall.qual.MustCall;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.checkerframework.common.basetype.BaseTypeChecker;
 import org.checkerframework.framework.qual.StubFiles;
 import org.checkerframework.framework.source.SupportedOptions;
@@ -19,10 +30,14 @@
 @SupportedOptions({
   MustCallChecker.NO_CREATES_MUSTCALLFOR,
   MustCallChecker.NO_LIGHTWEIGHT_OWNERSHIP,
-  MustCallChecker.NO_RESOURCE_ALIASES
+  MustCallChecker.NO_RESOURCE_ALIASES,
+  MustCallChecker.IGNORED_EXCEPTIONS
 })
 public class MustCallChecker extends BaseTypeChecker {
 
+  /** Creates a MustCallChecker. */
+  public MustCallChecker() {}
+
   /**
    * Disables @CreatesMustCallFor support. Not of interest to most users. Not documented in the
    * manual.
@@ -39,4 +54,180 @@ public class MustCallChecker extends BaseTypeChecker {
    * Disables @MustCallAlias support. Not of interest to most users. Not documented in the manual.
    */
   public static final String NO_RESOURCE_ALIASES = "noResourceAliases";
+
+  /**
+   * The exception types in this set are ignored in the CFG when determining if a resource leaks
+   * along an exceptional path. These kinds of errors fall into a few categories: runtime errors,
+   * errors that the JVM can issue on any statement, and errors that can be prevented by running
+   * some other CF checker.
+   */
+  public static final SetOfTypes DEFAULT_IGNORED_EXCEPTIONS =
+      SetOfTypes.anyOfTheseNames(
+          ImmutableSet.of(
+              // Any method call has a CFG edge for Throwable/RuntimeException/Error
+              // to represent run-time misbehavior. Ignore it.
+              Throwable.class.getCanonicalName(),
+              Error.class.getCanonicalName(),
+              RuntimeException.class.getCanonicalName(),
+              // Use the Nullness Checker to prove this won't happen.
+              NullPointerException.class.getCanonicalName(),
+              // These errors can't be predicted statically, so ignore them and assume
+              // they won't happen.
+              ClassCircularityError.class.getCanonicalName(),
+              ClassFormatError.class.getCanonicalName(),
+              NoClassDefFoundError.class.getCanonicalName(),
+              OutOfMemoryError.class.getCanonicalName(),
+              // It's not our problem if the Java type system is wrong.
+              ClassCastException.class.getCanonicalName(),
+              // It's not our problem if the code is going to divide by zero.
+              ArithmeticException.class.getCanonicalName(),
+              // Use the Index Checker to prevent these errors.
+              ArrayIndexOutOfBoundsException.class.getCanonicalName(),
+              NegativeArraySizeException.class.getCanonicalName(),
+              // Most of the time, this exception is infeasible, as the charset used
+              // is guaranteed to be present by the Java spec (e.g., "UTF-8").
+              // Eventually, this exclusion could be refined by looking at the charset
+              // being requested.
+              UnsupportedEncodingException.class.getCanonicalName()));
+
+  /**
+   * Command-line option for controlling which exceptions are ignored.
+   *
+   * @see #DEFAULT_IGNORED_EXCEPTIONS
+   * @see #getIgnoredExceptions()
+   */
+  public static final String IGNORED_EXCEPTIONS = "resourceLeakIgnoredExceptions";
+
+  /**
+   * A pattern that matches one or more consecutive commas, optionally preceded and followed by
+   * whitespace.
+   */
+  public static final Pattern COMMAS = Pattern.compile("\\s*(?:" + Pattern.quote(",") + "\\s*)+");
+
+  /**
+   * A pattern that matches an exception specifier for the {@link #IGNORED_EXCEPTIONS} option: an
+   * optional "=" followed by a qualified name. The whole thing can be padded with whitespace.
+   */
+  public static final Pattern EXCEPTION_SPECIFIER =
+      Pattern.compile(
+          "^\\s*" + "(" + Pattern.quote("=") + "\\s*" + ")?" + "(\\w+(?:\\.\\w+)*)" + "\\s*$");
+
+  /**
+   * The cached set of ignored exceptions parsed from {@link #IGNORED_EXCEPTIONS}. Caching this
+   * field prevents the checker from issuing duplicate warnings about missing exception types.
+   *
+   * @see #getIgnoredExceptions()
+   */
+  private @MonotonicNonNull SetOfTypes ignoredExceptions = null;
+
+  /**
+   * Get the set of exceptions that should be ignored. This set comes from the {@link
+   * #IGNORED_EXCEPTIONS} option if it was provided, or {@link #DEFAULT_IGNORED_EXCEPTIONS} if not.
+   *
+   * @return the set of exceptions to ignore
+   */
+  public SetOfTypes getIgnoredExceptions() {
+    SetOfTypes result = ignoredExceptions;
+    if (result == null) {
+      String ignoredExceptionsOptionValue = getOption(IGNORED_EXCEPTIONS);
+      result =
+          ignoredExceptionsOptionValue == null
+              ? DEFAULT_IGNORED_EXCEPTIONS
+              : parseIgnoredExceptions(ignoredExceptionsOptionValue);
+      ignoredExceptions = result;
+    }
+    return result;
+  }
+
+  /**
+   * Parse the argument given for the {@link #IGNORED_EXCEPTIONS} option. Warnings will be issued
+   * for any problems in the argument, for instance if any of the named exceptions cannot be found.
+   *
+   * @param ignoredExceptionsOptionValue the value given for {@link #IGNORED_EXCEPTIONS}
+   * @return the set of ignored exceptions
+   */
+  protected SetOfTypes parseIgnoredExceptions(String ignoredExceptionsOptionValue) {
+    String[] exceptions = COMMAS.split(ignoredExceptionsOptionValue);
+    List<SetOfTypes> sets = new ArrayList<>();
+    for (String e : exceptions) {
+      SetOfTypes set = parseExceptionSpecifier(e, ignoredExceptionsOptionValue);
+      if (set != null) {
+        sets.add(set);
+      }
+    }
+    return SetOfTypes.union(sets.toArray(new SetOfTypes[0]));
+  }
+
+  /**
+   * Parse a single exception specifier from the {@link #IGNORED_EXCEPTIONS} option and issue
+   * warnings if it does not parse. See {@link #EXCEPTION_SPECIFIER} for a description of the
+   * syntax.
+   *
+   * @param exceptionSpecifier the exception specifier to parse
+   * @param ignoredExceptionsOptionValue the whole value of the {@link #IGNORED_EXCEPTIONS} option;
+   *     only used for error reporting
+   * @return the parsed set of types, or null if the value does not parse
+   */
+  @SuppressWarnings({
+    // user input might not be a legal @CanonicalName, but it should be safe to pass to
+    // `SetOfTypes.anyOfTheseNames`
+    "signature:type.arguments.not.inferred",
+  })
+  protected @Nullable SetOfTypes parseExceptionSpecifier(
+      String exceptionSpecifier, String ignoredExceptionsOptionValue) {
+    Matcher m = EXCEPTION_SPECIFIER.matcher(exceptionSpecifier);
+    if (m.matches()) {
+      @Nullable String equalsSign = m.group(1);
+      String qualifiedName = m.group(2);
+
+      if (qualifiedName.equalsIgnoreCase("default")) {
+        return DEFAULT_IGNORED_EXCEPTIONS;
+      }
+      TypeMirror type = checkCanonicalName(qualifiedName);
+      if (type == null) {
+        // There is a chance that the user named a real type, but the class is not
+        // accessible for some reason. We'll issue a warning (in case this was a typo) but
+        // add the type as ignored anyway (in case it's just an inaccessible type).
+        //
+        // Note that if the user asked to ignore subtypes of this exception, this code won't
+        // do it because we can't know what those subtypes are. We have to treat this as if
+        // it were "=qualifiedName" even if no equals sign was provided.
+        message(
+            Diagnostic.Kind.WARNING,
+            "The exception '%s' appears in the -A%s=%s option, but it does not seem to exist",
+            exceptionSpecifier,
+            IGNORED_EXCEPTIONS,
+            ignoredExceptionsOptionValue);
+        return SetOfTypes.anyOfTheseNames(ImmutableSet.of(qualifiedName));
+      } else {
+        return equalsSign == null ? SetOfTypes.allSubtypes(type) : SetOfTypes.singleton(type);
+      }
+    } else if (!exceptionSpecifier.trim().isEmpty()) {
+      message(
+          Diagnostic.Kind.WARNING,
+          "The string '%s' appears in the -A%s=%s option,"
+              + " but it is not a legal exception specifier",
+          exceptionSpecifier,
+          IGNORED_EXCEPTIONS,
+          ignoredExceptionsOptionValue);
+    }
+    return null;
+  }
+
+  /**
+   * Check if the given String refers to an actual type.
+   *
+   * @param s any string
+   * @return the referenced type, or null if it does not exist
+   */
+  @SuppressWarnings({
+    "signature:argument", // `s` is not a qualified name, but we pass it to getTypeElement
+  })
+  protected @Nullable TypeMirror checkCanonicalName(String s) {
+    TypeElement elem = getProcessingEnvironment().getElementUtils().getTypeElement(s);
+    if (elem == null) {
+      return null;
+    }
+    return types.getDeclaredType(elem);
+  }
 }