MPMD-288 Fixed NullPointerException #11
Conversation
this line throws NPE when file.list is null https://docs.oracle.com/javase/8/docs/api/java/io/File.html#list-- For me this occurs on jar files like `~/.m2/repository/org/apache/logging/log4j/log4j-api/2.11.1/log4j-api-2.11.1.jar`
|
Thanks for the PR! Also I've added the pull request template after you opened the PR. Can you check the points listed there? |
| @@ -769,7 +769,7 @@ private void configureTypeResolution( PMDConfiguration configuration ) throws Ma | |||
| for ( String path : projectCompileClasspath ) | |||
| { | |||
| File pathFile = new File( path ); | |||
| if ( !pathFile.exists() || pathFile.list().length == 0 ) | |||
| if ( !pathFile.exists() || pathFile.list() == null || pathFile.list().length == 0 ) | |||
There was a problem hiding this comment.
This fix is not correct.
Because the second invocation may return null.
You have to assign the result of list() to a local variable.
There was a problem hiding this comment.
I'm not following/understanding your feedback.
If the second clause is true, the third clause never executes.
If the second clause is false, the third clause won't throw an NPE.
Can you suggest code that is better for this? (Or describe why a local variable is better?)
There was a problem hiding this comment.
If the first call to list() returns a non-null value the jvm will evaluate the third condition but a new call to list() is not guaranteed to be null. (In real world it won't be null I know)
Tools like spotbugs usually find this kind of potential problems.
I am just nitpicking.
If you call list() only once you won't have problems
There was a problem hiding this comment.
- yes, you are correct.
- See updates in c64e1f6
- See https://issues.apache.org/jira/browse/MPMD-288
- I ran
mvn clean verifyandmvn -Prun-its clean verify(both completed successfully) - Note that contrary to your directions, github warns that my commit message subject is too long (if I use the subject matching the jira ticket)
see https://issues.apache.org/jira/browse/MPMD-288 Fixes from @eolivelli's feedback
|
In order to merge your oatcg please amend the commit message including the JIRA id, do a 'git history' and follow the style of other commit messages |
|
Just squash to the commit with the jira id |
adangel
changed the title
Closes #11 Fixed NullPointerException this line throws NPE when file.list is null https://docs.oracle.com/javase/8/docs/api/java/io/File.html#list-- For me this occurs on jar files like `~/.m2/repository/org/apache/logging/log4j/log4j-api/2.11.1/log4j-api-2.11.1.jar` Contributed by: Wil Carmon
| if ( !pathFile.exists() || pathFile.list().length == 0 ) | ||
| String[] children = pathFile.list(); | ||
|
|
||
| if ( !pathFile.exists() || children == null || children.length == 0 ) |
There was a problem hiding this comment.
@wcarmon I think, the logic should actually be
| if ( !pathFile.exists() || children == null || children.length == 0 ) | |
| if ( !pathFile.exists() || ( children != null && children.length == 0 ) ) |
However, I'm wondering how a jar file can end up in this place. I expected here something like the paths "project-a/target/classes" or "project-a/target/generated-classes" or so.
You've mentioned, you get the null pointer when path is ~/.m2/repository/org/apache/logging/log4j/log4j-api/2.11.1/log4j-api-2.11.1.jar. Do you use log4j-api as a normal dependency or is there something special in your project?
I've for now squashed and pushed your change to a new branch, let's see what Jenkins says: https://builds.apache.org/view/M-R/view/Maven/job/maven-box/job/maven-pmd-plugin/job/MPMD-288/
There was a problem hiding this comment.
I've changed it to only consider non-existing files or empty directories - Jenkins is still happy: https://builds.apache.org/view/M-R/view/Maven/job/maven-box/job/maven-pmd-plugin/job/MPMD-288/2/
So, I'll merge this PR, thanks again!
Closes #11 Fixed NullPointerException this line throws NPE when file.list is null https://docs.oracle.com/javase/8/docs/api/java/io/File.html#list-- For me this occurs on jar files like `~/.m2/repository/org/apache/logging/log4j/log4j-api/2.11.1/log4j-api-2.11.1.jar` Contributed by: Wil Carmon
this line throws NullPointerException when
File.list()returnsnullSee related javadoc
For me this occurs on jar files like
~/.m2/repository/org/apache/logging/log4j/log4j-api/2.11.1/log4j-api-2.11.1.jar