Skip to content

PHP 8.4 modernization batch: native lazy objects, property hooks, str_*/array_any + latent strpos bug fixes - #612

Merged
lisachenko merged 3 commits into
masterfrom
claude/php85-audit-fix-modernization
Aug 28, 2026
Merged

PHP 8.4 modernization batch: native lazy objects, property hooks, str_*/array_any + latent strpos bug fixes#612
lisachenko merged 3 commits into
masterfrom
claude/php85-audit-fix-modernization

Conversation

@lisachenko

Copy link
Copy Markdown
Member

Modernization batch (issues #606, #607, #609)

Three commits, one per issue. Gates on every commit: php8.5 vendor/bin/phpunit green, php8.4 vendor/bin/phpunit green, PHPStan level 10 clean (no new baseline entries).

Task 1 — native lazy objects (Partially fixes #606)

Container: materializeService() now materializes services registered via addLazyService() as native lazy proxies (ReflectionClass::newLazyProxy()). Retrieval hands out a typed, instanceof-correct instance of the service class; the registered factory only runs on the first real interaction with the object. registerAspect() by class name gains an eager validation step (implements Aspect, default-constructibility) that runs when the entry materializes, so misconfiguration still surfaces on retrieval while construction stays deferred.

CachedAspectLoader: the @property + __get('loader') ghost pattern becomes a hooked property that memoizes the container lookup in its backing store; the magic method, its RuntimeException fallthrough and #[AllowDynamicProperties] are removed.

Behavior-parity notes for the lazy-object change:

  • Public API unchanged (add/addLazyService/getService/getValue/has/getServicesByInterface/registerAspect); has() and interface/tag introspection behave as before — materialization still tags by interface and registers the class file as a resource, without running the factory.
  • Nothing is autoloaded or constructed at registration time, exactly as before.
  • Classes PHP cannot make lazy fall back to the previous eager construction path: internal classes and their non-stdClass subclasses, abstract classes, enums, readonly classes before PHP 8.5, non-class ids — and notably classes without instance properties, because PHP creates lazy objects of property-less classes as already initialized, which would silently skip the factory (covered by a dedicated test).
  • Invalid aspect registrations (registerAspect(stdClass::class), missing factory for required ctor args) still throw AspectException on retrieval, not later.
  • A factory returning an incompatible object still surfaces as AspectException ("is not properly registered" / "returned an incompatible object"), now at initialization time.
  • Timing shifts by design: factory side effects (e.g. FilterInjectorTransformer registering the stream filter, CachedAspectLoader reading kernel options) run at first use of the service instead of first retrieval — always before the service can do anything.

Item 3 of #606 (ReflectionConstructorInvocation::proceed()) was evaluated and skipped, as the issue anticipated: a lazy-ghost sequence there would either have to be initialized immediately (no gain over the current explicit sequence) or defer constructor side effects and change observable behavior around getThis()/Before advices; newLazyGhost() also rejects classes (internal, property-less) that newInstanceWithoutConstructor() + explicit ctor invoke handles today. Left for a follow-up, hence "Partially fixes".

docs/php84-limitations.md now accurately describes what the container does with lazy objects.

Tests: ContainerTest gained laziness-semantics coverage — factory not run at registration/retrieval/enumeration, instanceof and isUninitializedLazyObject() correct before initialization, first method call initializes and delegates, property-less services construct eagerly through their (possibly re-registered) factory.

Task 2 — property hooks instead of internal __get/__set (Fixes #607)

  • StreamMetaData: the magic source pair becomes a native hooked property — get still rebuilds the source from the token stream, set still triggers the existing E_USER_DEPRECATED notice and re-tokenizes. @property-read docblock removed; all in-repo accesses are reads and pass unchanged; new StreamMetaDataTest covers both hook paths.
  • LazyAdvisorAccessor: #[AllowDynamicProperties] + __get dynamic-property cache replaced by a typed array<string, Interceptor> cache behind the existing getInterceptor(); its only call site (InterceptorInjector) already used getInterceptor(), no ->{$name} magic access exists in the repo.
  • AbstractAttribute: __get/__set existed only to throw BadMethodCallException; nothing in the repo relies on that, both removed.
  • CachedAspectLoader.__get was intentionally handled in Task 1 (per issue scoping).

Task 3 — str_*/array_any/array_all + latent strpos bugs (Fixes #609)

Bug fixes with regression tests:

  • Enumerator::getInPaths() used strpos($path, $root, 0) === false — a substring-anywhere test — as a prefix check, so an include path merely containing the root elsewhere (e.g. /somewhere/base/other for root /base) was wrongly accepted. Now !str_starts_with(...); EnumeratorTest::testIncludePathMerelyContainingRootDirectoryIsRejected proves the fixed behavior (plus a positive prefix sanity test).
  • WeavingTransformerTest assertFalse(strpos($proxyContent, '\\\\Exception')) (also passes on a match at offset 0) → assertStringNotContainsString().

Modernizations: PHP_OS 'WIN' sniff → PHP_OS_FAMILY === 'Windows'; Enumerator filter prefix check → str_starts_with(); include/exclude fnmatch loops → array_any(); PathResolver/AdviceMatcherstr_contains(); OrPointcut/AndPointcut matches()array_any()/array_all(); AbstractInterceptedPropertyGenerator union-type scan → array_any().

Deprecation cleanup: ReflectionFilenameTest single-argument ReflectionProperty::setValue() → two-argument form for the static property.

Note: src/Instrument/Transformer/WeavingTransformer.php is deliberately untouched — it is being rewritten on another branch, so its array_find/end()/strpos sites are excluded from this PR by design.

Test evidence

Gate Result
php8.4 vendor/bin/phpunit OK — 2500 tests, 2987 assertions, 0 deprecations (1 pre-existing skip: a PHP ≥ 8.5 feature test)
php8.5 vendor/bin/phpunit OK — 2500 tests, 2990 assertions, 0 deprecations
php8.6 (8.6.0beta2) vendor/bin/phpunit (informational) OK — 2500 tests, 2990 assertions, 0 deprecations
php8.5 vendor/bin/phpstan analyze (level 10) No errors, no new baseline entries

Before this batch the suite ended with 1 deprecation (the setValue() call); it now ends with zero on all three PHP versions.

Partially fixes #606 (items 1, 2, 4; item 3 evaluated and deliberately skipped)
Fixes #607
Fixes #609


Generated by Claude Code

claude added 3 commits August 28, 2026 21:26
Container: materializeService() now creates services registered via
addLazyService() as native lazy proxies (ReflectionClass::newLazyProxy),
so retrieval hands out typed, instanceof-correct objects whose factory
only runs on the first real interaction. Classes PHP cannot make lazy
(internal classes and their non-stdClass subclasses, abstract classes,
enums, readonly classes before PHP 8.5, and classes without instance
properties - PHP creates those pre-initialized, which would silently
skip the factory) keep the previous eager construction path.

Aspect registration by class name gains an eager validation step
(implements Aspect, default-constructibility) that runs when the lazy
object materializes, so misconfiguration still surfaces on retrieval
while construction stays deferred until first use.

CachedAspectLoader: the @Property + __get() ghost pattern for the inner
loader is replaced with a hooked property that memoizes the container
lookup in its backing store; the magic method, its RuntimeException
fallthrough and #[AllowDynamicProperties] are gone.

ReflectionConstructorInvocation::proceed() (item 3 of the issue) is
deliberately left as newInstanceWithoutConstructor() + explicit
constructor invocation: a lazy-ghost sequence would either have to be
initialized immediately (no gain) or defer constructor side effects,
changing observable behavior, and newLazyGhost() rejects classes
(internal, property-less) that the current sequence handles.

docs/php84-limitations.md now accurately describes what the container
does with lazy objects.

Partially fixes #606 (items 1, 2 and 4; item 3 evaluated and skipped)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFMYyvE4hYRUoMS8mtKrHP
StreamMetaData: the magic __get/__set pair for the virtual 'source'
property becomes a native hooked property - reading still rebuilds the
source from the token stream, writing still triggers the existing
E_USER_DEPRECATED notice and re-tokenizes. The @property-read docblock
is gone; new StreamMetaDataTest covers both hook paths.

LazyAdvisorAccessor: #[AllowDynamicProperties] and the dynamic-property
__get cache are replaced by a typed private array cache behind the
existing getInterceptor() accessor (its only call site,
InterceptorInjector, already uses getInterceptor()).

AbstractAttribute: __get/__set existed only to throw
BadMethodCallException for unknown properties; nothing relies on that
behavior, so both are removed - attribute classes now expose only their
promoted readonly properties.

CachedAspectLoader's __get was converted separately as part of the lazy
objects work (#606).

Fixes #607

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFMYyvE4hYRUoMS8mtKrHP
Bug fixes:
- Enumerator::getInPaths() used strpos(...) === false - a
  substring-anywhere test - as a prefix check, so an include path that
  merely contained the root directory in the middle (e.g.
  '/somewhere/base/other' with root '/base') was wrongly accepted
  instead of rejected; now str_starts_with() with a regression test.
- WeavingTransformerTest asserted assertFalse(strpos(...)), which also
  passes for a match at offset 0; now assertStringNotContainsString().

Modernizations (no behavior change):
- Enumerator: PHP_OS 'WIN' prefix sniff -> PHP_OS_FAMILY === 'Windows';
  filter root check -> str_starts_with(); include/exclude fnmatch loops
  -> array_any() with a shared pattern matcher.
- PathResolver / AdviceMatcher: strpos(...) !== false -> str_contains().
- OrPointcut / AndPointcut: matches() loops -> array_any()/array_all().
- AbstractInterceptedPropertyGenerator: union-type array scan ->
  array_any().
- ReflectionFilenameTest: single-argument ReflectionProperty::setValue()
  (deprecated) -> two-argument form for the static property; the suite
  now finishes with zero PHP deprecations on 8.4, 8.5 and 8.6.

WeavingTransformer.php is deliberately untouched - it is being rewritten
on another branch, so its strpos/end() sites are out of scope here.

Fixes #609

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 28, 2026 21:39
@lisachenko
lisachenko merged commit 26604ed into master Aug 28, 2026
7 checks passed
@lisachenko
lisachenko deleted the claude/php85-audit-fix-modernization branch August 28, 2026 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants