JDK 12 RFR of JDK-8190886: package-info handling in RoundEnvironment.getElementsAnnotatedWith

[joe darcy]

Hello,

I had hoped that writing a few more test to cover the modules cases 
would be a simple exercise, but that is not how things worked out!

The resulting set of changes is now:

     http://cr.openjdk.java.net/~darcy/8190886.6/

Major differences between the prior version sent for review are in the files

src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacRoundEnvironment.java
test/langtools/tools/javac/processing/environment/round/TestElementsAnnotatedWith.java

The earlier iteration of the fix was adequate for package-info files; 
however, there are several complications related to modules the previous 
fix did not account for.

The getElementsAnnotatedWith methods with a java.lang.Class parameter 
need some way to bridge between the core reflection world and the 
compile-time reflection world of javax.lang.model. The way to do this is 
to construct a TypeElement for an annotation type corresponding to the 
Class object for the annotation type argument by mapping from the name 
of the annotation type to a type element via 
Elements.getTypeElement(name). A wrinkle is that the behavior of 
Elements.getTypeElement(name) was redefined in JDK 9 to account for 
modules since a name can correspond to multiple types post-modules. The 
revised fix first tries Elements.getTypeElement(name) and if that fails, 
including failing due to multiple results being found, it fails over to 
the two-argument version of Elements.getTypeElement which has a module 
parameter. The module parameter is set to a module whose name matches 
the name of the module of the Class, if such a Class is in a named module.

On the testing sides, as discussed in JEP 200 when the compiler is 
running in single-module mode (which includes when compiling a 
module-info file), all the files being compiled are assumed to be in 
that module even if they would not be in the module by virtue of their 
file system locations. This necessitates some of the testing conditions 
being conditionally excluded.

I'll leave any extension of this test to multi-module compilation mode 
for future work.

Thanks,

-Joe


On 7/24/2018 10:00 AM, Vicente Romero wrote:
> looks good, just minimal copyright for test:
>
> test/langtools/tools/javac/processing/environment/round/pkg/package-info.java 
>
>
> should be 2018
>
> Vicente
>
> On 07/24/2018 02:50 AM, joe darcy wrote:
>> Hello,
>>
>> Please review the changes to address
>>
>>     JDK-8190886: package-info handling in 
>> RoundEnvironment.getElementsAnnotatedWith
>>     http://cr.openjdk.java.net/~darcy/8190886.1/
>>
>> Contrary to the spec, the RoundEnvironment.getElementsAnnotatedWith 
>> implementation was traversing into packages (and modules). Given the 
>> phase of JDK 11 and at least the potential for behavioral 
>> compatibility impact, I'm proposing to fix this in 12 rather than 11. 
>> I'll file a CSR for the behavioral change in due course.
>>
>> The implementation change is small, just adding overrides of visitor 
>> methods which skip scanning packages and modules. To allow use of 
>> AnnotatedElementInfo in compilation units in a named package, the 
>> AnnotatedElementInfo type has to itself be in a named package rather 
>> than an unnamed one; therefore, a number of supporting changes are 
>> needed to other test files in the directory to support that type rename.
>>
>> The tests have not been updated to verify the behavior on modules, 
>> but that could be done too.
>>
>> I've verified the updated test fails with a promoted build of JDK 12 
>> but passes with a build with these javac modifications. All other 
>> langtools tests pass on a modified build too.
>>
>> I'll perform a copyright pass before pushing once the rest of the 
>> contents are reviewed.
>>
>> Thanks,
>>
>> -Joe
>>
>