Skip to content

Property interception breaks promoted constructor properties (parse error or silently lost assignments) #599

Description

@lisachenko

Summary

When a property pointcut matches a promoted constructor property, WeavingTransformer::commentOutInterceptedPropertiesInTraitBody() comments the promoted parameter out inside the constructor signature. Depending on formatting this yields either a parse error or a constructor that silently discards its arguments.

Found during the PHP 8.5 support audit (PR #597). Reproduces on PHP 8.4 / 8.5.10 / 8.6.0beta2.

Reproduction

Single-line constructor → parse error (the // prefix swallows the closing )):

class Collaborator
{
    public function __construct(public string $tag = 'default') {}
}

woven trait:

public function __construct(// public string $tag = 'default' // Moved by weaving interceptor to the {@see ...->tag})
{
}

Multi-line constructor → lints, but is semantically broken:

trait Php85CloneWith__AopProxied
{
    public function __construct(
        // public string $name = 'initial' // Moved by weaving interceptor ...,
        // public int $count = 0 // Moved by weaving interceptor ...,
    ) {
    }

The proxy re-declares the properties with interception hooks and its constructor keeps the parameter list (__construct(string $name = 'initial', int $count = 0)), but it delegates to the trait's __aop____construct(...) — whose parameters no longer exist. Arguments are silently dropped, the promoted assignment never happens, and any read ($this->name) hits an uninitialized typed property. The hook properties on the proxy also lose their default values (public string $name { instead of public string $name = 'initial' {).

Same failure mode with PHP 8.5 final promoted properties (final public string $identity = 'anon'), fixture tests/Fixtures/audit/src/Php85FinalPromotionAsymStatic.php.

Root cause

  • src/Instrument/Transformer/WeavingTransformer.phpcommentOutInterceptedPropertiesInTraitBody() (~line 550) treats a promoted param like a property declaration statement and comments out its token range inside the parameter list.
  • src/Proxy/Part/InterceptedPropertyGenerator.php builds the hook property without carrying over the promoted parameter's default.

Proposed fix

For an intercepted promoted property, demote instead of remove in the woven trait: rewrite public string $name = 'initial' to a plain parameter string $name = 'initial' and inject $this->name = $name; at the top of the constructor body (line numbers can be preserved by doing it within the same line/token budget, or accepting the proxy include boundary). The proxy-side hook property should carry the original default value. Alternatively (smaller but restrictive): exclude promoted properties from access() interception in AdviceMatcher the way static/readonly/hooked properties already are, and document the limitation — but that silently narrows pointcut semantics, so demotion is preferred.

Environment

  • goaop/framework master (4.0-dev), PHP 8.4 / 8.5.10 / 8.6.0beta2

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions