Update API version in visitors to ASM5

[retronym]

We are using "org.pantsbuild" % "jarjar" % "1.6.0" in the SBT build for Scala.

We see an error when we call JarJar with bytecode that contains the MethodParameters attribute.

This seems to be due to the fact that you updated to ASM5, but did not update all visitors in this project to the ASM5 api version.

The ASM5's ClassReader visits these parameters, ultimately with EmptyClassVisitor, which runs into:

    public void visitParameter(String name, int access) {
        if (api < Opcodes.ASM5) {
            throw new RuntimeException();
        }
        if (mv != null) {
            mv.visitParameter(name, access);
        }
    }

See:

https://docs.oracle.com/javase/tutorial/reflect/member/methodparameterreflection.html

for discussion of the MethodParameters classfile attribute. It is not enabled by default, one needs to enable it with:

javac -help 2>&1 | grep parameters
  -parameters                Generate metadata for reflection on method parameters

No test is included here as I'm not familiar with pants to know how to add this.

[retronym]

The stacktrace that we observed in the debugger:

java.lang.RuntimeException
    at org.objectweb.asm.MethodVisitor.visitParameter(Unknown Source)
    at org.objectweb.asm.MethodVisitor.visitParameter(Unknown Source)
    at org.objectweb.asm.ClassReader.b(Unknown Source)
    at org.objectweb.asm.ClassReader.accept(Unknown Source)
    at org.objectweb.asm.ClassReader.accept(Unknown Source)
    at org.pantsbuild.jarjar.KeepProcessor.process(KeepProcessor.java:70)
    at org.pantsbuild.jarjar.util.JarProcessorChain.process(JarProcessorChain.java:38)
    at org.pantsbuild.jarjar.MainProcessor.process(MainProcessor.java:115)

Without the debugger, we just saw:

Error reading scala/tools/nsc/interpreter/jline/FileBackedHistory$.class: null
Error reading scala/tools/nsc/interpreter/jline/FileBackedHistory.class: null

Due to the way that the exception handling in JarJar employs the try { _ } catch (Exception e) err.println(e.getMessage) error handling antipattern.

[stuhood]

Thanks for the PR @retronym !

If you're able to extend one of the test classes in src/test/java/org/pantsbuild/jarjar/integration/ to cover this path, it would be much appreciated. If not, that's fine too. You can run those tests with: ./pants test src/test/java/org/pantsbuild/jarjar/integration ...and you can run all of the tests with :: (a recursive target glob): ./pants test ::

[retronym]

@stuhood I tried to test drive the change in https://github.com/pantsbuild/jarjar/pull/19

I'm not totally happy with the resulting test, but have exhausted my budget of time, so have submitted as is.

[stuhood]

Thanks! Went ahead and merged #19