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: us...@infra.apache.org