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

Prism custom builder #354

Merged
merged 2 commits into from
Mar 19, 2025
Merged

Conversation

Earlopain
Copy link
Contributor

Context: ruby/prism#3443

I'd like for prism to emit ast for new ruby features, like it. It's incredibly easy for prism to emit itblock, it just needs a custom builder, with a few lines changed to do so. But since rubocop-ast already uses its own custom builder, they need to play nicely together for it to work.

The PR for prism just adds the builder class, without any changes. I'd like to explore that after this.

Verified

This commit was signed with the committer’s verified signature.
dtolnay David Tolnay
It may emit different ast when used with a ruby version
that `parser` doesn't understand, for example `itblock`
That's better than checking if requires succeed, especially now that
not only a parser may be missing
@Earlopain
Copy link
Contributor Author

Ah, right. The main gem specs for prism fail because it doesn't use the gemfile from over here and just uses the latest release anyways

@Earlopain
Copy link
Contributor Author

This is ready now, prism 1.4.0 got released which contains the necessary changes. One main rubocop spec currently fails for unrelated reasons (rubocop/rubocop#14000).

For a bit of context, in rubocop/rubocop#13617 (comment) I wrote what I believe should be the next steps.

@Earlopain Earlopain marked this pull request as ready for review March 18, 2025 19:56
@marcandre marcandre merged commit 18dd459 into rubocop:master Mar 19, 2025
39 of 41 checks passed
@marcandre
Copy link
Contributor

Cool, thanks. Released as v1.40

@Earlopain Earlopain deleted the prism-custom-builder branch March 19, 2025 11:04
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.

None yet

2 participants