Added Optional emptiness handler

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

@@ -68,6 +68,8 @@ public abstract class AbstractConfig implements Config {
 
   protected boolean isAcknowledgeRestrictive;
 
+  protected boolean checkOptionalEmptiness;
+
   protected boolean assertsEnabled;
 
   /**
@@ -182,6 +184,11 @@ public boolean acknowledgeRestrictiveAnnotations() {
     return isAcknowledgeRestrictive;
   }
 
+  @Override
+  public boolean checkOptionalEmptiness() {
+    return checkOptionalEmptiness;
+  }
+
   @Override
   public boolean assertsEnabled() {
     return assertsEnabled;

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

@@ -107,6 +107,9 @@ public interface Config {
    */
   boolean acknowledgeRestrictiveAnnotations();
 
+  /** @return true if Optional Emptiness Handler is to be used. */
+  boolean checkOptionalEmptiness();
+
   /**
    * @return the fully qualified name of a method which will take a @Nullable version of a value and
    *     return an @NonNull copy (likely through an unsafe downcast, but performing runtime checking

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

@@ -112,6 +112,11 @@ public boolean acknowledgeRestrictiveAnnotations() {
     throw new IllegalStateException(error_msg);
   }
 
+  @Override
+  public boolean checkOptionalEmptiness() {
+    throw new IllegalStateException(error_msg);
+  }
+
   @Override
   @Nullable
   public String getCastToNonNullMethod() {

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

@@ -55,6 +55,7 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig {
   static final String FL_UNANNOTATED_CLASSES = EP_FL_NAMESPACE + ":UnannotatedClasses";
   static final String FL_ACKNOWLEDGE_RESTRICTIVE =
       EP_FL_NAMESPACE + ":AcknowledgeRestrictiveAnnotations";
+  static final String FL_CHECK_OPTIONAL_EMPTINESS = EP_FL_NAMESPACE + ":CheckOptionalEmptiness";
   static final String FL_SUPPRESS_COMMENT = EP_FL_NAMESPACE + ":AutoFixSuppressionComment";
   /** --- JarInfer configs --- */
   static final String FL_JI_ENABLED = EP_FL_NAMESPACE + ":JarInferEnabled";
@@ -132,6 +133,7 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig {
     isExhaustiveOverride = flags.getBoolean(FL_EXHAUSTIVE_OVERRIDE).orElse(false);
     isSuggestSuppressions = flags.getBoolean(FL_SUGGEST_SUPPRESSIONS).orElse(false);
     isAcknowledgeRestrictive = flags.getBoolean(FL_ACKNOWLEDGE_RESTRICTIVE).orElse(false);
+    checkOptionalEmptiness = flags.getBoolean(FL_CHECK_OPTIONAL_EMPTINESS).orElse(false);
     treatGeneratedAsUnannotated = flags.getBoolean(FL_GENERATED_UNANNOTATED).orElse(false);
     assertsEnabled = flags.getBoolean(FL_ASSERTS_ENABLED).orElse(false);
     fieldAnnotPattern =

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

@@ -365,8 +365,7 @@ 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.context));
+        .addEnvironmentMapping(tree, analysis.getLocalVarInfoBefore(state.getPath(), state));
   }
 
   private Symbol.MethodSymbol getSymbolOfSuperConstructor(
@@ -1933,7 +1932,17 @@ private Description matchDereference(
       return Description.NO_MATCH;
     }
     if (mayBeNullExpr(state, baseExpression)) {
-      String message = "dereferenced expression " + baseExpression.toString() + " is @Nullable";
+      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";
+      }
       return createErrorDescriptionForNullAssignment(
           MessageTypes.DEREFERENCE_NULLABLE,
           derefExpression,

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

@@ -19,13 +19,17 @@
 package com.uber.nullaway.dataflow;
 
 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;
@@ -47,6 +51,8 @@ public final class AccessPathNullnessAnalysis {
 
   private final DataFlow dataFlow;
 
+  private static String OPTIONAL_PATH = "java.util.Optional";
+
   // Use #instance to instantiate
   private AccessPathNullnessAnalysis(
       Predicate<MethodInvocationNode> methodReturnsNonNull,
@@ -151,11 +157,11 @@ public Set<Element> getNonnullStaticFieldsBefore(TreePath path, Context context)
    * Get nullness info for local variables before some node
    *
    * @param path tree path to some AST node within a method / lambda / initializer
-   * @param context Javac context
+   * @param state visitor state
    * @return nullness info for local variables just before the node
    */
-  public NullnessStore getLocalVarInfoBefore(TreePath path, Context context) {
-    NullnessStore store = dataFlow.resultBefore(path, context, nullnessPropagation);
+  public NullnessStore getLocalVarInfoBefore(TreePath path, VisitorState state) {
+    NullnessStore store = dataFlow.resultBefore(path, state.context, nullnessPropagation);
     if (store == null) {
       return NullnessStore.empty();
     }
@@ -169,6 +175,23 @@ public NullnessStore getLocalVarInfoBefore(TreePath path, Context context) {
                   || e.getKind().equals(ElementKind.LOCAL_VARIABLE);
             }
           }
+
+          // 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;
         });
   }

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

@@ -166,4 +166,9 @@ public void onDataflowVisitLambdaResultExpression(
       ExpressionTree tree, NullnessStore thenStore, NullnessStore elseStore) {
     // NoOp
   }
+
+  @Override
+  public boolean checkIfOptionalGetCall(ExpressionTree expr, VisitorState state) {
+    return false;
+  }
 }

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

@@ -207,4 +207,13 @@ public void onDataflowVisitLambdaResultExpression(
       h.onDataflowVisitLambdaResultExpression(tree, thenStore, elseStore);
     }
   }
+
+  @Override
+  public boolean checkIfOptionalGetCall(ExpressionTree expr, VisitorState state) {
+    boolean ifOptionalGetCall = false;
+    for (Handler h : handlers) {
+      ifOptionalGetCall |= h.checkIfOptionalGetCall(expr, state);
+    }
+    return ifOptionalGetCall;
+  }
 }

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

@@ -270,6 +270,15 @@ NullnessHint onDataflowVisitMethodInvocation(
   void onDataflowVisitLambdaResultExpression(
       ExpressionTree tree, NullnessStore thenStore, NullnessStore elseStore);
 
+  /**
+   * Called while creating the error message on a possible null/empty optional deference.
+   *
+   * @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().
+   */
+  boolean checkIfOptionalGetCall(ExpressionTree expr, VisitorState state);
+
   /**
    * A three value enum for handlers implementing onDataflowVisitMethodInvocation to communicate
    * their knowledge of the method return nullability to the the rest of NullAway.

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

@@ -50,6 +50,9 @@ public static Handler buildDefault(Config config) {
     handlerListBuilder.add(new RxNullabilityPropagator());
     handlerListBuilder.add(new ContractHandler());
     handlerListBuilder.add(new ApacheThriftIsSetHandler());
+    if (config.checkOptionalEmptiness()) {
+      handlerListBuilder.add(new OptionalEmptinessHandler());
+    }
     return new CompositeHandler(handlerListBuilder.build());
   }
 

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

@@ -289,6 +289,7 @@ private static class DefaultLibraryModels implements LibraryModels {
                     "javax.lang.model.util.Elements",
                     "getDocComment(javax.lang.model.element.Element)"),
                 0)
+            .put(methodRef("java.util.Optional", "<T>of(T)"), 0)
             .put(methodRef("java.util.Deque", "addFirst(E)"), 0)
             .put(methodRef("java.util.Deque", "addLast(E)"), 0)
             .put(methodRef("java.util.Deque", "offerFirst(E)"), 0)

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

@@ -0,0 +1,138 @@
+/*
+ * Copyright (c) 2018 Uber Technologies, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package com.uber.nullaway.handlers;
+
+import com.google.common.base.Preconditions;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.Tree;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Type;
+import com.sun.tools.javac.code.Types;
+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 java.util.Optional;
+import javax.annotation.Nullable;
+import javax.lang.model.element.Element;
+import javax.lang.model.element.ElementKind;
+import org.checkerframework.dataflow.cfg.node.MethodInvocationNode;
+import org.checkerframework.dataflow.cfg.node.Node;
+
+/**
+ * Handler to better handle {@code isPresent()} methods in code generated for Optionals. With this
+ * handler, we learn appropriate Emptiness facts about the relevant property from these calls.
+ */
+public class OptionalEmptinessHandler extends BaseNoOpHandler {
+
+  private static String OPTIONAL_PATH = "java.util.Optional";
+
+  @Nullable private Optional<Type> optionalType;
+
+  @Override
+  public boolean onOverrideMayBeNullExpr(
+      NullAway analysis, ExpressionTree expr, VisitorState state, boolean exprMayBeNull) {
+    if (expr.getKind() == Tree.Kind.METHOD_INVOCATION
+        && optionalIsGetCall((Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.getTypes())) {
+      return true;
+    }
+    return exprMayBeNull;
+  }
+
+  @Override
+  public void onMatchTopLevelClass(
+      NullAway analysis, ClassTree tree, VisitorState state, Symbol.ClassSymbol classSymbol) {
+    if (optionalType == null) {
+      optionalType =
+          Optional.ofNullable(state.getTypeFromString(OPTIONAL_PATH))
+              .map(state.getTypes()::erasure);
+    }
+  }
+
+  @Override
+  public NullnessHint onDataflowVisitMethodInvocation(
+      MethodInvocationNode node,
+      Types types,
+      Context context,
+      AccessPathNullnessPropagation.SubNodeValues inputs,
+      AccessPathNullnessPropagation.Updates thenUpdates,
+      AccessPathNullnessPropagation.Updates elseUpdates,
+      AccessPathNullnessPropagation.Updates bothUpdates) {
+    Symbol.MethodSymbol symbol = ASTHelpers.getSymbol(node.getTree());
+
+    if (optionalIsGetCall(symbol, types)) {
+      return NullnessHint.HINT_NULLABLE;
+    } else if (optionalIsPresentCall(symbol, types)) {
+      Element getter = null;
+      Node base = node.getTarget().getReceiver();
+      for (Symbol elem : symbol.owner.getEnclosedElements()) {
+        if (elem.getKind().equals(ElementKind.METHOD)
+            && elem.getSimpleName().toString().equals("get")) {
+          getter = elem;
+        }
+      }
+      updateNonNullAPsForElement(thenUpdates, getter, base);
+    }
+    return NullnessHint.UNKNOWN;
+  }
+
+  @Override
+  public boolean checkIfOptionalGetCall(ExpressionTree expr, VisitorState state) {
+    if (expr.getKind() == Tree.Kind.METHOD_INVOCATION
+        && optionalIsGetCall((Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.getTypes())) {
+      return true;
+    }
+    return false;
+  }
+
+  private void updateNonNullAPsForElement(
+      AccessPathNullnessPropagation.Updates updates, @Nullable Element elem, Node base) {
+    if (elem != null) {
+      AccessPath ap = AccessPath.fromBaseAndElement(base, elem);
+      if (ap != null) {
+        updates.set(ap, Nullness.NONNULL);
+      }
+    }
+  }
+
+  private boolean optionalIsPresentCall(Symbol.MethodSymbol symbol, Types types) {
+    Preconditions.checkNotNull(optionalType);
+    // noinspection ConstantConditions
+    return optionalType.isPresent()
+        && symbol.getSimpleName().toString().equals("isPresent")
+        && symbol.getParameters().length() == 0
+        && types.isSubtype(symbol.owner.type, optionalType.get());
+  }
+
+  private boolean optionalIsGetCall(Symbol.MethodSymbol symbol, Types types) {
+    Preconditions.checkNotNull(optionalType);
+    // noinspection ConstantConditions
+    return optionalType.isPresent()
+        && symbol.getSimpleName().toString().equals("get")
+        && symbol.getParameters().length() == 0
+        && types.isSubtype(symbol.owner.type, optionalType.get());
+  }
+}