MJAVADOC-485 Upgrade to commons-lang3

[marschall]

Patch for https://issues.apache.org/jira/browse/MJAVADOC-485

[petteyg]

1. Why not lang3?
2. Why import a class that isn't used anywhere in the file?

[marschall]

Must have missed this.

[Tunaki]

The plugin is compatible with Java 6, but Commons Lang 3.6 requires Java 7. We can only upgrade to 3.5 for now.

[marschall]

3.6 added quite a lot of support for Java 9, do you know how well 3.6 works with Java 9?

[marschall]

Since Maven 3.3+ require JDK 1.7 or above to execute is there a chance that the Java 6 requirement can be lifted any time soon? Java 6 went end of life over four years ago.

[Tunaki]

The support for Java 6 is because the plugin can be used with all Maven 3.x versions. Unless there is a strong reason to require Java 7 (like a 3rd party dependency that has to be updated to a version requiring Java 7 in order to fix bugs or implement new features), it'd be preferable to keep with this compability for now.

[marschall]

I checked 3.5 with JDK 9 ea 175 and it seems to work fine.

[petteyg]

Still importing a class that is not used anywhere in the code.

[Tunaki]

This raises an exception when building the project on 9+175 (with mvn clean verify -Prun-its for example)

Caused by: java.util.regex.PatternSyntaxException: Unrecognized version of Javadoc: '9' near index 47
(?s).*?[^a-zA-Z]([0-9]+\.?[0-9]*)(\.([0-9]+))?.*
                                               ^
	at org.apache.maven.plugin.javadoc.JavadocUtil.parseJavadocVersion(JavadocUtil.java:674)

This method is intended to parse the output of javadoc -J-fullversion and it works for Javadoc 9+175 (output being java full version "9+175"). I think a new float parseJavaVersion(String) is needed, that works for the output of java -version. Or perhaps we should get rid of all those float comparisons and rely on Commons Lang JavaVersion (and atLeast).

[michael-o]

@Tunaki Unfortunately, JavaVersion works with floats too. How will this make it any better? Moreover, on needs to extract the version string first before passing to JavaVersion.

[Tunaki]

@michael-o I just noticed there is a new Runtime.Version class in Java 9 which can parse and compare Java version strings. Do you think we can use with reflection in place of JavaVersion?

But as far as upgrading to commons-lang3, having a new method float parseJavaVersion(String), that is similar to parseJavadocVersion but with the regex being ([0-9]+\\.?[0-9]*)(\\.([0-9]+)), looks like the way to go. We can look at using Runtime.Version or not later.

[michael-o]

I wouldn't use it for two reasons:

1. I don't like reflection personally, it is just ugly code
2. It work 9+ only

We can of course add more logic to Commons Lang parsing arbitrary strings from 6 to 9.

[marschall]

@Tunaki refactoring to JavaVersion would also have been my preference but that's tricky for two reasons

1. AbstractJavadocMojo.version is Maven container injected, I don't know how we could customise the injection process Mapping Complex Objects does not seem to be a solution
2. Unfortunately both the parsing code and the code for accessing the parsed float is not public in commons-lang3. We ultimately need the parsed float in AbstractJavadocMojo.addStandardDocletOptions.

Initially I copy and pasted the parsing code from commons-lang 2.6 but that was quite a bit of code.

[rfscholte]

I think we need to get rid of the float, in the end it will cause issues (java versions were never floats). Instead we need a Comparable JavaVersion (just like Aether/Artifact Resolver has). Is should be as simple as parsing the String to a JavaVersion instance and use its compareTo.