Use default method in Paranamer interface

[mjustin]

With one exception (NullParanamer), all implementations of the single-argument version of Paranamer.lookupParameterNames have the same implementation, delegating to the two argument version with true as the second argument. This is also a sensible default for user-defined Paranamer instances. Now that Paranamer requires a minimum of Java 8, it seems like a switch to a default method might be appropriate. This PR implements this change.

Incidentally, not a big deal if you don't want to go this route. Figured I'd throw it out there to see what your thoughts are on this approach.

That said, the fact that NullParanamer has different results for Paranamer.lookupParameterNames(methodOrConstructor, true) and Paranamer.lookupParameterNames (methodOrConstructor) does feel like a little bit of a code smell.

I have confirmed that this change does not break compilation of the build (mvn clean install -DskipTests=true). I have not confirmed that unit tests pass, since unit tests appear to be broken in master. I built on macOS 10.14.5 with Maven 3.8.2 & Java 8 using AdoptOpenJDK 1.8.0_282:

$ mvn clean install
[...]

[INFO] 
[INFO] --- maven-surefire-plugin:2.19.1:test (default-test) @ paranamer-generator ---

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.thoughtworks.paranamer.generator.OldQDoxParanamerTestCase
Tests run: 7, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.291 sec <<< FAILURE! - in com.thoughtworks.paranamer.generator.OldQDoxParanamerTestCase
testGenericClassGeneration(com.thoughtworks.paranamer.generator.OldQDoxParanamerTestCase)  Time elapsed: 0.106 sec  <<< FAILURE!
org.junit.ComparisonFailure: 
expected:<...bo 
elephantArrays E[],java.lang.String th...> but was:<...bo 
elephantArrays E[[]],java.lang.String th...>
	at com.thoughtworks.paranamer.generator.OldQDoxParanamerTestCase.testGenericClassGeneration(OldQDoxParanamerTestCase.java:92)

testSimpleClassGeneration(com.thoughtworks.paranamer.generator.OldQDoxParanamerTestCase)  Time elapsed: 0.026 sec  <<< FAILURE!
org.junit.ComparisonFailure: 
expected:<...nger 
longArray long[] longs 
setMap java....> but was:<...nger 
longArray long[[]] longs 
setMap java....>
	at com.thoughtworks.paranamer.generator.OldQDoxParanamerTestCase.testSimpleClassGeneration(OldQDoxParanamerTestCase.java:83)

Running com.thoughtworks.paranamer.generator.QDoxParanamerTestCase
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.016 sec <<< FAILURE! - in com.thoughtworks.paranamer.generator.QDoxParanamerTestCase
testFoo(com.thoughtworks.paranamer.generator.QDoxParanamerTestCase)  Time elapsed: 0.014 sec  <<< FAILURE!
org.junit.ComparisonFailure: 
expected:<...nger 
longArray long[] longs 
setMap java....> but was:<...nger 
longArray long[[]] longs 
setMap java....>
	at com.thoughtworks.paranamer.generator.QDoxParanamerTestCase.testFoo(QDoxParanamerTestCase.java:76)


Results :

Failed tests: 
  OldQDoxParanamerTestCase.testGenericClassGeneration:92 expected:<...bo 
elephantArrays E[],java.lang.String th...> but was:<...bo 
elephantArrays E[[]],java.lang.String th...>
  OldQDoxParanamerTestCase.testSimpleClassGeneration:83 expected:<...nger 
longArray long[] longs 
setMap java....> but was:<...nger 
longArray long[[]] longs 
setMap java....>
  QDoxParanamerTestCase.testFoo:76 expected:<...nger 
longArray long[] longs 
setMap java....> but was:<...nger 
longArray long[[]] longs 
setMap java....>

Tests run: 8, Failures: 3, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for ParaNamer Parent 2.8.1-SNAPSHOT:
[INFO] 
[INFO] ParaNamer Parent ................................... SUCCESS [  0.214 s]
[INFO] ParaNamer Generator ................................ FAILURE [  1.894 s]
[INFO] ParaNamer Maven plugin ............................. SKIPPED
[INFO] ParaNamer Core ..................................... SKIPPED
[INFO] ParaNamer Ant ...................................... SKIPPED
[INFO] ParaNamer Integration Tests ........................ SKIPPED
[INFO] ParaNamer IT 011: can use maven plugin defaults .... SKIPPED
[INFO] ParaNamer Distribution ............................. SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  2.693 s
[INFO] Finished at: 2021-08-28T21:43:54-05:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.19.1:test (default-test) on project paranamer-generator: There are test failures.
[ERROR] 
[ERROR] Please refer to /path/to/paranamer/paranamer-generator/target/surefire-reports for the individual test results.
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <args> -rf :paranamer-generator

[paul-hammant]

If we're changing the API we're bump to v3.0, right?

If we're changing the API for JDK 8 and above, why wouldn't developers use the JDK built-in way wholly instead of Paranamer?

[mjustin]

If we're changing the API we're bump to v3.0, right?

Changing an interface method to default doesn't break binary compatibility:

Adding a default method, or changing a method from abstract to default, does not break compatibility with pre-existing binaries, but may cause an IncompatibleClassChangeError if a pre-existing binary attempts to invoke the method. This error occurs if the qualifying type, T, is a subtype of two interfaces, I and J, where both I and J declare a default method with the same signature and result, and neither I nor J is a subinterface of the other.

In other words, adding a default method is a binary-compatible change because it does not introduce errors at link time, even if it introduces errors at compile time or invocation time. In practice, the risk of accidental clashes occurring by introducing a default method are similar to those associated with adding a new method to a non-final class. In the event of a clash, adding a method to a class is unlikely to trigger a LinkageError, but an accidental override of the method in a child can lead to unpredictable method behavior. Both changes can cause errors at compile time.

That being said, since it does introduce the (remote) possibility of error at linkage or compile time, it perhaps should not be done this lightly, especially since this change doesn't add much. I think I can see the value in not making the changes proposed by this PR.

[mjustin]

If we're changing the API for JDK 8 and above, why wouldn't developers use the JDK built-in way wholly instead of Paranamer?

I've since answered this question at #45 (comment)