docs

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

@@ -223,7 +223,8 @@ public NullnessStore getNullnessInfoBeforeNewContext(
     if (store == null) {
       return NullnessStore.empty();
     }
-    Predicate<AccessPath> handlerPredicate = handler.getAccessPathPredForSavedContext(path, state);
+    Predicate<AccessPath> handlerPredicate =
+        handler.getAccessPathPredicateForNestedMethod(path, state);
     return store.filterAccessPaths(
         (ap) -> {
           boolean allAPNonRootElementsAreFinalFields = true;

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

@@ -201,7 +201,8 @@ public Optional<ErrorMessage> onExpressionDereference(
   }
 
   @Override
-  public Predicate<AccessPath> getAccessPathPredForSavedContext(TreePath path, VisitorState state) {
+  public Predicate<AccessPath> getAccessPathPredicateForNestedMethod(
+      TreePath path, VisitorState state) {
     return CompositeHandler.FALSE_AP_PREDICATE;
   }
 

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

@@ -254,14 +254,26 @@ public Optional<ErrorMessage> onExpressionDereference(
     return Optional.empty();
   }
 
+  /**
+   * An AccessPath predicate that always returns false. Used for optimizing
+   * getAccessPathPredicateForNestedMethod.
+   */
   static final Predicate<AccessPath> FALSE_AP_PREDICATE = ap -> false;
+
+  /**
+   * An AccessPath predicate that always returns true. Used for optimizing
+   * getAccessPathPredicateForNestedMethod.
+   */
   static final Predicate<AccessPath> TRUE_AP_PREDICATE = ap -> true;
 
   @Override
-  public Predicate<AccessPath> getAccessPathPredForSavedContext(TreePath path, VisitorState state) {
+  public Predicate<AccessPath> getAccessPathPredicateForNestedMethod(
+      TreePath path, VisitorState state) {
     Predicate<AccessPath> filter = FALSE_AP_PREDICATE;
     for (Handler h : handlers) {
-      Predicate<AccessPath> curFilter = h.getAccessPathPredForSavedContext(path, state);
+      Predicate<AccessPath> curFilter = h.getAccessPathPredicateForNestedMethod(path, state);
+      // here we do some optimization, to try to avoid unnecessarily returning a deeply nested
+      // Predicate object (which would be more costly to test)
       if (curFilter != FALSE_AP_PREDICATE) {
         if (curFilter == TRUE_AP_PREDICATE) {
           return curFilter;

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

@@ -329,15 +329,16 @@ Optional<ErrorMessage> onExpressionDereference(
       ExpressionTree expr, ExpressionTree baseExpr, VisitorState state);
 
   /**
-   * Called when the store access paths are filtered for local variable information before an
-   * expression.
+   * Called when determining which access path nullability information should be preserved when
+   * analyzing a nested method, i.e., a lambda expression or a method in an anonymous or local
+   * class.
    *
-   * @param path The access path that needs to be checked if filtered.
+   * @param path The tree path to the node for the nested method.
    * @param state The current visitor state.
-   * @return true if the nullability information for this accesspath should be treated as part of
-   *     the surrounding context when processing a lambda expression or anonymous class declaration.
+   * @return A predicate that determines which access paths should be preserved when analyzing the
+   *     nested method.
    */
-  Predicate<AccessPath> getAccessPathPredForSavedContext(TreePath path, VisitorState state);
+  Predicate<AccessPath> getAccessPathPredicateForNestedMethod(TreePath path, VisitorState state);
 
   /**
    * Called during dataflow analysis initialization to register structurally immutable types.

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

@@ -165,7 +165,8 @@ private boolean isOptionalContentNullable(
   }
 
   @Override
-  public Predicate<AccessPath> getAccessPathPredForSavedContext(TreePath path, VisitorState state) {
+  public Predicate<AccessPath> getAccessPathPredicateForNestedMethod(
+      TreePath path, VisitorState state) {
     return ap -> {
       if (ap.getElements().size() == 1) {
         final Element e = ap.getRoot();

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

@@ -23,7 +23,7 @@ public class SynchronousCallbackHandler extends BaseNoOpHandler {
 
   /**
    * Maps method name to full information about the corresponding methods and what parameter is the
-   * relevant callback
+   * relevant callback. We key on method name to quickly eliminate most cases when doing a lookup.
    */
   private static final ImmutableMap<String, ImmutableMap<MethodRef, Integer>>
       METHOD_NAME_TO_SIG_AND_PARAM_INDEX =
@@ -44,7 +44,8 @@ public class SynchronousCallbackHandler extends BaseNoOpHandler {
       Suppliers.typeFromString("java.util.stream.Stream");
 
   @Override
-  public Predicate<AccessPath> getAccessPathPredForSavedContext(TreePath path, VisitorState state) {
+  public Predicate<AccessPath> getAccessPathPredicateForNestedMethod(
+      TreePath path, VisitorState state) {
     Tree leafNode = path.getLeaf();
     Preconditions.checkArgument(
         leafNode instanceof ClassTree || leafNode instanceof LambdaExpressionTree,

nullaway/src/test/java/com/uber/nullaway/SyncLambdasTests.java

@@ -4,7 +4,7 @@
 
 /**
  * Tests for cases where lambdas or anonymous class methods are invoked nearly synchronously, so it
- * is reasonable to propagat more nullability information to their bodies.
+ * is reasonable to propagate more nullability information to their bodies.
  */
 public class SyncLambdasTests extends NullAwayTestsBase {
 
@@ -26,6 +26,7 @@ public void forEachOnMap() {
             "        }",
             "        this.resolved = new HashMap<>();",
             "        this.target.forEach((key, value) -> {",
+            "            // no error here as info gets propagated",
             "            this.resolved.put(key, value);",
             "        });",
             "    }",
@@ -50,6 +51,7 @@ public void forEachOnHashMap() {
             "        }",
             "        this.resolved = new HashMap<>();",
             "        this.target.forEach((key, value) -> {",
+            "            // no error here as info gets propagated",
             "            this.resolved.put(key, value);",
             "        });",
             "    }",
@@ -79,6 +81,7 @@ public void otherForEach() {
             "        }",
             "        this.resolved = new MyMap<>();",
             "        this.target.forEach((key, value) -> {",
+            "            // error since this is a custom type, not inheriting from java.util.Map",
             "            // BUG: Diagnostic contains: dereferenced expression this.resolved is @Nullable",
             "            this.resolved.put(key, value);",
             "        });",
@@ -125,6 +128,7 @@ public void streamMethods() {
             "            throw new IllegalArgumentException();",
             "        }",
             "        List<Object> l = new ArrayList<>();",
+            "        // this.f being non-null gets propagated to all callback lambdas",
             "        l.stream().filter(v -> this.f.toString().equals(v.toString()))",
             "         .map(v -> this.f.toString())",
             "         .forEach(v -> System.out.println(this.f.hashCode() + v.toString()));",