Rule Change: no-fallthrough should not permit "falls through" comment when code will not fall through.

[kirkwaiblinger]

What rule do you want to change?

no-fallthrough

What change do you want to make?

Generate more warnings

How do you think the change should be implemented?

A new default behavior

Example code

const aVariable = "something";
switch (aVariable) {
  case 1:
    console.log('did something in case 1 that will not fall through to case 2!')
    break;
    // fallthrough
  case 2:
    console.log('did something in case 2!')
    break;
}

What does the rule currently do for this code?

No warnings

What will the rule do after it's changed?

the // fallthrough comment should be flagged, because it incorrectly implies to a reader that a fallthrough can/will occur.

Participation

• I am willing to submit a pull request to implement this change.

Additional comments

This very similar in concept to --report-unused-disable-directives, so I'm not terribly sure whether it ought to flag as part of the rule per se, or when running --report-unused-disable-directives. But I think that having this would be especially valuable for cases with a multiple branches, where it doesn't literally terminate in a break/return. For example, something like

function myBigConfusingSwitch() {
  const aVariable = "something";
  switch (aVariable) {
    case 1:
      if (Math.random() > 0.5) {
        if (Math.random() > 0.5) {
          console.log('do something!')
          return 42;
        } else if (Math.random() > 0.5) {
          console.log('do something!');
          break;
        } else {
          break;
        }
      } else {
        return 4242
      }
      // fallthrough
    case 2:
      // did I fall through or not?
      console.log('do something in case 2!')
      break;
  }
}

Note that if I put code where the fallthrough comment is, it would get flagged for being unreachable

PS I discovered this organically, making this very mistake during refactoring

[Tanujkanti4441]

Hi @kirkwaiblinger, thanks for the issue, i think reporting comments shouldn't be the default behavior of no-fallthrough rule as it mainly reports fallthrough from switch cases, but an option can be added for this.

[mdjermanovic]

the // fallthrough comment should be flagged, because it incorrectly implies to a reader that a fallthrough can/will occur

Makes sense to me 👍

I agree with @Tanujkanti4441 that this behavior should be behind an option.

@eslint/eslint-team thoughts about this?

[JoshuaKGoldberg]

👍 agreed this is a good idea for an opt-in.

[fasttime]

It looks like there is consensus to add the ability to flag useless // fallthrough comments behind an option. @kirkwaiblinger feel free to submit a PR.