search

MutabilityDetector / MutabilityDetector

Lightweight analysis tool for detecting mutability in Java classes
Apache License 2.0
240 stars 76 forks source link

Does not work with jdk 10 #116

Closed joahuber closed 5 years ago

joahuber commented 6 years ago

Hi I tried to upgrade to jdk 10 but the check fails with an exception in the asm library. I tried to upgrade to the latest asm-version but it didn't help. I attached a zipped example project. I assume that the class file format doesn`t match anymore. But it seems to be bit more involved than just updating the libraries.

        at org.mutabilitydetector.checkers.UnhandledExceptionBuilder.unhandledException(UnhandledExceptionBuilder.java:60)
        at org.mutabilitydetector.checkers.CheckerRunner.attemptRecovery(CheckerRunner.java:104)
        at org.mutabilitydetector.checkers.CheckerRunner.run(CheckerRunner.java:72)
        at org.mutabilitydetector.checkers.AllChecksRunner.runCheckers(AllChecksRunner.java:71)
        at org.mutabilitydetector.ThreadUnsafeAnalysisSession.requestAnalysis(ThreadUnsafeAnalysisSession.java:125)
        at org.mutabilitydetector.ThreadUnsafeAnalysisSession.resultFor(ThreadUnsafeAnalysisSession.java:111)
        at org.mutabilitydetector.unittesting.MutabilityAsserter.getResultFor(MutabilityAsserter.java:190)
        at org.mutabilitydetector.unittesting.MutabilityAsserter.assertImmutable(MutabilityAsserter.java:108)
        at org.mutabilitydetector.unittesting.MutabilityAssert.assertImmutable(MutabilityAssert.java:672)
        at ch.hslu.plab.sw03.mutability.MutabilityTest.checkPersonIsImmutable(MutabilityTest.java:12)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:564)
        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
        at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252)
        at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141)
        at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:564)
        at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
        at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
        at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
        at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
        at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)
Caused by: java.lang.IllegalArgumentException
        at org.mutabilitydetector.internal.org.objectweb.asm.ClassReader.<init>(ClassReader.java:170)
        at org.mutabilitydetector.internal.org.objectweb.asm.ClassReader.<init>(ClassReader.java:153)
        at org.mutabilitydetector.internal.org.objectweb.asm.ClassReader.<init>(ClassReader.java:424)
        at org.mutabilitydetector.checkers.CheckerRunner.analyse(CheckerRunner.java:89)
        at org.mutabilitydetector.checkers.CheckerRunner.analyseFromClassLoader(CheckerRunner.java:85)
        at org.mutabilitydetector.checkers.CheckerRunner.run(CheckerRunner.java:69)
        ... 36 more

plab_public.zip

Grundlefleck commented 5 years ago

Hi, thanks for reporting.

I've taken a brief look and I think you are essentially stuck: an ASM upgrade needs to happen, but as a consumer of the library you can't do that upgrade because ASM is repackaged into the MutabilityDetector jar, into another namespace. There's no way, as far as I know, of getting around that, except by rebuilding Mutability Detector with a newer version.

I need to figure out a way to use ASM that isn't going to require a release on every Java version upgrade, because I can't really keep up with it. I will spend some time figuring out if I can delegate the choice of ASM version to the consumer of the library. That way the user can upgrade ASM when they also upgrade their JDK, not when I decide I've rolled out of bed the right way and decided I've found some time to cut another release :)

barclay-reg commented 5 years ago

Hi @Grundlefleck what about shading all dependencies, except asm? Another way could be to offer two distributions, one (full) shaded, another one without any shading.

joahuber commented 5 years ago

Hi @Grundlefleck I finally found some time to look into it again, I try to set some time apart to find a solution for it. If this very useful project should have a future, we better keep up with the crazy 6-month releases of Java.

Grundlefleck commented 5 years ago

Hey @joahuber, I've had a play around to see if there was anything sneaky that could be done to stick with one single distributed JAR while still allowing future JDK versions with no dev effort. I can't see a way to offer both "can keep up with Java" and "CLI usage with no dependencies" at the same time.

I think two distributions, as @barclay-reg suggests, one for unit test and runtime (no shading) and one for CLI (full shading) is the way to go. Or if anybody knows any really neat hacks with newer versions of Java where you can bootstrap your own dependencies from a script, that would be nice too, but we could figure that out later.

org.mutabilitydetector:MutabilityDetector should remain as the unit-testing, runtime dependency, org.mutabilitydetector:MutabilityDetector-cli should be created as a new artifact, that allows convenient running on the command line

@joahuber how's your POM skills? Do you think this approach is something you could work on, changing the project to be able to produce both artifacts from the same codebase?

joahuber commented 5 years ago

Hi @Grundlefleck Thanks for guiding me in the right direction. My Maven-Skills are a bit dated, but I think I will manage. What keeps me busy at the moment is the fact that the latest ASM version is not just a drop-in replacement for the currently used one. There are a couple of breaking changes which I first have to figure out, as well in your upstream project ASM-NonClassloadingExtensions. This fact will also limit the feasability of a shading-approach since this would only work for non-breaking changes. I'll keep you posted.

Grundlefleck commented 5 years ago

@joahuber in the meantime, until such times as the artifacts are split, I've released version 0.10.2 which is compatible up to JDK 11. It's already available in Maven Central.

... having done that, I'm now not so convinced that it will be possible to just let clients upgrade ASM underneath MutabilityDetector as a way to keep up to date with JDK releases. For example, in getting compatible with JDK11, I had to go to ASM 7, and to get to ASM I had to make changes to MutabilityDetector, so I don't know if it's as easy as I had previously thought. I will probably open another issue to ponder further, but for now a minor upgrade solves this issue.