enable rule no-case-declarations to prevent usage of undeclared variables

[nifgraup]

What version of standard?
12.0.1

What operating system, Node.js, and npm version?
N/A

What did you expect to happen?
This code should be considered an error:

switch (foo) {
  case 1:
    let x = 1
    // some code using x
    break
  case 2:
    x = 2 // ReferenceError on runtime but no lint error
    // some code using x
    break
}

What actually happened?
Code was ported to ES6 & Standard at the same time. Originally it looked like this (simplified):

var x
switch (foo) {
  case 1:
    x = 1
    // some code using x
    break
  case 2:
    x = 2
    // some code using x
    break
}

During refactoring, var x was removed which triggers two no-undef errors. First error is solved with let x = 1 but that makes the second error not to report although x is not declared in case 2.

Adding rule https://eslint.org/docs/rules/no-case-declarations prevents this situation from happening.

[LinusU]

I think we should enable no-case-declarations, it seems like a nice rule, but are you sure that that would fix your problem?

It seems like no-case-declarations would not complain when you wrap the case body in { } (which creates a new lexical scope) 🤔

[nifgraup]

It will fix it because no-undef will start complaining about case 2 once case 1 is wrapped in {}.

[LinusU]

I don't understand how enabling no-case-declaration could affect the behavior of no-undef 🤔 are you sure about that?

[nifgraup]

Wrapping the declaration, let x = 1, in {} is what makes no-undef think x is not defined in x = 2.

[nifgraup]

I see now that the example case statements I wrote were wrong, they were not supposed to have {} in them. I fixed the description.

[brodybits]

I wonder if you should raise a *new* issue that describes the scenario that standard should check for. I think closed issues are more likely to be missed and ignored.

…

On Wed, Oct 10, 2018, 4:40 PM Björgvin Ragnarsson ***@***.***> wrote: I see now that the example case statements I wrote were wrong, they were not supposed to have {} in them. I fixed the description. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1211 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABfNUFI_gEEaWtxXPTQ_mcAHNatN8RP9ks5ujltWgaJpZM4XUzH2> .

[LinusU]

Actually, I did not mean to close this, sorry about that, must have hit the wrong button

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[brodybits]

Can we pin this one to avoid auto-closing?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[brodybits]

please?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[brodybits]

Bump

[feross]

@brodybits Sorry this wasn't tagged to prevent closing.

I personally like this rule. I prefer to enclose case statements with braces to create a new lexical scope. It comes in handy when you want to reuse a variable name in multiple cases, like this:

switch (x) {
  case 1: {
    const y = x.y
    console.log(y)
    break
  }
  case 2: {
    const y = x.y // normally would be an error for redeclaring y. the braces make a new lexical scope
    console.log(y * 2)
  }
}

[feross]

This rule will ship in standard v14. Thanks everyone for the feedback :)