RFR: 8284220: TypeMirror#toString omits enclosing class names after JDK-8281238 [v4]

Liam Miller-Cushon cushon at openjdk.java.net
Tue Apr 26 17:11:46 UTC 2022


On Tue, 26 Apr 2022 04:35:49 GMT, Joe Darcy <darcy at openjdk.org> wrote:

>> Liam Miller-Cushon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into JDK-8284220
>>  - Refactor test based on review feedback
>>  - 8284220: TypeMirror#toString omits enclosing class names after JDK-8281238
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java line 1046:
> 
>> 1044:                         buf.append(".");
>> 1045:                     }
>> 1046:                     ListBuffer<Name> names = new ListBuffer<>();
> 
> There may be a more idiomataic way to javac to construct this string, but offhand I don't know what it is.

I made this as idiomatic as I could, I think part of the issue here is that that the pretty-printing logic is operating on names at a slightly lower level than we normally do, so the existing ways of getting simple or qualified names aren't sufficient. Suggestions are welcome if you think of a way to clean this up at all.

> test/langtools/tools/javac/processing/model/type/AnnotatedTypeToString/AnnotatedTypeToString.java line 1:
> 
>> 1: /*
> 
> From personal experience with similar tests, they can look obscure when revisting them months in the future. I suggest putting a sentence or two description of what the test is trying to do. Also, if all the types can fit in one file, that can be easier to understand.

Thanks, I added some more context to the test `@summary` and also added javadoc on the processor, suggestions for additional documentation improvements are welcome.

> if all the types can fit in one file, that can be easier to understand.

I considered that, but `JavacTestingAbstractProcessor` is in the default packages, and I want the test to exercise printing types that are not in the default package, so I don't see a good way to put everything in the same file.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8084


More information about the compiler-dev mailing list