Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Allow 'break' in switch case + curly braces #1161

Closed
wants to merge 1 commit into from

4 participants

@aknow

For issue #858

@cowwoc

You need to also support return inside a block, not just break.

@aknow

I have add the same handle for return in switch. How about this version?

@Skalman

I have no idea whether it's outside the scope of this issue, but:

  • It should also handle throw
  • It should perform complete flow analysis and figure out whether all blocks eventually break out of the switch
switch (v) {
    case 1: {
        if (a) {
            // ...
            break;
        } else {
            // ...
            break;
        }
    }
    case 2: {
        // ...
    }
}
@cowwoc

@Skalman My guess is that adding support for throw is trivial but doing a complete flow analysis is going to be more complex. Perhaps we can treat the two as separate issues?

@Skalman

@cowwoc Yeah, I realize that flow analysis is a huge undertaking. Just wanted to mention that JSHint should do that too. ;-)

@aknow

OK. Add the throw.
Flow analysis might be a big job.

@valueof
Owner

The code looks good but we will need a test case (see tests/unit for more info) to merge it in. Thanks!

@aknow

Test case done.

@valueof valueof referenced this pull request from a commit
@aknow aknow Fixed #858: Allow 'break' and blocks in switch cases.
Closes #1161.

Signed-off-by: Anton Kovalyov <anton@kovalyov.net>
50306c9
@valueof
Owner

Hm, it seems that GitHub doesn't auto-close bugs unless commits are in master. Oh well. I've merged your patch, thanks!

@valueof valueof closed this
@jugglinmike jugglinmike referenced this pull request from a commit in jugglinmike/jshint
@aknow aknow Fixed #858: Allow 'break' and blocks in switch cases.
Closes #1161.

Signed-off-by: Anton Kovalyov <anton@kovalyov.net>
bbb202f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 16, 2013
  1. @aknow
This page is out of date. Refresh to see the latest.
Showing with 54 additions and 16 deletions.
  1. +18 −4 src/jshint.js
  2. +36 −12 tests/unit/parser.js
View
22 src/jshint.js
@@ -1578,7 +1578,15 @@ var JSHINT = (function () {
// Is it a lonely block?
if (t.id === "{") {
- block(true, true);
+ // Is it a switch case block?
+ //
+ // switch (foo) {
+ // case bar: { <= here.
+ // ...
+ // }
+ // }
+ var iscase = (funct["(verb)"] === "case" && state.tokens.curr.value === ":");
+ block(true, true, false, false, iscase);
return;
}
@@ -1707,11 +1715,13 @@ var JSHINT = (function () {
* Parses a single block. A block is a sequence of statements wrapped in
* braces.
*
- * ordinary - true for everything but function bodies and try blocks.
+ * ordinary - true for everything but function bodies and try blocks.
* stmt - true if block can be a single statement (e.g. in if/for/while).
* isfunc - true if block is a function body
+ * isfatarrow -
+ * iscase - true if block is a switch case block
*/
- function block(ordinary, stmt, isfunc, isfatarrow) {
+ function block(ordinary, stmt, isfunc, isfatarrow, iscase) {
var a,
b = inblock,
old_indent = indent,
@@ -1824,7 +1834,11 @@ var JSHINT = (function () {
delete funct["(nolet)"];
}
- funct["(verb)"] = null;
+ // Don't clear and let it propagate out if it is "break", "return", or "throw" in switch case
+ if (!(iscase && ["break", "return", "throw"].indexOf(funct["(verb)"]) != -1)) {
+ funct["(verb)"] = null;
+ }
+
if (!ordinary || !state.option.funcscope) scope = s;
inblock = b;
if (ordinary && state.option.noempty && (!a || a.length === 0)) {
View
48 tests/unit/parser.js
@@ -341,7 +341,7 @@ exports.regexp = function (test) {
run.test(code, {esnext: true});
run.test(code, {moz: true});
-
+
TestRun(test).test("var y = Math.sqrt(16) / 180;", {es3: true});
TestRun(test).test("var y = Math.sqrt(16) / 180;", {}); // es5
TestRun(test).test("var y = Math.sqrt(16) / 180;", {esnext: true});
@@ -597,7 +597,7 @@ exports.testIdentifiers = function (test) {
run.test(src, {unused: true }); // es5
run.test(src, {esnext: true, unused: true });
run.test(src, {moz: true, unused: true });
-
+
test.done();
};
@@ -1026,7 +1026,7 @@ exports["test: destructuring globals as esnext"] = function (test) {
"[ a, [ [ [ b ], c ], d ] ] = [ 1, [ [ [ 2 ], 3], 4 ] ];",
"[ a, { foo : b } ] = [ 2, { foo : 1 } ];",
];
-
+
TestRun(test)
.addError(4, "'z' is not defined.")
.test(code, {esnext: true, unused: true, undef: true});
@@ -1044,7 +1044,7 @@ exports["test: destructuring globals as es5"] = function (test) {
"[ a, [ [ [ b ], c ], d ] ] = [ 1, [ [ [ 2 ], 3], 4 ] ];",
"[ a, { foo : b } ] = [ 2, { foo : 1 } ];",
];
-
+
TestRun(test)
.addError(4, "'z' is not defined.")
.addError(2, "'destructuring assignment' is only available in JavaScript 1.7.")
@@ -1055,7 +1055,7 @@ exports["test: destructuring globals as es5"] = function (test) {
.addError(7, "'destructuring assignment' is only available in JavaScript 1.7.")
.addError(8, "'destructuring assignment' is only available in JavaScript 1.7.")
.test(code, {unused: true, undef: true}); // es5
-
+
test.done();
};
exports["test: destructuring globals as legacy JS"] = function (test) {
@@ -1069,7 +1069,7 @@ exports["test: destructuring globals as legacy JS"] = function (test) {
"[ a, [ [ [ b ], c ], d ] ] = [ 1, [ [ [ 2 ], 3], 4 ] ];",
"[ a, { foo : b } ] = [ 2, { foo : 1 } ];",
];
-
+
TestRun(test)
.addError(4, "'z' is not defined.")
.addError(2, "'destructuring assignment' is only available in JavaScript 1.7.")
@@ -1080,7 +1080,7 @@ exports["test: destructuring globals as legacy JS"] = function (test) {
.addError(7, "'destructuring assignment' is only available in JavaScript 1.7.")
.addError(8, "'destructuring assignment' is only available in JavaScript 1.7.")
.test(code, {es3: true, unused: true, undef: true});
-
+
test.done();
};
exports["test: destructuring globals with syntax error"] = function (test) {
@@ -1091,7 +1091,7 @@ exports["test: destructuring globals with syntax error"] = function (test) {
"[ a, b; c ] = [ 1, 2, 3 ];",
"[ a, b, c ] = [ 1, 2; 3 ];"
];
-
+
TestRun(test)
.addError(4, "Expected ']' to match '[' from line 4 and instead saw ';'.")
.addError(4, "Expected an assignment or function call and instead saw an expression.")
@@ -1116,7 +1116,7 @@ exports["test: destructuring globals with syntax error"] = function (test) {
.addError(5, "Expected an assignment or function call and instead saw an expression.")
.addError(2, "'z' is not defined.")
.test(code, {esnext: true, unused: true, undef: true});
-
+
test.done();
};
@@ -2311,7 +2311,7 @@ exports["test: mozilla generator as esnext"] = function (test) {
" print(g.next());"
];
TestRun(test)
- .addError(4,
+ .addError(4,
"A yield statement shall be within a generator function (with syntax: `function*`)")
.test(code, {esnext: true, unused: true, undef: true, predef: ["print", "Iterator"]});
@@ -3145,7 +3145,7 @@ exports["automatic comma insertion GH-950"] = function (test) {
.addError(8, "Line breaking error 'return'.")
.addError(9, "Label 'a' on 1 statement.")
.addError(9, "Expected an assignment or function call and instead saw an expression.");
-
+
run.test(code, {es3: true, asi: true});
run.test(code, {asi: true}); // es5
run.test(code, {esnext: true, asi: true});
@@ -3541,7 +3541,7 @@ exports["test for GH-1103"] = function (test) {
test.done();
delete Array.prototype.ohnoes;
-};
+};
exports["test for GH-1105"] = function (test) {
var code = [
@@ -3715,3 +3715,27 @@ exports["test for line breaks with 'yield'"] = function (test) {
run.test(code, {moz: true});
test.done();
};
+
+exports["test for 'break' in switch case + curly braces"] = function (test) {
+ var code = [
+ "switch (foo) {",
+ " case 1: { break; }",
+ " case 2: { return; }",
+ " case 3: { throw 'Error'; }",
+ " case 11: {",
+ " while (true) {",
+ " break;",
+ " }",
+ " }",
+ " default: break;",
+ "}"
+ ];
+
+ // No error for case 1, 2, 3.
+ var run = TestRun(test)
+ .addError(9, "Expected a 'break' statement before 'default'.");
+
+ run.test(code);
+ test.done();
+};
+
Something went wrong with that request. Please try again.