Proposal: Allow annotations on module directives

[Ron Pressler]

Before getting to any implementation issue, you haven’t explained what problem you’re trying to solve and why it’s a big-enough problem to merit solving. Getting to the bottom of that is usually the hardest (and longest) part of any feature.

— Ron

> On 11 Nov 2024, at 16:30, Alessandro Autiero <alautiero at gmail.com> wrote:
> 
> Summary
> This proposal suggests allowing annotations on module directives in a module-info, similar to how module declarations can currently be annotated. This enhancement, while narrow in scope, would give developers more flexibility when designing libraries deeply integrated with Java's built-in module system. I have built a proof of concept on Github which is already fully working and completely backwards compatible. I still haven't:
>     • Build automated integration tests
>     • Added support for the new ClassFile API 
> Before doing that though I wanted to collect some feedback on my implementation, especially on:
>     • Whether this change makes sense to the maintainers
>     • The new bytecode instructions I introduced and whether better alternatives exist
>     • The uses() accessor in ModuleDescriptor
> All changes I made are documented in the respective commits I made, but if you have any questions please let me know.
> 
> Bytecode
> My implementation introduces two new bytecode attributes:
>     • ModuleDirectivesRuntimeInvisibleAnnotations
>     • ModuleDirectivesRuntimeVisibleAnnotations
> which both contain, for each type of module directive, the number of directives that were annotated and for each directive that was annotated:
>     • The index of the directive relative to all the directives of the same type that are present in the module
>     • The number of annotations that were attached to the directive
>     • The metadata of the annotation in the standard format used in the bytecode
> 
> Before working on this implementation, I had never designed bytecode instructions so my approach might be suboptimal, so if any better alternatives exist, I'll implement the changes as soon as possible. Other approaches I considered:
>     • Using RuntimeInvisibleAnnotations and RuntimeVisibleAnnotations 
> Module directives are stored in the Module attribute alongside the module name, flags and version, instead of in a specialized table like class fields or local variables. For this reason, they cannot declare their own attributes(nested attributes are not supported by the JVM). 
>     • Modifying the Module attribute to contain the annotations directly in the existing directives metadata
> While this approach doesn't require additional bytecode attributes, it breaks completely existing tooling that parses a module-info because of the same constraints explained for the previous approach.
> In conclusion, while my proposed implementation is fully backwards compatible, it introduces two new bytecode attributes.
> 
> Public API changes
> All changes are backwards-compatible, but I've included some reasoning on some design choices I made.
> javax.lang.model.element.ModuleElement$Directive
>     • 
> Implement AnnotatedConstruct to support annotations during compile-time processing
>     • Directive does not currently implement Element and cannot do so in the future as it already defines a method named getKind which clashes with Element's getKind because of their different return types(DirectiveKind and ElementKind respectively). Implementing Element would also require ElementVisitor to implement DirectiveVisitor. I don't think that it's necessary for Directive to implement Element as it wasn't intended to do so when Java 9 was introduced, so I see no reason or backwards-compatible way to do so.
> 
> java.lang.module.ModuleDescriptor
>     • 
> Added a new sealed common class named Directive that Exports, Requires, Opens, Uses and Provides extend. This class contains an annotations() accessor to allow callers to access annotations
>     • Added a new Uses class that represents uses directives. This introduces a slight backwards compatibility issue as an accessor named uses() already exists in ModuleDescriptor and it returns a Set<String> instead of a Set<Uses>. For now I added an accessor named used() that returns a Set<Uses>, but usedServices could be a better name. I don't think that changing uses() to return Set<Uses> is an option as that would break backwards compatibility and having an odd-named accessor shouldn't be that much of an issue if we find a fitting name.
>