fix: detect mutations within assignments expressions (alternative approach)#12429
Merged
dummdidumm merged 8 commits intomainfrom Jul 15, 2024
Merged
fix: detect mutations within assignments expressions (alternative approach)#12429dummdidumm merged 8 commits intomainfrom
dummdidumm merged 8 commits intomainfrom
Conversation
This enhances our "variable was mutated" detection to also recognize that `foo` in `[foo[1]] = [..]` was mutated. This allows us to be more granular about detecting mutations to each expressions in legacy mode, which fixes #12169
🦋 Changeset detectedLatest commit: a80ea10 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
5 tasks
dummdidumm
approved these changes
Jul 15, 2024
trueadm
pushed a commit
that referenced
this pull request
Jul 16, 2024
…roach) (#12429) This enhances our "variable was mutated" detection to also recognize that `foo` in `[foo[1]] = [..]` was mutated. This allows us to be more granular about detecting mutations to each expressions in legacy mode, which fixes #12169 --------- Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
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.
Alternative to #12411.
We use the
extract_identifiersutility to find the identifiers in patterns, like these:Patterns also occur in e.g. function declarations and assignment expressions. But there are two kinds of pattern — binding patterns and assignment patterns — and they're subtly different. Binding patterns can ultimately only contain identifiers (the things being declared), while assignment patterns can contain any lvalue (i.e.
IdentifierorMemberExpressionin ESTree terminology).The drawback of #12411 is that it lumps identifiers and member expressions together. This works, but it has a cost — it means that the bindings referenced in an assignment pattern are always marked as reassigned (rather than merely mutated) even if they're member expressions. This means that the compiler sees something like this...
...and marks
arrayas being reassigned, which means that we have to create a source for it:It's better if we can differentiate between these things. That's what this PR does: it expands the pattern unwrapping logic to include member expression nodes, and updates the
AssignmentExpressionlogic to use it. The existingextract_identifiersfunction is kept with the same API, by callingunwrap_patternunder the hood but filtering out non-identifier nodes.Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.Tests and linting
pnpm testand lint the project withpnpm lint