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

Fix #116 : Java 16 support for ReflectionUtils #123

Merged

Conversation

axel3rd
Copy link
Contributor

@axel3rd axel3rd commented Jan 4, 2021

Proposal for #116. It is more a workaround, could break backward compatibility.

If updating fields accessibility becomes really forbidden in Java 16 for "Class", the usage of Reflection should be updated according that.
Functionally, the impact would be minor. Generally the usage of Reflection is on "personal / business" classes, not for concrete use case directly on "Class".

@axel3rd axel3rd marked this pull request as draft January 8, 2021 18:51
@axel3rd axel3rd force-pushed the fix-116-java16-support-workaround branch from cad7c2a to 0fbb275 Compare January 10, 2021 18:53
@axel3rd axel3rd marked this pull request as ready for review January 10, 2021 18:56
@axel3rd axel3rd requested a review from mkarg January 10, 2021 18:56
@@ -218,6 +219,14 @@ private static void gatherVariablesAndValuesIncludingSuperclasses( Object object

Class<?> clazz = object.getClass();

if ( Integer.parseInt( System.getProperty( "java.specification.version" ).split( "\\." )[0] ) >= 11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have anything better in place here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have anything better in place here?

Float.parseFloat( System.getProperty( "java.specification.version" ) )

But Runtime.Version (doc) cannot be used, exist since Java 9 (plexus-utils always compatible with Java 7). And without adding some third-part tooling as dependencies, I don't see another way 😢.

I will update commit with Float instead Integer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will update commit with Float instead Integer.

Done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-o : Or did you want a better implementation around System.getProperty( "java.specification.version" ) if value is null/blanck/notParsable ?

@axel3rd axel3rd force-pushed the fix-116-java16-support-workaround branch from 0fbb275 to 0fa66d5 Compare January 10, 2021 21:54
@axel3rd axel3rd requested a review from michael-o January 10, 2021 21:54
@axel3rd
Copy link
Contributor Author

axel3rd commented Feb 26, 2021

Is this PR not relevant ? 😢

@@ -1,5 +1,7 @@
package org.codehaus.plexus.util;

import java.lang.reflect.AccessibleObject;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why this import has been moved here?

@olamy
Copy link
Member

olamy commented Apr 4, 2021

I guess we do not have much choice..
I'm ok to merge it.
@michael-o ?

@michael-o
Copy link
Member

I guess we do not have much choice..
I'm ok to merge it.
@michael-o ?

No opinion, go ahead.

@olamy
Copy link
Member

olamy commented Apr 4, 2021

@axel3rd yes it is still relevant. Just please fix the import and we're good to go! Thanks

@axel3rd axel3rd force-pushed the fix-116-java16-support-workaround branch from 0fa66d5 to 4e4c88a Compare April 4, 2021 18:10
@axel3rd
Copy link
Contributor Author

axel3rd commented Apr 4, 2021

@axel3rd yes it is still relevant. Just please fix the import and we're good to go! Thanks

Good catch for the import 😕, done.

@olamy olamy merged commit 6f11ee1 into codehaus-plexus:master Apr 5, 2021
@axel3rd axel3rd deleted the fix-116-java16-support-workaround branch April 5, 2021 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants