Checkstyle: RedundantModifierCheck throws NullPointerException

[Fernal73]

Check documentation:
RedundantModifier

````
$ cat TestClass.java
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.Writer;
import java.net.Socket;
import java.nio.charset.StandardCharsets;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Locale;
import java.util.concurrent.Callable;

public class DaytimeTask implements Runnable, Callable {
private static final String UTF_8 = StandardCharsets.UTF_8.name();
private final Socket connection;

DaytimeTask(Socket connection) {
this.connection = connection;
}

@Override
public Void call() {
run();
return null;
}

@Override
@SuppressWarnings("PMD.LawOfDemeter")
public void run() {
try (Writer out =
new OutputStreamWriter(connection.getOutputStream(), UTF_8);
connection;) {
Date now = new Date();
SimpleDateFormat format =
new SimpleDateFormat("yy-MM-dd hh:mm:ss Z", Locale.getDefault());
out.write(ProcessHandle.current().pid() + " " + format.format(now)
+ "\r\n");
out.flush();
} catch (IOException ex) {
System.err.println(ex);
}
}
}

$ cat TestConfig.xml

"-//Puppy Crawl//DTD Check Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="TreeWalker">
<module name="RedundantModifier" />
</module>

$ java -jar checkstyle-8.33-all.jar -c TestConfig.xml TestClass.java
Starting audit...
com.puppycrawl.tools.checkstyle.api.CheckstyleException: Exception was thrown while processing TestClass.java
at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:311)
at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:221)
at com.puppycrawl.tools.checkstyle.Main.runCheckstyle(Main.java:408)
at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:331)
at com.puppycrawl.tools.checkstyle.Main.execute(Main.java:190)
at com.puppycrawl.tools.checkstyle.Main.main(Main.java:125)
Caused by: java.lang.NullPointerException
at com.puppycrawl.tools.checkstyle.checks.modifier.RedundantModifierCheck.checkForRedundantModifier(RedundantModifierCheck.java:424)
at com.puppycrawl.tools.checkstyle.checks.modifier.RedundantModifierCheck.processResources(RedundantModifierCheck.java:413)
at com.puppycrawl.tools.checkstyle.checks.modifier.RedundantModifierCheck.visitToken(RedundantModifierCheck.java:237)
at com.puppycrawl.tools.checkstyle.TreeWalker.notifyVisit(TreeWalker.java:340)
at com.puppycrawl.tools.checkstyle.TreeWalker.processIter(TreeWalker.java:451)
at com.puppycrawl.tools.checkstyle.TreeWalker.walk(TreeWalker.java:278)
at com.puppycrawl.tools.checkstyle.TreeWalker.processFiltered(TreeWalker.java:151)
at com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck.process(AbstractFileSetCheck.java:87)
at com.puppycrawl.tools.checkstyle.Checker.processFile(Checker.java:337)
at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:298)
... 5 more
Checkstyle ends with 1 errors.
````

---

Describe what you expect in detail.

Checkstyle should complete successfully either emitting violations or no error messages when there are none. No NullPointerException as above must occur.

---

First reported at Checkstyle Google group:

https://groups.google.com/forum/m/#!topic/checkstyle/OZvmwJypxvI

Links to additional files throwing similar exception in Google Group post.

[rnveach]

Exception occurs in 8.33 and not in 8.32 .

[Fernal73]

Thanks @rnveach , for stripping out the config and properties file. Verified that 8.32 does not have the above problem.

[rnveach]

The AST of the resource changed from https://github.com/checkstyle/checkstyle/issues/6332 .

| | | | |--SEMI -> ; [6:72] | | | | `--IDENT -> connection [7:9]

became

| | | | |--SEMI -> ; [6:72] | | | | `--RESOURCE -> RESOURCE [7:9] | | | | `--IDENT -> connection [7:9]

https://github.com/checkstyle/checkstyle/blob/2e8c277c181487d81231170ff4b959edc008114e/src/main/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheck.java#L424
Check was written with modifiers always being listed. This is not the case for this field and thus the exception occurs.

The code must be changed to fix the NPE and add the example as a test case.

[Fernal73]

Thanks, @rnveach, for printing out the diff in the ASTs from the two
versions. That beats running the source through the debugger.

The line number printed against the exception does not match the occurrence
of the error.

What I'm also concerned with is how auto closing the resource in the try
block of the run method precludes reuse of the Runnable (as it is).

[Fernal73]

This is the output in my errors output file for the same by setting haltOnException=false in the config file.

Starting audit...
[ERROR] /data/data/com.termux/files/home/LearnJava/TestCode/./cstests/DaytimeTask.java:1: Got an exception - java.lang.NullPointerException
    at com.puppycrawl.tools.checkstyle.checks.modifier.RedundantModifierCheck.checkForRedundantModifier(RedundantModifierCheck.java:424)
    at com.puppycrawl.tools.checkstyle.checks.modifier.RedundantModifierCheck.processResources(RedundantModifierCheck.java:413)
    at com.puppycrawl.tools.checkstyle.checks.modifier.RedundantModifierCheck.visitToken(RedundantModifierCheck.java:237)
    at com.puppycrawl.tools.checkstyle.TreeWalker.notifyVisit(TreeWalker.java:340)
    at com.puppycrawl.tools.checkstyle.TreeWalker.processIter(TreeWalker.java:451)
    at com.puppycrawl.tools.checkstyle.TreeWalker.walk(TreeWalker.java:278)
    at com.puppycrawl.tools.checkstyle.TreeWalker.processFiltered(TreeWalker.java:151)
    at com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck.process(AbstractFileSetCheck.java:87)
    at com.puppycrawl.tools.checkstyle.Checker.processFile(Checker.java:337)
    at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:298)
    at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:221)
    at com.puppycrawl.tools.checkstyle.Main.runCheckstyle(Main.java:408)
    at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:331)
    at com.puppycrawl.tools.checkstyle.Main.execute(Main.java:190)
    at com.puppycrawl.tools.checkstyle.Main.main(Main.java:125)
 [Checker]
Audit done.

[Fernal73]

As an aside, I'm open to Checkstyle using my repository to test their new rules when integrating pull requests. You might wish to override the existing suppressions when you do, though.

[romani]

fix is merged.