From bcc6384aa21be85629c8b94d46472b78a7a5dc25 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 24 Aug 2017 20:43:49 -0700 Subject: [PATCH 1/3] Fix #3709: throwing an if, for, switch or while should throw the returned value of the statement/loop --- lib/coffeescript/nodes.js | 6 +++- src/nodes.coffee | 9 ++++- test/exception_handling.coffee | 62 +++++++++++++++++++++++++++------- 3 files changed, 62 insertions(+), 15 deletions(-) diff --git a/lib/coffeescript/nodes.js b/lib/coffeescript/nodes.js index 1d5609bd96..2390460a21 100644 --- a/lib/coffeescript/nodes.js +++ b/lib/coffeescript/nodes.js @@ -4774,7 +4774,11 @@ compileNode(o) { var fragments; - fragments = this.expression.compileToFragments(o); + if (this.expression instanceof For || this.expression instanceof If || this.expression instanceof Switch || this.expression instanceof While) { + fragments = this.expression.compileClosure(o); + } else { + fragments = this.expression.compileToFragments(o); + } unshiftAfterComments(fragments, this.makeCode('throw ')); fragments.unshift(this.makeCode(this.tab)); fragments.push(this.makeCode(';')); diff --git a/src/nodes.coffee b/src/nodes.coffee index 677d4df028..73fa695713 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -3255,7 +3255,14 @@ exports.Throw = class Throw extends Base makeReturn: THIS compileNode: (o) -> - fragments = @expression.compileToFragments o + if @expression instanceof For or + @expression instanceof If or + @expression instanceof Switch or + @expression instanceof While + fragments = @expression.compileClosure o + else + fragments = @expression.compileToFragments o + unshiftAfterComments fragments, @makeCode 'throw ' fragments.unshift @makeCode @tab fragments.push @makeCode ';' diff --git a/test/exception_handling.coffee b/test/exception_handling.coffee index 3c3240b778..8d17931beb 100644 --- a/test/exception_handling.coffee +++ b/test/exception_handling.coffee @@ -89,10 +89,8 @@ test "try/catch with empty catch as last statement in a function body", -> catch err eq nonce, fn() - -# Catch leads to broken scoping: #1595 - -test "try/catch with a reused variable name.", -> +test "#1595: try/catch with a reused variable name", -> + # `catch` shouldn’t lead to broken scoping. do -> try inner = 5 @@ -100,11 +98,7 @@ test "try/catch with a reused variable name.", -> # nothing eq typeof inner, 'undefined' - -# Allowed to destructure exceptions: #2580 - -test "try/catch with destructuring the exception object", -> - +test "#2580: try/catch with destructuring the exception object", -> result = try missing.object catch {message} @@ -112,8 +106,6 @@ test "try/catch with destructuring the exception object", -> eq message, 'missing is not defined' - - test "Try catch finally as implicit arguments", -> first = (x) -> x @@ -130,8 +122,8 @@ test "Try catch finally as implicit arguments", -> catch e eq bar, yes -# Catch Should Not Require Param: #2900 -test "parameter-less catch clause", -> +test "#2900: parameter-less catch clause", -> + # `catch` should not require a parameter. try throw new Error 'failed' catch @@ -140,3 +132,47 @@ test "parameter-less catch clause", -> try throw new Error 'failed' catch finally ok true ok try throw new Error 'failed' catch then true + +test "#3709: throwing an if statement", -> + # `throw if` should return a closure around the `if` block, so that the + # output is valid JavaScript. + try + throw if no + new Error 'drat!' + else + new Error 'no escape!' + catch err + eq err.message, 'no escape!' + + try + throw if yes then new Error 'huh?' else null + catch err + eq err.message, 'huh?' + +test "#3709: throwing a switch statement", -> + i = 3 + try + throw switch i + when 2 + new Error 'not this one' + when 3 + new Error 'oh no!' + catch err + eq err.message, 'oh no!' + +test "#3709: throwing a for loop", -> + # `throw for` should return a closure around the `for` block, so that the + # output is valid JavaScript. + try + throw for i in [0..3] + i * 2 + catch err + arrayEq err, [0, 2, 4, 6] + +test "#3709: throwing a while loop", -> + i = 0 + try + throw while i < 3 + i++ + catch err + eq i, 3 From 18e1ec439d8bcac891849b5e7675a265014d495e Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 24 Aug 2017 20:51:28 -0700 Subject: [PATCH 2/3] =?UTF-8?q?Fix=20#3789:=20don=E2=80=99t=20throw=20a=20?= =?UTF-8?q?throw=20(unless=20it=E2=80=99s=20in=20a=20closure)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/coffeescript/nodes.js | 2 +- src/nodes.coffee | 1 + test/exception_handling.coffee | 6 ++++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/coffeescript/nodes.js b/lib/coffeescript/nodes.js index 2390460a21..320aac73fc 100644 --- a/lib/coffeescript/nodes.js +++ b/lib/coffeescript/nodes.js @@ -4774,7 +4774,7 @@ compileNode(o) { var fragments; - if (this.expression instanceof For || this.expression instanceof If || this.expression instanceof Switch || this.expression instanceof While) { + if (this.expression instanceof For || this.expression instanceof If || this.expression instanceof Switch || this.expression instanceof Throw || this.expression instanceof While) { fragments = this.expression.compileClosure(o); } else { fragments = this.expression.compileToFragments(o); diff --git a/src/nodes.coffee b/src/nodes.coffee index 73fa695713..3b3cbf6410 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -3258,6 +3258,7 @@ exports.Throw = class Throw extends Base if @expression instanceof For or @expression instanceof If or @expression instanceof Switch or + @expression instanceof Throw or @expression instanceof While fragments = @expression.compileClosure o else diff --git a/test/exception_handling.coffee b/test/exception_handling.coffee index 8d17931beb..416bdc534f 100644 --- a/test/exception_handling.coffee +++ b/test/exception_handling.coffee @@ -176,3 +176,9 @@ test "#3709: throwing a while loop", -> i++ catch err eq i, 3 + +test "#3789: throwing a throw", -> + try + throw throw throw new Error 'whoa!' + catch err + eq err.message, 'whoa!' From 8e7b3ef30e03916cad87cacbf5c1a0b7c802d638 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 25 Aug 2017 09:49:41 -0700 Subject: [PATCH 3/3] LEVEL_LIST works better than a list of node types --- lib/coffeescript/nodes.js | 6 +----- src/nodes.coffee | 10 +--------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/lib/coffeescript/nodes.js b/lib/coffeescript/nodes.js index 320aac73fc..6406e29c28 100644 --- a/lib/coffeescript/nodes.js +++ b/lib/coffeescript/nodes.js @@ -4774,11 +4774,7 @@ compileNode(o) { var fragments; - if (this.expression instanceof For || this.expression instanceof If || this.expression instanceof Switch || this.expression instanceof Throw || this.expression instanceof While) { - fragments = this.expression.compileClosure(o); - } else { - fragments = this.expression.compileToFragments(o); - } + fragments = this.expression.compileToFragments(o, LEVEL_LIST); unshiftAfterComments(fragments, this.makeCode('throw ')); fragments.unshift(this.makeCode(this.tab)); fragments.push(this.makeCode(';')); diff --git a/src/nodes.coffee b/src/nodes.coffee index 3b3cbf6410..f3b8990274 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -3255,15 +3255,7 @@ exports.Throw = class Throw extends Base makeReturn: THIS compileNode: (o) -> - if @expression instanceof For or - @expression instanceof If or - @expression instanceof Switch or - @expression instanceof Throw or - @expression instanceof While - fragments = @expression.compileClosure o - else - fragments = @expression.compileToFragments o - + fragments = @expression.compileToFragments o, LEVEL_LIST unshiftAfterComments fragments, @makeCode 'throw ' fragments.unshift @makeCode @tab fragments.push @makeCode ';'