Lombok: JavaParser fails with StringIndexOutOfBoundsException when lombok processor is enabled

[amishra-u]

What version of OpenRewrite are you using?

• OpenRewrite 8.41.2

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

I haven't debugged the exact root cause yet. the issue seems to occur when annotations are added or removed in the source code. This appears to result in incorrect character positions for the cursor.
(Will add more detail about the issue after debugging)

  @Getter
  @Builder
  public static class Request {
    @Nonnull private final List<TransportJobId> transportJobIds;
    @Nullable private final UnixTimeMillis estimatedReadyAt;
    @Nonnull private final RegionId regionId;
    @Nonnull private final boolean wasTriggeredFromFulfillmentOrder;
  }

What did you expect to see?

A valid compilation unit should be generated for the source file.

What did you see instead?

A ParseError instance is generated for the source file instead.

What is the full stack trace of any errors you encountered?

java.lang.StringIndexOutOfBoundsException: begin 5327, end 5315, length 5434
  java.base/java.lang.String.checkBoundsBeginEnd(String.java:3319)
  java.base/java.lang.String.substring(String.java:1874)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitAnnotation(ReloadableJava17ParserVisitor.java:143)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitAnnotation(ReloadableJava17ParserVisitor.java:72)
  com.sun.tools.javac.tree.JCTree$JCAnnotation.accept(JCTree.java:2601)
  com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convert(ReloadableJava17ParserVisitor.java:1604)
  ...

Are you interested in [contributing a fix to OpenRewrite]

Yes.

[knutwannheden]

This is basically the reason we haven't enabled the Lombok processor yet. It appears like it doesn't always adjust the offsets in the AST correctly. I suspect that us a bug in Lombok. So either we need to get it fixed there or we need to adjust our parser so that it tracks the cursor in the file without relying on the AST.

[knutwannheden]

I think we are actually very close to have this working, we just didn't find the time to complete it.

[knutwannheden]

If you feel like taking a look at it, I believe that this is where we need to fix things to no longer rely on the position from the AST:

rewrite/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java

Line 1682 in abb80e1

String prefix = source.substring(cursor, Math.max(cursor, getActualStartPosition((JCTree) t)));

[amishra-u]

Able to reproduce this bug with below unit test, @nonnull to field to cause the error.

    @Test
    void lombokBug() {
        rewriteRun(
          java(
            """
              import lombok.Getter;
              import javax.annotation.Nonnull;

              @Getter
              public class Foo {
                @Nonnull private final String foo;
              }
              """
          )
        );
    }

[timtebeek]

Thanks for the runnable example @amishra-u ! That should help zero in on the issue. Did you by chance look at a fix as well?

[amishra-u]

Thanks for the runnable example @amishra-u ! That should help zero in on the issue. Did you by chance look at a fix as well?

Yes, I’ve identified the root cause. It’s not an OpenRewrite bug, but rather an issue with the Lombok annotation processor. I added a workaround in OpenRewrite, which resolved the problem. I’m also looking into fixing the underlying issue in Lombok itself and will publish a PR soon.

Lombok incorrectly updates the annotation type’s position on a field to the variable’s position in certain scenarios. If you closely look after image it has correct position for annotation but only the annotation type position is updated to the variable’s position.
Before annotation processor:

After annotation processor:

[timtebeek]

Awesome work! Thanks for diving in and posting the outcome here. Keep us posted on any progress with regards to a workaround or fix upstream.

[amishra-u]

This method is buggy, it doesn't make deep copy.
https://github.com/projectlombok/lombok/blob/master/src/core/lombok/javac/handlers/JavacHandlerUtil.java#L2111

[knutwannheden]

This is also what I observed, but I never analyzed it in detail. Fixing this upstream would be awesome! Hopefully that will then address all such errors in Lombok.