Properly update location data when setting a call to use new
#4445
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is an upstream port of decaffeinate#24 In a case like `new A().b(c)`, the jison structure ends up being different from the resulting AST. To the jison parser, this is the `new` unary operator applied to the expression `A().b(c)`. When the unary operator is applied, the `Call.prototype.newInstance` function traverses into the leftmost function call and sets the `isNew` flag to true, and the `Op` constructor returns the `Call` node so that the call is used in place of the unary operator. However, the code wasn't updating the node location data, so this commit fixes that. It's sort of hard to get the location data in `newInstance`, so we set a flag on every affected node in `newInstance` and override `updateLocationDataIfMissing` (which is called with the location data after the fact) so that it updates just the starting position.
lydell
reviewed
Feb 17, 2017
|
|
||
| assertColumnRange assign, 0, 15 | ||
| assertColumnRange outerCall, 4, 15 | ||
| assertColumnRange innerValue, 4, 12 |
Collaborator
There was a problem hiding this comment.
Why 12 here? (I'm having a bit of a difficulty mapping outerCall, innerValue and innerCall to the corresponding parts of source.)
Contributor
Author
There was a problem hiding this comment.
outerCall is new B().c(d)
innerValue is new B().c
innerCall is new B()
All three of these needed to have their locations updated by this change, since previously they ignored the new on the left.
Collaborator
There was a problem hiding this comment.
Thanks, looks like I got those right after all. And when counting again, I get it to 12, too. False alarm!
Collaborator
|
@alangpierce I’ve merged this into the |
Contributor
Author
|
@GeoffreyBooth ok, I looked into it and put up #4447. |
EsrefDurna
added a commit
to EsrefDurna/coffeescript
that referenced
this pull request
Nov 12, 2025
…ocation-data Properly update location data when setting a call to use `new`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an upstream port of decaffeinate#24
In a case like
new A().b(c), the jison structure ends up being different fromthe resulting AST. To the jison parser, this is the
newunary operator appliedto the expression
A().b(c). When the unary operator is applied, theCall.prototype.newInstancefunction traverses into the leftmost function calland sets the
isNewflag to true, and theOpconstructor returns theCallnode so that the call is used in place of the unary operator. However, the code
wasn't updating the node location data, so this commit fixes that.
It's sort of hard to get the location data in
newInstance, so we set a flag onevery affected node in
newInstanceand overrideupdateLocationDataIfMissing(which is called with the location data after the fact) so that it updates just
the starting position.