VarFinalLocalVariable: Enforce var with final local variables

[joswinter]

The var reserved type was introduced in Java 10. Type inference is used in var keyword in which it detects automatically the datatype of a variable based on the surrounding context.

I would like to create a checkstyle check that enforces that all final local variables use the var reserved type. This is to avoid duplication of type references in statements. This code style is used in some projects, but it is currently difficult to enforce because there is no specific checkstyle rule for this.

Proposed configuration

<module name="VarFinalLocalVariable">
</module>

Incorrect style

For example, the following code would fail the VarFinalLocalVariable check:

    public int square() {
        final int a = 5;
        return a * a;
    }

The following warning could then be logged by checkstyle for example:

Final variable 'a' should be created via 'var' keyword to avoid duplication of type reference in statement.

Correct style

This would be the correct style which would not break the VarFinalLocalVariable rule.

    public int square() {
        final var a = 5;
        return a * a;
    }

Current approach

At the moment you can enforce final local variables to be declared with var by using the following MatchXpath rule:

<module name="MatchXpath">
  <property name="query" 
   value="//VARIABLE_DEF[ancestor::METHOD_DEF and (./MODIFIERS/FINAL) and not(./TYPE/IDENT[@text='var'])]" />
  <message key="matchxpath.match" 
   value="New instances should be created via 'var' keyword to avoid duplication of type reference in statement" />
</module>

[joswinter]

Let me know if you have any feedback or objections. If the issue is approved I can implement this myself and create a pull request.

[romani]

please read https://checkstyle.org/report_issue.html#How_to_request_new_Check.2FModule.3F pay attention to example link.

to approve issue we need to see design with all names and behavior on some basic code.

[nrmancuso]

@joswinter I am curious, if you have a stable MatchXPath module configuration to use, why create a new check? Are there some properties you would like to add, etc?

[joswinter]

@nrmancuso multiple reasons why I would like to add such a check:

• I would like to keep our checkstyle project config clean, I don't think the MatchXPath rules are very clear, I would prefer to define a rule with a nice name such as VarFinalLocalVariable
• I want to contribute and I want to make it easier for future users of checkstyle to define such a rule. It took me a while to understand and go over the AST and to create the MatchXPath rule.

[future2r]

Besides the working MatchXPath, there is more to it.

• Non-final variables may (and in some project should) also use the var keyword.

var s = parse();
s = s + "\n";

• There are final variables that cannot use the var keyword.

final String s;
try {
  s = parse();
} catch (ParseException ex) {
  throw new IOException(ex);
}

I think, it is worth implementing a new check.

[romani]

xpath is updated to avoid false positive of XpathMatch

$ javac Test.java 
$ cat Test.java 
public class Test {
  public void plugin() {
    int a = 5; // violation
    a = a * a;

    var s = parse();
    s = s + "\n";

    final String s2;
    try {
      s2 = parse();
    } catch (Exception ex) {
      throw new RuntimeException(ex);
    }
  }


  private String parse() { return ""; }
}

$ cat config.xml 
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
  <module name="TreeWalker">
    <module name="MatchXpath">
      <property name="query"
       value="//VARIABLE_DEF[ancestor::METHOD_DEF
                              and child::ASSIGN
                              and not(./TYPE/IDENT[@text='var'])]" />
      <message key="matchxpath.match"
       value="New instances should be created via 'var' keyword" />
    </module>
  </module>
</module>

$ java -jar checkstyle-10.20.0-all.jar -c config.xml Test.java 
Starting audit...
[ERROR] /var/tmp/Test.java:3:5: New instances should be created via var keyword [MatchXpath]
Audit done.
Checkstyle ends with 1 errors.

after fix of violation:

$ cat Test.java 
public class Test {
  public void plugin() {
    var a = 5;
    a = a * a;

    var s = parse();
    s = s + "\n";

    final String s2;
    try {
      s2 = parse();
    } catch (Exception ex) {
      throw new RuntimeException(ex);
    }
  }

  private String parse() { return ""; }
}

$ javac Test.java 
$ cat config.xml 
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
  <module name="TreeWalker">
    <module name="MatchXpath">
      <property name="query"
       value="//VARIABLE_DEF[ancestor::METHOD_DEF
                              and child::ASSIGN
                              and not(./TYPE/IDENT[@text='var'])]" />
      <message key="matchxpath.match"
       value="New instances should be created via 'var' keyword" />
    </module>
  </module>
</module>

$ java -jar checkstyle-10.20.0-all.jar -c config.xml Test.java 
Starting audit...
Audit done.

@future2r , please run on your sources this instance of MatchXpath and share what other false positives you see. if it works well, we should avoid writing java implementation of Check. We can add it to examples of MatchXpath Check so it will be easier accessible for users.