Skip to content

[java] Fix NPE in MethodTypeResolution#3269

Merged
oowekyala merged 4 commits into
pmd:masterfrom
adangel:fix-NPE-methodtyperesolution
May 9, 2021
Merged

[java] Fix NPE in MethodTypeResolution#3269
oowekyala merged 4 commits into
pmd:masterfrom
adangel:fix-NPE-methodtyperesolution

Conversation

@adangel

@adangel adangel commented May 7, 2021

Copy link
Copy Markdown
Member

Describe the PR

In one of the last pmd regression reports, strange new errors appeared, e.g.
https://chunk.io/pmd/0263b81c16b8417b993b6ea93f55d7be/diff/spring-framework/index.html#section-errors

Caused by: java.lang.NullPointerException
	at net.sourceforge.pmd.lang.java.typeresolution.MethodTypeResolution.isSubtypeable(MethodTypeResolution.java:692)
	at net.sourceforge.pmd.lang.java.typeresolution.MethodTypeResolution.isSubtypeable(MethodTypeResolution.java:656)
	at net.sourceforge.pmd.lang.java.typeresolution.MethodTypeResolution.selectMethodsFirstPhase(MethodTypeResolution.java:165)
	at net.sourceforge.pmd.lang.java.typeresolution.MethodTypeResolution.getBestMethodReturnType(MethodTypeResolution.java:381)
	at net.sourceforge.pmd.lang.java.typeresolution.ClassTypeResolver.visit(ClassTypeResolver.java:445)
	at net.sourceforge.pmd.lang.java.ast.ASTName.jjtAccept(ASTName.java:38)

Also other exceptions occurred, e.g.

Caused by: java.lang.TypeNotPresentException: Type javax.money.MonetaryAmount not present
	at java.base/sun.reflect.generics.factory.CoreReflectionFactory.makeNamedType(CoreReflectionFactory.java:117)
	at java.base/sun.reflect.generics.visitor.Reifier.visitClassTypeSignature(Reifier.java:125)
	at java.base/sun.reflect.generics.tree.ClassTypeSignature.accept(ClassTypeSignature.java:49)
	at java.base/sun.reflect.generics.visitor.Reifier.reifyTypeArguments(Reifier.java:68)
	at java.base/sun.reflect.generics.visitor.Reifier.visitClassTypeSignature(Reifier.java:138)
	at java.base/sun.reflect.generics.tree.ClassTypeSignature.accept(ClassTypeSignature.java:49)
	at java.base/sun.reflect.generics.repository.MethodRepository.computeReturnType(MethodRepository.java:75)
	at java.base/sun.reflect.generics.repository.MethodRepository.getReturnType(MethodRepository.java:66)
	at java.base/java.lang.reflect.Method.getGenericReturnType(Method.java:292)
	at net.sourceforge.pmd.lang.java.typeresolution.MethodTypeResolution.getTypeDefOfMethod(MethodTypeResolution.java:525)
	at net.sourceforge.pmd.lang.java.typeresolution.MethodTypeResolution.getApplicableMethods(MethodTypeResolution.java:475)
	at net.sourceforge.pmd.lang.java.typeresolution.ClassTypeResolver.getLocalApplicableMethods(ClassTypeResolver.java:480)
	at net.sourceforge.pmd.lang.java.typeresolution.ClassTypeResolver.visit(ClassTypeResolver.java:404)
	at net.sourceforge.pmd.lang.java.ast.ASTName.jjtAccept(ASTName.java:38)

Other types seem to be missing, too: Type org.reactivestreams.Publisher not present, Type org.junit.runner.Runner not present, Type javax.servlet.http.Part not present, Type org.eclipse.jetty.websocket.api.extensions.ExtensionConfig not present.

So something seems to be wrong with the auxclasspath the pmd-regression-tester is using.

This PR does two things now:

  1. It caches the missing types and just logs them. This helps that PMD doesn't crash anymore in case the auxclasspath is not complete. I could reproduce the case here: https://github.com/adangel/scratchpad/tree/master/pmd-npe-methodtyperesolution

It has one big downside, though: We don't see these problems anymore.... (only indirectly maybe through false positives/false negatives)

  1. Adding the .gradle/caches directory to github actions caches. I'm guessing, that this is the root cause, why the auxclasspath is not complete anymore: With refactoring the build scripts, I added ~/work/pmd/target/repositories as a cache directory. This contains the checkouts of pmd-regression-tester, e.g. the spring-framework. Now this has been cached, which means, we don't need to rebuild spring-framework again - but spring uses gradle... and gradle doesn't use the standard maven local repository cache. So I guess, the classpath.txt file created here
    ./gradlew createSquishClasspath -q > classpath.txt
    is also cached, which skips rebuilding spring here
    if test -e classpath.txt; then
    which means, that we didn't download any dependencies for spring...
    Not a problem for checkstyle, because they use maven and ~/.m2/repository is already cached.

Oh, I also renamed the cache-key and restore-key to make sure, no old cache is used. Old caches are not complete and can't be used anymore.

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by github actions)
  • Added (in-code) documentation (if needed)

@adangel adangel added the a:bug PMD crashes or fails to analyse a file. label May 7, 2021
@adangel adangel added this to the 6.35.0 milestone May 7, 2021
@ghost

ghost commented May 7, 2021

Copy link
Copy Markdown
1 Message
📖 This changeset changes 0 violations,
introduces 1877 new violations, 0 new errors and 0 new configuration errors,
removes 2264 violations, 15 errors and 0 configuration errors.
Full report
This changeset changes 0 violations,
introduces 1877 new violations, 0 new errors and 0 new configuration errors,
removes 2264 violations, 15 errors and 0 configuration errors.
Full report

Generated by 🚫 Danger

@oowekyala oowekyala left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

It has one big downside, though: We don't see these problems anymore.... (only indirectly maybe through false positives/false negatives)

It's still in the log though. I think this is fine.

@oowekyala oowekyala merged commit dca0bf9 into pmd:master May 9, 2021
@adangel adangel deleted the fix-NPE-methodtyperesolution branch May 14, 2021 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:bug PMD crashes or fails to analyse a file.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants