Skip to content

Fix attribute-class and new-in-initializer weaving fatals; typed consts, PARAMETER_WIDENING removal - #617

Merged
lisachenko merged 7 commits into
masterfrom
claude/php85-audit-fix-consts-cleanup
Aug 29, 2026
Merged

Fix attribute-class and new-in-initializer weaving fatals; typed consts, PARAMETER_WIDENING removal#617
lisachenko merged 7 commits into
masterfrom
claude/php85-audit-fix-consts-cleanup

Conversation

@lisachenko

Copy link
Copy Markdown
Member

Summary

Two confirmed bug fixes plus the approved modernization batch, in four commits (one per task). All gates green on PHP 8.4 and 8.5 (phpunit + PHPStan level 10 with no new baseline entries); the full suite also passes on PHP 8.6.0beta2 (informational).

1. Fixes #615 — weaving an attribute class fatals

Class-level attributes survive class→trait conversion since #613, but #[\Attribute] and #[\AllowDynamicProperties] are compile-time invalid on traits ("Cannot apply #[\Attribute] to trait"), so weaving any attribute class was a fatal at woven-trait load.

WeavingTransformer now removes exactly these attribute entries from the woven trait tokens (resolved names from the class node's attrGroups):

  • a group consisting only of incompatible attributes is blanked out entirely, preserving contained newlines (line numbers stay intact for XDebug);
  • in a grouped attribute (#[\Attribute, SomethingElse]) only the incompatible entry plus one adjacent comma is removed.

The proxy class still copies the original attribute groups from the AST (AttributeGroupsGenerator), so reflection on the proxied attribute class keeps reporting #[\Attribute(...)] with original arguments — asserted in the new test.

Tests: new golden fixture php80-attribute-class.php/-woven.php (attribute class with args, grouped attribute, #[\AllowDynamicProperties]), updated php80-class-attribute-woven.php golden, proxy attribute assertion.

2. Fixes #616 — new-in-initializer default copied onto hook property

An intercepted promoted property like private Collaborator $service = new Collaborator('x') produced a proxy hook property carrying the new default — a compile error ("New expressions are not supported in this context"): new is legal in parameter defaults but illegal in property initializers.

AbstractInterceptedPropertyGenerator now skips the AST default when it contains any PhpParser\Node\Expr\New_ (NodeFinder). The hook property stays uninitialized; the demoted constructor parameter keeps the default, the injected $this->prop = $prop; assignment routes the value through the set hook, and hasPotentiallyUninitializedTypedProperty() now reports true so the isInitialized() guards cover the pre-construction window.

Tests: targeted WeavingTransformerTest assertions (woven + proxy content, proxy parseability) with fixture php81-new-in-initializer.php, plus a functional runtime test (ClassWeavingTest::testNewInInitializerPromotedPropertyWeaving) that weaves NewInInitializerClass in a subprocess, instantiates it without arguments and confirms the constructor default materializes (['seed']). Verified to fail before the fix.

3. Fixes #608 — typed class constants + #[\Override]

  • Every class/interface constant in src/ outside the Fix enum const-expression case values, extend pointcut expressiveness, add PHP 8.5 limitations doc #614-reserved files now has a native type: AspectContainer prefixes, Features flags, Pointcut::KIND_*, generator FLAG_*/VISIBILITY_*, TypeGenerator::BUILTIN_TYPES (array), CachePathManager cache file names, stream-filter identifiers and cache suffixes.
  • #[\Override] added to ~110 overriding methods across src/ (verified exhaustively via reflection): transformer transform() implementations, SourceTransformingLoader::filter(), the AbstractJoinpoint/AbstractInvocation/AbstractInterceptor hierarchies, Pointcut::matches()/getKind() implementations, container/aspect-loader extensions, console commands, proxy generators, node visitors. Docblocks that were only {@inheritDoc} were replaced by the attribute; docblocks adding @return/@throws info were kept. No #[\Override] added under tests/Fixtures/ or tests/Stubs/ (the weaver's Override-stripping paths exercise those deliberately).

4. Partially addresses #610 — with #614 covering the rest

  • [BC BREAK] Features::PARAMETER_WIDENING removed together with the whole $useParameterWidening/$useTypeWidening/$useWidening plumbing through WeavingTransformer, all four proxy generators, InterceptedMethodGenerator, InterceptedConstructorGenerator, FunctionParameterList and MethodGenerator/FunctionGenerator/ParameterGenerator::fromReflection(). The flag has been a no-op concern since the PHP 7.2 baseline; generated proxies always keep original parameter types. CHANGELOG entry added under 4.0.0 (unreleased). Kernel configurations passing the flag must drop it.
  • Constructor property promotion for declare-then-assign constructors in src/Instrument/ (BaseSourceTransformer, Enumerator, CachePathManager, AopComposerLoader, WeavingTransformer) and src/Proxy/Generator/ (ClassGenerator, TraitGenerator, PropertyGenerator, ParameterGenerator, ValueGenerator, DocBlockGenerator, MethodGenerator, FunctionGenerator), readonly where never reassigned. Left as-is: properties fed through array_values() or derived from other constructor arguments ($container, $options-derived fields), and FileGenerator (no constructor).
  • Restored golden coverage of general PHP 8.0–8.3 syntax through the current weaver: new fixture php80-82-syntax.php + -woven.php + -proxy.php covering constructor promotion (non-intercepted property), new-in-initializer parameter default, named arguments, match, nullsafe operator, enum usage in a method body, readonly property, first-class callable and a typed class constant; wired into WeavingTransformerTest. No fixtures were deleted.

Deferred until #614 merges (do-not-touch in this PR)

BC note

Features::PARAMETER_WIDENING (int 128) no longer exists and the proxy-generator constructors / fromReflection() factories lost their widening parameter. Anyone constructing these classes directly or passing the feature flag must update (4.0 changelog line included).

Test evidence


Generated by Claude Code

claude added 4 commits August 28, 2026 22:02
Fixes #615

Weaving an attribute class fatally failed at woven-trait load time:
class-level attributes are preserved during class-to-trait conversion
(issue #598), but #[\Attribute] and #[\AllowDynamicProperties] are
compile-time invalid on traits ("Cannot apply #[\Attribute] to trait").

WeavingTransformer now removes these attribute entries from the woven
trait tokens while converting a class to a trait:
- a group consisting only of incompatible attributes is blanked out
  entirely, keeping any newlines so line numbers stay intact;
- in a grouped attribute (e.g. `#[\Attribute, SomethingElse]`) only the
  incompatible entry plus one adjacent comma is removed.

The proxy class still copies the original attribute groups from the AST
via AttributeGroupsGenerator, so reflection on the proxied attribute
class keeps reporting #[\Attribute(...)] with its original arguments.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFMYyvE4hYRUoMS8mtKrHP
Fixes #616

An intercepted promoted constructor property with a new-in-initializer
default (e.g. `private Collaborator $service = new Collaborator('x')`)
produced a proxy hook property carrying that default, which is a compile
error at proxy load time: `new` is legal in constructor parameter
defaults but illegal in property initializers ("New expressions are not
supported in this context").

AbstractInterceptedPropertyGenerator now skips the AST default when the
default expression contains any `new` expression. The hook property
stays uninitialized: the demoted constructor parameter keeps the `new`
default in the woven trait, the injected `$this->prop = $prop;`
assignment routes the value through the proxy set hook, and the
isInitialized() guards (hasPotentiallyUninitializedTypedProperty() now
reports true for this case) cover the pre-construction window.

Covered by a targeted WeavingTransformerTest assertion set (woven +
proxy content, proxy parseability) and a functional runtime test that
weaves the class in a subprocess, instantiates it without arguments and
confirms the constructor default still materializes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFMYyvE4hYRUoMS8mtKrHP
Fixes #608

Typed constants: every class/interface constant in src/ now declares
its native type (string/int/array) — AspectContainer prefixes,
Features flags, Pointcut KIND_* kinds, proxy generator FLAG_* and
VISIBILITY_* constants, TypeGenerator::BUILTIN_TYPES, cache file names
in CachePathManager, stream filter identifiers and cache suffixes in
the Instrument transformers/loaders.

#[\Override]: added to every overriding method in framework src/ code —
transformer transform() implementations, SourceTransformingLoader::
filter(), the AbstractJoinpoint/AbstractInvocation/AbstractInterceptor
hierarchies, Pointcut matches()/getKind() implementations, container
and aspect loader extensions, console commands, proxy generators and
node visitors. Docblocks consisting solely of {@inheritdoc} were
replaced by the attribute; docblocks that add information (@return,
@throws) were kept.

Files touched by the open PR #614 (EnumProxyGenerator, EnumGenerator,
ReturnTypePointcut, ModifierPointcut, PointcutGrammar,
PointcutParseTable) are intentionally left untouched; their constants
and overrides remain for a follow-up after #614 merges. Fixture classes
under tests/ deliberately receive no #[\Override] (the weaver's
Override-stripping paths exercise them).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFMYyvE4hYRUoMS8mtKrHP
…ntax coverage

Partially addresses #610 — covers item 1 (flag + plumbing removal), the
coverage-restoration half of item 2, and the constructor-promotion part
of item 4. The open PR #614 carries the orphaned-fixture deletions, the
PHPStan 8.5 CI job (item 3) and tests/functions.php typing; the
static-singleton/extract() cleanup of item 4 remains open.

[BC BREAK] Removed the Features::PARAMETER_WIDENING flag constant and the
entire parameter-widening plumbing ($useParameterWidening/$useTypeWidening/
$useWidening) threaded through WeavingTransformer, ClassProxyGenerator,
TraitProxyGenerator, EnumProxyGenerator, FunctionProxyGenerator,
InterceptedMethodGenerator, InterceptedConstructorGenerator,
FunctionParameterList and MethodGenerator/FunctionGenerator/
ParameterGenerator::fromReflection(). The feature was a PHP 7.0/7.1
compatibility aid that is dead weight on the PHP 8.4+ baseline: generated
proxies always keep the original parameter types. Callers passing the flag
must drop it from their kernel options (documented in CHANGELOG.md).
EnumProxyGenerator received only the minimal constructor-arg/property
removal (file is otherwise reserved for PR #614).

Constructor property promotion for declare-then-assign constructors in
src/Instrument/ (BaseSourceTransformer, Enumerator, CachePathManager,
AopComposerLoader, WeavingTransformer) and src/Proxy/Generator/
(ClassGenerator, TraitGenerator, PropertyGenerator, ParameterGenerator,
ValueGenerator, DocBlockGenerator, MethodGenerator, FunctionGenerator),
readonly where the property is never reassigned. Properties fed through
array_values() or derived from other arguments stay declared.

Restored golden-file coverage of general PHP 8.0-8.3 syntax through the
current weaver: new fixture php80-82-syntax.php (+ -woven/-proxy goldens)
covering constructor promotion, new-in-initializer parameter default,
named arguments, match, nullsafe, enum usage in a method body, readonly
property, first-class callable and a typed class constant, wired into
WeavingTransformerTest.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFMYyvE4hYRUoMS8mtKrHP
Comment thread src/Aop/Framework/AbstractInterceptor.php Outdated
Keep the typed class constants; the attribute-everywhere approach adds
churn without runtime benefit, so all 110 added #[\Override] lines are
dropped. The weaver's own Override-stripping logic and its docs are
untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFMYyvE4hYRUoMS8mtKrHP
@lisachenko lisachenko changed the title Fix attribute-class and new-in-initializer weaving fatals; typed consts, #[\Override], PARAMETER_WIDENING removal Fix attribute-class and new-in-initializer weaving fatals; typed consts, PARAMETER_WIDENING removal Aug 29, 2026
claude added 2 commits August 29, 2026 08:45
The audit branch (#597) merged with KNOWN_GAPS still pinning #615/#616,
which this branch fixes — empty the gap list so the harness asserts the
fully-fixed state.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFMYyvE4hYRUoMS8mtKrHP
@lisachenko
lisachenko marked this pull request as ready for review August 29, 2026 08:46
@lisachenko
lisachenko merged commit 54b9bce into master Aug 29, 2026
8 checks passed
@lisachenko
lisachenko deleted the claude/php85-audit-fix-consts-cleanup branch August 29, 2026 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants