RFR: 8254023: A module declaration is not allowed to be a target of an annotation that lacks an @Target meta-annotation

[Joel Borggrén-Franck]

On Wed, 9 Dec 2020 06:35:56 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> the change looks good but I'm surprised that no other tests are failing and need changes, in particular tests like:  `test/langtools/tools/javac/annotations/repeatingAnnotations/combo/TargetAnnoCombo.java`
>
>> @vicente-romero-oracle It is difficult to identify which test should be changed, because all the tests passed. As you said, the `TargetAnnoCombo.java` maybe need to be changed. I want to revise it now. Should I do it?
> 
> sure please, I think that you should not only check that that test passes but also modify it, explicitly adding cases for the MODULE target. As a general comment this change is simple but the implications are not. I think the patch needs to add more tests. We also need to test that annotation processors can `see` annotations applied to modules, etc. We also need tests that check that target-less annotations, applied to modules are actually represented as such in the corresponding class file.

Hi,

I don't think TargetAnnoCombo should be updated, at least not for this fix. It is an old hard to understand test that predates modules.

It would be good to do further testing as Vicente writes. I suggest:

1) A test that the module info compiles, you already have this.
2) A test that the class file contains the bytes for the annotation might make sense. As Jon has pointed out in different review threads there is the toolbox, perhaps that could be of use? There are some examples of toolbox tests in  `jdk/test/langtools/tools/javac/modules` though I don't have a good example of one that verifies the contents of the class file. Perhaps @vicente-romero-oracle can point to an example? 
3) There should already be tests that annotation processing can pick up annotations on module declarations, it looks like `jdk/test/langtools/tools/javac/modules/AnnotationProcessing.java` contains some of this. See if you can add a case with an annotation without @Target.

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

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