fix(ppi): strip trailing :: from bareword method invocants#517
Merged
Conversation
In Perl, `Foo::->bar()` is equivalent to `Foo->bar()`: the trailing `::` on a bareword class-method invocant is part of the syntactic form and is stripped before the class name is passed as the method's first argument. PerlOnJava was keeping the `::`, so PPI's `PPI::Token::Whitespace::->new(...)` idiom (very common throughout PPI's test suite) was looking up methods in the package `"PPI::Token::Whitespace::"` and failing. This is the root cause of early aborts in `t/ppi_token_whitespace.t` and contributes to many related PPI subtest failures. Strip a trailing `::` from the bareword class name in both backends where the method invocant is an `IdentifierNode` (JVM backend in `Dereference.handleArrowOperator`, interpreter backend in `CompileBinaryOperator` method-call path). Only the bareword-literal form is stripped; a runtime string `"Foo::"` used as invocant is not touched. Adds a unit regression test and records the broader PPI investigation in dev/modules/ppi.md (including the separate refcount/DESTROY bug that still blocks PPI's while/until/for lexing, which needs its own coordinated fix). Generated with [Devin](https://app.devin.ai/) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
::from bareword method invocants soFoo::->bar()isequivalent to
Foo->bar()and passes"Foo"(not"Foo::") as thefirst argument, matching real Perl.
Dereference.handleArrowOperatorCompileBinaryOperatormethod-call path"Foo::"used as invocant is left alone.
Why
PPI's test suite uses the idiom
PPI::Token::Whitespace::->new(...)heavily. Before this fix, PerlOnJava looked up methods in the package
"PPI::Token::Whitespace::"and failed, which abortedt/ppi_token_whitespace.ton line 1 and contributed to many related PPIsubtest failures.
Discovered while investigating
./jcpan -t PPI. Seedev/modules/ppi.mdfor the full investigation notes, including aseparate refcount/DESTROY bug (RC2) that still blocks PPI's
while/until/for lexing and requires a coordinated container-store
refcount-parity pass across
push/pop/shift/splice/hash-store,plus rebalancing of
unit/refcount/*.t. That work is deferred to itsown project.
Test plan
make(full unit tests) passes.src/test/resources/unit/method_call_trailing_colons.t(5 subtests) passes.
t/ppi_token_whitespace.tfrom the PPI CPAN distribution nowpasses 6/6.
Foo::->bar()prints[Foo::]. After: prints[Foo],matching
perl.Generated with Devin