Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup #1

Closed
wants to merge 8 commits into from
64 changes: 31 additions & 33 deletions lib/coffee-script/lexer.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions lib/coffee-script/nodes.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

63 changes: 31 additions & 32 deletions src/lexer.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -141,38 +141,37 @@ exports.Lexer = class Lexer
else
'IDENTIFIER'

if tag is 'IDENTIFIER'
if @seenFor and id is 'from'
tag = 'FORFROM'
@seenFor = no
else if (id in JS_KEYWORDS or id in COFFEE_KEYWORDS) and
not (@exportSpecifierList and id in COFFEE_KEYWORDS)
tag = id.toUpperCase()
if tag is 'WHEN' and @tag() in LINE_BREAK
tag = 'LEADING_WHEN'
else if tag is 'FOR'
@seenFor = yes
else if tag is 'UNLESS'
tag = 'IF'
else if tag is 'IMPORT'
@seenImport = yes
else if tag is 'EXPORT'
@seenExport = yes
else if tag in UNARY
tag = 'UNARY'
else if tag in RELATION
if tag isnt 'INSTANCEOF' and @seenFor
tag = 'FOR' + tag
@seenFor = no
else
tag = 'RELATION'
if @value() is '!'
poppedToken = @tokens.pop()
id = '!' + id

if id in RESERVED
@error "reserved word '#{id}'", length: id.length

if tag is 'IDENTIFIER' and (id in JS_KEYWORDS or id in COFFEE_KEYWORDS) and
not (@exportSpecifierList and id in COFFEE_KEYWORDS)
tag = id.toUpperCase()
if tag is 'WHEN' and @tag() in LINE_BREAK
tag = 'LEADING_WHEN'
else if tag is 'FOR'
@seenFor = yes
else if tag is 'UNLESS'
tag = 'IF'
else if tag is 'IMPORT'
@seenImport = yes
else if tag is 'EXPORT'
@seenExport = yes
else if tag in UNARY
tag = 'UNARY'
else if tag in RELATION
if tag isnt 'INSTANCEOF' and @seenFor
tag = 'FOR' + tag
@seenFor = no
else
tag = 'RELATION'
if @value() is '!'
poppedToken = @tokens.pop()
id = '!' + id
else if id is 'from' and @seenFor
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this break if tag is 'PROPERTY'?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add a test for that :) This change didn't break any tests.

If it needs that check, I would just change this to else if tag is 'IDENTIFIER' and id is 'from' and @seenFor

tag = 'FORFROM'
@seenFor = no

if tag is 'IDENTIFIER' and id in RESERVED
@error "reserved word '#{id}'", length: id.length

unless tag is 'PROPERTY'
if id in COFFEE_ALIASES
alias = id
Expand Down
26 changes: 13 additions & 13 deletions src/nodes.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -2251,16 +2251,16 @@ exports.For = class For extends While
@object = !!source.object
@from = !!source.from
@index.error 'cannot use index with for-from' if @from and @index

[@name, @index] = [@index, @name] if @object

@index.error 'index cannot be a pattern matching expression' if @index instanceof Value
@range = @source instanceof Value and @source.base instanceof Range and not @source.properties.length and not @from
@pattern = @name instanceof Value
@index.error 'indexes do not apply to range loops' if @range and @index
@name.error 'cannot pattern match over range loops' if @range and @pattern
@name.error 'cannot use own with for-in' if @own and not @object

@returns = false

children: ['body', 'source', 'guard', 'step']
Expand Down Expand Up @@ -2298,14 +2298,14 @@ exports.For = class For extends While
forPartFragments = source.compileToFragments merge o,
{index: ivar, name, @step, isComplex: isComplexOrAssignable}
else
svar = @source.compile o, LEVEL_LIST
svar = @source.compile o, LEVEL_LIST
if (name or @own) and @source.unwrap() not instanceof IdentifierLiteral
defPart += "#{@tab}#{ref = scope.freeVariable 'ref'} = #{svar};\n"
svar = ref
if not @from
defPart += "#{@tab}#{ref = scope.freeVariable 'ref'} = #{svar};\n"
svar = ref
unless @from
if name and not @pattern
namePart = "#{name} = #{svar}[#{kvar}]"
if not @object
namePart = "#{name} = #{svar}[#{kvar}]"
unless @object
defPart += "#{@tab}#{step};\n" if step isnt stepVar
down = stepNum < 0
lvar = scope.freeVariable 'len' unless @step and stepNum? and down
Expand Down Expand Up @@ -2342,12 +2342,12 @@ exports.For = class For extends While
defPartFragments = [].concat @makeCode(defPart), @pluckDirectCall(o, body)
varPart = "\n#{idt1}#{namePart};" if namePart
if @object
forPartFragments = [@makeCode("#{kvar} in #{svar}")]
forPartFragments = [@makeCode("#{kvar} in #{svar}")]
guardPart = "\n#{idt1}if (!#{utility 'hasProp', o}.call(#{svar}, #{kvar})) continue;" if @own
if @from
forPartFragments = [@makeCode("#{kvar} of #{svar}")]
else if @from
forPartFragments = [@makeCode("#{kvar} of #{svar}")]
bodyFragments = body.compileToFragments merge(o, indent: idt1), LEVEL_TOP
if bodyFragments and (bodyFragments.length > 0)
if bodyFragments and bodyFragments.length > 0
bodyFragments = [].concat @makeCode("\n"), bodyFragments, @makeCode("\n")
[].concat defPartFragments, @makeCode("#{resultPart or ''}#{@tab}for ("),
forPartFragments, @makeCode(") {#{guardPart}#{varPart}"), bodyFragments,
Expand Down
3 changes: 1 addition & 2 deletions test/arrays.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,10 @@ test "for-from loops over Array", ->
eq 50, c

test "for-from comprehensions over Array", ->

array1 = (x + 10 for x from [10, 20, 30])
ok array1.join(' ') is '20 30 40'

array2 = (x for x from [30, 41, 57] when x %% 3 == 0)
array2 = (x for x from [30, 41, 57] when x %% 3 is 0)
ok array2.join(' ') is '30 57'

array1 = (b + 5 for [a, b] from [[20, 30], [40, 50]])
Expand Down
2 changes: 1 addition & 1 deletion test/compilation.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ test "ensure that carriage returns don't break compilation on Windows", ->
test "#3089 - don't mutate passed in options to compile", ->
opts = {}
CoffeeScript.compile '1 + 1', opts
ok !opts.scope
ok !opts.scope

test "--bare", ->
eq -1, CoffeeScript.compile('x = y', bare: on).indexOf 'function'
Expand Down
17 changes: 7 additions & 10 deletions test/generators.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -251,43 +251,40 @@ test "yield handles 'this' correctly", ->

throws -> y.next new Error "boom"


test "for-from loops over generators", ->
array1 = [50, 30, 70, 20]
gen = -> yield from array1

array2 = []
array3 = []
array4 = []

iterator = gen()
for x from iterator
array2.push(x)
break if x is 30

for x from iterator
array3.push(x)

for x from iterator
array4.push(x)

arrayEq array2, [50, 30]
# Different JS engines have different opinions on the value of array3:
# https://github.com/jashkenas/coffeescript/pull/4306#issuecomment-257066877
# As a temporary measure, either result is accepted.
ok array3.length == 0 or array3.join(',') == '70,20'
ok array3.length is 0 or array3.join(',') is '70,20'
arrayEq array4, []


test "for-from comprehensions over generators", ->

gen = ->
yield from [30, 41, 51, 60]

iterator = gen()
array1 = (x for x from iterator when x %% 2 == 1)
array1 = (x for x from iterator when x %% 2 is 1)
array2 = (x for x from iterator)

ok array1.join(' ') is '41 51'
ok array2.length is 0

3 changes: 1 addition & 2 deletions test/ranges.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ test "for-from loops over ranges", ->
array1 = []
for x from [20..30]
array1.push(x)
if x == 25
break
break if x is 25
arrayEq array1, [20, 21, 22, 23, 24, 25]

test "for-from comprehensions over ranges", ->
Expand Down