no-fallthrough has some false positive effects when multiple cases present

[pumano]

in some edge cases (looks like problem in || and &&):

    const notification = { type: 'PRIMARY' };

    switch (true) {
      case notification.type === 'PRIMARY' || notification.type === 'SECONDARY':
        // TODO
        break;
      case notification.type === 'SECONDARY':
        // TODO
        break;
      case notification.type === 'OUTLINE' || notification.type === 'SECONDARY':
        // TODO
        break;
      case notification.type === 'ETC':
        // TODO
        break;
    }

    const notification = { type: 'PRIMARY' };
    const other = true;

    switch (true) {
      case notification.type === 'PRIMARY' && other:
        // TODO
        break;
      case notification.type === 'SECONDARY':
        // TODO
        break;
      case notification.type === 'OUTLINE' && other:
        // TODO
        break;
      case notification.type === 'ETC':
        // TODO
        break;
    }

[kth496]

@Boshen

Could I take this issue?

I have reproduced the different behavior between eslint and oxlint for the no-fallthrough rule.

[kth496]

@Boshen @rzvxa I've been looking into this issue and am having the following difficulties.

In the switch-case syntax, when the condition contains a logical expression, the rule is not checked because the tests variable is not populated with the appropriate value. https://github.com/oxc-project/oxc/blob/a9260cf6d1b83917c7a61b25cabd2d40858b0fff/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs#L269

The tests is generated from the get_switch_semantic_cases function, and the comments above this function say to treat this function as a black box, which is not an easy situation to fix.

Can I get some help to resolve this issue?

[rzvxa]

@Boshen @rzvxa I've been looking into this issue and am having the following difficulties.

In the switch-case syntax, when the condition contains a logical expression, the rule is not checked because the tests variable is not populated with the appropriate value.

https://github.com/oxc-project/oxc/blob/a9260cf6d1b83917c7a61b25cabd2d40858b0fff/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs#L269

The tests is generated from the get_switch_semantic_cases function, and the comments above this function say to treat this function as a black box, which is not an easy situation to fix.

Can I get some help to resolve this issue?

Yeah, We have some limitations at the moment that makes accessing cases really difficult. Feel free to tinker with that demonic function to get it to work.

Let me know how I can help you with this.

[kth496]

@rzvxa I thought modifying that function was prohibited, I'll see if I can improve it, thanks.

[Boshen]

@kth496 do you have a PR? I'll let someone else work on this if you don't have the time.

[kth496]

@Boshen I have spent quite some time reviewing the codebase with great interest, but unfortunately, I was unable to make progress on the work. Could you kindly unassign me?

[vikiboss]

There seems to be something wrong with this case as well, but it works in ESLint (v9.17.0).

oxlint version: 0.15.5

// index.ts
export const getLangByCodeType = (codeType: string): string => {
  switch (codeType) {
    case 'Apache Maven':
    case 'Apache Ivy':
    default:
      return 'xml'

    case 'Gradle Groovy DSL':
    case 'Gradle Kotlin DSL':
      return 'groovy'

    case 'Scala SBT':
      return 'scala'

    case 'Composer':
      return 'json'
  }
}

// .oxlintrc.json
{
  "rules": {
    "no-fallthrough": ["error", { "allowEmptyCase": true }]
  }
}

[ityuany]

#11315

[camc314]

@vikiboss do you mind making a new issue for that case? i couldn't reproduce it, but if feels separate to the original one here

[vikiboss]

@vikiboss do you mind making a new issue for that case? i couldn't reproduce it, but if feels separate to the original one here

Sure! I'll open a new issue with a minimal reproduction repo to demonstrate the bug.

[vikiboss]

done. https://github.com/oxc-project/oxc/issues/11340

[camc314]

fixed via #14811