NullPointerException in AbstractHeaderCheck when cache file specified and no header file #3771

Closed
Tunaki opened this Issue Jan 28, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@Tunaki

Tunaki commented Jan 28, 2017

This affects Checkstyle 6.19, which is currently the last JDK 1.7 compatible version, and every version in the 7.x line. It was discovered trying to fix MCHECKSTYLE-322 for the Maven Checkstyle Plugin.

http://checkstyle.sourceforge.net/config_header.html#RegexpHeader#RegexpHeader

Given the following Java class

public class Main {
    public static void main(String[] args) { }
}

and the following config.xml Checkstyle configuration, making use of a cache file and a regexp header:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.2//EN" "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
<module name="Checker">
  <property name="cacheFile" value="cachefile"/>
  <module name="RegexpHeader">
    <property name="header" value="/* A Required Header */"/>
  </module>
</module>

With this set-up, and launching an audit:

$ java -jar checkstyle-6.19-all.jar -c config.xml Main.java
Exception in thread "main" java.lang.NullPointerException
    at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:212)
    at com.google.common.collect.SingletonImmutableSet.<init>(SingletonImmutableSet.java:43)
    at com.google.common.collect.ImmutableSet.of(ImmutableSet.java:60)
    at com.puppycrawl.tools.checkstyle.checks.header.AbstractHeaderCheck.getExternalResourceLocations(AbstractHeaderCheck.java:197)
    at com.puppycrawl.tools.checkstyle.Checker.getExternalResourceLocations(Checker.java:225)
    at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:187)
    at com.puppycrawl.tools.checkstyle.Main.runCheckstyle(Main.java:396)
    at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:330)
    at com.puppycrawl.tools.checkstyle.Main.main(Main.java:161)

A similar NullPointerException is thrown with current latest Checkstyle 7.4.

$ java -jar checkstyle-7.4-all.jar -c config.xml Main.java
Exception in thread "main" java.lang.NullPointerException
    at com.puppycrawl.tools.checkstyle.checks.header.AbstractHeaderCheck.getExternalResourceLocations(AbstractHeaderCheck.java:196)
    at com.puppycrawl.tools.checkstyle.Checker.lambda$getExternalResourceLocations$1(Checker.java:236)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(Unknown Source)
    at java.util.stream.ReferencePipeline$2$1.accept(Unknown Source)
    at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(Unknown Source)
    at java.util.stream.AbstractPipeline.copyInto(Unknown Source)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(Unknown Source)
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(Unknown Source)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(Unknown Source)
    at java.util.stream.AbstractPipeline.evaluate(Unknown Source)
    at java.util.stream.ReferencePipeline.forEach(Unknown Source)
    at com.puppycrawl.tools.checkstyle.Checker.getExternalResourceLocations(Checker.java:234)
    at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:202)
    at com.puppycrawl.tools.checkstyle.Main.runCheckstyle(Main.java:425)
    at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:359)
    at com.puppycrawl.tools.checkstyle.Main.main(Main.java:174)

The audit should not fail with a NullPointerException, but instead complete succesfully. The issue seems to be that Checker starts by populating the cache if present, which in turn invokes getExternalResourceLocations() on the RegexpHeaderCheck, but this fails with NPE because there was no configured headerFile and ImmutableSet.of requires non-null objects.

Is it possible to fix this issue and also backport it on the 6.x line? It would otherwise block a Checkstyle upgrade by the Maven Checkstyle Plugin to 6.18 at most.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jan 28, 2017

Member

The issue does look valid. We need to check for null when creating the external resource set for this check.

Is it possible to fix this issue and also backport it on the 6.x line

If you are stuck on an older version because of JDK8, I recommend you try https://github.com/rnveach/checkstyle-backport-jre6 to use the latest Checkstyle version (http://checkstyle.sourceforge.net/#Backport).
If you can't do that, you might want to think of turning off the cache as there were a few major bug fixes in 7.2 and 7.3 that fixed issues with the cache, almost making it pointless to have.

Member

rnveach commented Jan 28, 2017

The issue does look valid. We need to check for null when creating the external resource set for this check.

Is it possible to fix this issue and also backport it on the 6.x line

If you are stuck on an older version because of JDK8, I recommend you try https://github.com/rnveach/checkstyle-backport-jre6 to use the latest Checkstyle version (http://checkstyle.sourceforge.net/#Backport).
If you can't do that, you might want to think of turning off the cache as there were a few major bug fixes in 7.2 and 7.3 that fixed issues with the cache, almost making it pointless to have.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 28, 2017

Member

Is it possible to fix this issue and also backport it on the 6.x line?

we do not provide packports, as it is too expensive for our tiny team.
But you can easily do this yourself and have its own version your maven repository.

Member

romani commented Jan 28, 2017

Is it possible to fix this issue and also backport it on the 6.x line?

we do not provide packports, as it is too expensive for our tiny team.
But you can easily do this yourself and have its own version your maven repository.

@romani romani added the approved label Jan 28, 2017

@Tunaki

This comment has been minimized.

Show comment
Hide comment
@Tunaki

Tunaki Jan 28, 2017

Maven Checkstyle Plugin currently uses 6.11.2 with JDK 7, without the backport. The cache is always used so we can't really disable it (plus, users could configure the plugin with their own config.xml with a cache anyway). We'll update to 6.18 for now until we can move to JDK 8.

Tunaki commented Jan 28, 2017

Maven Checkstyle Plugin currently uses 6.11.2 with JDK 7, without the backport. The cache is always used so we can't really disable it (plus, users could configure the plugin with their own config.xml with a cache anyway). We'll update to 6.18 for now until we can move to JDK 8.

@rnveach rnveach self-assigned this Jan 28, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 28, 2017

Member

@Tunaki , as workaround, you can move your header to some jar as resource (temp solution), change configuration to use headerFile property and checkstyle will load template from file - http://checkstyle.sourceforge.net/property_types.html#uri .
Or put in web(url), checkstyle will load it from web on first run, is cache is on it should optimize process.

Member

romani commented Jan 28, 2017

@Tunaki , as workaround, you can move your header to some jar as resource (temp solution), change configuration to use headerFile property and checkstyle will load template from file - http://checkstyle.sourceforge.net/property_types.html#uri .
Or put in web(url), checkstyle will load it from web on first run, is cache is on it should optimize process.

rnveach added a commit to rnveach/checkstyle that referenced this issue Jan 28, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Jan 28, 2017

romani added a commit that referenced this issue Jan 28, 2017

@romani romani added the bug label Jan 28, 2017

@romani romani added this to the 7.5 milestone Jan 28, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 28, 2017

Member

fix is merged, will be released in 7.5.

Member

romani commented Jan 28, 2017

fix is merged, will be released in 7.5.

@romani romani closed this Jan 28, 2017

asfgit pushed a commit to apache/maven-plugins that referenced this issue Feb 12, 2017

Guillaume Boué
[MCHECKSTYLE-322] Update plugin to use 6.19 checkstyle (last version …
…that is based on jdk7)

Updating Checkstyle to 2.18, and not 2.19 because of a regression (see issue checkstyle/checkstyle#3771).

git-svn-id: https://svn.apache.org/repos/asf/maven/plugins/trunk@1782677 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit to apache/maven-plugins that referenced this issue Jun 11, 2017

Guillaume Boué
[MCHECKSTYLE-322] Update plugin to use 6.19 checkstyle (last version …
…that is based on jdk7)

Updating Checkstyle to 2.18, and not 2.19 because of a regression (see issue checkstyle/checkstyle#3771).

git-svn-id: https://svn.apache.org/repos/asf/maven/plugins/trunk/maven-checkstyle-plugin@1782677 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit to apache/maven-checkstyle-plugin that referenced this issue Dec 9, 2017

Guillaume Boué
[MCHECKSTYLE-322] Update plugin to use 6.19 checkstyle (last version …
…that is based on jdk7)

Updating Checkstyle to 2.18, and not 2.19 because of a regression (see issue checkstyle/checkstyle#3771).

git-svn-id: https://svn.apache.org/repos/asf/maven/plugins/trunk@1782677 13f79535-47bb-0310-9956-ffa450edef68
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment