Reject p(p a, &block => value) and similar
#3669
Conversation
| bool contains_keyword_splat = false; | ||
|
|
||
| if (pm_symbol_node_label_p(argument) || accept1(parser, PM_TOKEN_EQUAL_GREATER)) { | ||
| if (argument_allowed_for_bare_hash(parser, argument)){ |
There was a problem hiding this comment.
There is very similar code here
Line 18217 in ce4abe1
I tried a few things to find code samples which force me to do the same change there as well but I did not find such code. Left it alone for now
| ^~ unexpected '=>', expecting end-of-input | ||
| ^~ unexpected '=>', ignoring it | ||
|
|
||
| p[p a, x: b => value] |
There was a problem hiding this comment.
This is code which I assumed would still be accepted with this change (see previous comment). But it is correctly rejected.
|
|
||
| not !foo 1 | ||
|
|
||
| foo(bar baz, key => value) |
There was a problem hiding this comment.
Probably already tested somewhere 🤷
Earlopain
force-pushed
the
bare-hash-command-call
branch
from
02c4a77 to
f189649
Compare
tenderlove
left a comment
tenderlove
left a comment
There was a problem hiding this comment.
LGTM, but I wish we had a CI build that would test Prism PRs against CRuby. I'll try this PR locally just in case.
|
There is this But I guess you mean something a bit more comprehensive? |
Ya, I'd like it if the whole suite was run. But it may make CI take too long. We've had cases where those two tests passed just fine, but merging broke upstream CI. |
|
Yeah, I thought so. I ran test-all locally with this and apart from some |
Earlopain
force-pushed
the
bare-hash-command-call
branch
3 times, most recently
from
e32c909 to
587cb5c
Compare
Earlopain
force-pushed
the
bare-hash-command-call
branch
2 times, most recently
from
e9b7024 to
a3127f6
Compare
They were being parsed as `p((p a, &block) => value)`. When we get to this point, we must not just have parsed a command call, always consuming the `=>` is not correct. Closes [Bug #21622]
Earlopain
force-pushed
the
bare-hash-command-call
branch
from
a3127f6 to
796ab0e
Compare
|
This needs to be reverted: https://github.com/ruby/ruby/actions/runs/19615315949/job/56167057220 Seems I missed something, will revisit this later |
Redo of ruby#3669 with more tests
Redo of ruby#3669 with more tests
3 participants
They were being parsed as
p((p a, &block) => value). When we get to this point, we must not just have parsed a command call, always consuming the=>is not correct.Closes [Bug #21622]