FallThrough check doesn't correctly handle try-with-resources #3509

Closed
dgolub opened this Issue Oct 20, 2016 · 2 comments

Comments

Projects
None yet
3 participants
@dgolub

dgolub commented Oct 20, 2016

Here is updated description.

$ cat TestClass.java
public class TestClass {
    Object method() throws Exception {
        int variable = 0;

        switch (variable) {
        case 0:
            try (final Resource resource = getResource()) {
                return resource.doSomething();
            }
        case 1:
            return doSomethingElse();
        }

        return null;
    }

    private Resource getResource() {
        return new Resource();
    }

    private Object doSomethingElse() {
        return null;
    }

    public static class Resource implements AutoCloseable {
        @Override
        public void close() throws Exception {
        }

        public Object doSomething() {
            return null;
        }
    }
}

$ 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="FallThrough" />
    </module>
</module>

$ java -jar checkstyle-7.2-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[ERROR] TestClass.java:10:9: Fall through from previous branch of the switch statement. [FallThrough]
Audit done.
Checkstyle ends with 1 errors.
@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Dec 22, 2016

Member

@romani To me this looks valid.
Only 2 paths that can be taken, 1) we return inside the try with resource or 2) we throw an exception because something went wrong with the resource.
Eclipse doesn't flag it as a fall through either when the option is enabled.

Member

rnveach commented Dec 22, 2016

@romani To me this looks valid.
Only 2 paths that can be taken, 1) we return inside the try with resource or 2) we throw an exception because something went wrong with the resource.
Eclipse doesn't flag it as a fall through either when the option is enabled.

@rnveach rnveach self-assigned this Jan 5, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Jan 5, 2017

romani added a commit that referenced this issue Jan 5, 2017

@romani romani added this to the 7.5 milestone Jan 5, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 5, 2017

Member

fix is merged

Member

romani commented Jan 5, 2017

fix is merged

@romani romani closed this Jan 5, 2017

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