Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotation lookup on parameter in inner class constructor fails when using javac from JDK versions prior to 9 [SPR-16652] #21193

Closed
spring-projects-issues opened this issue Mar 28, 2018 · 6 comments
Assignees
Labels
in: core status: backported type: bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Mar 28, 2018

Sam Brannen opened SPR-16652 and commented

Overview

An issue in JUnit Jupiter has brought it to my attention that there is a bug in javac in JDK versions prior to JDK 9. The bug was fixed in JDK 9 (see linked JDK bug issues below).

JDK Bugs

For example, if an inner class is compiled using javac on JDK 8 and its constructor has parameters that are annotated, then a lookup for such annotations via java.lang.reflect.Parameter might fail with an exception similar to the following (if the constructor declares only one argument).

Caused by: java.lang.ArrayIndexOutOfBoundsException: 1
	at java.lang.reflect.Parameter.getDeclaredAnnotations(Parameter.java:305)
	at java.lang.reflect.Parameter.declaredAnnotations(Parameter.java:342)
	at java.lang.reflect.Parameter.getAnnotation(Parameter.java:287)
	at java.lang.reflect.AnnotatedElement.isAnnotationPresent(AnnotatedElement.java:258)

If the constructor declares multiple arguments, looking up an annotation on a parameter will actually search for the annotation on the parameter to its left in the constructor signature.

The following is a quote from one of the JDK bugs explaining the root cause.

Root cause is javac generates RuntimeVisibleParameterAnnotations without an annotation slot for the synthetic/mandated parameters.

Workaround

For JUnit Jupiter I have come up with the following workaround for finding the AnnotatedElement on which annotations should be searched.

private AnnotatedElement getAnnotatedElement() {
	Executable executable = getDeclaringExecutable();

	// Take into account a bug in javac in JDK 8:
	if (executable instanceof Constructor //
			&& isInnerClass(executable.getDeclaringClass()) //
			&& (executable.getParameters().length == executable.getParameterAnnotations().length + 1)) {
		return executable.getParameters()[this.index - 1];
	}

	return this.parameter;
}

Note that the above code snippet comes from the implementation of ParameterContext in JUnit Jupiter which is somewhat analogous to Spring's MethodParameter.

Also, please keep in mind that the Parameter returned by the above method could literally be the Parameter for the preceding constructor parameter... except that it sees the annotations from the subsequent parameter. Thus, it should only be used to look up annotations: it should not be used to retrieve the parameter's name, type, etc.

Deliverables

  • Revise annotation lookup methods in MethodParameter so that annotations are properly discovered on parameters in constructors for inner classes compiled using JDK < 9.

Affects: 4.3.14, 5.0.4

Issue Links:

Referenced from: commits 9244090, 5f6b042, 4b9e3a9, 53d0139

Backported to: 4.3.15

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 28, 2018

Sam Brannen commented

See (currently disabled) failing test introduced in the following commit.

4b9e3a9

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 28, 2018

Sam Brannen commented

Juergen Hoeller, I know this is not technically a bug in Spring, but I imagine some people might deem it as such since Spring usually works a lot of magic when it comes to annotations.

But feel free to change it to "improvement" if you like. ;-)

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 28, 2018

Sam Brannen commented

Oh yeah... in case you are currently running tests in spring-core using JDK 9+ in your IDE, you'll need to configure MethodParameterTests.annotatedConstructorParameterInInnerClass() to be compiled/run using Java 8 in order to see the failure.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 28, 2018

Juergen Hoeller commented

I've implemented a similar check comparing executable.getParameterCount() to executable.getParameterAnnotations().length and lowering the index by 1 when accessing the annotation array (i.e. nevertheless operating on executable.getParameterAnnotations(), never calling executable.getParameters() at all). And for this extra parameter 0 on an inner class, we always return an empty annotation array.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 29, 2018

Sam Brannen commented

Sounds good! (y)

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 29, 2018

Sam Brannen commented

Thanks for the quick turnaround, Juergen Hoeller!

I'll get to work on #21194 ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: backported type: bug
Projects
None yet
Development

No branches or pull requests

2 participants