Pmd: [java] NPE in PMD 6.8.0 InvalidSlf4jMessageFormat

[cynthux]

Affects PMD Version: 6.8.0

Rule:

Description:
NPE while applying rule InvalidSlf4jMessageFormat. Trace:
```
Exception applying rule InvalidSlf4jMessageFormat on file ...ClassName.java, continuing with next rule
java.lang.NullPointerException
at net.sourceforge.pmd.lang.java.rule.errorprone.InvalidSlf4jMessageFormatRule.expectedArguments(InvalidSlf4jMessageFormatRule.java:175)
at net.sourceforge.pmd.lang.java.rule.errorprone.InvalidSlf4jMessageFormatRule.visit(InvalidSlf4jMessageFormatRule.java:86)
at net.sourceforge.pmd.lang.java.ast.ASTName.jjtAccept(ASTName.java:35)
at net.sourceforge.pmd.lang.java.rule.JavaRuleChainVisitor.visit(JavaRuleChainVisitor.java:44)
at net.sourceforge.pmd.lang.rule.AbstractRuleChainVisitor.visitAll(AbstractRuleChainVisitor.java:96)
at net.sourceforge.pmd.RuleChain.apply(RuleChain.java:67)
at net.sourceforge.pmd.RuleSets.apply(RuleSets.java:140)
at net.sourceforge.pmd.SourceCodeProcessor.processSource(SourceCodeProcessor.java:184)
at net.sourceforge.pmd.SourceCodeProcessor.processSourceCode(SourceCodeProcessor.java:96)
at net.sourceforge.pmd.SourceCodeProcessor.processSourceCode(SourceCodeProcessor.java:51)
at net.sourceforge.pmd.processor.PmdRunnable.call(PmdRunnable.java:78)
at net.sourceforge.pmd.processor.MonoThreadProcessor.runAnalysis(MonoThreadProcessor.java:29)
at net.sourceforge.pmd.processor.AbstractPMDProcessor.processFiles(AbstractPMDProcessor.java:108)
at net.sourceforge.pmd.PMD.processFiles(PMD.java:329)
at net.sourceforge.pmd.ant.internal.PMDTaskImpl.doTask(PMDTaskImpl.java:191)
at net.sourceforge.pmd.ant.internal.PMDTaskImpl.execute(PMDTaskImpl.java:275)
at net.sourceforge.pmd.ant.PMDTask.execute(PMDTask.java:50)
...
at java.lang.Thread.run(Thread.java:748)

**Code Sample demonstrating the issue:**

```java
public enum ClassName {
    INSTANCE;

    private final Logger log = LoggerFactory.getLogger(ClassName.class);

    public void sendMessage(String message) {
        log.info(message);
    }

    public static void main(String[] args) {
        ClassName.INSTANCE.sendMessage("A message");
    }
}

Running PMD through: Gradle

[jsotuyod]

@cynthux thanks for the report.. this seems to be related to #1504 (fixed in 6.10.0), but a particular case for enums, which means #1504 probably doesn't fix it. I'll add a test case and follow up on this.

[cynthux]

Yes, I have tried to reproduce it with classes and I wasn't able to do it. It fails only with enums. Also, as the bug you referenced says, it occurs when the logger call has a variable as parameter (even if the variable is a local variable).

[rmartinus]

@cynthux @jsotuyod It looks like we need a null check in InvalidSlf4jMessageFormatRule (see below code), unless I'm missing something.

if (count == 0) {
    // look if the message is defined in a field
    final ASTClassOrInterfaceBody astClassOrInterfaceBody = node.getFirstParentOfType(ASTClassOrInterfaceBody.class);
    if (astClassOrInterfaceBody != null) {
        final List<ASTFieldDeclaration> fieldlist = astClassOrInterfaceBody
                .findDescendantsOfType(ASTFieldDeclaration.class);
    ...
    ...

[jsotuyod]

@rmartinus the null check would suppress the error, but not fix the underlying issue with enums.

What we should actually do is, in that very same snippet, change the node.getFirstParentOfType(ASTClassOrInterfaceBody.class); to node.getFirstParentOfAnyType(ASTClassOrInterfaceBody.class, ASTEnumBody.class);, so we can also properly handle enums.

The result would look like:

            if (count == 0) {
                // look if the message is defined in a field
                final List<ASTFieldDeclaration> fieldlist = node.getFirstParentOfAnyType(ASTClassOrInterfaceBody.class, ASTEnumBody.class)
                        .findDescendantsOfType(ASTFieldDeclaration.class);
                // only look for ASTVariableDeclarator that are Fields
                final List<ASTVariableDeclarator> fields = new ArrayList<>(fieldlist.size());
                for (final ASTFieldDeclaration astFieldDeclaration : fieldlist) {
                    fields.add(astFieldDeclaration.getFirstChildOfType(ASTVariableDeclarator.class));
                }
                count = getAmountOfExpectedArguments(variableName, fields);
            }

We would have to add test cases for both, the supplied snippet (no issues), and one defining a constant message as a field in an enum with improper argument count (1 violation).

[rmartinus]

@jsotuyod it makes sense to me. Can I create a PR for this?

[jsotuyod]

@rmartinus yes! Please do so!