#1249 Allow skip of individual SF_SWITCH_FALLTHROUGH

[David Harkness]

With the Checkstyle plugin you can use an inline comment to indicate that you mean for one case to fall through to the next. It would be great for FindBugs to honor the same comment.

However, that's my understanding is that since FindBugs processes the bytecode this isn't possible. Is there any other method that could work here?

Here's a ridiculous example demonstrating how it works with Checkstyle:

public boolean isSouth(Direction direction) {
    switch (direction) {
    case EAST: // fallthru
    case WEST: // fallthru
    case NORTH:
        return false;
    case SOUTH:
        return true;
    }
}

[Andrey Loskutov]

Hmm, I can't compile that example even if I define the enum by myself - return is missing at the end of the method.

Nevertheless, there is a system property "findbugs.sf.comment" you can set for FB, which allows him to check line comments as above, see [1]. Of course you must set "-sourcepath" argument to let FB know where the code is. Strange enough, I just found that not all command line options are documented, so please check [2] for currently available.

With all that, should we close this issue?

[1] http://findbugs.sourceforge.net/manual/analysisprops.html
[2] http://code.google.com/p/findbugs/source/browse/findbugs/src/java/edu/umd/cs/findbugs/TextUICommandLine.java

Regards,
Andrey

[David Harkness]

Sorry, it's been some time since I used the SF.net issue tracker. I forgot that it separates bugs and feature requests. Is there a way to move this and the other request I created about ignoring warnings using "serial"?

[William Pugh]

First, the example you show for CheckStyle isn't a bug and FindBugs wouldn't complain about it. Having multiple case statements for one block of code isn't a fall trough.

But as Andy mentioned, we will check this under the circumstance described. I guess the question is whether we should change that option to the default.

[David Harkness]

Ah, I wasn't aware that it was only considered "fall-through" if the case wasn't empty. My apologies for a doubly-failed example. Here is the actual code where I see the FindBugs warning and use the Checkstyle comment:

SystemPhase phase = shutdownService.shutdown();
StringBuilder status = new StringBuilder();
switch (phase) {
case SHUTDOWN:
    status.insert(0, "done\n\nShutdown complete");
    // fallthru
case JOB_QUEUE:
    status.insert(0, "done\n\nStopping thread pool...");
    // fallthru
case SESSIONS:
    status.insert(0, "\n\nBlocking new sessions...");
    // fallthru
case RUNNING:
    status.insert(0, "System shutdown initiated");
    // fallthru
default:
    break;
}
return status.toString();

[Andrey Loskutov]

I've fixed it for Eclipse, so that we will always honor the source code comments for switches.
See https://code.google.com/p/findbugs/source/detail?r=d13b9b59cdf17125cc50a2d51050f03fe826e7a1

For the command line see comments above regarding the system property you have to set.

Regards,
Andrey