Skip to content
This repository

Enable case statement fall through #18

Closed
nzakas opened this Issue February 18, 2011 · 10 comments

8 participants

Nicholas C. Zakas Anton Kovalyov Ben Alman Meinte Boersma Daghall Adam Crabtree Gregory Jacobs Jaime Pillora
Nicholas C. Zakas

One of things I've never been able to convince Crockford of is to allow case statement fallthroughs.

Currently, this is okay:

switch(foo){
case 1:
case 2:
doSomething();
}

But this is not:

switch(foo){
case 1:
doSomethingFirst();
case 2:
doSomething();
}

In a version of JSLint that I hacked before, I made a check for a comment /falls through/ to indication that you intend to fall through:

switch(foo){
case 1:
doSomethingFirst();
/falls through/
case 2:
doSomething();
}

I'd really like to see this included in JSHint, as it's been a pain in JSLint for a very long time for me.

Anton Kovalyov
Owner

Also, from #11 — JSHint probably should have an option to allow fall through all the way to the default.

Anton Kovalyov
Owner

Since more often than not case fall through is unintentional, I didn't add a separate option to just ignore the message. Instead, I re-used your approach with an explicit comment saying that fall through is intentional.

switch(foo) {
case 1:
  dosmth();
  /* falls through */
case 2:
  dosmth();
}

Related commit: 4a72da1.

Nicholas C. Zakas

Awesome, thanks!

Ben Alman

IIRC, Crockford makes a specific mention in his book why he dislikes case fall-through. The story is both amusing and somewhat delf-deprecating.

Meinte Boersma

Sorry to comment on this, but the warning is still triggered for cases with a throw at the end of a block. You can disable the warning with a /* falls through */ annotation, but the annotation is "disturbed" (in the sense that the warning is then triggered after all) by anything other than whitespace between the previous case, the annotation and the next case so I can't even comment on why the comment's there.

Daghall

The /* falls through */ comment instead of break; is an undocumented feature. Please add it to the docs.

Additional comments can be added on a line before /* falls through */.

Adam Crabtree

Seconded. Please add /*falls through*/ to the docs.

Adam Crabtree

Also, please add supprt for:

case 'none':
default:

While technically the 'none' case is unnecessary, it does add to the readability of the code.

Gregory Jacobs

+1 for adding info about this to the docs.

As a point of interest, I recently found a situation where "falls through" cases make sense. I used this for performing migrations of versioned data read from localStorage. Ex:

function migrate( version, data ) {
    switch( version ) {
        case 1 :
            data.new1 = data.old;  // convert data from version 1 to version 2
            delete data.old;
        case 2 :
            data.new2 = data.new1;  // convert data from version 2 to version 3
            delete data.new1;
    }
    return data;  // return data in version 3 format
}

As the data format evolves, the code can be maintained by adding cases for migrating older versions, and all migrations to bring a particular version up to the latest are applied.

Joscha Feth joscha referenced this issue in NV/CSSOM March 19, 2014
Merged

JSHint fix #73

Jaime Pillora

For those looking for the .jshintrc object key:

"-W086": true, //allow fall-through
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.