nacx commented on a change in pull request #76:
URL: https://github.com/apache/jclouds/pull/76#discussion_r445645864
##########
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:
Instead of doing this, could we just do `if (!ctor.isAccessible())
ctor.setAccessible(true)`? Probably this is better for the general case and
makes the code safer (and this comment irrelevant)?
We can keep the core java class checks if needed, but it looks a bit safer
to just check if it is accessible, then try setting it. And if not accessible
and a core Java class, we can probably log a warning message as something is
probably gonna fail later as we can't set the proper access level tot he method?
----------------------------------------------------------------
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:
us...@infra.apache.org
