Fix problem with dataflow inside lambdas

checker/src/main/java/org/checkerframework/checker/rlccalledmethods/RLCCalledMethodsAnnotatedTypeFactory.java

@@ -2,6 +2,8 @@
 
 import com.google.common.collect.BiMap;
 import com.google.common.collect.HashBiMap;
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.LambdaExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.NewClassTree;
@@ -11,6 +13,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Queue;
 import java.util.Set;
 import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.Element;
@@ -47,6 +50,7 @@
 import org.checkerframework.dataflow.cfg.node.LocalVariableNode;
 import org.checkerframework.dataflow.cfg.node.MethodInvocationNode;
 import org.checkerframework.dataflow.cfg.node.Node;
+import org.checkerframework.framework.flow.CFAbstractAnalysis.FieldInitialValue;
 import org.checkerframework.framework.flow.CFStore;
 import org.checkerframework.framework.source.SourceChecker;
 import org.checkerframework.framework.type.AnnotatedTypeMirror;
@@ -56,6 +60,7 @@
 import org.checkerframework.javacutil.ElementUtils;
 import org.checkerframework.javacutil.TreeUtils;
 import org.checkerframework.javacutil.TypeSystemError;
+import org.plumelib.util.IPair;
 
 /**
  * The type factory for the RLCCalledMethodsChecker. The main difference between this and the Called
@@ -143,7 +148,39 @@ public AnnotationMirror createCalledMethods(String... val) {
   }
 
   @Override
-  public void postAnalyze(ControlFlowGraph cfg) {
+  protected ControlFlowGraph analyze(
+      Queue<IPair<ClassTree, @Nullable AccumulationStore>> classQueue,
+      Queue<IPair<LambdaExpressionTree, @Nullable AccumulationStore>> lambdaQueue,
+      UnderlyingAST ast,
+      List<FieldInitialValue<AccumulationValue>> fieldValues,
+      @Nullable ControlFlowGraph cfg,
+      boolean isInitializationCode,
+      boolean updateInitializationStore,
+      boolean isStatic,
+      @Nullable AccumulationStore capturedStore) {
+    // This is a workaround for a bug that I tried and failed to fix.
+    // See checker/tests/resourceleak/RLLambda.java.
+    // This code really belongs in postAnalyze, but this code only works correctly when called after
+    // a method is analyzed the first time and before any containing lambdas are analyzed.
+    // This workaround means there could be false positives when the type of a method invocation
+    // depends on dataflow in a lambda.
+
+    if (cfg != null) {
+      // The cfg is not null, so the analysis has been run before.  Don't rerun it.
+      return cfg;
+    }
+    cfg =
+        super.analyze(
+            classQueue,
+            lambdaQueue,
+            ast,
+            fieldValues,
+            cfg,
+            isInitializationCode,
+            updateInitializationStore,
+            isStatic,
+            capturedStore);
+    assert root != null : "@AssumeAssertion(nullness): at this point root is always nonnull";
     rlc.setRoot(root);
     MustCallConsistencyAnalyzer mustCallConsistencyAnalyzer = new MustCallConsistencyAnalyzer(rlc);
     mustCallConsistencyAnalyzer.analyze(cfg);
@@ -156,8 +193,8 @@ public void postAnalyze(ControlFlowGraph cfg) {
       }
     }
 
-    super.postAnalyze(cfg);
     tempVarToTree.clear();
+    return cfg;
   }
 
   @Override

checker/tests/nullness/Issue6629.java

@@ -0,0 +1,22 @@
+package open.falsepos;
+
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Issue6629 {
+
+  void method(Stream<String> stream) {
+    stream
+        .map(
+            x -> {
+              Object o = new Object();
+              return new Other(o);
+            })
+        .collect(Collectors.toList());
+  }
+
+  static class Other {
+
+    public Other(Object o) {}
+  }
+}

checker/tests/nullness/flow/LambdaFlow.java

@@ -0,0 +1,89 @@
+package open;
+
+import java.util.function.Function;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+public class LambdaFlow {
+
+  void flowIntoLambda(@Nullable String param) {
+    if (param != null) {
+      Function<String, @NonNull String> b = s -> param;
+    }
+  }
+
+  void flowOutOfLambda(@Nullable String param) {
+    Function<String, @NonNull String> d =
+        s -> {
+          if (param != null) {
+            return param;
+          } else {
+            return "";
+          }
+        };
+  }
+
+  void flowOutOfLambda2(@Nullable String param) {
+    @NonNull String f =
+        foo(
+            s -> {
+              if (param != null) {
+                return param;
+              } else {
+                return "";
+              }
+            });
+  }
+
+  void flowIntoThenOutOfLambda(@Nullable String param) {
+    if (param != null) {
+      @NonNull String h = foo(s -> param);
+    }
+  }
+
+  // Same examples above but with a field instead of a parameter.
+
+  @Nullable String field;
+
+  void flowIntoLambdaField() {
+    if (field != null) {
+      Function<String, @NonNull String> a =
+          // :: error: (return)
+          s -> field;
+    }
+  }
+
+  void flowOutOfLambda() {
+    Function<String, @NonNull String> c =
+        s -> {
+          if (field != null) {
+            return field;
+          } else {
+            return "";
+          }
+        };
+  }
+
+  void flowOutOfLambda2() {
+    @NonNull String e =
+        foo(
+            s -> {
+              if (field != null) {
+                return field;
+              } else {
+                return "";
+              }
+            });
+  }
+
+  void flowIntoThenOutOfLambda() {
+    if (field != null) {
+      // :: error: (type.arguments.not.inferred) :: error: (assignment)
+      @NonNull String g = foo(s -> field);
+    }
+  }
+
+  <T> T foo(Function<String, T> func) {
+    return func.apply("");
+  }
+}

checker/tests/resourceleak/RLLambda.java

@@ -0,0 +1,59 @@
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.function.Function;
+import java.util.function.Predicate;
+import java.util.stream.Stream;
+import org.checkerframework.checker.builder.qual.CalledMethods;
+import org.checkerframework.checker.mustcall.qual.InheritableMustCall;
+import org.checkerframework.checker.mustcall.qual.MustCall;
+
+public class RLLambda {
+  @InheritableMustCall("a")
+  class Foo {
+    void a() {}
+
+    Foo b() {
+      return this;
+    }
+
+    void c(@CalledMethods("a") Foo this) {}
+  }
+
+  Foo makeFoo() {
+    return new Foo();
+  }
+
+  void innerfunc3() {
+
+    Foo f = makeFoo();
+    f.a();
+    Function<@MustCall Foo, @CalledMethods("a") @MustCall Foo> innerfunc =
+        st -> {
+          // :: error: (required.method.not.called)
+          Foo fn1 = new Foo();
+          Foo fn2 = makeFoo();
+          fn2.a();
+          // The need for this cast is undesirable, but is a consequence of our approach
+          // to generic types. In this case, this cast is clearly safe (a() has already
+          // been called, so the obligation is satisfied on the returned value, as
+          // intended).
+          // :: warning: cast.unsafe
+          return ((@MustCall Foo) fn2);
+        };
+
+    innerfunc.apply(f);
+  }
+
+  private void prepJdkFromFile(Path root) {
+    try (Stream<Path> walk = Files.walk(root)) {
+      Predicate<Path> pathPredicate = p -> false;
+    } catch (IOException e) {
+      throw new MyException("prepJdkFromFile", e);
+    }
+  }
+
+  public static class MyException extends RuntimeException {
+    public MyException(String s, Exception e) {}
+  }
+}