Skip to content
This repository has been archived by the owner on May 19, 2018. It is now read-only.

[7.0] Change RestProperty/SpreadProperty to RestElement/SpreadElement #384

Merged
merged 2 commits into from
Feb 28, 2017

Conversation

hzoo
Copy link
Member

@hzoo hzoo commented Feb 26, 2017

  • need to fix a failing test

    estree-throws › es2015/uncategorised/288
    /Users/hzhu/dev/babylon/test/utils/runFixtureTests.js:92

    91: if (opts.throws) {
    92: throw new Error("Expected error message: " + opts.throws + ". But parsing succeeded.");
    93: } else {

    Error: /Users/hzhu/dev/babylon/test/fixtures/es2015/uncategorised/288/actual.js: Expected error message: Invalid left-hand side in assignment expression (1:1). But parsing succeeded.

Odd that [...a, b] = c is parsing as a SpreadElement when it should be a RestElement?

@hzoo hzoo added this to the 7.0.0 milestone Feb 26, 2017
@hzoo hzoo changed the base branch from master to 7.0 February 26, 2017 00:12
@codecov
Copy link

codecov bot commented Feb 26, 2017

Codecov Report

Merging #384 into 7.0 will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##              7.0    #384      +/-   ##
=========================================
+ Coverage   97.88%   97.9%   +0.02%     
=========================================
  Files          20      20              
  Lines        3444    3443       -1     
  Branches      910     910              
=========================================
  Hits         3371    3371              
  Misses         30      30              
+ Partials       43      42       -1
Impacted Files Coverage Δ
src/parser/lval.js 98.63% <100%> (-0.01%)
src/parser/expression.js 97.5% <100%> (+0.15%)
src/parser/statement.js 97.92% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe2d2a9...9d78cff. Read the comment docs.

@@ -34,8 +34,8 @@ pp.toAssignable = function (node, isBinding, contextDescription) {
this.toAssignable(node.value, isBinding, contextDescription);
break;

case "SpreadProperty":
node.type = "RestProperty";
case "SpreadElement":
Copy link
Member

Choose a reason for hiding this comment

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

SpreadElement should only be converted to RestElement if isBinding === true and otherwise fail.

Previously we were not converting SpreadElement at all, but now we need to in some cases (the cases that were previously SpreadProperty)

Copy link
Member Author

Choose a reason for hiding this comment

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

If you have time to finish this up, go ahead 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants