rewrite icon indicating copy to clipboard operation
rewrite copied to clipboard

`maybeAddImport` fails to recognise usages in `try` statements

Open greg-at-moderne opened this issue 10 months ago • 2 comments

What version of OpenRewrite are you using?

Current main = c5cc221bd0a335c98a5665f30a2316a33db1992e

What is the smallest, simplest way to reproduce the problem?

The following test case added to JavaVisitorTest:

    @Test
    void maybeAddImportInTry() {
        rewriteRun(
          spec -> spec
            .afterTypeValidationOptions(TypeValidation.none())
            .typeValidationOptions(TypeValidation.none())
            .recipes(toRecipe(() -> new JavaIsoVisitor<>() {
                @Override
                public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext p) {
                    if (method.getBody().getStatements().isEmpty()) {
                        maybeAddImport("java.io.PrintWriter");
                        JavaTemplate t = JavaTemplate.builder("try (PrintWriter pw = new PrintWriter(\"/tmp/file\")) { pw.println(\"A\"); }").build();
                        return t.apply(getCursor(), method.getCoordinates().replaceBody());
                    }
                    return super.visitMethodDeclaration(method, p);
                }
          })),
          java(
              """
              public class A {
                void test() {
                }
              }
              """,
              """
              import java.io.PrintWriter;
              
              public class A {
                void test() {
                    try (PrintWriter pw = new PrintWriter("/tmp/file")) {
                        pw.println("A");
                    }
                }
              }
              """
          )
      );
    }

What did you see instead?

Fails with no import added.

Workaround

Change the maybeAddImport line to:

                        maybeAddImport("java.io.PrintWriter", false);

and the test passes.

Context

It seems like org.openrewrite.java.AddImport#hasReference doesn't honour try statements.

greg-at-moderne avatar Mar 18 '25 11:03 greg-at-moderne

It looks like your JavaTemplate.builder is missing a call to imports; could that factor in here? Those snippets are compiled in isolation, so they need their classpath (beyond JDK) and imports added to the builder. The best way to troubleshoot these is through doBeforeParseTemplate(System.out::println)

timtebeek avatar Mar 18 '25 11:03 timtebeek

It looks like your JavaTemplate.builder is missing a call to imports; could that factor in here?

It does. But the problem is still relevant even if I don't use JavaTemplate. JavaTemplate seemed the easiest/smallest to put into the test.

Also the test fails with no import added with the following visitor code as well:

                    if (method.getBody().getStatements().isEmpty()) {
                        JavaTemplate t = JavaTemplate.builder("try (PrintWriter pw = new PrintWriter(\"/tmp/file\")) { pw.println(\"A\"); }")
                          .imports("java.io.PrintWriter")
                          .build();
                        return t.apply(getCursor(), method.getCoordinates().replaceBody());
                    }
                    return super.visitMethodDeclaration(method, p);

which I guess might be another problem.

greg-at-moderne avatar Mar 18 '25 11:03 greg-at-moderne

There appears to have been an issue with the test above; the JavaTemplate is missing .imports("java.io.PrintWriter"); this works:

    @Issue("https://github.com/openrewrite/rewrite/issues/5187")
    @Test
    void maybeAddImportInTryWithResourcesViaJavaTemplate() {
        rewriteRun(
          spec -> spec
            .recipes(toRecipe(() -> new JavaIsoVisitor<ExecutionContext>() {
                @Override
                public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext p) {
                    if (method.getBody() != null && method.getBody().getStatements().isEmpty()) {
                        maybeAddImport("java.io.PrintWriter", false);
                        JavaTemplate t = JavaTemplate.builder("try (PrintWriter pw = new PrintWriter(\"/tmp/file\")) { pw.println(\"A\"); }")
                          .imports("java.io.PrintWriter")
                          .build();
                        return t.apply(getCursor(), method.getCoordinates().replaceBody());
                    }
                    return super.visitMethodDeclaration(method, p);
                }
            })),
          java(
            """
            public class A {
                void test() {
                }
            }
            """,
            """
            import java.io.PrintWriter;
            
            public class A {
                void test() {
                    try (PrintWriter pw = new PrintWriter("/tmp/file")) {
                        pw.println("A");
                    }
                }
            }
            """
          )
        );
    }

timtebeek avatar Jul 09 '25 10:07 timtebeek