Added Optional emptiness handler

nullaway/src/main/java/com/uber/nullaway/Config.java

@@ -107,7 +107,12 @@ public interface Config {
    */
   boolean acknowledgeRestrictiveAnnotations();
 
-  /** @return true if Optional Emptiness Handler is to be used. */
+  /**
+   * @return true if Optional Emptiness Handler is to be used. When Optional.get() method is called
+   *     on an empty optional, program will crash with an exception. This handler warns on possible
+   *     cases where Optional.get() call is made on an empty optional. Nullaway determines if an
+   *     optional is non-empty based on Optional.isPresent() call.
+   */
   boolean checkOptionalEmptiness();
 
   /**

nullaway/src/main/java/com/uber/nullaway/NullAway.java

@@ -365,7 +365,8 @@ private void updateEnvironmentMapping(Tree tree, VisitorState state) {
     // from the nested scope, so the program point doesn't matter
     // 2. we keep info on all locals rather than just effectively final ones for simplicity
     EnclosingEnvironmentNullness.instance(state.context)
-        .addEnvironmentMapping(tree, analysis.getLocalVarInfoBefore(state.getPath(), state));
+        .addEnvironmentMapping(
+            tree, analysis.getLocalVarInfoBefore(state.getPath(), state, handler));
   }
 
   private Symbol.MethodSymbol getSymbolOfSuperConstructor(
@@ -1932,21 +1933,16 @@ private Description matchDereference(
       return Description.NO_MATCH;
     }
     if (mayBeNullExpr(state, baseExpression)) {
-      String message;
-      if (handler.checkIfOptionalGetCall(baseExpression, state)) {
-        final int exprStringSize = baseExpression.toString().length();
-        // Name of the optional is extracted from the expression
-        message =
-            "Optional "
-                + baseExpression.toString().substring(0, exprStringSize - 6)
-                + " can be empty, dereferenced get() call on it";
-      } else {
-        message = "dereferenced expression " + baseExpression.toString() + " is @Nullable";
-      }
+      final String message =
+          "dereferenced expression " + baseExpression.toString() + " is @Nullable";
+      ErrorMessage errorMessage = new ErrorMessage(MessageTypes.DEREFERENCE_NULLABLE, message);
+
+      handler.getErrorMessage(baseExpression, state, errorMessage);
+
       return createErrorDescriptionForNullAssignment(
-          MessageTypes.DEREFERENCE_NULLABLE,
+          errorMessage.messageType,
           derefExpression,
-          message,
+          errorMessage.message,
           baseExpression,
           state.getPath());
     }
@@ -2346,7 +2342,23 @@ public enum MessageTypes {
     UNBOX_NULLABLE,
     NONNULL_FIELD_READ_BEFORE_INIT,
     ANNOTATION_VALUE_INVALID,
-    CAST_TO_NONNULL_ARG_NONNULL;
+    CAST_TO_NONNULL_ARG_NONNULL,
+    GET_ON_EMPTY_OPTIONAL;
+  }
+
+  public static class ErrorMessage {
+    MessageTypes messageType;
+    String message;
+
+    ErrorMessage(MessageTypes messageType, String message) {
+      this.messageType = messageType;
+      this.message = message;
+    }
+
+    public void updateErrorMessage(MessageTypes messageType, String message) {
+      this.messageType = messageType;
+      this.message = message;
+    }
   }
 
   @AutoValue

nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java

@@ -311,7 +311,7 @@ public static boolean isMapPut(Symbol.MethodSymbol symbol, Types types) {
    * root of an access path; either a variable {@link javax.lang.model.element.Element} or <code>
    * this</code> (enclosing method receiver)
    */
-  static final class Root {
+  public static final class Root {
 
     /** does this represent the receiver? */
     private final boolean isMethodReceiver;

nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java

@@ -21,15 +21,12 @@
 import com.google.common.collect.ImmutableList;
 import com.google.errorprone.VisitorState;
 import com.sun.source.util.TreePath;
-import com.sun.tools.javac.code.Symbol;
-import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.util.Context;
 import com.uber.nullaway.Config;
 import com.uber.nullaway.Nullness;
 import com.uber.nullaway.handlers.Handler;
 import java.util.Collections;
 import java.util.LinkedHashSet;
-import java.util.Optional;
 import java.util.Set;
 import java.util.function.Predicate;
 import javax.annotation.Nullable;
@@ -160,7 +157,7 @@ public Set<Element> getNonnullStaticFieldsBefore(TreePath path, Context context)
    * @param state visitor state
    * @return nullness info for local variables just before the node
    */
-  public NullnessStore getLocalVarInfoBefore(TreePath path, VisitorState state) {
+  public NullnessStore getLocalVarInfoBefore(TreePath path, VisitorState state, Handler handler) {
     NullnessStore store = dataFlow.resultBefore(path, state.context, nullnessPropagation);
     if (store == null) {
       return NullnessStore.empty();
@@ -176,30 +173,14 @@ public NullnessStore getLocalVarInfoBefore(TreePath path, VisitorState state) {
             }
           }
 
-          // a filter for Optional get() call
-          if (ap.getElements().size() == 1) {
-            AccessPath.Root root = ap.getRoot();
-            if (!root.isReceiver() && (ap.getElements().get(0) instanceof Symbol.MethodSymbol)) {
-              final Element e = root.getVarElement();
-              final Symbol.MethodSymbol g = (Symbol.MethodSymbol) ap.getElements().get(0);
-              final Optional<Type> tbaseType =
-                  Optional.ofNullable(state.getTypeFromString(OPTIONAL_PATH))
-                      .map(state.getTypes()::erasure);
-              return e.getKind().equals(ElementKind.LOCAL_VARIABLE)
-                  && g.getSimpleName().toString().equals("get")
-                  && g.getParameters().length() == 0
-                  && tbaseType.isPresent()
-                  && state.getTypes().isSubtype(g.owner.type, tbaseType.get());
-            }
-          }
-          return false;
+          return handler.filterApForLocalVarInfoBefore(ap, state);
         });
   }
 
   /**
    * @param path tree path of static method, or initializer block
    * @param context Javac context
-   * @return fields guaranteed to be nonnull at exit of static method (or initializer block)
+   * @return fields guaranteed to be nonnull at exit of static method (or initialize r block)
    */
   public Set<Element> getNonnullStaticFieldsAtExit(TreePath path, Context context) {
     NullnessStore nullnessResult = dataFlow.finalResult(path, context, nullnessPropagation);

nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java

@@ -35,6 +35,7 @@
 import com.sun.tools.javac.code.Types;
 import com.sun.tools.javac.util.Context;
 import com.uber.nullaway.NullAway;
+import com.uber.nullaway.dataflow.AccessPath;
 import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
 import com.uber.nullaway.dataflow.NullnessStore;
 import java.util.List;
@@ -168,7 +169,13 @@ public void onDataflowVisitLambdaResultExpression(
   }
 
   @Override
-  public boolean checkIfOptionalGetCall(ExpressionTree expr, VisitorState state) {
+  public void getErrorMessage(
+      ExpressionTree expr, VisitorState state, NullAway.ErrorMessage errorMessage) {
+    // NoOp
+  }
+
+  @Override
+  public boolean filterApForLocalVarInfoBefore(AccessPath accessPath, VisitorState state) {
     return false;
   }
 }

nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java

@@ -36,6 +36,7 @@
 import com.sun.tools.javac.code.Types;
 import com.sun.tools.javac.util.Context;
 import com.uber.nullaway.NullAway;
+import com.uber.nullaway.dataflow.AccessPath;
 import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
 import com.uber.nullaway.dataflow.NullnessStore;
 import java.util.List;
@@ -209,11 +210,19 @@ public void onDataflowVisitLambdaResultExpression(
   }
 
   @Override
-  public boolean checkIfOptionalGetCall(ExpressionTree expr, VisitorState state) {
-    boolean ifOptionalGetCall = false;
+  public void getErrorMessage(
+      ExpressionTree expr, VisitorState state, NullAway.ErrorMessage errorMessage) {
     for (Handler h : handlers) {
-      ifOptionalGetCall |= h.checkIfOptionalGetCall(expr, state);
+      h.getErrorMessage(expr, state, errorMessage);
     }
-    return ifOptionalGetCall;
+  }
+
+  @Override
+  public boolean filterApForLocalVarInfoBefore(AccessPath accessPath, VisitorState state) {
+    boolean shouldFilter = false;
+    for (Handler h : handlers) {
+      shouldFilter |= h.filterApForLocalVarInfoBefore(accessPath, state);
+    }
+    return shouldFilter;
   }
 }

nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java

@@ -36,6 +36,7 @@
 import com.sun.tools.javac.util.Context;
 import com.uber.nullaway.NullAway;
 import com.uber.nullaway.Nullness;
+import com.uber.nullaway.dataflow.AccessPath;
 import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
 import com.uber.nullaway.dataflow.NullnessStore;
 import java.util.List;
@@ -275,9 +276,21 @@ void onDataflowVisitLambdaResultExpression(
    *
    * @param expr The AST node for the expression being matched.
    * @param state The current visitor state.
-   * @return If the method call in the expression is to Optional.get().
+   * @param errorMessage error message string and type of the error wrapped in {@link
+   *     com.uber.nullaway.NullAway.ErrorMessage}.
    */
-  boolean checkIfOptionalGetCall(ExpressionTree expr, VisitorState state);
+  void getErrorMessage(ExpressionTree expr, VisitorState state, NullAway.ErrorMessage errorMessage);
+
+  /**
+   * Called when the store access paths are filtered for local variable information before an
+   * expression.
+   *
+   * @param accessPath The access path that needs to be checked if filtered.
+   * @param state The current visitor state.
+   * @return true if the accesspath should be filtered to be included in the local variable
+   *     information.
+   */
+  boolean filterApForLocalVarInfoBefore(AccessPath accessPath, VisitorState state);
 
   /**
    * A three value enum for handlers implementing onDataflowVisitMethodInvocation to communicate

nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java

@@ -100,10 +100,31 @@ public NullnessHint onDataflowVisitMethodInvocation(
   }
 
   @Override
-  public boolean checkIfOptionalGetCall(ExpressionTree expr, VisitorState state) {
+  public void getErrorMessage(
+      ExpressionTree expr, VisitorState state, NullAway.ErrorMessage errorMessage) {
     if (expr.getKind() == Tree.Kind.METHOD_INVOCATION
         && optionalIsGetCall((Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.getTypes())) {
-      return true;
+      final int exprStringSize = expr.toString().length();
+      // Name of the optional is extracted from the expression
+      final String message =
+          "Optional "
+              + expr.toString().substring(0, exprStringSize - 6)
+              + " can be empty, dereferenced get() call on it";
+      errorMessage.updateErrorMessage(NullAway.MessageTypes.GET_ON_EMPTY_OPTIONAL, message);
+    }
+  }
+
+  @Override
+  public boolean filterApForLocalVarInfoBefore(AccessPath accessPath, VisitorState state) {
+
+    if (accessPath.getElements().size() == 1) {
+      AccessPath.Root root = accessPath.getRoot();
+      if (!root.isReceiver() && (accessPath.getElements().get(0) instanceof Symbol.MethodSymbol)) {
+        final Element e = root.getVarElement();
+        final Symbol.MethodSymbol g = (Symbol.MethodSymbol) accessPath.getElements().get(0);
+        return e.getKind().equals(ElementKind.LOCAL_VARIABLE)
+            && optionalIsGetCall(g, state.getTypes());
+      }
     }
     return false;
   }