NoWhitespaceAfter errors handling array with @NotNull

[XenoAmess]

package sample;

import org.jetbrains.annotations.NotNull;

public class ArrayNotNull {

    byte[] bytes = new byte[0];

    public byte @NotNull [] getBytes() {
        return bytes;
    }

    int[] ints = new int[0];

    public int @NotNull [] getInts() {
        return ints;
    }

    Object[] objects = new Object[0];

    public Object @NotNull [] getObjects() {
        return objects;
    }

}

becomes something that cannot compile; replicated in #2910

package sample;
  
  import org.jetbrains.annotations.NotNull;
  
  public class ArrayNotNull {
  
      byte[] bytes = new byte[0];
  
      public byte @NotNullbyte @NotNull [] getBytes() {
          return bytes;
      }
  
      int[] ints = new int[0];
  
      public int @NotNullint @NotNull [] getInts() {
          return ints;
      }
  
      Object[] objects = new Object[0];
  
      public Object @NotNullObject[]getObjectstObjects() {
          return objects;
      }
  
  }

[mccartney]

After debugging, the bug doesn't seem to be with NoWhitespaceAfterStyle.
The test from #2910 fails the same way with:

          spec -> spec.parser(JavaParser.fromJavaVersion()),

[timtebeek]

Briefly explored here by setting a breakpoint in org.openrewrite.PrintOutputCapture#append(java.lang.String) to trace where the duplicated strings are coming from, using the dontWronglyHandleArray test. I briefly saw a prefix of tObject come by in a prefix, so there's likely a mix of issues with the parser and printer.

[sfc-gh-dtran]

After some further digging and stepping: it looks like part of the problem could lie in ReloadableJava17ParserVisitor.visitArrayType visitor: the parser incorrectly assumes that anything between the array type and the brackets (e.g. byte []) is whitespace, when there could be an annotation (e.g. byte @NotNull []).

As a result ReloadableJava17ParserVisitor.convert returns j = 'byte @NotNull []' for a given input tree t = byte[] (which I assume is incorrect, since convert should return a string representation consistent with the tree representation)

[timtebeek]

Thanks for looking into it some more @sfc-gh-dtran ; That indeed looks odd to see @NotNull as the whitespace on the array dimension, and a likely source of the issues that we're seeing. Maybe we'd need to ensure the cursor is read past that annotation location?

[sfc-gh-dtran]

I think the decision to be made is here in JavaPrinter.visitAnnotatedType:

public J visitAnnotatedType(AnnotatedType annotatedType, PrintOutputCapture<P> p) {
    beforeSyntax(annotatedType, Space.Location.ANNOTATED_TYPE_PREFIX, p);
    visit(annotatedType.getAnnotations(), p);
    visit(annotatedType.getTypeExpression(), p);
    afterSyntax(annotatedType, p);
    return annotatedType;
}

When annotatedType is byte @NotNullbyte @NotNull [], the semantic problems of the parser are that:

• annotatedType.annotations
  • .annotationType = NotNull
  • .prefix.whitespace = "byte "
• annotatedType.typeExpression.dimensions[0].element.whitespace = " @NotNull "

so that when visit(annotatedType.getAnnotations(), p) is called, byte @NotNull is printed, followed by visit(annotatedType.getTypeExpression(), p) which appends byte @NotNull []. A logical fix should agree with section 9.7.4 in these docs

I think what needs to be decided is what the following fields should look like for a type @A Object @B [] @C []:

• annotatedType.annotations
• annotatedType.typeExpression.dimensions (ArrayList<JRightPadded>)
  • (do we need to add a new annotation member to each element of annotatedTypeExpression.dimensions to contain information of the annotation for each dimension? Currently @B and @C is stored under dimensions[0].element.whitespace, but semantically this doesn't make much sense since it isn't whitespace