Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NullPointerException thrown for BugPattern ScheduledTransactionTrace when using error-prone-support on jbanking #626

Closed
2 tasks done
marcwrobel opened this issue May 14, 2023 · 6 comments
Labels
bug Something isn't working
Milestone

Comments

@marcwrobel
Copy link

Describe the bug

While experimenting Error Prone Support on jbanking I got a java.lang.NullPointerException in tech.picnic.errorprone.bugpatterns.ScheduledTransactionTrace.

Considering the ScheduledTransactionTrace bug pattern is located in error-prone-contrib I opened an issue here instead of https://github.com/google/error-prone.

  • I have verified that the issue is reproducible against the latest version
    of the project.
  • I have searched through existing issues to verify that this issue is not
    already known.

Minimal Reproducible Example

The code that triggered the issue is located at https://github.com/marcwrobel/jbanking/blob/error-prone/src/main/java/fr/marcwrobel/jbanking/calendar/DayOfWeekInMonthHoliday.java. The bug can be reproduced using https://github.com/marcwrobel/jbanking/tree/error-prone.

Logs
git clone [email protected]:marcwrobel/jbanking.git
cd jbanking
git checkout error-prone

(error-prone u=)$ mvn clean install
Apache Maven 3.9.2 (c9616018c7a021c1c39be70fb2843d6f5f9b8a1c)
Maven home: /home/mwrobel/.asdf/installs/maven/3.9.2
Java version: 17.0.7, vendor: Eclipse Adoptium, runtime: /home/mwrobel/.asdf/installs/java/temurin-17.0.7+7
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "6.1.0-0.deb11.6-amd64", arch: "amd64", family: "unix"
911 [INFO] Error stacktraces are turned on.
1001 [INFO] Scanning for projects...
1215 [INFO] 
1215 [INFO] -----------------------< fr.marcwrobel:jbanking >-----------------------
1215 [INFO] Building jbanking 4.2.0-SNAPSHOT
1215 [INFO]   from pom.xml
1215 [INFO] --------------------------------[ jar ]---------------------------------
1396 [INFO] 
1397 [INFO] --- clean:3.2.0:clean (default-clean) @ jbanking ---
1464 [INFO] Deleting /home/mwrobel/projects/jbanking/target
1465 [INFO] 
1466 [INFO] --- flatten:1.3.0:clean (flatten.clean) @ jbanking ---
1744 [INFO] Deleting /home/mwrobel/projects/jbanking/.flattened-pom.xml
1745 [INFO] 
1745 [INFO] --- enforcer:3.1.0:enforce (enforce-maven) @ jbanking ---
1855 [INFO] 
1855 [INFO] --- formatter:2.16.0:format (default) @ jbanking ---
2010 [INFO] Using 'UTF-8' encoding to format source files.
2039 [INFO] Number of files to be formatted: 98
3400 [INFO] Successfully formatted:          0 file(s)
3400 [INFO] Fail to format:                  0 file(s)
3400 [INFO] Skipped:                         98 file(s)
3400 [INFO] Read only skipped:               0 file(s)
3400 [INFO] Approximate time taken:          1s
3400 [INFO] 
3400 [INFO] --- resources:3.3.0:resources (default-resources) @ jbanking ---
3466 [INFO] Copying 0 resource
3467 [INFO] Copying 1 resource
3472 [INFO] 
3473 [INFO] --- flatten:1.3.0:flatten (flatten) @ jbanking ---
3485 [INFO] Generating flattened POM of project fr.marcwrobel:jbanking:jar:4.2.0-SNAPSHOT...
3548 [INFO] 
3548 [INFO] --- compiler:3.10.1:compile (default-compile) @ jbanking ---
3814 [INFO] Changes detected - recompiling the module!
3816 [INFO] Compiling 49 source files to /home/mwrobel/projects/jbanking/target/classes
6490 [INFO] -------------------------------------------------------------
6490 [WARNING] COMPILATION WARNING : 
6490 [INFO] -------------------------------------------------------------
6490 [WARNING] bootstrap class path not set in conjunction with -source 8
6490 [INFO] 1 warning
6490 [INFO] -------------------------------------------------------------
6491 [INFO] -------------------------------------------------------------
6491 [ERROR] COMPILATION ERROR : 
6491 [INFO] -------------------------------------------------------------
6491 [ERROR] /home/mwrobel/projects/jbanking/src/main/java/fr/marcwrobel/jbanking/calendar/DayOfWeekInMonthHoliday.java:[39,10] An unhandled exception was thrown by the Error Prone static analysis plugin.
     Please report this at https://github.com/google/error-prone/issues/new and include the following:
  
     error-prone version: 2.19.1
     BugPattern: ScheduledTransactionTrace
     Stack Trace:
     java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)" because "msym.visiblePackages" is null
  	at jdk.compiler/com.sun.tools.javac.code.Symtab.lookupPackage(Symtab.java:695)
  	at jdk.compiler/com.sun.tools.javac.code.Symtab.lookupPackage(Symtab.java:676)
  	at jdk.compiler/com.sun.tools.javac.code.ClassFinder.loadClass(ClassFinder.java:425)
  	at tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary.canLoadClass(ThirdPartyLibrary.java:91)
  	at tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary.isKnownClass(ThirdPartyLibrary.java:84)
  	at tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary.canIntroduceUsage(ThirdPartyLibrary.java:74)
  	at tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary.lambda$new$60d6954b$1(ThirdPartyLibrary.java:59)
  	at com.google.errorprone.VisitorState$Cache.get(VisitorState.java:702)
  	at tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary.isIntroductionAllowed(ThirdPartyLibrary.java:70)
  	at tech.picnic.errorprone.bugpatterns.ScheduledTransactionTrace.matchMethod(ScheduledTransactionTrace.java:55)
  	at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:449)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:739)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:150)
  	at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:953)
  	at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:86)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
  	at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:96)
  	at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:111)
  	at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:119)
  	at jdk.compiler/com.sun.source.util.TreeScanner.visitClass(TreeScanner.java:203)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:548)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:150)
  	at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:860)
  	at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:86)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
  	at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:111)
  	at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:119)
  	at jdk.compiler/com.sun.source.util.TreeScanner.visitCompilationUnit(TreeScanner.java:152)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:560)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:150)
  	at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:614)
  	at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:60)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:58)
  	at com.google.errorprone.scanner.ErrorProneScannerTransformer.apply(ErrorProneScannerTransformer.java:43)
  	at com.google.errorprone.ErrorProneAnalyzer.finished(ErrorProneAnalyzer.java:156)
  	at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:132)
  	at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1394)
  	at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1341)
  	at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:933)
  	at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.lambda$doCall$0(JavacTaskImpl.java:104)
  	at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.invocationHelper(JavacTaskImpl.java:152)
  	at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.doCall(JavacTaskImpl.java:100)
  	at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.call(JavacTaskImpl.java:94)
  	at org.codehaus.plexus.compiler.javac.JavaxToolsCompiler.compileInProcess(JavaxToolsCompiler.java:136)
  	at org.codehaus.plexus.compiler.javac.JavacCompiler.performCompile(JavacCompiler.java:182)
  	at org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:1209)
  	at org.apache.maven.plugin.compiler.CompilerMojo.execute(CompilerMojo.java:198)
  	at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:126)
  	at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2(MojoExecutor.java:342)
  	at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute(MojoExecutor.java:330)
  	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:213)
  	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:175)
  	at org.apache.maven.lifecycle.internal.MojoExecutor.access$000(MojoExecutor.java:76)
  	at org.apache.maven.lifecycle.internal.MojoExecutor$1.run(MojoExecutor.java:163)
  	at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute(DefaultMojosExecutionStrategy.java:39)
  	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:160)
  	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:105)
  	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:73)
  	at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:53)
  	at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:118)
  	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:261)
  	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:173)
  	at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:101)
  	at org.apache.maven.cli.MavenCli.execute(MavenCli.java:910)
  	at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:283)
  	at org.apache.maven.cli.MavenCli.main(MavenCli.java:206)
  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
  	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
  	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
  	at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:283)
  	at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:226)
  	at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:407)
  	at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:348)
6492 [INFO] 1 error
6492 [INFO] -------------------------------------------------------------
6492 [INFO] ------------------------------------------------------------------------
6493 [INFO] BUILD FAILURE
6493 [INFO] ------------------------------------------------------------------------
6494 [INFO] Total time:  5.559 s
6494 [INFO] Finished at: 2023-05-14T18:32:32+02:00
6494 [INFO] ------------------------------------------------------------------------

Expected behavior

A successful build, without NullPointerException.

Setup

  • Operating system : Debian GNU/Linux 11 (bullseye)
  • Java version : 17.0.7 (Eclipse Adoptium)
  • Error Prone version : 2.19.1
  • Error Prone Support version : 0.11.0

Additional context

None

@marcwrobel marcwrobel added the bug Something isn't working label May 14, 2023
@Stephan202
Copy link
Member

Tnx for the report @marcwrobel! I just checked out the repository and branch, and can indeed reproduce the issue. Will have a look!

@Stephan202
Copy link
Member

@marcwrobel to get you unblocked, the following patch avoids the issue (it disables the only three checks that rely on the ThirdPartyLibrary class):

diff --git a/pom.xml b/pom.xml
index bbd0c3e..e9677c7 100644
--- a/pom.xml
+++ b/pom.xml
@@ -96,6 +96,10 @@
               <arg>
                 -Xplugin:ErrorProne
                 <!-- More Error Prone flags on https://errorprone.info/docs/flags. -->
+                <!-- Workaround for https://github.com/PicnicSupermarket/error-prone-support/issues/626. -->
+                -Xep:CollectorMutability:OFF
+                -Xep:FluxImplicitBlock:OFF
+                -Xep:ScheduledTransactionTrace:OFF
               </arg>
               <arg>-XDcompilePolicy=simple</arg>
             </compilerArgs>

As for the proper fix: I see where the NPE happens, and we could avoid it using an extra if-statement, but the fix I have so far isn't strictly correct. (Because if I drop <scope>test</scope> from the project's Guava dependency declaration, then relevant field remains null, so testing for that would then cause ThirdPartyLibrary.GUAVA.isIntroductionAllowed(state) to return false, which would be incorrect.) Requires more thinking.

Stephan202 added a commit that referenced this issue May 14, 2023
When targeting Java 8, `unnamedModule` is not properly initialized,
causing an NPE when trying to load a class from it. In that context
`noModule` should be used instead.

Resolves #626.
@Stephan202
Copy link
Member

I filed #627, which should properly resolve the issue. Thanks again for reporting @marcwrobel.

NB: Good to keep in mind that this library is rather opinionated about use of Guava and AssertJ. I imagine that especially the former may not be a good fit for jbanking, as it is a zero-dependency library. It may help to omit the refaster-runner dependency for now, because for Refaster rules we currently do not have classpath-conditional logic. (Changing that is on our TODO list, but there's no ETA.)

@marcwrobel
Copy link
Author

Thanks for your prompt response @Stephan202.

NB: Good to keep in mind that this library is rather opinionated about use of Guava and AssertJ. I imagine that especially the former may not be a good fit for jbanking, as it is a zero-dependency library.

Yep, I just saw that. I am thinking about moving to AssertJ, so at least it may help for that migration.

@rickie rickie closed this as completed in 03af058 May 16, 2023
@Stephan202
Copy link
Member

Hey @marcwrobel, version 0.11.1 is now available on Maven Central :)

@Stephan202 Stephan202 added this to the 0.11.1 milestone May 16, 2023
@marcwrobel
Copy link
Author

Hi @Stephan202, just tested the new version and everything works fine now. Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants