more refactoring and docs

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

@@ -50,10 +50,10 @@
 import com.uber.nullaway.dataflow.AccessPathElement;
 import com.uber.nullaway.dataflow.AccessPathNullnessAnalysis;
 import com.uber.nullaway.dataflow.NullnessStore;
-import com.uber.nullaway.handlers.stream.CollectlikeMethodRecord;
+import com.uber.nullaway.handlers.stream.CollectLikeMethodRecord;
+import com.uber.nullaway.handlers.stream.MapLikeMethodRecord;
 import com.uber.nullaway.handlers.stream.MapOrCollectLikeMethodRecord;
 import com.uber.nullaway.handlers.stream.MapOrCollectMethodToFilterInstanceRecord;
-import com.uber.nullaway.handlers.stream.MaplikeMethodRecord;
 import com.uber.nullaway.handlers.stream.StreamTypeRecord;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
@@ -214,8 +214,8 @@ public void onMatchMethodInvocation(
             ClassTree annonClassBody = ((NewClassTree) argTree).getClassBody();
             // Ensure that this `new B() ...` has a custom class body, otherwise, we skip for now.
             if (annonClassBody != null) {
-              MaplikeMethodRecord methodRecord = streamType.getMaplikeMethodRecord(methodSymbol);
-              handleMapOrCollectAnonClass(
+              MapLikeMethodRecord methodRecord = streamType.getMaplikeMethodRecord(methodSymbol);
+              handleMapOrCollectAnonClassBody(
                   methodRecord,
                   annonClassBody,
                   t -> observableCallToInnerMethodOrLambda.put(tree, t));
@@ -226,20 +226,22 @@ public void onMatchMethodInvocation(
             observableCallToInnerMethodOrLambda.put(tree, argTree);
           }
         } else {
-          CollectlikeMethodRecord collectlikeMethodRecord =
+          CollectLikeMethodRecord collectlikeMethodRecord =
               streamType.getCollectlikeMethodRecord(methodSymbol);
           if (collectlikeMethodRecord != null && methodSymbol.getParameters().length() == 1) {
-            handleCollectCall(tree, methodSymbol, collectlikeMethodRecord);
+            handleCollectCall(tree, collectlikeMethodRecord);
           }
         }
       }
     }
   }
 
+  /**
+   * Handles a call to a collect-like method. If the argument to the method is supported, updates
+   * the {@link #collectCallToInnerMethodsOrLambdas} map appropriately.
+   */
   private void handleCollectCall(
-      MethodInvocationTree tree,
-      Symbol.MethodSymbol unused,
-      CollectlikeMethodRecord collectlikeMethodRecord) {
+      MethodInvocationTree tree, CollectLikeMethodRecord collectlikeMethodRecord) {
     ExpressionTree argTree = tree.getArguments().get(0);
     if (argTree instanceof MethodInvocationTree) {
       MethodInvocationTree collectInvokeArg = (MethodInvocationTree) argTree;
@@ -260,7 +262,7 @@ private void handleCollectCall(
             ClassTree anonClassBody = ((NewClassTree) factoryMethodArg).getClassBody();
             // Ensure that this `new B() ...` has a custom class body, otherwise, we skip for now.
             if (anonClassBody != null) {
-              handleMapOrCollectAnonClass(
+              handleMapOrCollectAnonClassBody(
                   collectlikeMethodRecord,
                   anonClassBody,
                   t -> collectCallToInnerMethodsOrLambdas.put(tree, t));
@@ -298,7 +300,7 @@ private void handleChainFromFilter(
       // Check for a map method (which might be a pass-through method or the first method after a
       // pass-through chain)
       if (observableCallToInnerMethodOrLambda.containsKey(outerCallInChain)) {
-        // Update mapToFilterMap
+        // Update mapOrCollectRecordToFilterMap
         Symbol.MethodSymbol mapMethod = ASTHelpers.getSymbol(outerCallInChain);
         if (streamType.isMapMethod(mapMethod)) {
           MapOrCollectMethodToFilterInstanceRecord record =
@@ -308,9 +310,8 @@ private void handleChainFromFilter(
               observableCallToInnerMethodOrLambda.get(outerCallInChain), record);
         }
       } else if (collectCallToInnerMethodsOrLambdas.containsKey(outerCallInChain)) {
-        // Update collectToFilterMap
         Symbol.MethodSymbol collectMethod = ASTHelpers.getSymbol(outerCallInChain);
-        CollectlikeMethodRecord collectlikeMethodRecord =
+        CollectLikeMethodRecord collectlikeMethodRecord =
             streamType.getCollectlikeMethodRecord(collectMethod);
         if (collectlikeMethodRecord != null) {
           for (Tree innerMethodOrLambda :
@@ -351,7 +352,11 @@ private void handleFilterLambda(
     handleChainFromFilter(streamType, observableDotFilter, lambdaTree, state);
   }
 
-  private void handleMapOrCollectAnonClass(
+  /**
+   * If the relevant inner method from the method record is found in the class body, the consumer is
+   * called with the corresponding MethodTree.
+   */
+  private void handleMapOrCollectAnonClassBody(
       MapOrCollectLikeMethodRecord methodRecord, ClassTree anonClassBody, Consumer<Tree> consumer) {
     for (Tree t : anonClassBody.getMembers()) {
       if (t instanceof MethodTree

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

@@ -65,6 +65,8 @@ public static StreamNullabilityPropagator getJavaStreamNullabilityPropagator() {
                 "accept",
                 ImmutableSet.of(0))
             .withMapMethodAllFromName("flatMap", "apply", ImmutableSet.of(0))
+            // Names and relevant arguments of all the methods of java.util.stream.Stream that
+            // behave like .collect(...) for the purposes of this checker
             .withCollectMethodFromSignature(
                 "<R,A>collect(java.util.stream.Collector<? super T,A,R>)",
                 "java.util.stream.Collectors",

nullaway/src/main/java/com/uber/nullaway/handlers/stream/CollectlikeMethodRecord.java → nullaway/src/main/java/com/uber/nullaway/handlers/stream/CollectLikeMethodRecord.java

@@ -28,12 +28,14 @@
 
 /**
  * An immutable model describing a collect-like method from a stream-based API, such as {@link
- * java.util.stream.Stream#collect(Collector)}.
+ * java.util.stream.Stream#collect(Collector)}. Such methods are distinguished from map-like methods
+ * in that they take a {@link Collector} instance as an argument. We match specific factory methods
+ * that create a {@link Collector} and track the arguments to those factory methods.
  */
 @AutoValue
-public abstract class CollectlikeMethodRecord implements MapOrCollectLikeMethodRecord {
+public abstract class CollectLikeMethodRecord implements MapOrCollectLikeMethodRecord {
 
-  public static CollectlikeMethodRecord create(
+  public static CollectLikeMethodRecord create(
       String collectorFactoryMethodClass,
       String collectorFactoryMethodSignature,
       ImmutableSet<Integer> argsToCollectorFactoryMethod,

nullaway/src/main/java/com/uber/nullaway/handlers/stream/MaplikeMethodRecord.java → nullaway/src/main/java/com/uber/nullaway/handlers/stream/MapLikeMethodRecord.java

@@ -24,7 +24,7 @@
 import com.google.common.collect.ImmutableSet;
 
 /** An immutable model describing a map-like method from a stream-based API such as RxJava. */
-public class MaplikeMethodRecord implements MapOrCollectLikeMethodRecord {
+public class MapLikeMethodRecord implements MapOrCollectLikeMethodRecord {
 
   private final String innerMethodName;
 
@@ -40,7 +40,7 @@ public ImmutableSet<Integer> argsFromStream() {
     return argsFromStream;
   }
 
-  public MaplikeMethodRecord(String innerMethodName, ImmutableSet<Integer> argsFromStream) {
+  public MapLikeMethodRecord(String innerMethodName, ImmutableSet<Integer> argsFromStream) {
     this.innerMethodName = innerMethodName;
     this.argsFromStream = argsFromStream;
   }

nullaway/src/main/java/com/uber/nullaway/handlers/stream/StreamModelBuilder.java

@@ -46,9 +46,9 @@ public class StreamModelBuilder {
   private @Nullable TypePredicate tp = null;
   private ImmutableSet.Builder<String> filterMethodSigs;
   private ImmutableSet.Builder<String> filterMethodSimpleNames;
-  private ImmutableMap.Builder<String, MaplikeMethodRecord> mapMethodSigToRecord;
-  private ImmutableMap.Builder<String, MaplikeMethodRecord> mapMethodSimpleNameToRecord;
-  private ImmutableMap.Builder<String, CollectlikeMethodRecord> collectMethodSigToRecord;
+  private ImmutableMap.Builder<String, MapLikeMethodRecord> mapMethodSigToRecord;
+  private ImmutableMap.Builder<String, MapLikeMethodRecord> mapMethodSimpleNameToRecord;
+  private ImmutableMap.Builder<String, CollectLikeMethodRecord> collectMethodSigToRecord;
   private ImmutableSet.Builder<String> passthroughMethodSigs;
   private ImmutableSet.Builder<String> passthroughMethodSimpleNames;
 
@@ -150,7 +150,7 @@ public StreamModelBuilder withFilterMethodAllFromName(String methodSimpleName) {
   public StreamModelBuilder withMapMethodFromSignature(
       String methodSig, String innerMethodName, ImmutableSet<Integer> argsFromStream) {
     this.mapMethodSigToRecord.put(
-        methodSig, new MaplikeMethodRecord(innerMethodName, argsFromStream));
+        methodSig, new MapLikeMethodRecord(innerMethodName, argsFromStream));
     return this;
   }
 
@@ -168,7 +168,7 @@ public StreamModelBuilder withMapMethodFromSignature(
   public StreamModelBuilder withMapMethodAllFromName(
       String methodSimpleName, String innerMethodName, ImmutableSet<Integer> argsFromStream) {
     this.mapMethodSimpleNameToRecord.put(
-        methodSimpleName, new MaplikeMethodRecord(innerMethodName, argsFromStream));
+        methodSimpleName, new MapLikeMethodRecord(innerMethodName, argsFromStream));
     return this;
   }
 
@@ -181,7 +181,7 @@ public StreamModelBuilder withCollectMethodFromSignature(
       ImmutableSet<Integer> argsFromStream) {
     this.collectMethodSigToRecord.put(
         collectMethodSig,
-        CollectlikeMethodRecord.create(
+        CollectLikeMethodRecord.create(
             collectorFactoryMethodClass,
             collectorFactoryMethodSig,
             argsToCollectorFactoryMethod,
@@ -235,7 +235,7 @@ public StreamModelBuilder withPassthroughMethodAllFromName(String methodSimpleNa
   public StreamModelBuilder withUseAndPassthroughMethodFromSignature(
       String passthroughMethodSig, String innerMethodName, ImmutableSet<Integer> argsFromStream) {
     this.mapMethodSigToRecord.put(
-        passthroughMethodSig, new MaplikeMethodRecord(innerMethodName, argsFromStream));
+        passthroughMethodSig, new MapLikeMethodRecord(innerMethodName, argsFromStream));
     this.passthroughMethodSigs.add(passthroughMethodSig);
     return this;
   }
@@ -255,7 +255,7 @@ public StreamModelBuilder withUseAndPassthroughMethodFromSignature(
   public StreamModelBuilder withUseAndPassthroughMethodAllFromName(
       String methodSimpleName, String innerMethodName, ImmutableSet<Integer> argsFromStream) {
     this.mapMethodSimpleNameToRecord.put(
-        methodSimpleName, new MaplikeMethodRecord(innerMethodName, argsFromStream));
+        methodSimpleName, new MapLikeMethodRecord(innerMethodName, argsFromStream));
     this.passthroughMethodSimpleNames.add(methodSimpleName);
     return this;
   }

nullaway/src/main/java/com/uber/nullaway/handlers/stream/StreamTypeRecord.java

@@ -44,10 +44,10 @@ public class StreamTypeRecord {
   // Names and relevant arguments of all the methods of this type that behave like .map(...) for
   // the purposes of this checker (the listed arguments are those that take the potentially
   // filtered objects from the stream)
-  private final ImmutableMap<String, MaplikeMethodRecord> mapMethodSigToRecord;
-  private final ImmutableMap<String, MaplikeMethodRecord> mapMethodSimpleNameToRecord;
+  private final ImmutableMap<String, MapLikeMethodRecord> mapMethodSigToRecord;
+  private final ImmutableMap<String, MapLikeMethodRecord> mapMethodSimpleNameToRecord;
 
-  private final ImmutableMap<String, CollectlikeMethodRecord> collectMethodSigToRecord;
+  private final ImmutableMap<String, CollectLikeMethodRecord> collectMethodSigToRecord;
 
   // List of methods of java.util.stream.Stream through which we just propagate the nullability
   // information of the last call, e.g. m() in Observable.filter(...).m().map(...) means the
@@ -62,9 +62,9 @@ public StreamTypeRecord(
       TypePredicate typePredicate,
       ImmutableSet<String> filterMethodSigs,
       ImmutableSet<String> filterMethodSimpleNames,
-      ImmutableMap<String, MaplikeMethodRecord> mapMethodSigToRecord,
-      ImmutableMap<String, MaplikeMethodRecord> mapMethodSimpleNameToRecord,
-      ImmutableMap<String, CollectlikeMethodRecord> collectMethodSigToRecord,
+      ImmutableMap<String, MapLikeMethodRecord> mapMethodSigToRecord,
+      ImmutableMap<String, MapLikeMethodRecord> mapMethodSimpleNameToRecord,
+      ImmutableMap<String, CollectLikeMethodRecord> collectMethodSigToRecord,
       ImmutableSet<String> passthroughMethodSigs,
       ImmutableSet<String> passthroughMethodSimpleNames) {
     this.typePredicate = typePredicate;
@@ -91,8 +91,8 @@ public boolean isMapMethod(Symbol.MethodSymbol methodSymbol) {
         || mapMethodSimpleNameToRecord.containsKey(methodSymbol.getQualifiedName().toString());
   }
 
-  public MaplikeMethodRecord getMaplikeMethodRecord(Symbol.MethodSymbol methodSymbol) {
-    MaplikeMethodRecord record = mapMethodSigToRecord.get(methodSymbol.toString());
+  public MapLikeMethodRecord getMaplikeMethodRecord(Symbol.MethodSymbol methodSymbol) {
+    MapLikeMethodRecord record = mapMethodSigToRecord.get(methodSymbol.toString());
     if (record == null) {
       record =
           castToNonNull(
@@ -102,7 +102,7 @@ record =
   }
 
   @Nullable
-  public CollectlikeMethodRecord getCollectlikeMethodRecord(Symbol.MethodSymbol methodSymbol) {
+  public CollectLikeMethodRecord getCollectlikeMethodRecord(Symbol.MethodSymbol methodSymbol) {
     return collectMethodSigToRecord.get(methodSymbol.toString());
   }