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

New: Rest Parameter (refs: #10) #57

Merged
merged 1 commit into from
Feb 10, 2015
Merged

New: Rest Parameter (refs: #10) #57

merged 1 commit into from
Feb 10, 2015

Conversation

xjamundx
Copy link
Contributor

  • test from harmony ported over
  • additional tests
  • error tests

try {
assert.deepEqual(result, expected);
} catch (e) {
throw result;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't mean to commit this, but it's really helpful for debugging. (will remove :))

@xjamundx
Copy link
Contributor Author

Just have one broken test now. Some slightly different line numbers for arrowFunctions:


      AssertionError: expected { Object (type, body, ...) } to deeply equal { Object (type, body, ...) }
      + expected - actual

                       "column": 26
                       "line": 1
                     }
                     "start": {
      +                "column": 19
      -                "column": 20
                       "line": 1
                     }
                   }
                   "range": [
      +              19
      -              20
                     26
                   ]
                   "type": "SequenceExpression"
                 }

@xjamundx
Copy link
Contributor Author

Works now and a bunch of Spread stuff in there, because there is some overlap. Will attack spread next.

* @param {boolean} expression True if the arrow function is created via an expression.
* Always false for declarations, but kept here to be in sync with
* FunctionExpression objects.
* @returns {ASTNode} An ASTNode representing the entire arrow function expression
*/
createArrowFunctionExpression: function (params, defaults, body, expression) {
createArrowFunctionExpression: function (params, defaults, body, rest, expression) {
Copy link
Member

Choose a reason for hiding this comment

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

What about FunctionExpression and FunctionDeclaration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was actually already there, but I'll add some more tests!

@nzakas
Copy link
Member

nzakas commented Feb 10, 2015

Looks like you're missing changes to support rest params in FunctionDeclaration and FunctionExpression.

@xjamundx
Copy link
Contributor Author

We now have tests for var x = function(...b) {} and function x(...b){} and a few variations....

@nzakas
Copy link
Member

nzakas commented Feb 10, 2015

Huh, secret implementation details revealed. Looks good!

nzakas added a commit that referenced this pull request Feb 10, 2015
New: Rest Parameter (refs: #10)
@nzakas nzakas merged commit c714fad into eslint:master Feb 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants