Null pointer exception with records in RequireThisCheck

[Vyom-Yadav]

I have read check documentation: https://checkstyle.sourceforge.io/config_coding.html#RequireThis
I have downloaded the latest checkstyle from: https://checkstyle.org/cmdline.html#Download_and_Run
I have executed the cli and showed it below, as cli describes the problem better than 1,000 words

/var/tmp $ javac Test.java

/var/tmp $ cat config.xml

<!DOCTYPE module PUBLIC
        "-//Puppy Crawl//DTD Check Configuration 1.2//EN"
        "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">

<module name="Checker">
  <module name="TreeWalker">
    <module name="RequireThisCheck"/>
  </module>
</module>

/var/tmp $ cat Test.java

public record Test(int a) {

    void method() {
        String a = "BUG";
        a = a.substring(0, 1);
    }
}

/var/tmp $ RUN_LOCALE="-Duser.language=en -Duser.country=US"
/var/tmp $ java $RUN_LOCALE -jar checkstyle-10.3.1-all.jar -c config.xml Test.java
Starting audit...
com.puppycrawl.tools.checkstyle.api.CheckstyleException: Exception was thrown while processing Test.java
        at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:305)
        at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:224)
        at com.puppycrawl.tools.checkstyle.Main.runCheckstyle(Main.java:407)
        at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:330)
        at com.puppycrawl.tools.checkstyle.Main.execute(Main.java:189)
        at com.puppycrawl.tools.checkstyle.Main.main(Main.java:126)
Caused by: java.lang.NullPointerException
        at com.puppycrawl.tools.checkstyle.checks.coding.RequireThisCheck$ClassFrame.hasFinalField(RequireThisCheck.java:1557)
        at com.puppycrawl.tools.checkstyle.checks.coding.RequireThisCheck.canAssignValueToClassField(RequireThisCheck.java:932)
        at com.puppycrawl.tools.checkstyle.checks.coding.RequireThisCheck.getClassFrameWhereViolationIsFound(RequireThisCheck.java:731)
        at com.puppycrawl.tools.checkstyle.checks.coding.RequireThisCheck.getFieldWithoutThis(RequireThisCheck.java:517)
        at com.puppycrawl.tools.checkstyle.checks.coding.RequireThisCheck.processIdent(RequireThisCheck.java:469)
        at com.puppycrawl.tools.checkstyle.checks.coding.RequireThisCheck.visitToken(RequireThisCheck.java:403)
        at com.puppycrawl.tools.checkstyle.TreeWalker.notifyVisit(TreeWalker.java:335)
        at com.puppycrawl.tools.checkstyle.TreeWalker.processIter(TreeWalker.java:406)
        at com.puppycrawl.tools.checkstyle.TreeWalker.walk(TreeWalker.java:273)
        at com.puppycrawl.tools.checkstyle.TreeWalker.processFiltered(TreeWalker.java:154)
        at com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck.process(AbstractFileSetCheck.java:87)
        at com.puppycrawl.tools.checkstyle.Checker.processFile(Checker.java:331)
        at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:292)
        ... 5 more
Checkstyle ends with 1 errors.

This was discovered at #11805 (comment).

Relevant AST-

RECORD_DEF -> RECORD_DEF 
 |--MODIFIERS -> MODIFIERS
 |   `--LITERAL_PUBLIC -> public
 |--LITERAL_RECORD -> record
 |--IDENT -> Test
 |--LPAREN -> (
 |--RECORD_COMPONENTS -> RECORD_COMPONENTS
 |   `--RECORD_COMPONENT_DEF -> RECORD_COMPONENT_DEF
 |       |--ANNOTATIONS -> ANNOTATIONS
 |       |--TYPE -> TYPE
 |       |   `--LITERAL_INT -> int
 |       `--IDENT -> a

Reason:

The main reason for this happening is due to the fact that record instance members (record components) don't have any modifier (they are implicitly private and final). AST does not show any TokenTypes.MODIFIERS for record components (which is correct) and the condition at

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheck.java

Lines 1529 to 1540 in b87a6c0

	public boolean hasFinalField(final DetailAST instanceMember) {
	boolean result = false;
	for (DetailAST member : instanceMembers) {
	final DetailAST mods = member.getParent().findFirstToken(TokenTypes.MODIFIERS);
	final boolean finalMod = mods.findFirstToken(TokenTypes.FINAL) != null;
	if (finalMod && isAstSimilar(member, instanceMember)) {
	result = true;
	break;
	}
	}
	return result;
	}

expects TokenTypes.MODIFIERS so we get an NPE.

[rnveach]

Regression never caught this?

[Vyom-Yadav]

Regression never caught this?

Support for records was added in #8579, the reports present there are with
<property name="checkFields" value="false"/> so it passed at that time or maybe this file wasn't present at that time.

[rnveach]

How did we find it now? Did we create this class ourselves?

[nrmancuso]

@rnveach see #11805 (comment), I have created a new projects file at checkstyle/contribution#613

[rnveach]

Fix was merged