Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
pumano opened this issue Oct 10, 2024 · 4 comments
Open
Assignees
Labels
A-linter Area - Linter C-bug Category - Bug good first issue Experience Level - Good for newcomers

Comments

@pumano
Copy link
Contributor

pumano commented Oct 10, 2024

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;
    }
@pumano pumano added the C-bug Category - Bug label Oct 10, 2024
@pumano pumano changed the title no-fallthrough has some false positive effects no-fallthrough has some false positive effects when multiple cases present Oct 10, 2024
@DonIsaac DonIsaac added the A-linter Area - Linter label Oct 10, 2024
@Boshen Boshen added the good first issue Experience Level - Good for newcomers label Oct 10, 2024
@kth496
Copy link

kth496 commented Oct 12, 2024

@Boshen

Could I take this issue?

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

@kth496
Copy link

kth496 commented Oct 14, 2024

@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.

let (cfg_ids, tests, default, exit) = get_switch_semantic_cases(ctx, node, switch);

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
Copy link
Contributor

rzvxa commented Oct 15, 2024

@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.

let (cfg_ids, tests, default, exit) = get_switch_semantic_cases(ctx, node, switch);

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
Copy link

kth496 commented Oct 17, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter C-bug Category - Bug good first issue Experience Level - Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants