jflex
jflex copied to clipboard
jflex-de
jflex has error-prone fall-through
jflex/jflex-unicode-maven-plugin/target/generated-sources/jflex/jflex/ArchaicLineBreakScanner.java:702: error: [FallThrough] Execution may fall through from the previous case; add a // fall through comment before this line if it was deliberate
case 11: break;
^
see http://errorprone.info/bugpattern/FallThrough
The fall-through violations seem to be in generated code -- we could automatically add the // fall through comment in the generator to indicate that this is on purpose.
Unfortunately, that's not enough.
At the end of emitActions, I have added a fall-through comment
println(" { " + action.content);
println(" } ");
println(" // fall through ");
The problem is that action.content can contain a return or a throw statement ; in which case the fall-through comment is not legitimate anymore.
Right, that's actually what that specific fall through is for: to guard for the cases where there is no return or throw and the scan just continues. Do we get warnings for fall through comments that are spurious? If not, we could add another line with something like
// allowed to fall through if action does not return or throw
Curious... why use fall through to a new case instead of adding an explicit break; ?
// fall through is the exact opposite of break. It means the case falls to the following case statement, and that the developer has not forgotten the break.
I understand, but I noticed quite a few cases that fall-through to cases that only break; and do nothing else. I was just about to edit my post because the answer is probably because you don't know whether the action code included a return or break, and without examining it the safest thing is to fall-through to a separate case that breaks. Blindly inserting a break after an action could result in an unreachable statement otherwise.
Sorry for the noise.
There are still violations https://travis-ci.org/jflex-de/jflex/jobs/431900782
I also believe that JFlex 1.7.0 has fall-through violations that 1.6.1 didn't have.