False positive in MissingSwitchDefault for switch statement where null label is present

[KalleBeneus]

Check documentation https://checkstyle.sourceforge.io/checks/coding/missingswitchdefault.html#Description

# Run with java 21
/var/tmp $ javac MissingSwitchDefaultTest.java

/var/tmp $ cat config.xml

#<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
        "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
        "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
  <module name="TreeWalker">
    <module name="MissingSwitchDefault">
   </module>
  </module>
</module>

/var/tmp $ cat MissingSwitchDefaultTest.java

class MissingSwitchDefaultTest {
    public void switchWithNullTest(SwitchInput i) {
        switch (i) {
            case FIRST -> System.out.println("FIRST");
            case SECOND -> System.out.println("SECOND");
            case null -> System.out.println("NULL");
        }
    }

    enum SwitchInput {
        FIRST,
        SECOND
    }
}

/var/tmp $ java -jar checkstyle-10.12.5-all.jar -c config.xml MissingSwitchDefaultTest.java
Starting audit...
[ERROR] /Users/kxbh697/temp/checkstyle/MissingSwitchDefaultTest.java:3:9:
        switch without "default" clause. [MissingSwitchDefault]
Audit done.
Checkstyle ends with 1 errors.

I'd expect this to have no violation. The documentation states that This check does not validate switch statements that use pattern or null labels, but here missing default is reported even though null case is present. The compiler correctly throws an error if case for FIRST or SECOND is removed, so exhaustiveness is being checked.

[aayushRedHat]

I am on it

[aayushRedHat]

@romani I have a doubt regarding this issue, The check is MissingSwitchDefault which seems to check whether the default keyword is present or not in the switch statement that's why a violation is raised. Can you please highlight this point This check does not validate switch statements that use pattern or null labels. Rationale: Switch statements that use pattern or null labels are checked by the compiler for exhaustiveness. This means that all possible inputs must be covered.

[rnveach]

@aayushRedHat You seemed to have copied the wording from our documentation.

https://checkstyle.org/checks/coding/missingswitchdefault.html#Description

This check does not validate switch statements that use pattern or null labels.

If this statement applies to the code example, then we are not expecting any violations. User has reported there is a violation.

[nrmancuso]

Closed via #14070

[romani]

@nrmancuso , this is defect/bug.

[Auto81]

v10.12.7 is raising a False positive in MissingSwitchDefault when using an exhaustive enum switch statement. Very similar to this issue, should I open a new issue?

class MissingSwitchDefaultTest {
    public void exhaustiveSwitchEnum(SwitchInput i) {
        switch (i) {
            case FIRST -> System.out.println("FIRST");
            case SECOND -> System.out.println("SECOND");
        }
    }

    enum SwitchInput {
        FIRST,
        SECOND
    }
}

[romani]

@Auto81, please create new issue on this

[nrmancuso]

v10.12.7 is raising a False positive in MissingSwitchDefault when using an exhaustive enum switch statement. Very similar to this issue, should I open a new issue?

class MissingSwitchDefaultTest {
    public void exhaustiveSwitchEnum(SwitchInput i) {
        switch (i) {
            case FIRST -> System.out.println("FIRST");
            case SECOND -> System.out.println("SECOND");
        }
    }

    enum SwitchInput {
        FIRST,
        SECOND
    }
}

@Auto81 This is not an "exhaustive switch" as enforced by the compiler. While this is conceptually "exhaustive" as written, this switch is semantically equivalent to:

        switch (i) {
            case FIRST: System.out.println("FIRST"); break;
            case SECOND: System.out.println("SECOND"); break;
        }

Consider:

➜  src cat Test.java
public class Test {
        public void exhaustiveSwitchEnum(SwitchInput i) {
        switch (i) {
            case FIRST -> System.out.println("FIRST");
        }
    }

    enum SwitchInput {
        FIRST,
        SECOND
    }
}
➜  src javac Test.java
➜  src echo $?
0

Also consider:

➜  src cat Test.java
public class Test {
        public void exhaustiveSwitchEnum(SwitchInput i) {
        switch (i) {
            case FIRST -> System.out.println("FIRST");
            case SECOND -> System.out.println("SECOND");
            default -> System.out.println("default");
        }
    }

    enum SwitchInput {
        FIRST,
        SECOND
    }
}
➜  src javac Test.java
➜  src echo $?        
0

Both examples above are compilable, and it is advisable in your case above to have a default branch since the addition of a new enum would not have been caught otherwise in this case:

➜  src cat Test.java  
public class Test {
        public void exhaustiveSwitchEnum(SwitchInput i) {
        switch (i) {
            case FIRST -> System.out.println("FIRST");
            case SECOND -> System.out.println("SECOND");
        }
    }

    enum SwitchInput {
        FIRST,
        SECOND,
        THIRD
    }
}
➜  src javac Test.java
➜  src echo $?        
0

Edit: TLDR; the arrow syntax in switch statements on non-pattern case label elements is not exhaustive.

[va1erian]

@nrmancuso Sorry but I don't understand what constitute an exhaustive switch then, in what case does checkstyle accept omitting the default case?

[nrmancuso]

@nrmancuso Sorry but I don't understand what constitute an exhaustive switch then, in what case does checkstyle accept omitting the default case?

@va1erian thanks for your question. You can omit the default case when the switch statement or expression is exhaustive by Java's definition. This includes all switch expressions, and any switch statements that have Pattern or null labels.

This is an example of an exhaustive switch:

➜  src cat Test.java 
public class Test {
    void m(Object o) {
        switch (o) {
            case Integer i -> {
            }
            case Boolean b -> {
            }
        }
    }
}
➜  src javac Test.java
Test.java:3: error: the switch statement does not cover all possible input values
        switch (o) {
        ^
1 error
➜  src 
➜  src cat Test.java  
public class Test {
    void m(Object o) {
        switch (o) {
            case Integer i -> {
            }
            case Boolean b -> {
            }
            case Object o2 -> {
            }
        }
    }
}
➜  src javac Test.java
➜  src 

In the case above, we would not want Checkstyle to tell us we need a default label, because exhaustiveness is enforced by the compiler. Indeed it does not:

➜  src cat config.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">
  <module name="TreeWalker">
    <module name="MissingSwitchDefault">
   </module>
  </module>
</module>

➜  src cat Test.java 
public class Test {
    void m(Object o) {
        switch (o) {
            case Integer i -> {
            }
            case Boolean b -> {
            }
            case Object o2 -> {
            }
        }
    }
}

➜  src java -jar checkstyle-10.12.1-all.jar -c config.xml Test.java
Starting audit...
Audit done.

A good litmus test on our side going forward for this check as the switch grammar continues to evolve will be guided by the compiler; if a given switch meets the criteria to be checked by the compiler for exhaustiveness (i.e. will throw an error like the example above), then we have no need to require a default label.

[va1erian]

@nrmancuso thank you for the "exhaustive" explanation ;) much appreciated