diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceStackWithDeque.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceStackWithDeque.java index e0509264c..5dd664146 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceStackWithDeque.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceStackWithDeque.java @@ -55,6 +55,16 @@ public TreeVisitor getVisitor() { @Override public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { J.VariableDeclarations.NamedVariable v = super.visitVariable(variable, ctx); + if (v.getInitializer() == null) { + return v; + } + + Expression initializer = (Expression) new ChangeType("java.util.Stack", "java.util.ArrayDeque", false) + .getVisitor().visitNonNull(v.getInitializer(), ctx, getCursor().getParentOrThrow()); + if (initializer == v.getInitializer()) { + // Not a `Stack`, so skip the data flow analysis below, which is costly on every variable in the file + return v; + } DataFlowSpec returned = new DataFlowSpec() { @Override @@ -68,9 +78,8 @@ public boolean isSink(DataFlowNode sinkNode) { } }; - if (v.getInitializer() != null && FindLocalFlowPaths.noneMatch(getCursor(), returned)) { - v = v.withInitializer((Expression) new ChangeType("java.util.Stack", "java.util.ArrayDeque", false) - .getVisitor().visitNonNull(v.getInitializer(), ctx, getCursor().getParentOrThrow())); + if (FindLocalFlowPaths.noneMatch(getCursor(), returned)) { + v = v.withInitializer(initializer); getCursor().putMessageOnFirstEnclosing(J.VariableDeclarations.class, "replace", true); } diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceStackWithDequeTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceStackWithDequeTest.java index 2c2a92a7e..fa6ae3086 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceStackWithDequeTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceStackWithDequeTest.java @@ -209,4 +209,45 @@ void test(java.util.List result) { ); } + @Issue("https://github.com/openrewrite/rewrite-analysis/issues/113") + @Test + void doNotFailOnDataFlowThroughNestedMethodInvocations() { + // Regression: dataflow was run for every initialized variable in the file, not just the `Stack`. + // On `Optional.ofNullable(a).orElse(opt.get())` the flow engine ping-ponged between the + // "argument to select" and "select to argument" steps until the stack overflowed. + rewriteRun( + //language=java + java( + """ + import java.util.Optional; + import java.util.Stack; + + class Test { + String test(Optional opt) { + Stack stack = new Stack<>(); + stack.push(1); + String a = "x"; + return Optional.ofNullable(a).orElse(opt.get()); + } + } + """, + """ + import java.util.ArrayDeque; + import java.util.Deque; + import java.util.Optional; + import java.util.Stack; + + class Test { + String test(Optional opt) { + Deque stack = new ArrayDeque<>(); + stack.push(1); + String a = "x"; + return Optional.ofNullable(a).orElse(opt.get()); + } + } + """ + ) + ); + } + }