New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LocalFinalVariableName: not validating try-with-resource variables (new Acceptable token) #3348

Closed
rnveach opened this Issue Jul 14, 2016 · 3 comments

Comments

Projects
None yet
3 participants
@rnveach
Member

rnveach commented Jul 14, 2016

$ cat TestClass.java
import java.io.BufferedReader;
import java.io.FileInputStream;
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;

public class Test {
    void method() throws Exception {
        final String fileName = "Test";
        final BufferedReader br = new BufferedReader(new InputStreamReader(
                new FileInputStream(fileName), StandardCharsets.UTF_8));
        try {
        }
        finally {
            br.close();
        }
    }

    void method2() throws Exception {
        final String fileName = "Test";
        try (BufferedReader br = new BufferedReader(new InputStreamReader(
                new FileInputStream(fileName), StandardCharsets.UTF_8))) {
        }
        finally {

        }
    }

    void method3() throws Exception {
        final String fileName = "Test";
        try (final BufferedReader br = new BufferedReader(new InputStreamReader(
                new FileInputStream(fileName), StandardCharsets.UTF_8))) {
        }
        finally {

        }
    }
}

$ cat TestConfig.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
    <module name="LocalFinalVariableName">
      <property name="format" value="^(id)|([a-z][a-z0-9][a-zA-Z0-9]+)$"/>
    </module>
    </module>
</module>

$ java -jar checkstyle-7.0-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[ERROR] TestClass.java:9:30: Name 'br' must match pattern '^(id)|([a-z][a-z0-9][a-zA-Z0-9]+)$'. [LocalFinalVariableName]
Audit done.
Checkstyle ends with 1 errors.

All methods have similar code, just 2 are using try-with-resources and one is using the old style try and local variable.
Since they are all similar, I am expecting similar violations on each one but the only one that shows a violation is the old style try.
Try-with-resource is implicitly declared final by 14.20.3 in the JLS

A variable declared in a resource specification is implicitly declared  final
(§4.12.4) if it is not explicitly declared  final .

I consider try-with-resource variables to be local, so they should fall under the validation of this Check as there is no other check specifically for it.
I expect lines 20 and 30 to have similar violations as line 9.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 27, 2017

Member

@Nimfadora , please help us with this issue.

in addition to fix documentation in xdoc folder should be extended to let user know that we treat variables in try-with-resources as final too.

Member

romani commented Mar 27, 2017

@Nimfadora , please help us with this issue.

in addition to fix documentation in xdoc folder should be extended to let user know that we treat variables in try-with-resources as final too.

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Mar 27, 2017

Contributor

@romani, ok, I'm on it.

Contributor

Nimfadora commented Mar 27, 2017

@romani, ok, I'm on it.

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Mar 28, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Mar 28, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Mar 28, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Apr 11, 2017

yevhen-strizhnev pushed a commit to Nimfadora/checkstyle that referenced this issue Apr 19, 2017

yevhen-strizhnev pushed a commit to Nimfadora/checkstyle that referenced this issue Apr 19, 2017

yevhen-strizhnev pushed a commit to Nimfadora/checkstyle that referenced this issue Apr 19, 2017

yevhen-strizhnev pushed a commit to Nimfadora/checkstyle that referenced this issue Apr 19, 2017

yevhen-strizhnev pushed a commit to Nimfadora/checkstyle that referenced this issue Apr 19, 2017

yevhen-strizhnev pushed a commit to Nimfadora/checkstyle that referenced this issue Apr 19, 2017

yevhen-strizhnev pushed a commit to Nimfadora/checkstyle that referenced this issue Apr 19, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Apr 19, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Apr 19, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Apr 19, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Apr 19, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Apr 20, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Apr 20, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Apr 20, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Apr 20, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue May 7, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue May 7, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue May 7, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue May 7, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue May 7, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue May 7, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue May 7, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue May 7, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue May 7, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue May 7, 2017

romani added a commit that referenced this issue May 7, 2017

@romani romani added the new feature label May 7, 2017

@romani romani added this to the 7.8 milestone May 7, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani May 7, 2017

Member

fix is merged, new token was added to getAcceptableTokens

Member

romani commented May 7, 2017

fix is merged, new token was added to getAcceptableTokens

@romani romani closed this May 7, 2017

@romani romani changed the title from LocalFinalVariableName: not validating try-with-resource variables to LocalFinalVariableName: not validating try-with-resource variables (new Acceptable token) May 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment