Skip to content
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

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

Closed
dododge opened this issue Nov 24, 2021 · 2 comments

Comments

@dododge
Copy link

dododge commented Nov 24, 2021

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 '}'?
@jingjing-0919
Copy link

I’m trying to fix this.

jingjing-0919 added a commit to jingjing-0919/error-prone that referenced this issue Apr 24, 2022
copybara-service bot pushed a commit that referenced this issue Sep 15, 2023
copybara-service bot pushed a commit that referenced this issue Sep 15, 2023
copybara-service bot pushed a commit that referenced this issue Sep 15, 2023
copybara-service bot pushed a commit that referenced this issue Sep 15, 2023
@cushon
Copy link
Collaborator

cushon commented Sep 15, 2023

b8c865c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants