fix: keep $ on nested variables in "${...}" interpolation - #2520
Open
Dextheking1 wants to merge 1 commit into
Open
Dextheking1 wants to merge 1 commit into
Dextheking1 wants to merge 1 commit into
Conversation
Printing `"${foo[$index_var]}"` swallowed the dollar sign of the nested
variable, producing `"${foo[index_var]}"` and silently changing semantics
(the variable becomes a bare constant). The `variable` printer dropped `$`
for any variable whose parent was an offsetlookup under a curly-simple
encapsedpart, but only the base variable of the lookup (which directly
follows the `${` emitted by the encapsedpart printer) may lose its `$`;
a variable in the offset position is nested and keeps its own `$`.
Closes prettier#2177
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Bug:
"${foo[$index_var]}"was printed as"${foo[index_var]}"— the$of the nested offset variable was swallowed, silently changing semantics (variable becomes a bare constant).Fix: in the
variableprinter (src/printer.mjs), the$was dropped for any variable whose parent was anoffsetlookupunder a curly-simpleencapsedpart. That is only correct for the base variable of the lookup, which directly follows the${emitted by the encapsedpart printer. The condition is now restricted toparent.what === node, so a variable in the offset position keeps its$.Tests: added
"text ${foo[$index_var]} text"totests/encapsed/encapsed.php; snapshots updated withjest -u. Full suite: 101 suites / 595 tests pass, plusAST_COMPARE=1round-trip on the encapsed suite. Verified output keeps$index_varand that the pre-fix code reproduced the bug ("${foo[index_var]}").Closes #2177