UseStaticImport removes type in Javadoc links

[Bananeweizen]

UseStaticImport shortens Javadoc link tags if they match the given methodPattern. The link is broken afterwards, Javadoc does not resolve static imports.

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

    @Issue("https://github.com/openrewrite/rewrite/issues/3663")
    @Test
    void javadocLinkUnchanged() {
        rewriteRun(
          spec -> spec.recipe(new UseStaticImport("java.util.Collections emptyList()")),
          java(
            """
            import java.util.Collections;
            import java.util.List;

            public class WithJavadoc {
                /**
                 * This method uses {@link Collections#emptyList()}.
                 */
                public void mustNotChangeTheJavadocAbove() {
                    List<Object> list = Collections.emptyList();
                }
            }
            """,
            """
            import java.util.List;

            import static java.util.Collections.emptyList;

            public class WithJavadoc {
                /**
                 * This method uses {@link Collections#emptyList()}.
                 */
                public void mustNotChangeTheJavadocAbove() {
                    List<Object> list = emptyList();
                }
            }
            """
          )
        );
    }

What did you expect to see?

Only the call inside the method should change.

What did you see instead?

    import java.util.List;

    import static java.util.Collections.emptyList;

    public class WithJavadoc {
        /**
         * This method uses {@link emptyList()}.
         */
        public void mustNotChangeTheJavadocAbove() {
            List<Object> list = emptyList();
        }
    }

Both the Javadoc and the method call have changed. The Javadoc is now invalid and will lead to an "invalid reference" error during Javadoc generation.

Are you interested in contributing a fix to OpenRewrite?

No. I'm not sure how Javadoc is handled by the visitors.

[timtebeek]

Thanks for the detailed report! Indeed better to not update the JavaDoc here. Surprised we even did that originally. From the recipe implementation I don't immediately see how we're removing that either, but with your tests we can start to step through. https://github.com/openrewrite/rewrite/blob/2173a7125fd4360b71a998c70b6eb9b6431eedba/rewrite-java/src/main/java/org/openrewrite/java/UseStaticImport.java#L60-L76

[Bananeweizen]

I meanwhile know this happens because the JavadocVisitor detects a method reference and delegates to the JavaVisitor at https://github.com/openrewrite/rewrite/blob/e908b09f87d945f5ce0198a0f8f826347732864e/rewrite-java/src/main/java/org/openrewrite/java/JavadocVisitor.java#L271.

To fix this, the visitor for the recipe would have to be disabled temporarily as long as it's stack trace is inside the javadoc visit. So an additional class along these lines seems reasonable:

    private class UseStaticImportJavadocVisitor extends JavadocVisitor<ExecutionContext> {
        public UseStaticImportJavadocVisitor() {
            super(new JavaVisitor<>());
        }

        @Override
        public Javadoc visitDocComment(Javadoc.DocComment javadoc, ExecutionContext executionContext) {
            try {
                inJavadoc = true;
                return super.visitDocComment(javadoc, executionContext);
            }
            finally {
                inJavadoc = false;
            }
        }
    }

The inJavadoc field could now be used by the JavaVisitor.

@timtebeek However, I have not yet found an example how to combine 2 different visitors via composition to get one resulting visitor that visits the implementations of both. Is there some utility available or would this rather mean to fall back to implementing a single visitor that derives from the common super class of JavaVisitor and JavadocVisitor?

[timtebeek]

Thanks for figuring it out and picking up the fix!