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