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

Make MutationVisitor visit all nodes #470

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

spect88
Copy link

@spect88 spect88 commented Jul 30, 2024

Hi, I tried to use MutationVisitor in my gem, but realized that it doesn't traverse the entire AST.

The most surprising thing perhaps was that anything assigned to a variable wouldn't be traversed. Looking at the documentation, this doesn't seem to be intended.

The test case demonstrates 3 important node types that would ignore
their children:
- Assign
- Assoc
- IfOp

The fact that MutationVisitor wouldn't traverse anything that's assigned
to a variable, a hash key, or inside a ternary operator was very
limiting.
Copy link
Member

@kddnewton kddnewton left a comment

Choose a reason for hiding this comment

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

I've accept this change, but note that the tree in this repository is going to be replaced soon by the one in prism, and I would suggest migrating to that.

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