RFR: 8043226: Better diagnostics for non-applicable type annotations [v5]

[Vicente Romero]

On Fri, 3 May 2024 17:00:15 GMT, Liam Miller-Cushon <cushon at openjdk.org> wrote:

>> Hi,
>> 
>> Please consider this improvement to diagnostics on inadmissable type annotations.
>> 
>> [JDK-8057683: clarify error messages trying to annotate scoping](https://bugs.openjdk.org/browse/JDK-8057683) is closely related to JDK-8043226 and I think could be made a duplicate of that bug. [JDK-8057683: improve ordering of errors with type annotations](https://bugs.openjdk.org/browse/JDK-8057683) is also closely related, and this PR partially fixes the issues described in that bug.
>> 
>> I have some notes on details of the proposed changes below.
>> 
>> ---
>> 
>> Currently javac reports 'scoping construct cannot be annotated' diagnostics for type annotations at locations where they are not admissable. 
>> 
>> As discussed in [JDK-8043226](https://bugs.openjdk.org/browse/JDK-8043226) and [JDK-8057683](https://bugs.openjdk.org/browse/JDK-8057683), the current language is unclear. The 'scoping construct' language was used in JSR-308 discussions but didn't end up in the final specification, JLS §9.7.4 talks about where annotations are 'admissable', and that language is mirrored in JVMS §4.7.20.2.
>> 
>> This change updates the diagnostics to state that type annotations 'not admissible at this location' and removed the reference to 'scoping constructs'. Additionally, the diagnostic now includes an explanation of a location in the type where annotations would be admissible, to make it easier to understand how to annotate qualified type names.
>> 
>> Before:
>> 
>> 
>> test/langtools/tools/javac/annotations/typeAnnotations/failures/CantAnnotateScoping.java:38: error: scoping construct cannot be annotated with type-use annotation: @TA
>>     @TA Outer.SInner osi;
>>              ^
>> 
>> 
>> After:
>> 
>> 
>> test/langtools/tools/javac/annotations/typeAnnotations/failures/CantAnnotateScoping.java:38: error: type annotations are not admissible at this location: @TA
>>     @TA Outer.SInner osi;
>>              ^
>>   (to annotate a qualified type, write Outer. at TA SInner)
>> 
>> 
>> ---
>> 
>> Attribution currently assumes `@TA java.lang.Object` in `List<@TA java.lang.Object>` will be a type (not a package), and reports a resolution failure when it can't find a class named `java`.
>> 
>> This change modifies attribution to search for packages as well as types when resolving annotated types, and relies on the subsequent error checking for annotated types to report that type annotations are not admissible at that location.
>> 
>> This also improves the ordering of the diagnostic in the output, since...
>
> 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 six additional commits since the last revision:
> 
>  - Adjust diagnostic message based on review feedback
>  - Merge branch 'master' into JDK-8043226
>  - Merge branch 'master' into JDK-8043226
>  - Merge branch 'master' into JDK-8043226
>  - Update src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>    
>    Co-authored-by: Werner Dietl <wdietl at gmail.com>
>  - 8043226: Better diagnostics for non-applicable type annotations

lgtm

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

Marked as reviewed by vromero (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16592#pullrequestreview-2094806961