openrewrite / rewrite

Automated mass refactoring of source code.
https://docs.openrewrite.org
Apache License 2.0
2.11k stars 314 forks source link

NPE on CatchClauseOnlyRethrows line 67 when run against JReleaser #2589

Closed timtebeek closed 1 year ago

timtebeek commented 1 year ago

Wanted to run the common static analysis recipes against JReleaser after seeing he recently added SonarCloud scanning. Figured see if I can help out (with automation) as we met a few times over the summer; but after struggling a bit with the Gradle setup I ran up against a NPE in CatchClauseOnlyRethrows on line 67.

Steps to reproduce:

  1. git clone git@github.com:jreleaser/jreleaser.git
  2. cd jreleaser
  3. sdk use java 11.0.17-tem (Java 17 fails, so no helpful NPEs)
  4. cat > init.gradle (instructions with some changes)

    initscript {
      repositories {
          maven { url "https://plugins.gradle.org/m2" }
      }
    
      dependencies {
          classpath("org.openrewrite:plugin:5.33.2")
      }
    }
    
    addListener(new BuildInfoPluginListener())
    
    allprojects {
      project.afterEvaluate {
          if (!project.plugins.hasPlugin(org.openrewrite.gradle.RewritePlugin)) {
              project.plugins.apply(org.openrewrite.gradle.RewritePlugin)
          }
      }
    }
    
    class BuildInfoPluginListener extends BuildAdapter {
    
      def void projectsLoaded(Gradle gradle) {
          Project root = gradle.getRootProject()
          if (!"buildSrc".equals(root.name)) {
              root.allprojects {
                  apply {
                      apply plugin: org.openrewrite.gradle.RewritePlugin
                  }
              }
          }
      }
    }
  5. ./gradlew -d --init-script init.gradle rewriteRun -Drewrite.activeRecipes=org.openrewrite.java.cleanup.CommonStaticAnalysis

Expected: a successful run with maybe some changes.

Actual:

2023-01-03T13:25:46.848+0100 [INFO] [org.openrewrite.gradle.isolated.ResourceParser] Finished parsing 92 other sources from /home/tim/Documents/workspace/jreleaser in 146 milliseconds (1 millisecond per source)
2023-01-03T13:25:46.848+0100 [WARN] [org.openrewrite.gradle.isolated.DefaultProjectParser] There were problems parsing 3 sources:
2023-01-03T13:25:46.848+0100 [WARN] [org.openrewrite.gradle.isolated.DefaultProjectParser]   build.gradle
2023-01-03T13:25:46.848+0100 [WARN] [org.openrewrite.gradle.isolated.DefaultProjectParser]   jreleaser.yml
2023-01-03T13:25:46.848+0100 [WARN] [org.openrewrite.gradle.isolated.DefaultProjectParser]   buildSrc/src/main/groovy/org/jreleaser/gradle/shadow/ConfigureShadowRelocation.groovy
2023-01-03T13:25:46.848+0100 [WARN] [org.openrewrite.gradle.isolated.DefaultProjectParser] Execution will continue but these files are unlikely to be affected by refactoring recipes
2023-01-03T13:25:46.848+0100 [LIFECYCLE] [org.openrewrite.gradle.isolated.DefaultProjectParser] All sources parsed, running active recipes: org.openrewrite.java.cleanup.CommonStaticAnalysis
2023-01-03T13:25:51.239+0100 [LIFECYCLE] [org.openrewrite.gradle.RewriteRunTask] 
2023-01-03T13:25:51.239+0100 [WARN] [org.openrewrite.gradle.RewriteRunTask] Error during rewrite run
org.openrewrite.internal.RecipeRunException: java.lang.NullPointerException
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:327)
        at org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:359)
        at org.openrewrite.java.JavaVisitor.visitMethodDeclaration(JavaVisitor.java:813)
        at org.openrewrite.java.JavaIsoVisitor.visitMethodDeclaration(JavaIsoVisitor.java:225)
        at org.openrewrite.java.JavaIsoVisitor.visitMethodDeclaration(JavaIsoVisitor.java:31)
        at org.openrewrite.java.tree.J$MethodDeclaration.acceptJava(J.java:3365)
        at org.openrewrite.java.tree.J.accept(J.java:64)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:279)
        at org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:359)
        at org.openrewrite.java.JavaVisitor.visitRightPadded(JavaVisitor.java:1258)
        at org.openrewrite.java.JavaVisitor.lambda$visitBlock$4(JavaVisitor.java:366)
        at org.openrewrite.internal.ListUtils.lambda$map$0(ListUtils.java:141)
        at org.openrewrite.internal.ListUtils.map(ListUtils.java:123)
        at org.openrewrite.internal.ListUtils.map(ListUtils.java:141)
        at org.openrewrite.java.JavaVisitor.visitBlock(JavaVisitor.java:365)
        at org.openrewrite.java.JavaIsoVisitor.visitBlock(JavaIsoVisitor.java:94)
        at org.openrewrite.java.cleanup.CatchClauseOnlyRethrows$1.visitBlock(CatchClauseOnlyRethrows.java:62)
        at org.openrewrite.java.cleanup.CatchClauseOnlyRethrows$1.visitBlock(CatchClauseOnlyRethrows.java:58)
        at org.openrewrite.java.cleanup.CatchClauseOnlyRethrows_1_GroovyVisitor.visitBlock(CatchClauseOnlyRethrows_1_GroovyVisitor.zig:81)
        at org.openrewrite.java.tree.J$Block.acceptJava(J.java:756)
        at org.openrewrite.java.tree.J.accept(J.java:64)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:279)
        at org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:359)
        at org.openrewrite.java.JavaVisitor.visitClassDeclaration(JavaVisitor.java:450)
        at org.openrewrite.java.tree.J$ClassDeclaration.acceptJava(J.java:1183)
        at org.openrewrite.java.tree.J.accept(J.java:64)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:279)
        at org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:359)
        at org.openrewrite.groovy.GroovyVisitor.lambda$visitCompilationUnit$0(GroovyVisitor.java:48)
        at org.openrewrite.internal.ListUtils.lambda$map$0(ListUtils.java:141)
        at org.openrewrite.internal.ListUtils.map(ListUtils.java:123)
        at org.openrewrite.internal.ListUtils.map(ListUtils.java:141)
        at org.openrewrite.groovy.GroovyVisitor.visitCompilationUnit(GroovyVisitor.java:48)
        at org.openrewrite.groovy.GroovyVisitor.visitJavaSourceFile(GroovyVisitor.java:41)
        at org.openrewrite.groovy.tree.G$CompilationUnit.acceptGroovy(G.java:173)
        at org.openrewrite.groovy.tree.G.accept(G.java:43)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:279)
        at org.openrewrite.RecipeScheduler.lambda$scheduleVisit$4(RecipeScheduler.java:222)
        at org.openrewrite.RecipeScheduler.lambda$mapAsync$0(RecipeScheduler.java:53)
        at org.openrewrite.scheduling.ForkJoinScheduler.lambda$schedule$0(ForkJoinScheduler.java:41)
        at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.exec(ForkJoinTask.java:1407)
        at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
        at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
        at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
        at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
        at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)
Caused by: java.lang.NullPointerException
        at org.openrewrite.java.cleanup.CatchClauseOnlyRethrows$1.lambda$visitBlock$1(CatchClauseOnlyRethrows.java:67)
        at org.openrewrite.internal.ListUtils.lambda$flatMap$1(ListUtils.java:199)
        at org.openrewrite.internal.ListUtils.flatMap(ListUtils.java:153)
        at org.openrewrite.internal.ListUtils.flatMap(ListUtils.java:199)
        at org.openrewrite.java.cleanup.CatchClauseOnlyRethrows$1.visitBlock(CatchClauseOnlyRethrows.java:63)
        at org.openrewrite.java.cleanup.CatchClauseOnlyRethrows$1.visitBlock(CatchClauseOnlyRethrows.java:58)
        at org.openrewrite.java.tree.J$Block.acceptJava(J.java:756)
        at org.openrewrite.java.tree.J.accept(J.java:64)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:279)
        ... 45 more

Aside from a fix for the NPE, it would be helpful to include in the output some reference to which file caused the issue in the first place. Especially when running on pre-helpful NPE versions of Java, that could help troubleshoot the problem and fix.

knutwannheden commented 1 year ago

Could even be interesting to catch and wrap the exception where the visit methods get called and then augment the wrapping exception with the line number of the object given to the visit method?

timtebeek commented 1 year ago

That could be a good way to do it indeed. As it stands such exceptions can be hard to debug.

timtebeek commented 1 year ago

Not sure what lines in jreleaser caused these issues, but the problem no longer occurs when I run it now. We could try to recreate the problem by checking out an old copy, and then use the new details about the visited element on exceptions if we really want to. But it I suggest we close this one for now, and wait for it to resurface if it ever does.