Skip to content

Conversation

eileencodes
Copy link
Member

@eileencodes eileencodes commented Sep 30, 2025

This PR is a followup to #3558 and fixes the following cases, which should throw a syntax error but were not:

A.print message: in 'BAR'

A.print message:
in 'BAR'

To fix this we check for methods calls with arguments and no parens that then call in.

Note I had claude write this because I wasn't sure where the problem was but then edited it to remove extra unnecessary code the robot wrote.

cc/ @tenderlove @kddnewton

This PR is a followup to ruby#3558 and fixes the following cases, which
should throw a syntax error but were not:

```ruby
A.print message: in 'BAR'

A.print message:
in 'BAR'
```

To fix this we check for methods calls with arguments and no parens that
then call `in`.

Note I had claude write this because I wasn't sure where the problem
was but then edited it to remove extra unnecessary code the robot wrote.
pm_call_node_t *cast = (pm_call_node_t *) node;
if (cast->arguments != NULL && cast->opening_loc.start == NULL && parser->current.type == PM_TOKEN_KEYWORD_IN) {
return node;
}
Copy link
Member

Choose a reason for hiding this comment

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

I need @kddnewton's opinion here, but I feel like testing for a specific token isn't the right fix and that we should be using binding power instead (but this is just a gut feeling). Is in the only keyword that has a problem?

Copy link
Member

@tompng tompng Oct 1, 2025

Choose a reason for hiding this comment

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

Is in the only keyword that has a problem?

Looks like in, =>, ., &. and all binary operators except + - ** & * are wrongly accepted after A.print message: but not after a.print message:

Copy link
Member

Choose a reason for hiding this comment

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

And ternary operator A.print message: ? 1 : 2

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @tenderlove that this is not the correct fix. The code that is supposed to handle this is here:

prism/src/prism.c

Lines 22253 to 22261 in ce4abe1

case PM_CALL_NODE:
// If we have a call node, then we need to check if it looks like a
// method call without parentheses that contains arguments. If it
// does, then it has different rules for parsing infix operators,
// namely that it only accepts composition (and/or) and modifiers
// (if/unless/etc.).
if ((pm_binding_powers[parser->current.type].left > PM_BINDING_POWER_COMPOSITION) && pm_call_node_command_p((pm_call_node_t *) node)) {
return node;
}

But parse_expression_prefix returns PM_CONSTANT_READ_NODE. So I feel like the fix would be to modify parse_expression_prefix so that it parses further instead of just one node deep. Or this check is also needed at some other place.

But I did not manage to actually fix it myself last I looked into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants