Fix printing of nested closure typecasts #7782
Closed
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
thorn0
reviewed
Mar 17, 2020
|
|
||
| // There's an annoying issue if both our node contains a typecast, and this | ||
| // ancestor contains a typecast. Unfortunately, both comments are applied | ||
| // to ancestor. So we need to detect this situation, and insert hack paren |
Member
There was a problem hiding this comment.
Unfortunately, we can't rely on what node the comment will be attached to. It's not always the ancestor. E.g. see #7775.
Contributor
Author
There was a problem hiding this comment.
Ahh, I see. This is attaching two casts onto the same expression. I can fix this in a follow up PR using the same paren comment hack, if you'd like?
Member
|
Please target the |
jridgewell
force-pushed
the
closure-type-casts-3
branch
from
b53b5ec to
ab0273b
Compare
jridgewell
force-pushed
the
closure-type-casts-3
branch
from
ab0273b to
a977fe6
Compare
Contributor
Author
Done. |
thorn0
reviewed
Mar 18, 2020
| function getParenStart(node) { | ||
| // Closure typecast comments only really make sense when _not_ using | ||
| // typescript or flow parsers, so we take advantage of the babel parser's | ||
| // parenthesized expressions. |
Member
There was a problem hiding this comment.
I forgot this feature was Babel-only. We should use Babel's createParenthesizedExpressions option then. This should give us nested parens in the AST. Could you look into that? Okay, I'm looking into that. #7791
3 tasks
Contributor
Author
|
Closing for #7791. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 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.
Fixes #6122.
This goes through the rather complicated steps to determine when parens surround a typecast comment, including when both the node and the ancestor have typecasts.
I had to add really hacky comment nodes in order to insert the parens in the correct locations. This is because using the normal
needsParenswould insert multiple typecasts comments before the first(. It order to get it right, we need to print the first typecast, then the first(, then the next typecast, so on. I was only able to get it right by splicing the comments in.changelog_unreleased/*/pr-XXXX.mdfile followingchangelog_unreleased/TEMPLATE.md.✨Try the playground for this PR✨