oxc-project / oxc

⚓ A collection of JavaScript tools written in Rust.
https://oxc.rs
MIT License
12.42k stars 456 forks source link

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

Open pumano opened 1 month ago

pumano commented 1 month ago

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 commented 1 month ago

@Boshen

Could I take this issue?

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

kth496 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

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