Repository navigation
Conversation
phcorp
marked this pull request as ready for review
November 8, 2026 20:08
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
PHP accepts any semi-reserved word as a method name in a trait adaptation, just as it does in a method declaration. tempest/highlight relies on this:
elephc rejected this with
Expected method or trait name in adaptation, andA::print insteadof Bfailed withExpected method name after 'TraitName::'. Every adaptation position now takes a keyword name, following PHP's grammar:Directly after
as, a modifier is still read as the visibility change (as public). After a visibility, the alias can be any word, modifiers included:as protected staticaliases the method tostatic().Repro
error: Expected method name after 'TraitName::' in adaptationABlBAlABlBAlEach adaptation form, compared across main, the branch and PHP 8.5:
match as protected matchContentfunction as f/static as st/namespace as ns(bare keyword target)T::enum as protected readonlyfoo as protected staticfoo as list,foo as protected listfoo as classfoo as staticTrait alias adaptation requires a visibility and/or alias name)How it works
src/parser/stmt/oop/traits.rs:Bare target (L180-L219). When the token cannot start a class-like name (
name_starts_atis false) but spells a word, the parser takes that word as the method. A name that can start a class name (A::foo,\Ns\A::foo,namespace\A::foo) still goes throughparse_name. A barenamespacethat is not followed by\falls through to the keyword path.Trait::methodtarget. Accepts any word through the same helper.Alias (L76-L90):
adaptation_barewordis a thin wrapper overkeyword_name::bareword_name_from_token, the helper that method declarations and->/::member names already use.Tests
test_parse_trait_adaptation_keyword_method_names(parser): covers the bare, qualified and visibility-only forms, andlist as private static, which aliases tostatic.test_trait_adaptation_keyword_method_names(codegen): runs the tempest/highlight shape and prints[m:x]|l|l.test_trait_insteadof_and_modifier_alias_with_keyword_names(codegen):A::print insteadof B,B::list insteadof A,B::print as functionandA::list as protected static. It printsA|Bl|B|Al, as PHP does.test_error_trait_alias_modifier_without_visibility_is_not_an_aliasandtest_error_trait_adaptation_scoped_target_requires_method_name(errors):foo as static;andT::$foo as bar;are still rejected.The parser and codegen tests fail on main. Runs on this branch:
cargo test --test parser_tests trait: 7 passedcargo test --test error_tests trait: 115 passedELEPHC_PHP_CHECK=1 cargo test --test codegen_tests traits: 18 passedNote
CI status: this PR comes from a fork, so the
CI,PDO Live DatabasesandPHP extension host SDKworkflows are waiting for maintainer approval (action_required). The focused tests above ran locally on macOS arm64, and the codegen tests were also compared with PHP 8.5.12 (ELEPHC_PHP_CHECK=1). This is a front-end change (parser/checker only) with no target-specific code.Limits
foo as static;is still a parse error. PHP parses it and then fails at compile time with a different message. Both reject it.$x->list(),K::print(),$x->match()) already worked on main and is unchanged. The magicianeval()parser already accepts keyword names in adaptations:evaloflist as protected print; TT::list as function;printsll.Follow-ups
tests/codegen/oop/traits.rs(EOF) andtests/error_tests/classes_traits.rs(after line 72). This PR's tests sit mid-file, away from those hunks. Scratch merges of fix(parser): accept semi-reserved keywords as method names in trait adaptations #2286 + fix(traits): an abstract trait method is a requirement, never a conflict with another trait's method #2289 and of fix(parser): accept semi-reserved keywords as method names in trait adaptations #2286 + fix(checker): a used trait's abstract methods are requirements an inherited method satisfies #2293 are clean. fix(traits): an abstract trait method is a requirement, never a conflict with another trait's method #2289 and fix(checker): a used trait's abstract methods are requirements an inherited method satisfies #2293 conflict with each other in both test files, because both add tests at the same spots. Whichever of them merges second needs a trivial rebase. This PR can merge before or after either one.