eslint / eslint

Find and fix problems in your JavaScript code.
https://eslint.org
MIT License
24.39k stars 4.4k forks source link

feat: `no-case-declarations` add suggestions #18388

Closed JoshuaKGoldberg closed 3 weeks ago

JoshuaKGoldberg commented 3 weeks ago

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update [ ] Bug fix (template) [ ] New rule (template) [ ] Changes an existing rule (template) [x] Add ~autofix~ suggestion to a rule [ ] Add a CLI option [ ] Add something to the core [ ] Other, please explain:

Fixes https://github.com/eslint/eslint/issues/18349

What changes did you make? (Give an overview)

Adds a suggestion fixer to no-case-declarations to add { } brackets around blocks.

Also adds a couple new tests to make sure switches with multiple violating cases report on the right case (consequent).

Is there anything you'd like reviewers to focus on?

I'd considered adding logic for friendlier { } bracket placement but then decided against the added complexity.

netlify[bot] commented 3 weeks ago

Deploy Preview for docs-eslint canceled.

Name Link
Latest commit 9dced471fbbb9c21f51a2a8d39d0dcd722264d15
Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/662967f07277fa0009093e1d
fasttime commented 3 weeks ago

When a declared identifier is used in another case block, applying the suggestion can change the logic, for example:

// ecmaVersion: "latest"
switch ("foo") {
    case "bar":
        function baz() { }
        break;
    default:
        baz();
}

compared to:

// ecmaVersion: "latest"
switch ("foo") {
    case "bar":
        { function baz() { }
        break; }
    default:
        baz(); // TypeError!
}

I assume this is acceptable for a suggestion, but maybe we should add a unit test to show that we are aware of the risk and we know what we are doing?