From 8292d25d2976aa630491ab5387c3a9da06d64db3 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sat, 8 Apr 2017 21:59:09 -0700 Subject: [PATCH] [CS2] Throw an error for ambiguous `get` or `set` keywords or function calls (#4484) * Throw an error for ambiguous `get` or `set` function calls or ES5 getter/setter keywords, to warn the user to use parentheses if they intend a function call (or to inform them that `get` or `set` cannot be used as a keyword) * Code golf * Catch get or set keyword before static method * DRY up getting the previous token * Throw an error if get or set are used as keywords before what looks like a function or method with an interpolated/dynamic name * Allow `get` or `set` parentheses-less function calls when first argument is a string without a colon (so a plain string, not a property accessor) * Revert "Allow `get` or `set` parentheses-less function calls when first argument is a string without a colon (so a plain string, not a property accessor)" This reverts commit 2d1addf5a4d2ebdb4039aee8d9f751d2c50b50da. * Optimization * No longer throw an error on `get` or `set` function calls to objects with dynamic property names (introduces a way to circumvent our check for trying to avoid the `get` or `set` keywords, but not worth the complications for this tiny edge case) --- lib/coffeescript/lexer.js | 52 +++++++----- src/lexer.coffee | 27 +++++-- src/nodes.coffee | 2 +- test/error_messages.coffee | 136 ++++++++++++++++++++++++++++++++ test/function_invocation.coffee | 44 ++++++++++- 5 files changed, 234 insertions(+), 27 deletions(-) diff --git a/lib/coffeescript/lexer.js b/lib/coffeescript/lexer.js index 46018cda4d..96dbd8a772 100644 --- a/lib/coffeescript/lexer.js +++ b/lib/coffeescript/lexer.js @@ -64,7 +64,7 @@ } identifierToken() { - var alias, colon, colonOffset, id, idLength, input, match, poppedToken, prev, ref, ref1, ref2, ref3, ref4, ref5, ref6, tag, tagToken; + var alias, colon, colonOffset, id, idLength, input, match, poppedToken, prev, prevprev, ref, ref1, ref2, ref3, ref4, ref5, ref6, ref7, tag, tagToken; if (!(match = IDENTIFIER.exec(this.chunk))) { return 0; } @@ -98,11 +98,11 @@ this.token('DEFAULT', id); return id.length; } - ref4 = this.tokens, prev = ref4[ref4.length - 1]; - tag = colon || (prev != null) && (((ref5 = prev[0]) === '.' || ref5 === '?.' || ref5 === '::' || ref5 === '?::') || !prev.spaced && prev[0] === '@') ? 'PROPERTY' : 'IDENTIFIER'; + prev = this.prev(); + tag = colon || (prev != null) && (((ref4 = prev[0]) === '.' || ref4 === '?.' || ref4 === '::' || ref4 === '?::') || !prev.spaced && prev[0] === '@') ? 'PROPERTY' : 'IDENTIFIER'; if (tag === 'IDENTIFIER' && (indexOf.call(JS_KEYWORDS, id) >= 0 || indexOf.call(COFFEE_KEYWORDS, id) >= 0) && !(this.exportSpecifierList && indexOf.call(COFFEE_KEYWORDS, id) >= 0)) { tag = id.toUpperCase(); - if (tag === 'WHEN' && (ref6 = this.tag(), indexOf.call(LINE_BREAK, ref6) >= 0)) { + if (tag === 'WHEN' && (ref5 = this.tag(), indexOf.call(LINE_BREAK, ref5) >= 0)) { tag = 'LEADING_WHEN'; } else if (tag === 'FOR') { this.seenFor = true; @@ -129,6 +129,15 @@ } else if (tag === 'IDENTIFIER' && this.seenFor && id === 'from' && isForFrom(prev)) { tag = 'FORFROM'; this.seenFor = false; + } else if (tag === 'PROPERTY' && prev) { + if (prev.spaced && (ref6 = prev[0], indexOf.call(CALLABLE, ref6) >= 0) && /^[gs]et$/.test(prev[1])) { + this.error(`'${prev[1]}' cannot be used as a keyword, or as a function call without parentheses`, prev[2]); + } else { + prevprev = this.tokens[this.tokens.length - 2]; + if (((ref7 = prev[0]) === '@' || ref7 === 'THIS') && prevprev && prevprev.spaced && /^[gs]et$/.test(prevprev[1])) { + this.error(`'${prevprev[1]}' cannot be used as a keyword, or as a function call without parentheses`, prevprev[2]); + } + } } if (tag === 'IDENTIFIER' && indexOf.call(RESERVED, id) >= 0) { this.error(`reserved word '${id}'`, { @@ -223,13 +232,14 @@ } stringToken() { - var $, attempt, delimiter, doc, end, heredoc, i, indent, indentRegex, match, quote, ref, regex, token, tokens; + var $, attempt, delimiter, doc, end, heredoc, i, indent, indentRegex, match, prev, quote, ref, regex, token, tokens; [quote] = STRING_START.exec(this.chunk) || []; if (!quote) { return 0; } - if (this.tokens.length && this.value() === 'from' && (this.seenImport || this.seenExport)) { - this.tokens[this.tokens.length - 1][0] = 'FROM'; + prev = this.prev(); + if (prev && this.value() === 'from' && (this.seenImport || this.seenExport)) { + prev[0] = 'FROM'; } regex = (function() { switch (quote) { @@ -335,7 +345,7 @@ } regexToken() { - var body, closed, end, flags, index, match, origin, prev, ref, ref1, ref2, regex, tokens; + var body, closed, end, flags, index, match, origin, prev, ref, ref1, regex, tokens; switch (false) { case !(match = REGEX_ILLEGAL.exec(this.chunk)): this.error(`regular expressions cannot begin with ${match[2]}`, { @@ -352,13 +362,13 @@ offsetInChunk: 1 }); index = regex.length; - ref = this.tokens, prev = ref[ref.length - 1]; + prev = this.prev(); if (prev) { - if (prev.spaced && (ref1 = prev[0], indexOf.call(CALLABLE, ref1) >= 0)) { + if (prev.spaced && (ref = prev[0], indexOf.call(CALLABLE, ref) >= 0)) { if (!closed || POSSIBLY_DIVISION.test(regex)) { return 0; } - } else if (ref2 = prev[0], indexOf.call(NOT_REGEX, ref2) >= 0) { + } else if (ref1 = prev[0], indexOf.call(NOT_REGEX, ref1) >= 0) { return 0; } } @@ -504,11 +514,11 @@ } whitespaceToken() { - var match, nline, prev, ref; + var match, nline, prev; if (!((match = WHITESPACE.exec(this.chunk)) || (nline = this.chunk.charAt(0) === '\n'))) { return 0; } - ref = this.tokens, prev = ref[ref.length - 1]; + prev = this.prev(); if (prev) { prev[match ? 'spaced' : 'newLine'] = true; } @@ -537,7 +547,7 @@ } literalToken() { - var match, message, origin, prev, ref, ref1, ref2, ref3, ref4, skipToken, tag, token, value; + var match, message, origin, prev, ref, ref1, ref2, ref3, skipToken, tag, token, value; if (match = OPERATOR.exec(this.chunk)) { [value] = match; if (CODE.test(value)) { @@ -547,17 +557,17 @@ value = this.chunk.charAt(0); } tag = value; - ref = this.tokens, prev = ref[ref.length - 1]; + prev = this.prev(); if (prev && indexOf.call(['=', ...COMPOUND_ASSIGN], value) >= 0) { skipToken = false; - if (value === '=' && ((ref1 = prev[1]) === '||' || ref1 === '&&') && !prev.spaced) { + if (value === '=' && ((ref = prev[1]) === '||' || ref === '&&') && !prev.spaced) { prev[0] = 'COMPOUND_ASSIGN'; prev[1] += '='; prev = this.tokens[this.tokens.length - 2]; skipToken = true; } if (prev && prev[0] !== 'PROPERTY') { - origin = (ref2 = prev.origin) != null ? ref2 : prev; + origin = (ref1 = prev.origin) != null ? ref1 : prev; message = isUnassignable(prev[1], origin[1]); if (message) { this.error(message, origin[2]); @@ -592,12 +602,12 @@ } else if (value === '?' && (prev != null ? prev.spaced : void 0)) { tag = 'BIN?'; } else if (prev && !prev.spaced) { - if (value === '(' && (ref3 = prev[0], indexOf.call(CALLABLE, ref3) >= 0)) { + if (value === '(' && (ref2 = prev[0], indexOf.call(CALLABLE, ref2) >= 0)) { if (prev[0] === '?') { prev[0] = 'FUNC_EXIST'; } tag = 'CALL_START'; - } else if (value === '[' && (ref4 = prev[0], indexOf.call(INDEXABLE, ref4) >= 0)) { + } else if (value === '[' && (ref3 = prev[0], indexOf.call(INDEXABLE, ref3) >= 0)) { tag = 'INDEX_START'; switch (prev[0]) { case '?': @@ -852,6 +862,10 @@ return token != null ? token[1] : void 0; } + prev() { + return this.tokens[this.tokens.length - 1]; + } + unfinished() { var ref; return LINE_CONTINUER.test(this.chunk) || ((ref = this.tag()) === '\\' || ref === '.' || ref === '?.' || ref === '?::' || ref === 'UNARY' || ref === 'MATH' || ref === 'UNARY_MATH' || ref === '+' || ref === '-' || ref === '**' || ref === 'SHIFT' || ref === 'RELATION' || ref === 'COMPARE' || ref === '&' || ref === '^' || ref === '|' || ref === '&&' || ref === '||' || ref === 'BIN?' || ref === 'THROW' || ref === 'EXTENDS'); diff --git a/src/lexer.coffee b/src/lexer.coffee index 3fe94fbfed..1eab163d65 100644 --- a/src/lexer.coffee +++ b/src/lexer.coffee @@ -132,7 +132,7 @@ exports.Lexer = class Lexer @token 'DEFAULT', id return id.length - [..., prev] = @tokens + prev = @prev() tag = if colon or prev? and @@ -170,6 +170,16 @@ exports.Lexer = class Lexer isForFrom(prev) tag = 'FORFROM' @seenFor = no + # Throw an error on attempts to use `get` or `set` as keywords, or + # what CoffeeScript would normally interpret as calls to functions named + # `get` or `set`, i.e. `get({foo: function () {}})` + else if tag is 'PROPERTY' and prev + if prev.spaced and prev[0] in CALLABLE and /^[gs]et$/.test(prev[1]) + @error "'#{prev[1]}' cannot be used as a keyword, or as a function call without parentheses", prev[2] + else + prevprev = @tokens[@tokens.length - 2] + if prev[0] in ['@', 'THIS'] and prevprev and prevprev.spaced and /^[gs]et$/.test(prevprev[1]) + @error "'#{prevprev[1]}' cannot be used as a keyword, or as a function call without parentheses", prevprev[2] if tag is 'IDENTIFIER' and id in RESERVED @error "reserved word '#{id}'", length: id.length @@ -237,8 +247,9 @@ exports.Lexer = class Lexer # If the preceding token is `from` and this is an import or export statement, # properly tag the `from`. - if @tokens.length and @value() is 'from' and (@seenImport or @seenExport) - @tokens[@tokens.length - 1][0] = 'FROM' + prev = @prev() + if prev and @value() is 'from' and (@seenImport or @seenExport) + prev[0] = 'FROM' regex = switch quote when "'" then STRING_SINGLE @@ -318,7 +329,7 @@ exports.Lexer = class Lexer [regex, body, closed] = match @validateEscapes body, isRegex: yes, offsetInChunk: 1 index = regex.length - [..., prev] = @tokens + prev = @prev() if prev if prev.spaced and prev[0] in CALLABLE return 0 if not closed or POSSIBLY_DIVISION.test regex @@ -440,7 +451,7 @@ exports.Lexer = class Lexer whitespaceToken: -> return 0 unless (match = WHITESPACE.exec @chunk) or (nline = @chunk.charAt(0) is '\n') - [..., prev] = @tokens + prev = @prev() prev[if match then 'spaced' else 'newLine'] = true if prev if match then match[0].length else 0 @@ -468,7 +479,7 @@ exports.Lexer = class Lexer else value = @chunk.charAt 0 tag = value - [..., prev] = @tokens + prev = @prev() if prev and value in ['=', COMPOUND_ASSIGN...] skipToken = false @@ -758,6 +769,10 @@ exports.Lexer = class Lexer [..., token] = @tokens token?[1] + # Get the previous token in the token stream. + prev: -> + @tokens[@tokens.length - 1] + # Are we in the midst of an unfinished expression? unfinished: -> LINE_CONTINUER.test(@chunk) or diff --git a/src/nodes.coffee b/src/nodes.coffee index 4dd66e5b17..44afec3b02 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -775,7 +775,7 @@ exports.Call = class Call extends Base constructor: (@variable, @args = [], @soak) -> super() - @isNew = false + @isNew = no if @variable instanceof Value and @variable.isNotCallable() @variable.error "literal is not a function" diff --git a/test/error_messages.coffee b/test/error_messages.coffee index bbf7a02706..17d823df6f 100644 --- a/test/error_messages.coffee +++ b/test/error_messages.coffee @@ -1340,3 +1340,139 @@ test "new with 'super'", -> class extends A then foo: -> new super() ^^^^^ ''' + +test "getter keyword in object", -> + assertErrorFormat ''' + obj = + get foo: -> + ''', ''' + [stdin]:2:3: error: 'get' cannot be used as a keyword, or as a function call without parentheses + get foo: -> + ^^^ + ''' + +test "setter keyword in object", -> + assertErrorFormat ''' + obj = + set foo: -> + ''', ''' + [stdin]:2:3: error: 'set' cannot be used as a keyword, or as a function call without parentheses + set foo: -> + ^^^ + ''' + +test "getter keyword in inline implicit object", -> + assertErrorFormat 'obj = get foo: ->', ''' + [stdin]:1:7: error: 'get' cannot be used as a keyword, or as a function call without parentheses + obj = get foo: -> + ^^^ + ''' + +test "setter keyword in inline implicit object", -> + assertErrorFormat 'obj = set foo: ->', ''' + [stdin]:1:7: error: 'set' cannot be used as a keyword, or as a function call without parentheses + obj = set foo: -> + ^^^ + ''' + +test "getter keyword in inline explicit object", -> + assertErrorFormat 'obj = {get foo: ->}', ''' + [stdin]:1:8: error: 'get' cannot be used as a keyword, or as a function call without parentheses + obj = {get foo: ->} + ^^^ + ''' + +test "setter keyword in inline explicit object", -> + assertErrorFormat 'obj = {set foo: ->}', ''' + [stdin]:1:8: error: 'set' cannot be used as a keyword, or as a function call without parentheses + obj = {set foo: ->} + ^^^ + ''' + +test "getter keyword in function", -> + assertErrorFormat ''' + f = -> + get foo: -> + ''', ''' + [stdin]:2:3: error: 'get' cannot be used as a keyword, or as a function call without parentheses + get foo: -> + ^^^ + ''' + +test "setter keyword in function", -> + assertErrorFormat ''' + f = -> + set foo: -> + ''', ''' + [stdin]:2:3: error: 'set' cannot be used as a keyword, or as a function call without parentheses + set foo: -> + ^^^ + ''' + +test "getter keyword in inline function", -> + assertErrorFormat 'f = -> get foo: ->', ''' + [stdin]:1:8: error: 'get' cannot be used as a keyword, or as a function call without parentheses + f = -> get foo: -> + ^^^ + ''' + +test "setter keyword in inline function", -> + assertErrorFormat 'f = -> set foo: ->', ''' + [stdin]:1:8: error: 'set' cannot be used as a keyword, or as a function call without parentheses + f = -> set foo: -> + ^^^ + ''' + +test "getter keyword in class", -> + assertErrorFormat ''' + class A + get foo: -> + ''', ''' + [stdin]:2:3: error: 'get' cannot be used as a keyword, or as a function call without parentheses + get foo: -> + ^^^ + ''' + +test "setter keyword in class", -> + assertErrorFormat ''' + class A + set foo: -> + ''', ''' + [stdin]:2:3: error: 'set' cannot be used as a keyword, or as a function call without parentheses + set foo: -> + ^^^ + ''' + +test "getter keyword in inline class", -> + assertErrorFormat 'class A then get foo: ->', ''' + [stdin]:1:14: error: 'get' cannot be used as a keyword, or as a function call without parentheses + class A then get foo: -> + ^^^ + ''' + +test "setter keyword in inline class", -> + assertErrorFormat 'class A then set foo: ->', ''' + [stdin]:1:14: error: 'set' cannot be used as a keyword, or as a function call without parentheses + class A then set foo: -> + ^^^ + ''' + +test "getter keyword before static method", -> + assertErrorFormat ''' + class A + get @foo = -> + ''', ''' + [stdin]:2:3: error: 'get' cannot be used as a keyword, or as a function call without parentheses + get @foo = -> + ^^^ + ''' + +test "setter keyword before static method", -> + assertErrorFormat ''' + class A + set @foo = -> + ''', ''' + [stdin]:2:3: error: 'set' cannot be used as a keyword, or as a function call without parentheses + set @foo = -> + ^^^ + ''' diff --git a/test/function_invocation.coffee b/test/function_invocation.coffee index 7baf0b09ac..99e1a4b362 100644 --- a/test/function_invocation.coffee +++ b/test/function_invocation.coffee @@ -675,7 +675,7 @@ test "Non-callable literals shouldn't compile", -> cantCompile '[1..10][2..9] 2' cantCompile '[1..10][2..9](2)' -test 'implicit invocation with implicit object literal', -> +test "implicit invocation with implicit object literal", -> f = (obj) -> eq 1, obj.a f @@ -706,3 +706,45 @@ test 'implicit invocation with implicit object literal', -> else "#{a}": 1 eq 2, obj.a + +test "get and set can be used as function names when not ambiguous with `get`/`set` keywords", -> + get = (val) -> val + set = (val) -> val + eq 2, get(2) + eq 3, set(3) + eq 'a', get('a') + eq 'b', set('b') + + get = ({val}) -> val + set = ({val}) -> val + eq 4, get({val: 4}) + eq 5, set({val: 5}) + eq 'c', get({val: 'c'}) + eq 'd', set({val: 'd'}) + +test "get and set can be used as variable and property names", -> + get = 2 + set = 3 + eq 2, get + eq 3, set + + {get} = {get: 4} + {set} = {set: 5} + eq 4, get + eq 5, set + +test "get and set can be used as class method names", -> + class A + get: -> 2 + set: -> 3 + + a = new A() + eq 2, a.get() + eq 3, a.set() + + class B + @get = -> 4 + @set = -> 5 + + eq 4, B.get() + eq 5, B.set()