Skip to content

Conversation

@eregon
Copy link
Member

@eregon eregon commented Jan 20, 2026

@eregon eregon requested review from Earlopain and kddnewton January 20, 2026 20:21
@eregon
Copy link
Member Author

eregon commented Jan 21, 2026

Thinking more about it, it seems far better to just check node.arguments&.arguments&.any?, that's much safer and does the correct decision based on the AST.
I'll update to that.

@eregon eregon force-pushed the check-if-array-before-calling-any branch from 5406f51 to 413616f Compare January 21, 2026 09:48
@eregon eregon changed the title Check if arguments is an Array before calling #any? in Ripper translator Check using Prism nodes if a command call has any arguments in Ripper translator Jan 21, 2026
@eregon
Copy link
Member Author

eregon commented Jan 21, 2026

Done, this feels like a proper fix.

… translator

* We don't know what `on_*` events might return so we cannot assume it's an Array.
* See ruby#3838 (comment)
@eregon eregon force-pushed the check-if-array-before-calling-any branch from 413616f to bed4271 Compare January 21, 2026 09:57
@eregon
Copy link
Member Author

eregon commented Jan 21, 2026

Added a test as well and confirmed it fails before the fix (with 132 failures).

@eregon
Copy link
Member Author

eregon commented Jan 21, 2026

@Earlopain could you review this?
With this merged truffleruby/truffleruby#4102 will have no more failures in MRI tests than before.

We should touch these as little as possible and just pass them along
Copy link
Collaborator

@Earlopain Earlopain left a comment

Choose a reason for hiding this comment

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

I tweaked it slightly, see added commit. I'm positively surprised it's only needed in so few places. Feel free to merge if the addition looks good to you.

BTW, this also fixes the very simple case of "foo bar". It's now correctly reported on_command instead of on_fcall.

@eregon eregon merged commit 5ea83ca into ruby:main Jan 21, 2026
66 checks passed
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.

Ripper translation layer relies on upstream Ripper and is not fully compatible

2 participants