Add parameter-related coherent API methods

[sarxos]

Hello Jandex Team!

Right now Jandex API for MethodInfo does not expose MethodParameterInfo directly. Some w/a for this limitation is to create custom class which acts the same way as the MethodParameterInfo, but why do we have to reinvent the wheel if we can add missing methods directly in Jandex API.

There were some discussions in quarkusio/quarkus#8054 and quarkusio/quarkus#2493 and this PR is to mitigate the root issue, i.e. it is to add missing methods in MethodInfo and MethodParameterInfo classes and, as the result, make Jandex API more user friendly.

@mkouba This is to address some of the problems we discussed in issues mentioned above.

To summarise. This PR adds the following methods:

• MethodInfo.parametersList() - list method parameters (infos not types)
• MethodInfo.methodAnnotations() - list method annotations (without param annotations)
• MethodParameterInfo.annotations() - list annotations defined on parameter
• MethodParameterInfo.annotation(DotName) - get parameter annotation with given name
• MethodParameterInfo.hasAnnotation(DotName) - is parameter annotated with given annotation

Unit tests included.

Please let me know in case of any issues.

[sarxos]

@n1hility @mkouba Please let me know if you have any comments in regards to this PR.

[mkouba]

I like the idea. One thing we should probably think through is whether to cache the list of MethodParameterInfos for the MethodInfo.parametersList() method?

[sarxos]

Hi @mkouba

I thought about this as well. My idea was to create all lists in constructors, but I dropped it because: 1) it seemed to be a bit redundant since we create it even if user won't use it, and 2) we pass this as the argument to MethodParameterInfo before constructor is completed, which is an antipattern. The other option would be to lazy initialize this list on the first use but this would break class immutability (only declared in javadoc, not enforced by finals, not sure why) and I'm not a great fan of having volatile fields or synchronization constructs when it's not really necessary.

[mkouba]

the argument to MethodParameterInfo before constructor is completed, which is an antipattern

I wouldn't consider this a problem.

only declared in javadoc, not enforced by finals, not sure why

Some of the jandex constructs are effectively immutable, i.e. the state is modified after it's created but before it's published.

and I'm not a great fan of having volatile fields or synchronization constructs when it's not really necessary

+1

[sarxos]

@mkouba Ok then, if you don't see it as a problem, then I won't too. I will send new patchset where I initialize these list in constructors.

[mkouba]

@sarxos we should probably wait for review from @n1hility ;-).

[sarxos]

@mkouba Never mind :) I just tried and it's impossible to have all these lists cached. We can cache some, but not all. For example when I try to initialize the list in MethodParameterInfo constructor I'm getting NPE because MethodInfo.methodInternal is not initialized yet. This is due to how reading is done in InderReaderV2. I will leave this code as it is.

[MikeEdgar]

+1

It may also be useful for the MethodParameterInfo class to implement equals and hashCode so that it equates to the same logical parameters obtained by other means.

[Ladicek]

#64 has a discussion of a similar API proposal.

[Ladicek]

Superseded by #182.