JnRouvignac commented on a change in pull request #76:
URL: https://github.com/apache/jclouds/pull/76#discussion_r445868817
##########
File path: core/src/main/java/org/jclouds/reflect/Reflection2.java
##########
@@ -153,15 +153,23 @@
.newBuilder().build(new CacheLoader<TypeToken<?>, Set<Invokable<?,
?>>>() {
public Set<Invokable<?, ?>> load(TypeToken<?> key) {
ImmutableSet.Builder<Invokable<?, ?>> builder =
ImmutableSet.<Invokable<?, ?>> builder();
- for (Constructor<?> ctor :
key.getRawType().getDeclaredConstructors()) {
- ctor.setAccessible(true);
+ Class<?> raw = key.getRawType();
+ for (Constructor<?> ctor : raw.getDeclaredConstructors()) {
+ if (!coreJavaClass(raw)) {
+ // In JDK 11 up to 14, the only uses for
`java.beans.ConstructorProperties` annotation
+ // are in the `java.awt`, `java.beans` and `javax.swing`
packages.
+ // Since these constructors are public, there is no need
to call `setAccessible()`
Review comment:
I read again the code and your comment, and I think you are saying I
could drop the last sentence of the comment and then do the following?
```java
if (!ctor.canAccess(null) && !coreJavaClass(raw)) {
// In JDK 11 up to 14, the only uses for
`java.beans.ConstructorProperties` annotation
// are in the `java.awt`, `java.beans` and
`javax.swing` packages.
ctor.setAccessible(true);
}
```
We need to test it, but that should work indeed. There would be less
spurious warnings for regular cases.
The check on `coreJavaClass()` is still required in our case though.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
