From 05b3707506efd1cfdb05ed6d328a28a47ea507e2 Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Mon, 12 Jan 2015 20:10:54 +0100 Subject: [PATCH] Fix #1316: Interpolate interpolations safely Instead of compiling to `"" + + (+"-");`, `"#{+}-"'` now gives an appropriate error message: [stdin]:1:5: error: unexpected end of interpolation "#{+}-" ^ This is done by _always_ (instead of just sometimes) wrapping the interpolations in parentheses in the lexer. Unnecessary parentheses won't be output anyway. I got tired of updating the tests in test/location.coffee (which I had enough of in #3770), which relies on implementation details (the exact amount of tokens generated for a given string of code) to do their testing, so I refactored them to be less fragile. --- lib/coffee-script/lexer.js | 18 +++--- src/lexer.coffee | 18 +++--- test/error_messages.coffee | 66 +++++++++++++++++++++ test/interpolation.coffee | 3 + test/location.coffee | 115 +++++++++++-------------------------- 5 files changed, 118 insertions(+), 102 deletions(-) diff --git a/lib/coffee-script/lexer.js b/lib/coffee-script/lexer.js index e1f7fcc7..6892680a 100644 --- a/lib/coffee-script/lexer.js +++ b/lib/coffee-script/lexer.js @@ -570,7 +570,7 @@ }; Lexer.prototype.matchWithInterpolations = function(str, regex, end, offsetInChunk) { - var column, index, line, nested, strPart, tokens, _ref2, _ref3, _ref4; + var close, column, index, line, nested, open, strPart, tokens, _ref2, _ref3, _ref4; tokens = []; while (true) { strPart = regex.exec(str)[0]; @@ -587,14 +587,12 @@ untilBalanced: true }), nested = _ref3.tokens, index = _ref3.index; index += 1; - nested.shift(); - nested.pop(); - if (((_ref4 = nested[0]) != null ? _ref4[0] : void 0) === 'TERMINATOR') { - nested.shift(); - } - if (nested.length > 1) { - nested.unshift(this.makeToken('(', '(', offsetInChunk + 1, 0)); - nested.push(this.makeToken(')', ')', offsetInChunk + 1 + index, 0)); + open = nested[0], close = nested[nested.length - 1]; + open[0] = open[1] = '('; + close[0] = close[1] = ')'; + close.origin = ['', 'end of interpolation', close[2]]; + if (((_ref4 = nested[1]) != null ? _ref4[0] : void 0) === 'TERMINATOR') { + nested.splice(1, 1); } tokens.push(['TOKENS', nested]); str = str.slice(index); @@ -622,7 +620,7 @@ tag = token[0], value = token[1]; switch (tag) { case 'TOKENS': - if (value.length === 0) { + if (value.length === 2) { continue; } locationToken = value[0]; diff --git a/src/lexer.coffee b/src/lexer.coffee index e712e67a..8b6e9172 100644 --- a/src/lexer.coffee +++ b/src/lexer.coffee @@ -499,16 +499,16 @@ exports.Lexer = class Lexer # Skip the trailing `}`. index += 1 - # Remove leading and trailing `{` and `}`. - nested.shift() - nested.pop() + # Turn the leading and trailing `{` and `}` into parentheses. Unnecessary + # parentheses will be removed later. + [open, ..., close] = nested + open[0] = open[1] = '(' + close[0] = close[1] = ')' + close.origin = ['', 'end of interpolation', close[2]] # Remove leading 'TERMINATOR' (if any). - nested.shift() if nested[0]?[0] is 'TERMINATOR' + nested.splice 1, 1 if nested[1]?[0] is 'TERMINATOR' - if nested.length > 1 - nested.unshift @makeToken '(', '(', offsetInChunk + 1, 0 - nested.push @makeToken ')', ')', offsetInChunk + 1 + index, 0 # Push a fake 'TOKENS' token, which will get turned into real tokens later. tokens.push ['TOKENS', nested] @@ -535,8 +535,8 @@ exports.Lexer = class Lexer [tag, value] = token switch tag when 'TOKENS' - # Optimize out empty interpolations. - continue if value.length is 0 + # Optimize out empty interpolations (an empty pair of parentheses). + continue if value.length is 2 # Push all the tokens in the fake 'TOKENS' token. These already have # sane location data. locationToken = value[0] diff --git a/test/error_messages.coffee b/test/error_messages.coffee index 2aa9e7f1..20d89255 100644 --- a/test/error_messages.coffee +++ b/test/error_messages.coffee @@ -121,6 +121,72 @@ test "#1096: unexpected generated tokens", -> ^ ''' +test "#1316: unexpected end of interpolation", -> + assertErrorFormat ''' + "#{+}" + ''', ''' + [stdin]:1:5: error: unexpected end of interpolation + "#{+}" + ^ + ''' + assertErrorFormat ''' + "#{++}" + ''', ''' + [stdin]:1:6: error: unexpected end of interpolation + "#{++}" + ^ + ''' + assertErrorFormat ''' + "#{-}" + ''', ''' + [stdin]:1:5: error: unexpected end of interpolation + "#{-}" + ^ + ''' + assertErrorFormat ''' + "#{--}" + ''', ''' + [stdin]:1:6: error: unexpected end of interpolation + "#{--}" + ^ + ''' + assertErrorFormat ''' + "#{~}" + ''', ''' + [stdin]:1:5: error: unexpected end of interpolation + "#{~}" + ^ + ''' + assertErrorFormat ''' + "#{!}" + ''', ''' + [stdin]:1:5: error: unexpected end of interpolation + "#{!}" + ^ + ''' + assertErrorFormat ''' + "#{not}" + ''', ''' + [stdin]:1:7: error: unexpected end of interpolation + "#{not}" + ^ + ''' + assertErrorFormat ''' + "#{5) + (4}_" + ''', ''' + [stdin]:1:5: error: unmatched ) + "#{5) + (4}_" + ^ + ''' + # #2918 + assertErrorFormat ''' + "#{foo.}" + ''', ''' + [stdin]:1:8: error: unexpected end of interpolation + "#{foo.}" + ^ + ''' + test "#3325: implicit indentation errors", -> assertErrorFormat ''' i for i in a then i diff --git a/test/interpolation.coffee b/test/interpolation.coffee index ca92930e..f16354f9 100644 --- a/test/interpolation.coffee +++ b/test/interpolation.coffee @@ -137,6 +137,9 @@ eq 'multiline nested "interpolations" work', """multiline #{ )()}" } work""" +eq 'function(){}', "#{->}".replace /\s/g, '' +ok /^a[\s\S]+b$/.test "a#{=>}b" +ok /^a[\s\S]+b$/.test "a#{ (x) -> x ** 2 }b" # Regular Expression Interpolation diff --git a/test/location.coffee b/test/location.coffee index e0f5f0f4..c57c8225 100644 --- a/test/location.coffee +++ b/test/location.coffee @@ -19,20 +19,18 @@ test "Verify location of generated tokens", -> tokens = CoffeeScript.tokens "a = 79" eq tokens.length, 4 + [aToken, equalsToken, numberToken] = tokens - aToken = tokens[0] eq aToken[2].first_line, 0 eq aToken[2].first_column, 0 eq aToken[2].last_line, 0 eq aToken[2].last_column, 0 - equalsToken = tokens[1] eq equalsToken[2].first_line, 0 eq equalsToken[2].first_column, 2 eq equalsToken[2].last_line, 0 eq equalsToken[2].last_column, 2 - numberToken = tokens[2] eq numberToken[2].first_line, 0 eq numberToken[2].first_column, 4 eq numberToken[2].last_line, 0 @@ -59,11 +57,19 @@ test "Verify location of generated tokens (with indented first line)", -> eq numberToken[2].last_line, 0 eq numberToken[2].last_column, 7 -test 'Verify locations in string interpolation (in "string")', -> - tokens = CoffeeScript.tokens '"a#{b}c"' +getMatchingTokens = (str, wantedTokens...) -> + tokens = CoffeeScript.tokens str + matchingTokens = [] + i = 0 + for token in tokens + if token[1].replace(/^'|'$/g, '"') is wantedTokens[i] + i++ + matchingTokens.push token + eq wantedTokens.length, matchingTokens.length + matchingTokens - eq tokens.length, 8 - [openParen, a, firstPlus, b, secondPlus, c, closeParen] = tokens +test 'Verify locations in string interpolation (in "string")', -> + [a, b, c] = getMatchingTokens '"a#{b}c"', '"a"', 'b', '"c"' eq a[2].first_line, 0 eq a[2].first_column, 1 @@ -81,10 +87,7 @@ test 'Verify locations in string interpolation (in "string")', -> eq c[2].last_column, 6 test 'Verify locations in string interpolation (in "string", multiple interpolation)', -> - tokens = CoffeeScript.tokens '"#{a}b#{c}"' - - eq tokens.length, 8 - [{}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '"#{a}b#{c}"', 'a', '"b"', 'c' eq a[2].first_line, 0 eq a[2].first_column, 3 @@ -102,10 +105,7 @@ test 'Verify locations in string interpolation (in "string", multiple interpolat eq c[2].last_column, 8 test 'Verify locations in string interpolation (in "string", multiple interpolation and line breaks)', -> - tokens = CoffeeScript.tokens '"#{a}\nb\n#{c}"' - - eq tokens.length, 8 - [{}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '"#{a}\nb\n#{c}"', 'a', '" b "', 'c' eq a[2].first_line, 0 eq a[2].first_column, 3 @@ -123,10 +123,7 @@ test 'Verify locations in string interpolation (in "string", multiple interpolat eq c[2].last_column, 2 test 'Verify locations in string interpolation (in "string", multiple interpolation and starting with line breaks)', -> - tokens = CoffeeScript.tokens '"\n#{a}\nb\n#{c}"' - - eq tokens.length, 8 - [{}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '"\n#{a}\nb\n#{c}"', 'a', '" b "', 'c' eq a[2].first_line, 1 eq a[2].first_column, 2 @@ -144,10 +141,7 @@ test 'Verify locations in string interpolation (in "string", multiple interpolat eq c[2].last_column, 2 test 'Verify locations in string interpolation (in "string", multiple interpolation and starting with line breaks)', -> - tokens = CoffeeScript.tokens '"\n\n#{a}\n\nb\n\n#{c}"' - - eq tokens.length, 8 - [{}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '"\n\n#{a}\n\nb\n\n#{c}"', 'a', '" b "', 'c' eq a[2].first_line, 2 eq a[2].first_column, 2 @@ -165,10 +159,7 @@ test 'Verify locations in string interpolation (in "string", multiple interpolat eq c[2].last_column, 2 test 'Verify locations in string interpolation (in "string", multiple interpolation and starting with line breaks)', -> - tokens = CoffeeScript.tokens '"\n\n\n#{a}\n\n\nb\n\n\n#{c}"' - - eq tokens.length, 8 - [{}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '"\n\n\n#{a}\n\n\nb\n\n\n#{c}"', 'a', '" b "', 'c' eq a[2].first_line, 3 eq a[2].first_column, 2 @@ -186,10 +177,7 @@ test 'Verify locations in string interpolation (in "string", multiple interpolat eq c[2].last_column, 2 test 'Verify locations in string interpolation (in """string""", line breaks)', -> - tokens = CoffeeScript.tokens '"""a\n#{b}\nc"""' - - eq tokens.length, 8 - [{}, a, {}, b, {}, c, {}, {}] = tokens + [a, b, c] = getMatchingTokens '"""a\n#{b}\nc"""', '"a\\n"', 'b', '"\\nc"' eq a[2].first_line, 0 eq a[2].first_column, 3 @@ -207,10 +195,7 @@ test 'Verify locations in string interpolation (in """string""", line breaks)', eq c[2].last_column, 0 test 'Verify locations in string interpolation (in """string""", starting with a line break)', -> - tokens = CoffeeScript.tokens '"""\n#{b}\nc"""' - - eq tokens.length, 6 - [{}, b, {}, c] = tokens + [b, c] = getMatchingTokens '"""\n#{b}\nc"""', 'b', '"\\nc"' eq b[2].first_line, 1 eq b[2].first_column, 2 @@ -223,10 +208,7 @@ test 'Verify locations in string interpolation (in """string""", starting with a eq c[2].last_column, 0 test 'Verify locations in string interpolation (in """string""", starting with line breaks)', -> - tokens = CoffeeScript.tokens '"""\n\n#{b}\nc"""' - - eq tokens.length, 8 - [{}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '"""\n\n#{b}\nc"""', '"\\n"', 'b', '"\\nc"' eq a[2].first_line, 0 eq a[2].first_column, 3 @@ -244,10 +226,7 @@ test 'Verify locations in string interpolation (in """string""", starting with l eq c[2].last_column, 0 test 'Verify locations in string interpolation (in """string""", multiple interpolation)', -> - tokens = CoffeeScript.tokens '"""#{a}\nb\n#{c}"""' - - eq tokens.length, 8 - [{}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '"""#{a}\nb\n#{c}"""', 'a', '"\\nb\\n"', 'c' eq a[2].first_line, 0 eq a[2].first_column, 5 @@ -265,10 +244,7 @@ test 'Verify locations in string interpolation (in """string""", multiple interp eq c[2].last_column, 2 test 'Verify locations in string interpolation (in """string""", multiple interpolation, and starting with line breaks)', -> - tokens = CoffeeScript.tokens '"""\n\n#{a}\n\nb\n\n#{c}"""' - - eq tokens.length, 10 - [{}, {}, {}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '"""\n\n#{a}\n\nb\n\n#{c}"""', 'a', '"\\n\\nb\\n\\n"', 'c' eq a[2].first_line, 2 eq a[2].first_column, 2 @@ -286,10 +262,7 @@ test 'Verify locations in string interpolation (in """string""", multiple interp eq c[2].last_column, 2 test 'Verify locations in string interpolation (in """string""", multiple interpolation, and starting with line breaks)', -> - tokens = CoffeeScript.tokens '"""\n\n\n#{a}\n\n\nb\n\n\n#{c}"""' - - eq tokens.length, 10 - [{}, {}, {}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '"""\n\n\n#{a}\n\n\nb\n\n\n#{c}"""', 'a', '"\\n\\n\\nb\\n\\n\\n"', 'c' eq a[2].first_line, 3 eq a[2].first_column, 2 @@ -307,10 +280,7 @@ test 'Verify locations in string interpolation (in """string""", multiple interp eq c[2].last_column, 2 test 'Verify locations in heregex interpolation (in ///regex///, multiple interpolation)', -> - tokens = CoffeeScript.tokens '///#{a}b#{c}///' - - eq tokens.length, 11 - [{}, {}, {}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '///#{a}b#{c}///', 'a', '"b"', 'c' eq a[2].first_line, 0 eq a[2].first_column, 5 @@ -328,10 +298,7 @@ test 'Verify locations in heregex interpolation (in ///regex///, multiple interp eq c[2].last_column, 10 test 'Verify locations in heregex interpolation (in ///regex///, multiple interpolation)', -> - tokens = CoffeeScript.tokens '///a#{b}c///' - - eq tokens.length, 11 - [{}, {}, {}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '///a#{b}c///', '"a"', 'b', '"c"' eq a[2].first_line, 0 eq a[2].first_column, 3 @@ -349,10 +316,7 @@ test 'Verify locations in heregex interpolation (in ///regex///, multiple interp eq c[2].last_column, 8 test 'Verify locations in heregex interpolation (in ///regex///, multiple interpolation and line breaks)', -> - tokens = CoffeeScript.tokens '///#{a}\nb\n#{c}///' - - eq tokens.length, 11 - [{}, {}, {}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '///#{a}\nb\n#{c}///', 'a', '"b"', 'c' eq a[2].first_line, 0 eq a[2].first_column, 5 @@ -370,10 +334,7 @@ test 'Verify locations in heregex interpolation (in ///regex///, multiple interp eq c[2].last_column, 2 test 'Verify locations in heregex interpolation (in ///regex///, multiple interpolation and line breaks)', -> - tokens = CoffeeScript.tokens '///#{a}\n\n\nb\n\n\n#{c}///' - - eq tokens.length, 11 - [{}, {}, {}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '///#{a}\n\n\nb\n\n\n#{c}///', 'a', '"b"', 'c' eq a[2].first_line, 0 eq a[2].first_column, 5 @@ -391,10 +352,7 @@ test 'Verify locations in heregex interpolation (in ///regex///, multiple interp eq c[2].last_column, 2 test 'Verify locations in heregex interpolation (in ///regex///, multiple interpolation and line breaks)', -> - tokens = CoffeeScript.tokens '///a\n\n\n#{b}\n\n\nc///' - - eq tokens.length, 11 - [{}, {}, {}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '///a\n\n\n#{b}\n\n\nc///', '"a"', 'b', '"c"' eq a[2].first_line, 0 eq a[2].first_column, 3 @@ -412,10 +370,7 @@ test 'Verify locations in heregex interpolation (in ///regex///, multiple interp eq c[2].last_column, 0 test 'Verify locations in heregex interpolation (in ///regex///, multiple interpolation and line breaks and starting with linebreak)', -> - tokens = CoffeeScript.tokens '///\n#{a}\nb\n#{c}///' - - eq tokens.length, 11 - [{}, {}, {}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '///\n#{a}\nb\n#{c}///', 'a', '"b"', 'c' eq a[2].first_line, 1 eq a[2].first_column, 2 @@ -433,10 +388,7 @@ test 'Verify locations in heregex interpolation (in ///regex///, multiple interp eq c[2].last_column, 2 test 'Verify locations in heregex interpolation (in ///regex///, multiple interpolation and line breaks and starting with linebreak)', -> - tokens = CoffeeScript.tokens '///\n\n\n#{a}\n\n\nb\n\n\n#{c}///' - - eq tokens.length, 11 - [{}, {}, {}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '///\n\n\n#{a}\n\n\nb\n\n\n#{c}///', 'a', '"b"', 'c' eq a[2].first_line, 3 eq a[2].first_column, 2 @@ -454,10 +406,7 @@ test 'Verify locations in heregex interpolation (in ///regex///, multiple interp eq c[2].last_column, 2 test 'Verify locations in heregex interpolation (in ///regex///, multiple interpolation and line breaks and starting with linebreak)', -> - tokens = CoffeeScript.tokens '///\n\n\na\n\n\n#{b}\n\n\nc///' - - eq tokens.length, 11 - [{}, {}, {}, a, {}, b, {}, c] = tokens + [a, b, c] = getMatchingTokens '///\n\n\na\n\n\n#{b}\n\n\nc///', '"a"', 'b', '"c"' eq a[2].first_line, 0 eq a[2].first_column, 3