NPE while sorting imports using v1.4.1

[fralalonde]

[ERROR] Failed to execute goal net.revelc.code:impsort-maven-plugin:1.4.1:sort (default-cli) on project SbiComCnPat: Execution default-cli of goal net.revelc.code:impsort-maven-plugin:1.4.1:sort failed.: NullPointerException -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal net.revelc.code:impsort-maven-plugin:1.4.1:sort (default-cli) on project SbiComCnPat: Execution default-cli of goal net.revelc.code:impsort-maven-plugin:1.4.1:sort failed.
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:215)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:156)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:148)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117)
    at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call (MultiThreadedBuilder.java:190)
    at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call (MultiThreadedBuilder.java:186)
    at java.util.concurrent.FutureTask.run (FutureTask.java:266)
    at java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:511)
    at java.util.concurrent.FutureTask.run (FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:624)
    at java.lang.Thread.run (Thread.java:748)
Caused by: org.apache.maven.plugin.PluginExecutionException: Execution default-cli of goal net.revelc.code:impsort-maven-plugin:1.4.1:sort failed.
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:148)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:210)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:156)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:148)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117)
    at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call (MultiThreadedBuilder.java:190)
    at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call (MultiThreadedBuilder.java:186)
    at java.util.concurrent.FutureTask.run (FutureTask.java:266)
    at java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:511)
    at java.util.concurrent.FutureTask.run (FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:624)
    at java.lang.Thread.run (Thread.java:748)
Caused by: java.lang.NullPointerException
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0 (Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance (NativeConstructorAccessorImpl.java:62)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance (DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance (Constructor.java:423)
    at java.util.concurrent.ForkJoinTask.getThrowableException (ForkJoinTask.java:598)
    at java.util.concurrent.ForkJoinTask.reportException (ForkJoinTask.java:677)
    at java.util.concurrent.ForkJoinTask.invoke (ForkJoinTask.java:735)
    at java.util.stream.ReduceOps$ReduceOp.evaluateParallel (ReduceOps.java:714)
    at java.util.stream.AbstractPipeline.evaluate (AbstractPipeline.java:233)
    at java.util.stream.ReferencePipeline.reduce (ReferencePipeline.java:546)
    at net.revelc.code.impsort.maven.plugin.AbstractImpSortMojo.execute (AbstractImpSortMojo.java:251)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:137)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:210)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:156)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:148)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117)
    at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call (MultiThreadedBuilder.java:190)
    at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call (MultiThreadedBuilder.java:186)
    at java.util.concurrent.FutureTask.run (FutureTask.java:266)
    at java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:511)
    at java.util.concurrent.FutureTask.run (FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:624)
    at java.lang.Thread.run (Thread.java:748)
Caused by: java.lang.NullPointerException
    at java.lang.String.split (String.java:2337)
    at java.lang.String.split (String.java:2422)
    at net.revelc.code.impsort.ImpSort.parseFile (ImpSort.java:85)
    at net.revelc.code.impsort.maven.plugin.AbstractImpSortMojo.lambda$execute$3 (AbstractImpSortMojo.java:228)
    at java.util.stream.ReferencePipeline$3$1.accept (ReferencePipeline.java:193)
    at java.util.stream.ReferencePipeline$3$1.accept (ReferencePipeline.java:193)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept (ForEachOps.java:183)
    at java.util.stream.ReferencePipeline$3$1.accept (ReferencePipeline.java:193)
    at java.util.Spliterators$ArraySpliterator.forEachRemaining (Spliterators.java:948)
    at java.util.stream.AbstractPipeline.copyInto (AbstractPipeline.java:482)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto (AbstractPipeline.java:472)
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential (ForEachOps.java:150)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential (ForEachOps.java:173)
    at java.util.stream.AbstractPipeline.evaluate (AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.forEach (ReferencePipeline.java:485)
    at java.util.stream.ReferencePipeline$7$1.accept (ReferencePipeline.java:272)
    at java.util.Spliterators$ArraySpliterator.forEachRemaining (Spliterators.java:948)
    at java.util.stream.AbstractPipeline.copyInto (AbstractPipeline.java:482)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto (AbstractPipeline.java:472)
    at java.util.stream.ReduceOps$ReduceTask.doLeaf (ReduceOps.java:747)
    at java.util.stream.ReduceOps$ReduceTask.doLeaf (ReduceOps.java:721)
    at java.util.stream.AbstractTask.compute (AbstractTask.java:327)
    at java.util.concurrent.CountedCompleter.exec (CountedCompleter.java:731)
    at java.util.concurrent.ForkJoinTask.doExec (ForkJoinTask.java:289)
    at java.util.concurrent.ForkJoinPool$WorkQueue.runTask (ForkJoinPool.java:1056)
    at java.util.concurrent.ForkJoinPool.runWorker (ForkJoinPool.java:1692)
    at java.util.concurrent.ForkJoinWorkerThread.run (ForkJoinWorkerThread.java:157)

[fralalonde]

Going back to v1.3.0 of the plugin solves the issue. I do not know what source file is causing the error but could look into it.

Suggested enhancement: Any error related to a specific source file should display the path of the offending file and even the offending line if possible.

[fralalonde]

impsort plugin configured with <joinStaticWithNonStatic>true</joinStaticWithNonStatic> and runs with

mvn -e -T4 -Dimpsort.removeUnused=true net.revelc.code:impsort-maven-plugin:sort

[ctubbsii]

Do you have example code to test this with (preferably in the form of the minimal Maven project with the plugin configured) to reproduce this error?

[dwalluck]

Sorry for all these notifications. I think I found the issue and I was working on a PR for it. It happens when a file is empty. As this is a valid Java file, we shouldn't throw an exception of it. I was able to solve that.

But, then I got to thinking, what about a one-line Java file without a line-ending? This is a valid Java file, too, but I am unsure how to handle it, as the code really works off of the assumption that a newline is a delimiter, rather than, say, a semicolon.

So that part didn't work, but perhaps I should split that into a separate PR and submit the one for zero-length files.

[ctubbsii]

@dwalluck Perhaps it'd be better if this maven plugin simply skipped any files where the detected line ending is UNKNOWN. In any such case that I can think of, there wouldn't be any valid import lines for this plugin to sort. If there are zero lines, there is nothing to do. If there's one line without a line ending, then it is one of the following cases:

1. It has a bunch of statements on a single line, and should probably be run through something like the formatter-maven-plugin first, or
2. It has no import statements, in which case this plugin has nothing to do, or
3. It has an import statement, but no code, in which case, there's nothing for this to do (other than possibly remove the import).

So, we could just emit a warning for files where the detected line ending is UNKNOWN and just immediately skip it.

[dwalluck]

The way the code is currently written, parseFile() must return a Result. I reordered the statements so that it won't try to split a null string and it will return importDeclarations.isEmpty() == true for zero-length file.

We could reorder it to do the line-ending detection first, but then we also must change LineEnding to handle a zero-length file and return UNKNOWN. Finally, I think some logic must be earlier, in the mojo itself (at least, this is the only file that has access to the Maven log to actually log a warning currently).

I think it would be OK to skip any file where the ending is UNKNOWN (or missing). What does the code currently do for a file that has multiple import statements on one line? If it can't handle this case either, I guess we must skip it for now, or it requires some code rewrite.

[ctubbsii]

We could reorder it to do the line-ending detection first, but then we also must change LineEnding to handle a zero-length file and return UNKNOWN. Finally, I think some logic must be earlier, in the mojo itself (at least, this is the only file that has access to the Maven log to actually log a warning currently).

@dwalluck I think it already detects and returns UNKNOWN. I think the NullPointerException is when it tries to do LineEnding.getChars() and passes null to file.split on line 85.

[ctubbsii]

I think it would be OK to skip any file where the ending is UNKNOWN (or missing). What does the code currently do for a file that has multiple import statements on one line? If it can't handle this case either, I guess we must skip it for now, or it requires some code rewrite.

We already don't handle this case well. See #1 ; in any case, such a situation should probably pass through a formatter first, before using this plugin.

[ctubbsii]

The way the code is currently written, parseFile() must return a Result. I reordered the statements so that it won't try to split a null string and it will return importDeclarations.isEmpty() == true for zero-length file.

I haven't seen your code, since you haven't created a pull request yet. But, we can have special logic that returns a special Result object or throw a special exception type that doesn't fail the entire execution. It's the smallest change I can think of. But, I can take a look at your proposed changes if you open a PR. Feel free to leave it as a "Draft" PR, if you're still experimenting.

[ctubbsii]

Fixed in #33 , but now it fails differently. See discussion there for proposed feature to ignore failures.