error-prone icon indicating copy to clipboard operation
error-prone copied to clipboard

MissingDefault gives false positives about default case comments in new-style switches

Open dododge opened this issue 4 years ago • 1 comments

This is in error-prone 2.10.0.

If I use a new-style switch statement with a default case, MissingDefault will usually complain that the default case doesn't have a comment. I think the intended behavior is that it's only supposed to do that if the default case does nothing and exits the switch, but it's giving the warnings for switches that do have code in the default case.

Even if you add a comment to the default case to try to stop the warning, it usually doesn't recognize that a comment is there unless it's in a very specific location. See examples below.

package something;

@SuppressWarnings("SwitchDefault") // Suppress SwitchDefault crashes, issue #2705
public class Example
{
    public static void nothing1() { }
    public static void nothing2() { }
    public static void nothing3() { }

    public static void foo(int i) {

        // False positive?
        // [MissingDefault] Default case should be documented with a comment
        switch (i) {
            case 0 -> nothing1();
            case 1 -> nothing2();
            default -> nothing3();
        }

        // False positive
        // [MissingDefault] Default case should be documented with a comment
        switch (i) {
            case 0 -> nothing1();
            case 1 -> nothing2();

            // Here is a comment.
            default -> {
                nothing3();
            }
        }
        // False positive
        // [MissingDefault] Default case should be documented with a comment
        switch (i) {
            case 0 -> nothing1();
            case 1 -> nothing2();

            default ->
                // Here is a comment.
                nothing3();
        }

        // False positive
        // [MissingDefault] Default case should be documented with a comment
        switch (i) {
            case 0 -> nothing1();
            case 1 -> nothing2();

            default -> // Here is a comment.
                nothing3();
        }

        // False positive
        // [MissingDefault] Default case should be documented with a comment
        switch (i) {
            case 0 -> nothing1();
            case 1 -> nothing2();

            default -> {
                // Here is a comment.
                nothing3();
            }
        }

        // No warning for this switch.
        switch (i) {
            case 0 -> nothing1();
            case 1 -> nothing2();

            default -> nothing3(); // Here is a comment.
        }

    }

    private Example() { }
}

Only one of those switches makes it through cleanly; the other five get an unexpected warning:

[WARNING] /tmp/test/src/main/java/something/Example.java:[17,12] [MissingDefault] Default case should be documented with a comment
    (see https://errorprone.info/bugpattern/MissingDefault)
  Did you mean 'default -> nothing3();'?
[WARNING] /tmp/test/src/main/java/something/Example.java:[27,12] [MissingDefault] Default case should be documented with a comment
    (see https://errorprone.info/bugpattern/MissingDefault)
  Did you mean '}'?
[WARNING] /tmp/test/src/main/java/something/Example.java:[37,12] [MissingDefault] Default case should be documented with a comment
    (see https://errorprone.info/bugpattern/MissingDefault)
  Did you mean 'nothing3();'?
[WARNING] /tmp/test/src/main/java/something/Example.java:[48,12] [MissingDefault] Default case should be documented with a comment
    (see https://errorprone.info/bugpattern/MissingDefault)
  Did you mean 'nothing3();'?
[WARNING] /tmp/test/src/main/java/something/Example.java:[58,12] [MissingDefault] Default case should be documented with a comment
    (see https://errorprone.info/bugpattern/MissingDefault)
  Did you mean '}'?

dododge avatar Nov 24 '21 14:11 dododge

I’m trying to fix this.

jingjing-0919 avatar Apr 24 '22 17:04 jingjing-0919