diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c1808cc..97cb0e06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Changelog * [Feature] **PHP 8.1+ enum interception** — instance and static methods on both unit (pure) and backed enums can now be intercepted by aspects. The enum body is extracted into a trait (`Foo__AopProxied`); a proxy enum re-declares the cases and dispatches intercepted methods via per-method `static $__joinPoint` caching. Built-in enum methods (`cases`, `from`, `tryFrom`) and initialization joinpoints are never woven. * [Feature] `self::` in proxied classes now resolves to the proxy class naturally (via PHP trait semantics), removing the need for `SelfValueTransformer`. * [Feature] **First-class callable syntax** — generated proxy code and invocation constructors use PHP 8.1+ first-class callable syntax (`$this->__aop__method(...)`, `parent::method(...)`, `\func(...)`) to reference original method and function bodies, eliminating the need for `Closure::bind` at construction time. +* [BC BREAK] Removed `Features::PARAMETER_WIDENING` and the parameter-widening code path in the proxy generators. The feature was a PHP 7.0/7.1 compatibility aid (parameter type widening, wiki.php.net/rfc/parameter-no-type-variance) that has been a no-op concern since the PHP 7.2 baseline: generated proxies always keep the original parameter types. Remove the flag from `AspectKernel::configureAop()` options if you passed it. * [BC BREAK] Removed DeclareError support, including the `DeclareError` attribute, `DeclareErrorInterceptor`, and `PointcutBuilder::declareError()`. Use `Before` or `Around` interceptors to emit user warnings or throw exceptions instead. * [BC BREAK] Removed support for the "dynamic" pointcut (`dynamic(public Foo->method*(*))`), including `MagicMethodDynamicPointcut`, `DynamicInvocationMatcherInterceptor`, the `Pointcut::KIND_DYNAMIC` constant and the `$instanceOrScope`/`$arguments` parameters of `Pointcut::matches()`. Use a traditional execution pointcut for the magic methods instead, e.g. `execution(public Foo->__call(*))` or `execution(public Foo::__callStatic(*))`, and check the invoked method name from `$invocation->getArguments()[0]` inside the advice. * [Removed] `SelfValueTransformer` and `SelfValueVisitor` — no longer needed with the trait-based engine. diff --git a/src/Aop/AGENTS.md b/src/Aop/AGENTS.md index ba208f5f..14909dc2 100644 --- a/src/Aop/AGENTS.md +++ b/src/Aop/AGENTS.md @@ -49,4 +49,3 @@ Proxy generators use TypeGenerator::renderTypeForPhpDoc() to emit V as 2nd gener Interface with bitmask constants: - INTERCEPT_FUNCTIONS=1, INTERCEPT_INITIALIZATIONS=2, INTERCEPT_INCLUDES=4 - PREBUILT_CACHE=64 — assume cache already prepared, skip freshness checks -- PARAMETER_WIDENING=128 — enable parameter widening for PHP>=7.2 diff --git a/src/Aop/Features.php b/src/Aop/Features.php index d093beca..255cb432 100644 --- a/src/Aop/Features.php +++ b/src/Aop/Features.php @@ -21,19 +21,19 @@ interface Features * Enables interception of system function. * By default this feature is disabled, because this option is very expensive. */ - public const INTERCEPT_FUNCTIONS = 1; + public const int INTERCEPT_FUNCTIONS = 1; /** * Enables interception of "new" operator in the source code * By default this feature is disabled, because it's very tricky */ - public const INTERCEPT_INITIALIZATIONS = 2; + public const int INTERCEPT_INITIALIZATIONS = 2; /** * Enables interception of "include"/"require" operations in legacy code * By default this feature is disabled, because only composer should be used */ - public const INTERCEPT_INCLUDES = 4; + public const int INTERCEPT_INCLUDES = 4; /** * Trust the cache built at deploy time (`bin/aspect cache:warmup:aop`) unconditionally @@ -44,12 +44,5 @@ interface Features * rebuild the cache on every deployment. Also usable for read-only file systems * (GAE, phar, etc). */ - public const PREBUILT_CACHE = 64; - - /** - * Enables usage of parameter widening for PHP>=7.2.0 - * - * @see https://wiki.php.net/rfc/parameter-no-type-variance - */ - public const PARAMETER_WIDENING = 128; + public const int PREBUILT_CACHE = 64; } diff --git a/src/Aop/Pointcut.php b/src/Aop/Pointcut.php index b3545ad1..ff2373f2 100644 --- a/src/Aop/Pointcut.php +++ b/src/Aop/Pointcut.php @@ -42,15 +42,15 @@ */ interface Pointcut { - public const KIND_METHOD = 1; - public const KIND_PROPERTY = 2; - public const KIND_CLASS = 4; - public const KIND_TRAIT = 8; - public const KIND_FUNCTION = 16; - public const KIND_INIT = 32; - public const KIND_STATIC_INIT = 64; - public const KIND_ALL = 127; - public const KIND_INTRODUCTION = 512; + public const int KIND_METHOD = 1; + public const int KIND_PROPERTY = 2; + public const int KIND_CLASS = 4; + public const int KIND_TRAIT = 8; + public const int KIND_FUNCTION = 16; + public const int KIND_INIT = 32; + public const int KIND_STATIC_INIT = 64; + public const int KIND_ALL = 127; + public const int KIND_INTRODUCTION = 512; /** * Returns the kind of point filter diff --git a/src/Bridge/Doctrine/MetadataLoadInterceptor.php b/src/Bridge/Doctrine/MetadataLoadInterceptor.php index a5386aae..5eee2b36 100644 --- a/src/Bridge/Doctrine/MetadataLoadInterceptor.php +++ b/src/Bridge/Doctrine/MetadataLoadInterceptor.php @@ -28,9 +28,6 @@ */ final class MetadataLoadInterceptor implements EventSubscriber { - /** - * {@inheritdoc} - */ public function getSubscribedEvents(): array { return [ diff --git a/src/Console/Command/BaseAspectCommand.php b/src/Console/Command/BaseAspectCommand.php index 7fdc32e3..54f47f9d 100644 --- a/src/Console/Command/BaseAspectCommand.php +++ b/src/Console/Command/BaseAspectCommand.php @@ -31,9 +31,6 @@ class BaseAspectCommand extends Command */ protected AspectKernel $aspectKernel; - /** - * {@inheritDoc} - */ protected function configure(): void { $this->addArgument('loader', InputArgument::REQUIRED, 'Path to the aspect loader file'); diff --git a/src/Console/Command/CacheWarmupCommand.php b/src/Console/Command/CacheWarmupCommand.php index 190b0872..0071cba4 100644 --- a/src/Console/Command/CacheWarmupCommand.php +++ b/src/Console/Command/CacheWarmupCommand.php @@ -23,9 +23,6 @@ */ class CacheWarmupCommand extends BaseAspectCommand { - /** - * {@inheritDoc} - */ protected function configure(): void { parent::configure(); @@ -43,9 +40,6 @@ protected function configure(): void ; } - /** - * {@inheritDoc} - */ protected function execute(InputInterface $input, OutputInterface $output): int { $this->loadAspectKernel($input, $output); diff --git a/src/Console/Command/DebugAdvisorCommand.php b/src/Console/Command/DebugAdvisorCommand.php index 7b385166..ac4f3184 100644 --- a/src/Console/Command/DebugAdvisorCommand.php +++ b/src/Console/Command/DebugAdvisorCommand.php @@ -33,9 +33,6 @@ */ class DebugAdvisorCommand extends BaseAspectCommand { - /** - * {@inheritDoc} - */ protected function configure(): void { parent::configure(); @@ -51,9 +48,6 @@ protected function configure(): void ; } - /** - * {@inheritDoc} - */ protected function execute(InputInterface $input, OutputInterface $output): int { $this->loadAspectKernel($input, $output); diff --git a/src/Console/Command/DebugAspectCommand.php b/src/Console/Command/DebugAspectCommand.php index 292a5005..efa7b7bf 100644 --- a/src/Console/Command/DebugAspectCommand.php +++ b/src/Console/Command/DebugAspectCommand.php @@ -27,9 +27,6 @@ */ class DebugAspectCommand extends BaseAspectCommand { - /** - * {@inheritDoc} - */ protected function configure(): void { parent::configure(); @@ -45,9 +42,6 @@ protected function configure(): void ; } - /** - * {@inheritDoc} - */ protected function execute(InputInterface $input, OutputInterface $output): int { $this->loadAspectKernel($input, $output); diff --git a/src/Console/Command/DebugWeavingCommand.php b/src/Console/Command/DebugWeavingCommand.php index af4025e0..5619449d 100644 --- a/src/Console/Command/DebugWeavingCommand.php +++ b/src/Console/Command/DebugWeavingCommand.php @@ -31,9 +31,6 @@ */ class DebugWeavingCommand extends BaseAspectCommand { - /** - * {@inheritDoc} - */ protected function configure(): void { parent::configure(); @@ -49,9 +46,6 @@ protected function configure(): void ; } - /** - * {@inheritDoc} - */ protected function execute(InputInterface $input, OutputInterface $output): int { $this->loadAspectKernel($input, $output); diff --git a/src/Core/AspectContainer.php b/src/Core/AspectContainer.php index afd926b5..f5ce0838 100644 --- a/src/Core/AspectContainer.php +++ b/src/Core/AspectContainer.php @@ -24,47 +24,47 @@ interface AspectContainer /** * Prefix for function interceptor */ - public const FUNCTION_PREFIX = 'func'; + public const string FUNCTION_PREFIX = 'func'; /** * Prefix for properties interceptor */ - public const PROPERTY_PREFIX = 'prop'; + public const string PROPERTY_PREFIX = 'prop'; /** * Prefix for method interceptor */ - public const METHOD_PREFIX = 'method'; + public const string METHOD_PREFIX = 'method'; /** * Prefix for static method interceptor */ - public const STATIC_METHOD_PREFIX = 'static'; + public const string STATIC_METHOD_PREFIX = 'static'; /** * Trait introduction prefix */ - public const INTRODUCTION_TRAIT_PREFIX = 'trait'; + public const string INTRODUCTION_TRAIT_PREFIX = 'trait'; /** * Interface introduction prefix */ - public const INTRODUCTION_INTERFACE_PREFIX = 'interface'; + public const string INTRODUCTION_INTERFACE_PREFIX = 'interface'; /** * Initialization prefix, is used for initialization pointcuts */ - public const INIT_PREFIX = 'init'; + public const string INIT_PREFIX = 'init'; /** * Initialization prefix, is used for initialization pointcuts */ - public const STATIC_INIT_PREFIX = 'staticinit'; + public const string STATIC_INIT_PREFIX = 'staticinit'; /** * Suffix, that will be added to all proxied class names */ - public const AOP_PROXIED_SUFFIX = '__AopProxied'; + public const string AOP_PROXIED_SUFFIX = '__AopProxied'; /** * Returns a service from the container. diff --git a/src/Instrument/ClassLoading/AopComposerLoader.php b/src/Instrument/ClassLoading/AopComposerLoader.php index 96b5e170..caa7dca5 100644 --- a/src/Instrument/ClassLoading/AopComposerLoader.php +++ b/src/Instrument/ClassLoading/AopComposerLoader.php @@ -28,18 +28,6 @@ */ class AopComposerLoader { - /** - * Instance of original autoloader - */ - protected ClassLoader $original; - - /** - * AOP kernel options - * - * @phpstan-var KernelOptions - */ - protected array $options; - /** * File enumerator */ @@ -79,11 +67,11 @@ class AopComposerLoader * * @phpstan-param KernelOptions $options Configuration options */ - public function __construct(ClassLoader $original, AspectContainer $container, array $options) - { - $this->options = $options; - $this->original = $original; - + public function __construct( + protected readonly ClassLoader $original, + AspectContainer $container, + protected readonly array $options + ) { $prefixes = $original->getPrefixes(); $excludePaths = $options['excludePaths']; diff --git a/src/Instrument/ClassLoading/CachePathManager.php b/src/Instrument/ClassLoading/CachePathManager.php index 35e90363..c437c368 100644 --- a/src/Instrument/ClassLoading/CachePathManager.php +++ b/src/Instrument/ClassLoading/CachePathManager.php @@ -29,21 +29,16 @@ class CachePathManager /** * Name of the file with full transformation metadata (build-time data, loaded lazily) */ - private const CACHE_FILE_NAME = '/_transformation.cache'; + private const string CACHE_FILE_NAME = '/_transformation.cache'; /** * Name of the file with the minimal runtime include map (originalPath => cacheUri|null) */ - private const INCLUDE_MAP_FILE_NAME = '/_include.cache'; + private const string INCLUDE_MAP_FILE_NAME = '/_include.cache'; /** @phpstan-var KernelOptions */ protected array $options; - /** - * Aspect kernel instance - */ - protected AspectKernel $kernel; - protected ?string $cacheDir = null; /** @@ -99,9 +94,8 @@ class CachePathManager */ protected array $newCacheState = []; - public function __construct(AspectKernel $kernel) + public function __construct(protected readonly AspectKernel $kernel) { - $this->kernel = $kernel; $options = $kernel->getOptions(); $this->options = $options; $this->appDir = $options['appDir']; diff --git a/src/Instrument/ClassLoading/SourceTransformingLoader.php b/src/Instrument/ClassLoading/SourceTransformingLoader.php index be8a4ad6..1f21fbda 100644 --- a/src/Instrument/ClassLoading/SourceTransformingLoader.php +++ b/src/Instrument/ClassLoading/SourceTransformingLoader.php @@ -41,12 +41,12 @@ class SourceTransformingLoader extends PhpStreamFilter /** * Php filter definition */ - public const PHP_FILTER_READ = 'php://filter/read='; + public const string PHP_FILTER_READ = 'php://filter/read='; /** * Default PHP filter name for registration */ - public const FILTER_IDENTIFIER = 'go.source.transforming.loader'; + public const string FILTER_IDENTIFIER = 'go.source.transforming.loader'; /** * String buffer @@ -140,9 +140,6 @@ public static function getId(): string return self::$filterId; } - /** - * {@inheritdoc} - */ public function filter($in, $out, &$consumed, $closing): int { while ($bucket = stream_bucket_make_writeable($in)) { diff --git a/src/Instrument/FileSystem/Enumerator.php b/src/Instrument/FileSystem/Enumerator.php index 2264cfd1..e9dbaef6 100644 --- a/src/Instrument/FileSystem/Enumerator.php +++ b/src/Instrument/FileSystem/Enumerator.php @@ -26,37 +26,18 @@ */ class Enumerator { - /** - * Path to the root directory, where enumeration should start - */ - private string $rootDirectory; - - /** - * List of additional include paths, should be below rootDirectory - * - * @var string[] - */ - private array $includePaths; - - /** - * List of additional exclude paths, should be below rootDirectory - * - * @var string[] - */ - private array $excludePaths; - /** * Initializes an enumerator * - * @param string $rootDirectory Path to the root directory - * @param string[] $includePaths List of additional include paths - * @param string[] $excludePaths List of additional exclude paths + * @param string $rootDirectory Path to the root directory, where enumeration should start + * @param string[] $includePaths List of additional include paths, should be below rootDirectory + * @param string[] $excludePaths List of additional exclude paths, should be below rootDirectory */ - public function __construct(string $rootDirectory, array $includePaths = [], array $excludePaths = []) - { - $this->rootDirectory = $rootDirectory; - $this->includePaths = $includePaths; - $this->excludePaths = $excludePaths; + public function __construct( + private readonly string $rootDirectory, + private readonly array $includePaths = [], + private readonly array $excludePaths = [] + ) { } /** diff --git a/src/Instrument/Transformer/BaseSourceTransformer.php b/src/Instrument/Transformer/BaseSourceTransformer.php index fae47394..331ece81 100644 --- a/src/Instrument/Transformer/BaseSourceTransformer.php +++ b/src/Instrument/Transformer/BaseSourceTransformer.php @@ -29,11 +29,6 @@ abstract class BaseSourceTransformer implements SourceTransformer */ protected array $options; - /** - * Aspect kernel instance - */ - protected AspectKernel $kernel; - /** * Aspect container instance */ @@ -42,9 +37,8 @@ abstract class BaseSourceTransformer implements SourceTransformer /** * Default constructor for transformer */ - public function __construct(AspectKernel $kernel) + public function __construct(protected readonly AspectKernel $kernel) { - $this->kernel = $kernel; $this->container = $kernel->getContainer(); $this->options = $kernel->getOptions(); } diff --git a/src/Instrument/Transformer/FilterInjectorTransformer.php b/src/Instrument/Transformer/FilterInjectorTransformer.php index 4403a53d..a74e215a 100644 --- a/src/Instrument/Transformer/FilterInjectorTransformer.php +++ b/src/Instrument/Transformer/FilterInjectorTransformer.php @@ -30,7 +30,7 @@ class FilterInjectorTransformer implements SourceTransformer /** * Php filter definition */ - public const PHP_FILTER_READ = 'php://filter/read='; + public const string PHP_FILTER_READ = 'php://filter/read='; /** * Name of the filter to inject diff --git a/src/Instrument/Transformer/WeavingTransformer.php b/src/Instrument/Transformer/WeavingTransformer.php index 4e11eaea..eefd79bf 100644 --- a/src/Instrument/Transformer/WeavingTransformer.php +++ b/src/Instrument/Transformer/WeavingTransformer.php @@ -14,7 +14,6 @@ use Go\Aop\Advisor; use Go\Aop\Aspect; -use Go\Aop\Features; use Go\Aop\Framework\AbstractJoinpoint; use Go\Core\AdviceMatcher; use Go\Core\AdviceMatcherInterface; @@ -41,43 +40,34 @@ */ class WeavingTransformer extends BaseSourceTransformer { - private const FUNCTIONS_CACHE_SUFFIX = '/_functions/'; + private const string FUNCTIONS_CACHE_SUFFIX = '/_functions/'; /** - * Advice matcher for class - */ - protected AdviceMatcherInterface $adviceMatcher; - - /** - * Should we use parameter widening for our decorators - */ - protected bool $useParameterWidening = false; - - /** - * Cache manager - */ - private CachePathManager $cachePathManager; - - /** - * Loader for aspects + * Class-level attributes that are compile-time invalid on traits. + * + * When a class is converted to a trait, these attribute entries must be removed from the + * woven trait tokens: PHP raises "Cannot apply #[\Attribute] to trait" (and the same for + * #[\AllowDynamicProperties]) at load time. The proxy class re-declares them from the AST + * via AttributeGroupsGenerator, so attribute classes keep working (issue #615). + * + * @var list */ - protected AspectLoader $aspectLoader; + private const array TRAIT_INCOMPATIBLE_ATTRIBUTES = ['Attribute', 'AllowDynamicProperties']; /** * Constructs a weaving transformer + * + * @param AdviceMatcherInterface $adviceMatcher Advice matcher for class + * @param CachePathManager $cachePathManager Cache manager + * @param AspectLoader $aspectLoader Loader for aspects */ public function __construct( AspectKernel $kernel, - AdviceMatcherInterface $adviceMatcher, - CachePathManager $cachePathManager, - AspectLoader $loader + protected readonly AdviceMatcherInterface $adviceMatcher, + private readonly CachePathManager $cachePathManager, + protected readonly AspectLoader $aspectLoader ) { parent::__construct($kernel); - $this->adviceMatcher = $adviceMatcher; - $this->cachePathManager = $cachePathManager; - $this->aspectLoader = $loader; - - $this->useParameterWidening = $kernel->hasFeature(Features::PARAMETER_WIDENING); } /** @@ -160,13 +150,13 @@ private function processSingleClass( if ($class->isTrait()) { $this->commentOutInterceptedPropertiesInTraitBody($class, $advices, $metadata); $this->adjustOriginalTrait($class, $metadata, $newClassName); - $childProxyGenerator = new TraitProxyGenerator($class, $newFqcn, $advices, $this->useParameterWidening); + $childProxyGenerator = new TraitProxyGenerator($class, $newFqcn, $advices); } elseif ($class->isEnum()) { $this->convertEnumToTrait($class, $advices, $metadata, $newClassName); - $childProxyGenerator = new EnumProxyGenerator($class, $newFqcn, $advices, $this->useParameterWidening); + $childProxyGenerator = new EnumProxyGenerator($class, $newFqcn, $advices); } else { $this->convertClassToTrait($class, $advices, $metadata, $newClassName); - $childProxyGenerator = new ClassProxyGenerator($class, $newFqcn, $advices, $this->useParameterWidening); + $childProxyGenerator = new ClassProxyGenerator($class, $newFqcn, $advices); } $classFileName = $class->getFileName(); @@ -335,6 +325,119 @@ private function convertClassToTrait( // match, PHP would raise a fatal error if #[\Override] were present on the alias. $this->commentOutInterceptedPropertiesInTraitBody($class, $advices, $streamMetaData); $this->stripOverrideAttributeFromInterceptedMethods($class, $advices, $streamMetaData); + $this->stripTraitIncompatibleClassAttributes($classNode, $streamMetaData); + } + + /** + * Removes class-level attributes that cannot be applied to traits (issue #615). + * + * `#[\Attribute]` and `#[\AllowDynamicProperties]` are compile-time invalid on traits, so + * weaving an attribute class would make the woven trait fatal at load time. The attribute + * entries are removed from the trait tokens only — the proxy class copies the original + * attribute groups from the AST (AttributeGroupsGenerator), so runtime reflection on the + * proxied class still reports them. + * + * In a multi-attribute group (e.g. `#[\Attribute, SomethingElse]`) only the incompatible + * entries are removed together with one adjacent comma; the rest of the group is kept. + * Newlines inside removed token ranges are preserved so that all subsequent declarations + * stay at their original line numbers (XDebug breakpoint mapping). + */ + private function stripTraitIncompatibleClassAttributes(ClassLike $classNode, StreamMetaData $streamMetaData): void + { + foreach ($classNode->attrGroups as $attrGroup) { + $incompatibleAttributes = []; + foreach ($attrGroup->attrs as $attribute) { + // Names are resolved by parser-reflection's NameResolver, so global attribute + // classes are FullyQualified nodes ('Attribute', 'AllowDynamicProperties'). + if (in_array(ltrim($attribute->name->toString(), '\\'), self::TRAIT_INCOMPATIBLE_ATTRIBUTES, true)) { + $incompatibleAttributes[] = $attribute; + } + } + if ($incompatibleAttributes === []) { + continue; + } + if (count($incompatibleAttributes) === count($attrGroup->attrs)) { + // Every attribute in the group is incompatible — blank out the whole group '#[...]' + $start = $attrGroup->getAttribute('startTokenPos'); + $end = $attrGroup->getAttribute('endTokenPos'); + if (is_int($start) && is_int($end)) { + $this->blankTokenRangePreservingNewlines($start, $end, $streamMetaData); + } + continue; + } + foreach ($incompatibleAttributes as $attribute) { + $start = $attribute->getAttribute('startTokenPos'); + $end = $attribute->getAttribute('endTokenPos'); + if (!is_int($start) || !is_int($end)) { + continue; + } + $this->blankTokenRangePreservingNewlines($start, $end, $streamMetaData); + $this->removeAdjacentAttributeComma($start, $end, $streamMetaData); + } + } + } + + /** + * Blanks out all tokens in [$start, $end], keeping only the newlines they contained. + * + * Token objects are kept in place (text emptied) instead of being unset, so the iteration + * order of the token stream is untouched and the line budget of the file is preserved. + */ + private function blankTokenRangePreservingNewlines(int $start, int $end, StreamMetaData $streamMetaData): void + { + for ($position = $start; $position <= $end; ++$position) { + if (!isset($streamMetaData->tokenStream[$position])) { + continue; + } + $text = $streamMetaData->tokenStream[$position]->text; + $streamMetaData->tokenStream[$position]->text = str_repeat("\n", substr_count($text, "\n")); + } + } + + /** + * Removes one comma adjacent to a removed attribute entry inside a multi-attribute group. + * + * Prefers the trailing comma (after $end); falls back to the leading comma (before $start) + * when the removed entry was the last one in the group. Whitespace next to the comma is + * dropped only when it holds no newline (line budget). + */ + private function removeAdjacentAttributeComma(int $start, int $end, StreamMetaData $streamMetaData): void + { + // Scan forward for a trailing comma, skipping blank/whitespace tokens + $position = $end + 1; + while (isset($streamMetaData->tokenStream[$position])) { + $token = $streamMetaData->tokenStream[$position]; + if ($token->text === ',') { + unset($streamMetaData->tokenStream[$position]); + $nextPosition = $position + 1; + if (isset($streamMetaData->tokenStream[$nextPosition])) { + $nextToken = $streamMetaData->tokenStream[$nextPosition]; + if ($nextToken->id === T_WHITESPACE && strpbrk($nextToken->text, "\r\n") === false) { + unset($streamMetaData->tokenStream[$nextPosition]); + } + } + + return; + } + if ($token->id !== T_WHITESPACE && $token->text !== '') { + break; + } + ++$position; + } + // No trailing comma — remove the leading one instead + $position = $start - 1; + while (isset($streamMetaData->tokenStream[$position])) { + $token = $streamMetaData->tokenStream[$position]; + if ($token->text === ',') { + unset($streamMetaData->tokenStream[$position]); + + return; + } + if ($token->id !== T_WHITESPACE && $token->text !== '') { + break; + } + --$position; + } } /** @@ -872,7 +975,7 @@ private function processFunctions( if (!file_exists($dirname)) { mkdir($dirname, $this->options['cacheFileMode'], true); } - $generator = new FunctionProxyGenerator($namespace, $functionAdvices, $this->useParameterWidening); + $generator = new FunctionProxyGenerator($namespace, $functionAdvices); file_put_contents($functionFileName, $generator->generate(), LOCK_EX); // For cache files we don't want executable bits by default chmod($functionFileName, $this->options['cacheFileMode'] & (~0111)); diff --git a/src/Proxy/ClassProxyGenerator.php b/src/Proxy/ClassProxyGenerator.php index b031e8cb..10b01f5f 100644 --- a/src/Proxy/ClassProxyGenerator.php +++ b/src/Proxy/ClassProxyGenerator.php @@ -51,11 +51,6 @@ class ClassProxyGenerator */ protected GeneratorInterface $generator; - /** - * Should parameter widening be used or not - */ - protected bool $useParameterWidening; - /** * Generates a proxy class that wraps the original class body (now a trait) via trait-use. * @@ -64,19 +59,16 @@ class ClassProxyGenerator * that trait, and aliases each intercepted method as `private __aop__` so the * overriding method body can delegate to the original via a Closure::bind proceed closure. * - * @param ReflectionClass $originalClass Original class reflection (before transformation) - * @param string $traitName FQCN of the generated trait (e.g. Ns\Foo__AopProxied) - * @param string[][][] $classAdviceNames List of advices for class - * @param bool $useParameterWidening Enables usage of parameter widening feature + * @param ReflectionClass $originalClass Original class reflection (before transformation) + * @param string $traitName FQCN of the generated trait (e.g. Ns\Foo__AopProxied) + * @param string[][][] $classAdviceNames List of advices for class */ public function __construct( ReflectionClass $originalClass, string $traitName, - array $classAdviceNames, - bool $useParameterWidening + array $classAdviceNames ) { - $this->adviceNames = $classAdviceNames; - $this->useParameterWidening = $useParameterWidening; + $this->adviceNames = $classAdviceNames; $dynamicMethodAdvices = $classAdviceNames[AspectContainer::METHOD_PREFIX] ?? []; $staticMethodAdvices = $classAdviceNames[AspectContainer::STATIC_METHOD_PREFIX] ?? []; @@ -244,11 +236,7 @@ protected function interceptMethods(ReflectionClass $originalClass, array $metho $reflectionMethod = $originalClass->getMethod($methodName); $methodBody = $this->getJoinpointInvocationBody($reflectionMethod, $originalClass); - $interceptedMethods[$methodName] = new InterceptedMethodGenerator( - $reflectionMethod, - $methodBody, - $this->useParameterWidening - ); + $interceptedMethods[$methodName] = new InterceptedMethodGenerator($reflectionMethod, $methodBody); } return $interceptedMethods; diff --git a/src/Proxy/EnumProxyGenerator.php b/src/Proxy/EnumProxyGenerator.php index 59f2267a..0c8c7f31 100644 --- a/src/Proxy/EnumProxyGenerator.php +++ b/src/Proxy/EnumProxyGenerator.php @@ -75,13 +75,11 @@ class EnumProxyGenerator extends ClassProxyGenerator * @param ReflectionClass $originalClass Original enum reflection (before transformation) * @param string $traitName FQCN of the generated trait (e.g. Ns\Foo__AopProxied) * @param string[][][] $classAdviceNames List of advices for enum - * @param bool $useParameterWidening Enables usage of parameter widening feature */ public function __construct( ReflectionClass $originalClass, string $traitName, - array $classAdviceNames, - bool $useParameterWidening + array $classAdviceNames ) { // Enums cannot be instantiated (no `new EnumClass()`) and cannot have properties, so // initialization and property-access join points must never be woven for enums. @@ -92,8 +90,7 @@ public function __construct( AspectContainer::STATIC_METHOD_PREFIX => true, ]); - $this->adviceNames = $classAdviceNames; - $this->useParameterWidening = $useParameterWidening; + $this->adviceNames = $classAdviceNames; $dynamicMethodAdvices = $classAdviceNames[AspectContainer::METHOD_PREFIX] ?? []; $staticMethodAdvices = $classAdviceNames[AspectContainer::STATIC_METHOD_PREFIX] ?? []; diff --git a/src/Proxy/FunctionProxyGenerator.php b/src/Proxy/FunctionProxyGenerator.php index 4dbb5824..34cf2bd6 100644 --- a/src/Proxy/FunctionProxyGenerator.php +++ b/src/Proxy/FunctionProxyGenerator.php @@ -43,16 +43,14 @@ class FunctionProxyGenerator /** * Constructs functions stub class from namespace Reflection * - * @param ReflectionFileNamespace $namespace Reflection of namespace - * @param string[][][] $adviceNames List of function advices - * @param bool $useParameterWidening Enables usage of parameter widening feature + * @param ReflectionFileNamespace $namespace Reflection of namespace + * @param string[][][] $adviceNames List of function advices * * @throws ReflectionException If there is an advice for unknown function */ public function __construct( ReflectionFileNamespace $namespace, - array $adviceNames = [], - bool $useParameterWidening = false + array $adviceNames = [] ) { $this->adviceNames = $adviceNames; $this->fileGenerator = new FileGenerator(); @@ -65,7 +63,7 @@ public function __construct( foreach (array_keys($functionAdvices) as $functionName) { $functionReflection = new ReflectionFunction($functionName); $functionBody = $this->getJoinpointInvocationBody($functionReflection); - $funcGenerator = FunctionGenerator::fromReflection($functionReflection, $useParameterWidening); + $funcGenerator = FunctionGenerator::fromReflection($functionReflection); $funcGenerator->setBody($functionBody); $functionsContent[] = $funcGenerator->generate(); } diff --git a/src/Proxy/Generator/ClassGenerator.php b/src/Proxy/Generator/ClassGenerator.php index bd795c20..61702992 100644 --- a/src/Proxy/Generator/ClassGenerator.php +++ b/src/Proxy/Generator/ClassGenerator.php @@ -34,24 +34,13 @@ */ final class ClassGenerator implements GeneratorInterface { - public const FLAG_FINAL = 0b001; - public const FLAG_ABSTRACT = 0b010; - public const FLAG_READONLY = 0b100; + public const int FLAG_FINAL = 0b001; + public const int FLAG_ABSTRACT = 0b010; + public const int FLAG_READONLY = 0b100; private static ?Standard $printer = null; private static ?BuilderFactory $factory = null; - private string $name; - private ?string $namespace; - private ?int $flags; - private ?string $parentClass; - - /** @var string[] */ - private array $interfaces; - - /** @var PropertyNodeProvider[] */ - private array $properties; - /** @var MethodGenerator[] */ private array $methods; @@ -74,22 +63,21 @@ final class ClassGenerator implements GeneratorInterface * @param PropertyNodeProvider[] $properties * @param MethodGenerator[] $methods */ + /** + * @param string[] $interfaces + * @param PropertyNodeProvider[] $properties + * @param MethodGenerator[] $methods + */ public function __construct( - string $name, - ?string $namespace, - ?int $flags, - ?string $parentClass, - array $interfaces = [], - array $properties = [], + private readonly string $name, + private readonly ?string $namespace, + private readonly ?int $flags, + private readonly ?string $parentClass, + private readonly array $interfaces = [], + private readonly array $properties = [], array $methods = [], ) { - $this->name = $name; - $this->namespace = $namespace; - $this->flags = $flags; - $this->parentClass = $parentClass; - $this->interfaces = $interfaces; - $this->properties = $properties; - $this->methods = array_values($methods); + $this->methods = array_values($methods); } /** diff --git a/src/Proxy/Generator/DocBlockGenerator.php b/src/Proxy/Generator/DocBlockGenerator.php index d00554a4..2478d832 100644 --- a/src/Proxy/Generator/DocBlockGenerator.php +++ b/src/Proxy/Generator/DocBlockGenerator.php @@ -20,19 +20,16 @@ */ final class DocBlockGenerator { - private string $shortDescription; - private string $longDescription; - /** @var array tagName => list of tag content lines */ private array $tags = []; /** Holds a raw docblock string when constructed via fromDocComment() */ private ?string $rawDocComment = null; - public function __construct(string $shortDescription = '', string $longDescription = '') - { - $this->shortDescription = $shortDescription; - $this->longDescription = $longDescription; + public function __construct( + private readonly string $shortDescription = '', + private readonly string $longDescription = '' + ) { } /** diff --git a/src/Proxy/Generator/FunctionGenerator.php b/src/Proxy/Generator/FunctionGenerator.php index 39e58ee2..1873dcf5 100644 --- a/src/Proxy/Generator/FunctionGenerator.php +++ b/src/Proxy/Generator/FunctionGenerator.php @@ -35,7 +35,6 @@ final class FunctionGenerator private static ?Parser $parser = null; private static ?BuilderFactory $factory = null; - private string $name; private bool $returnsRef = false; private ?TypeGenerator $returnType = null; private ?DocBlockGenerator $docBlock = null; @@ -49,17 +48,14 @@ final class FunctionGenerator /** @var \PhpParser\Node\AttributeGroup[] */ private array $attributeGroups = []; - public function __construct(string $name) + public function __construct(private readonly string $name) { - $this->name = $name; } /** * Creates a FunctionGenerator from a reflection function. - * - * @param bool $useWidening When true, parameter types are omitted */ - public static function fromReflection(ReflectionFunction $function, bool $useWidening = false): self + public static function fromReflection(ReflectionFunction $function): self { $generator = new self($function->getShortName()); @@ -85,7 +81,7 @@ public static function fromReflection(ReflectionFunction $function, bool $useWid // Parameters foreach ($function->getParameters() as $reflectionParam) { - $generator->addParameter(ParameterGenerator::fromReflection($reflectionParam, $useWidening)); + $generator->addParameter(ParameterGenerator::fromReflection($reflectionParam)); } // Attributes: cloned from the AST when available (parser-reflection), so that diff --git a/src/Proxy/Generator/MethodGenerator.php b/src/Proxy/Generator/MethodGenerator.php index 930cb6c3..6e4248cc 100644 --- a/src/Proxy/Generator/MethodGenerator.php +++ b/src/Proxy/Generator/MethodGenerator.php @@ -33,15 +33,14 @@ */ final class MethodGenerator { - public const VISIBILITY_PUBLIC = 'public'; - public const VISIBILITY_PROTECTED = 'protected'; - public const VISIBILITY_PRIVATE = 'private'; + public const string VISIBILITY_PUBLIC = 'public'; + public const string VISIBILITY_PROTECTED = 'protected'; + public const string VISIBILITY_PRIVATE = 'private'; private static ?Standard $printer = null; private static ?Parser $parser = null; private static ?BuilderFactory $factory = null; - private string $name; private string $visibility = self::VISIBILITY_PUBLIC; private bool $static = false; private bool $final = false; @@ -60,17 +59,14 @@ final class MethodGenerator /** @var Stmt[]|null null for abstract methods */ private ?array $stmts = []; - public function __construct(string $name) + public function __construct(private readonly string $name) { - $this->name = $name; } /** * Creates a MethodGenerator from a reflection method. - * - * @param bool $useWidening When true, parameter types are omitted */ - public static function fromReflection(ReflectionMethod $method, bool $useWidening = false): self + public static function fromReflection(ReflectionMethod $method): self { $generator = new self($method->getName()); @@ -127,7 +123,7 @@ public static function fromReflection(ReflectionMethod $method, bool $useWidenin // Parameters foreach ($method->getParameters() as $reflectionParam) { - $generator->addParameter(ParameterGenerator::fromReflection($reflectionParam, $useWidening)); + $generator->addParameter(ParameterGenerator::fromReflection($reflectionParam)); } // Attributes: cloned from the AST when available (parser-reflection), so that diff --git a/src/Proxy/Generator/ParameterGenerator.php b/src/Proxy/Generator/ParameterGenerator.php index 1b6f79ca..af3581b8 100644 --- a/src/Proxy/Generator/ParameterGenerator.php +++ b/src/Proxy/Generator/ParameterGenerator.php @@ -27,40 +27,27 @@ final class ParameterGenerator private static ?Standard $printer = null; private static ?BuilderFactory $factory = null; - private string $name; - private ?TypeGenerator $type; - private bool $byRef; - private bool $variadic; - private ?ValueGenerator $defaultValue; - /** @var Node\AttributeGroup[] */ private array $attributeGroups = []; public function __construct( - string $name, - ?TypeGenerator $type = null, - bool $byRef = false, - bool $variadic = false, - ?ValueGenerator $defaultValue = null, + private readonly string $name, + private readonly ?TypeGenerator $type = null, + private readonly bool $byRef = false, + private readonly bool $variadic = false, + private ?ValueGenerator $defaultValue = null, ) { - $this->name = $name; - $this->type = $type; - $this->byRef = $byRef; - $this->variadic = $variadic; - $this->defaultValue = $defaultValue; } /** * Creates a ParameterGenerator from a reflection parameter. - * - * @param bool $useWidening When true, type declarations are omitted (for parameter widening) */ - public static function fromReflection(ReflectionParameter $param, bool $useWidening = false): self + public static function fromReflection(ReflectionParameter $param): self { $type = null; $defaultValue = null; - if (!$useWidening && $param->hasType()) { + if ($param->hasType()) { // If the parameter exposes its AST node (Go\ParserReflection\ReflectionParameter), // re-process the raw type node with TypeExpressionResolver(null, null) so that // 'self' and 'parent' keywords are preserved without PHP 8.5+ name resolution, diff --git a/src/Proxy/Generator/PropertyGenerator.php b/src/Proxy/Generator/PropertyGenerator.php index 2c0f1027..2e4c0bde 100644 --- a/src/Proxy/Generator/PropertyGenerator.php +++ b/src/Proxy/Generator/PropertyGenerator.php @@ -24,20 +24,18 @@ */ final class PropertyGenerator implements PropertyNodeProvider { - public const FLAG_PUBLIC = 0b0001; - public const FLAG_PROTECTED = 0b0010; - public const FLAG_PRIVATE = 0b0100; - public const FLAG_STATIC = 0b1000; - public const FLAG_READONLY = 0b0001_0000; - public const FLAG_PROTECTED_SET = 0b0010_0000; - public const FLAG_PRIVATE_SET = 0b0100_0000; - public const FLAG_FINAL = 0b1000_0000; + public const int FLAG_PUBLIC = 0b0001; + public const int FLAG_PROTECTED = 0b0010; + public const int FLAG_PRIVATE = 0b0100; + public const int FLAG_STATIC = 0b1000; + public const int FLAG_READONLY = 0b0001_0000; + public const int FLAG_PROTECTED_SET = 0b0010_0000; + public const int FLAG_PRIVATE_SET = 0b0100_0000; + public const int FLAG_FINAL = 0b1000_0000; private static ?Standard $printer = null; private static ?BuilderFactory $factory = null; - private string $name; - private int $flags; private mixed $defaultValue; private bool $hasDefault = false; @@ -52,10 +50,10 @@ final class PropertyGenerator implements PropertyNodeProvider /** @var list */ private array $hooks = []; - public function __construct(string $name, int $flags = self::FLAG_PUBLIC) - { - $this->name = $name; - $this->flags = $flags; + public function __construct( + private readonly string $name, + private readonly int $flags = self::FLAG_PUBLIC + ) { } public function setDefaultValue(mixed $defaultValue): void diff --git a/src/Proxy/Generator/TraitGenerator.php b/src/Proxy/Generator/TraitGenerator.php index b6fb2fde..bb8a6dea 100644 --- a/src/Proxy/Generator/TraitGenerator.php +++ b/src/Proxy/Generator/TraitGenerator.php @@ -36,17 +36,12 @@ final class TraitGenerator implements GeneratorInterface private static ?Standard $printer = null; private static ?BuilderFactory $factory = null; - private string $name; - private ?string $namespace; - /** @var MethodGenerator[] */ private array $methods; /** @var PropertyNode[] */ private array $properties; - private ?DocBlockGenerator $docBlock = null; - /** @var string[] used trait FQCNs */ private array $usedTraits = []; @@ -61,16 +56,13 @@ final class TraitGenerator implements GeneratorInterface * @param PropertyNode[] $properties */ public function __construct( - string $name, - ?string $namespace, + private readonly string $name, + private readonly ?string $namespace, array $methods = [], - ?DocBlockGenerator $docBlock = null, + private readonly ?DocBlockGenerator $docBlock = null, array $properties = [], ) { - $this->name = $name; - $this->namespace = $namespace; - $this->methods = array_values($methods); - $this->docBlock = $docBlock; + $this->methods = array_values($methods); $this->properties = array_values($properties); } diff --git a/src/Proxy/Generator/TypeGenerator.php b/src/Proxy/Generator/TypeGenerator.php index 72139f5e..6c95ea0f 100644 --- a/src/Proxy/Generator/TypeGenerator.php +++ b/src/Proxy/Generator/TypeGenerator.php @@ -38,7 +38,7 @@ final class TypeGenerator { /** @var list */ - private const BUILTIN_TYPES = [ + private const array BUILTIN_TYPES = [ 'int', 'float', 'string', 'bool', 'array', 'callable', 'object', 'iterable', 'void', 'null', 'never', 'mixed', 'false', 'true', 'self', 'static', 'parent', diff --git a/src/Proxy/Generator/ValueGenerator.php b/src/Proxy/Generator/ValueGenerator.php index 0352fc3f..517714b2 100644 --- a/src/Proxy/Generator/ValueGenerator.php +++ b/src/Proxy/Generator/ValueGenerator.php @@ -27,15 +27,13 @@ final class ValueGenerator { private static ?Standard $printer = null; - private mixed $value; private int $arrayDepth = 0; /** Pre-built AST expression node for defaults that can't be represented as PHP scalars. */ private ?Expr $astNode = null; - public function __construct(mixed $value) + public function __construct(private readonly mixed $value) { - $this->value = $value; } /** diff --git a/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php b/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php index 93b2cfc4..156c8f20 100644 --- a/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php +++ b/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php @@ -18,9 +18,11 @@ use Go\Proxy\Generator\TypeGenerator; use InvalidArgumentException; use PhpParser\Comment\Doc; +use PhpParser\Node\Expr\New_; use PhpParser\Node\Param; use PhpParser\Node\PropertyItem; use PhpParser\Node\Stmt\Property; +use PhpParser\NodeFinder; use ReflectionIntersectionType; use ReflectionNamedType; use ReflectionProperty; @@ -117,6 +119,13 @@ protected function hasPotentiallyUninitializedTypedProperty(): bool * * For promoted constructor properties the default lives on the Param node while * reflection's hasDefaultValue() reports false, so the AST node is authoritative. + * + * A default containing a `new` expression is never returned (issue #616): `new` is legal + * in a constructor parameter default but illegal in a property initializer, so copying it + * onto the proxy hook property would be a compile error ("New expressions are not + * supported in this context"). The hook property then stays uninitialized — the + * constructor assignment injected by the promoted-parameter demotion supplies the value, + * and the isInitialized() guard in the get hook covers the pre-construction window. */ private function getAstDefaultNode(): ?\PhpParser\Node\Expr { @@ -125,9 +134,16 @@ private function getAstDefaultNode(): ?\PhpParser\Node\Expr } $astNode = $this->property->getNode(); - return ($astNode instanceof PropertyItem || $astNode instanceof Param) + $default = ($astNode instanceof PropertyItem || $astNode instanceof Param) ? $astNode->default : null; + + if ($default !== null + && (new NodeFinder())->findFirstInstanceOf([$default], New_::class) !== null) { + return null; + } + + return $default; } protected function createFieldAccessDocComment(string $variableName = 'fieldAccess', bool $isNullable = false): Doc diff --git a/src/Proxy/Part/FunctionParameterList.php b/src/Proxy/Part/FunctionParameterList.php index 01055c73..e292cb94 100644 --- a/src/Proxy/Part/FunctionParameterList.php +++ b/src/Proxy/Part/FunctionParameterList.php @@ -28,13 +28,12 @@ final class FunctionParameterList /** * ParameterListGenerator constructor. * - * @param ReflectionFunctionAbstract $functionLike Instance of function or method - * @param bool $useTypeWidening Should generated parameters use type widening + * @param ReflectionFunctionAbstract $functionLike Instance of function or method */ - public function __construct(ReflectionFunctionAbstract $functionLike, bool $useTypeWidening = false) + public function __construct(ReflectionFunctionAbstract $functionLike) { foreach ($functionLike->getParameters() as $reflectionParameter) { - $this->generatedParameters[] = ParameterGenerator::fromReflection($reflectionParameter, $useTypeWidening); + $this->generatedParameters[] = ParameterGenerator::fromReflection($reflectionParameter); } } diff --git a/src/Proxy/Part/InterceptedConstructorGenerator.php b/src/Proxy/Part/InterceptedConstructorGenerator.php index a1a230a0..d1de841e 100644 --- a/src/Proxy/Part/InterceptedConstructorGenerator.php +++ b/src/Proxy/Part/InterceptedConstructorGenerator.php @@ -31,7 +31,6 @@ final class InterceptedConstructorGenerator * * @param ReflectionMethod|null $constructor Instance of original constructor or null * @param InterceptedMethodGenerator|null $constructorGenerator Constructor body generator (if present) - * @param bool $useTypeWidening Should generator use parameter widening for PHP>=7.2 * @param bool $constructorIsInTrait True when the original constructor is in the trait * (i.e. defined in the class itself, not inherited); * in that case the alias __aop____construct is used @@ -40,7 +39,6 @@ final class InterceptedConstructorGenerator public function __construct( ?ReflectionMethod $constructor = null, ?InterceptedMethodGenerator $constructorGenerator = null, - bool $useTypeWidening = false, bool $constructorIsInTrait = false ) { if ($constructor !== null) { @@ -52,7 +50,7 @@ public function __construct( } else { $constructorCallBody = 'parent::__construct(' . $splatPrefix . $callArguments->generate() . ');'; } - $generator = MethodGenerator::fromReflection($constructor, $useTypeWidening); + $generator = MethodGenerator::fromReflection($constructor); $generator->setBody($constructorCallBody); } else { $generator = $constructorGenerator->getGenerator(); diff --git a/src/Proxy/Part/InterceptedMethodGenerator.php b/src/Proxy/Part/InterceptedMethodGenerator.php index f2c3d88b..d33b8c4b 100644 --- a/src/Proxy/Part/InterceptedMethodGenerator.php +++ b/src/Proxy/Part/InterceptedMethodGenerator.php @@ -27,11 +27,10 @@ final class InterceptedMethodGenerator * * @param ReflectionMethod $reflectionMethod Instance of original method * @param string $body Method body - * @param bool $useTypeWidening Should generator use parameter widening for PHP>=7.2 */ - public function __construct(ReflectionMethod $reflectionMethod, string $body, bool $useTypeWidening = false) + public function __construct(ReflectionMethod $reflectionMethod, string $body) { - $this->generator = MethodGenerator::fromReflection($reflectionMethod, $useTypeWidening); + $this->generator = MethodGenerator::fromReflection($reflectionMethod); $this->generator->setBody($body); } diff --git a/src/Proxy/TraitProxyGenerator.php b/src/Proxy/TraitProxyGenerator.php index 697964f8..cb4c5f5e 100644 --- a/src/Proxy/TraitProxyGenerator.php +++ b/src/Proxy/TraitProxyGenerator.php @@ -40,11 +40,9 @@ class TraitProxyGenerator extends ClassProxyGenerator public function __construct( ReflectionClass $originalTrait, string $parentTraitName, - array $traitAdviceNames, - bool $useParameterWidening + array $traitAdviceNames ) { - $this->adviceNames = $traitAdviceNames; - $this->useParameterWidening = $useParameterWidening; + $this->adviceNames = $traitAdviceNames; $dynamicMethodAdvices = $traitAdviceNames[AspectContainer::METHOD_PREFIX] ?? []; $staticMethodAdvices = $traitAdviceNames[AspectContainer::STATIC_METHOD_PREFIX] ?? []; @@ -163,9 +161,6 @@ protected function getJoinpointInvocationBody(ReflectionMethod $method, ?Reflect BODY; } - /** - * {@inheritDoc} - */ public function addUse(string $use, ?string $useAlias = null): void { if ($use !== '' && $this->generator instanceof TraitGenerator) { @@ -173,9 +168,6 @@ public function addUse(string $use, ?string $useAlias = null): void } } - /** - * {@inheritDoc} - */ public function generate(): string { return $this->generator->generate(); diff --git a/tests/Fixtures/project/src/Application/NewInInitializerClass.php b/tests/Fixtures/project/src/Application/NewInInitializerClass.php new file mode 100644 index 00000000..723dc3cc --- /dev/null +++ b/tests/Fixtures/project/src/Application/NewInInitializerClass.php @@ -0,0 +1,25 @@ +bag->getArrayCopy(); + } +} diff --git a/tests/Fixtures/project/src/Aspect/PromotedPropertyInterceptAspect.php b/tests/Fixtures/project/src/Aspect/PromotedPropertyInterceptAspect.php index a603c085..b0fda5f1 100644 --- a/tests/Fixtures/project/src/Aspect/PromotedPropertyInterceptAspect.php +++ b/tests/Fixtures/project/src/Aspect/PromotedPropertyInterceptAspect.php @@ -23,4 +23,10 @@ public function beforePromotedTagAccess(FieldAccess $access): void { // No-op: registration is asserted by functional tests } + + #[Pointcut\Before("access(private Go\Tests\TestProject\Application\NewInInitializerClass->bag)")] + public function beforeNewInInitializerBagAccess(FieldAccess $access): void + { + // No-op: registration is asserted by functional tests + } } diff --git a/tests/Functional/ClassWeavingTest.php b/tests/Functional/ClassWeavingTest.php index 85efa9c6..fade09f8 100644 --- a/tests/Functional/ClassWeavingTest.php +++ b/tests/Functional/ClassWeavingTest.php @@ -18,8 +18,11 @@ use Go\Tests\TestProject\Application\FinalClass; use Go\Tests\TestProject\Application\FooInterface; use Go\Tests\TestProject\Application\Main; +use Go\Tests\TestProject\Application\NewInInitializerClass; use Go\Tests\TestProject\Application\PromotedPropertyClass; use Go\Tests\TestProject\Application\SingleLinePromotedClass; +use Symfony\Component\Process\PhpExecutableFinder; +use Symfony\Component\Process\Process; class ClassWeavingTest extends BaseFunctionalTestCase { @@ -108,6 +111,41 @@ public function testPromotedPropertyWeaving(): void ); } + /** + * An intercepted promoted property whose default is a new-in-initializer expression + * must weave into loadable code (issue #616): the proxy hook property must not carry + * the `new` default (illegal in property initializers). The runtime subprocess loads + * the woven class, instantiates it without arguments and reads the property, proving + * that the constructor default still materializes through the injected assignment. + */ + public function testNewInInitializerPromotedPropertyWeaving(): void + { + $this->assertPropertyWoven( + NewInInitializerClass::class, + 'bag', + 'Go\\Tests\\TestProject\\Aspect\\PromotedPropertyInterceptAspect->beforeNewInInitializerBagAccess' + ); + + $phpExecutable = (new PhpExecutableFinder())->find(); + $script = sprintf( + 'include %s; $instance = new %s(); echo implode(",", $instance->getBagItems());', + var_export($this->configuration['frontController'], true), + '\\' . NewInInitializerClass::class + ); + $process = new Process( + [$phpExecutable, '-r', $script], + null, + ['GO_AOP_CONFIGURATION' => $this->getConfigurationName()] + ); + $process->run(); + + $this->assertTrue( + $process->isSuccessful(), + 'Loading the woven class failed: ' . $process->getOutput() . $process->getErrorOutput() + ); + $this->assertSame('seed', trim($process->getOutput())); + } + public function testArrayPropertyInterceptionAllowsIndirectModification(): void { $this->assertPropertyWoven( diff --git a/tests/Instrument/Transformer/Php85AuditScratchTest.php b/tests/Instrument/Transformer/Php85AuditScratchTest.php index fbdc1106..41c6125b 100644 --- a/tests/Instrument/Transformer/Php85AuditScratchTest.php +++ b/tests/Instrument/Transformer/Php85AuditScratchTest.php @@ -118,18 +118,11 @@ public static function fixtureNames(): array * A fix PR that resolves one of these MUST remove the entry (the test then asserts success). */ private const KNOWN_GAPS = [ - // #598-#603 are all fixed on master. Remaining follow-ups (#615/#616, fixed by PR #617): - // #[\Attribute] on a trait only became a compile error in PHP 8.5, - // so these three are gaps on 8.5+ but weave cleanly on 8.4 - 'ConstAttr' => 'https://github.com/goaop/framework/issues/615', - 'ExprAttr' => 'https://github.com/goaop/framework/issues/615', - 'RichAttr' => 'https://github.com/goaop/framework/issues/615', - // new-in-initializer default copied onto the proxy hook property - 'Php81NewInInitializers' => 'https://github.com/goaop/framework/issues/616', + // All audit gaps (#598-#603, #615, #616) are fixed — every fixture must weave cleanly. ]; - /** Fixtures whose KNOWN_GAPS entry applies only on PHP >= 8.5 (see above). */ - private const GAP_ONLY_ON_85 = ['ConstAttr' => true, 'ExprAttr' => true, 'RichAttr' => true]; + /** Fixtures whose KNOWN_GAPS entry applies only on PHP >= 8.5. */ + private const GAP_ONLY_ON_85 = []; #[DataProvider('fixtureNames')] public function testWeaveAndLint(string $name): void diff --git a/tests/Instrument/Transformer/WeavingTransformerTest.php b/tests/Instrument/Transformer/WeavingTransformerTest.php index 13de7881..cba7537f 100644 --- a/tests/Instrument/Transformer/WeavingTransformerTest.php +++ b/tests/Instrument/Transformer/WeavingTransformerTest.php @@ -457,6 +457,90 @@ public function testWeaverKeepsClassLevelAttributesOnWovenTrait(): void $this->assertEquals($expected, $actual); } + /** + * Golden-file coverage of general PHP 8.0-8.3 syntax through the current weaver + * (issue #610): constructor promotion (non-intercepted property), new-in-initializer + * parameter default, named arguments, match expression, nullsafe operator, enum usage + * in a method body, readonly property, first-class callable and a typed class constant. + * Only the class is woven — the enum in the same file must stay untouched. + */ + public function testWeaverForPhp80To82Syntax(): void + { + $adviceMatcher = $this->createMock(AdviceMatcherInterface::class); + $adviceMatcher + ->method('getAdvicesForClass') + ->willReturnCallback(function (ReflectionClass $refClass) { + // Weave only the target class — the enum stays untouched + if ($refClass->getShortName() !== 'TestPhp80To82SyntaxClass') { + return []; + } + $advices = []; + foreach ($refClass->getMethods() as $method) { + $advisorId = "advisor.{$refClass->name}->{$method->name}"; + $advices[AspectContainer::METHOD_PREFIX][$method->name][$advisorId] = true; + } + return $advices; + }); + $adviceMatcher + ->method('getAdvicesForFunctions') + ->willReturn([]); + + $loader = $this + ->getMockBuilder(AspectLoader::class) + ->setConstructorArgs([$this->getContainerMock()]) + ->getMock(); + $transformer = new WeavingTransformer( + $this->kernel, + $adviceMatcher, + $this->cachePathManager, + $loader + ); + + $metadata = $this->loadTestMetadata('php80-82-syntax'); + $transformer->transform($metadata); + + $actual = $this->normalizeWhitespaces($metadata->source); + $expected = $this->normalizeWhitespaces($this->loadTestMetadata('php80-82-syntax-woven')->source); + $this->assertEquals($expected, $actual); + + $matches = []; + $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $actual, $matches)); + $actualProxyContent = $this->normalizeWhitespaces((string) file_get_contents('vfs://' . $matches[1])); + $expectedProxyContent = $this->normalizeWhitespaces($this->loadTestMetadata('php80-82-syntax-proxy')->source); + $this->assertEquals($expectedProxyContent, $actualProxyContent); + } + + /** + * Attribute classes must be weavable (issue #615): #[\Attribute] and + * #[\AllowDynamicProperties] are compile-time invalid on traits, so they must be removed + * from the woven trait tokens. In a grouped attribute only the incompatible entry is + * removed. The proxy class must keep the original attribute groups (copied from the AST). + */ + public function testWeaverStripsAttributeClassMarkersFromWovenTrait(): void + { + $metadata = $this->loadTestMetadata('php80-attribute-class'); + $this->transformer->transform($metadata); + + $actual = $this->normalizeWhitespaces($metadata->source); + $expected = $this->normalizeWhitespaces($this->loadTestMetadata('php80-attribute-class-woven')->source); + $this->assertEquals($expected, $actual); + + // Incompatible attributes must be gone from every woven trait + $this->assertStringNotContainsString('#[\Attribute', $actual); + $this->assertStringNotContainsString('\AllowDynamicProperties', $actual); + // The compatible part of the grouped attribute must survive + $this->assertStringContainsString('#[\FakeMarkerAttr]', $actual); + + // The proxy (last class in the file wins the shared cache path) must keep #[\Attribute(...)] + $matches = []; + $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $actual, $matches)); + $proxyContent = (string) file_get_contents('vfs://' . $matches[1]); + $this->assertStringContainsString( + '#[\Attribute(\Attribute::TARGET_CLASS | \Attribute::IS_REPEATABLE)]', + $proxyContent + ); + } + public function testWeaverMovesInterceptedPropertiesToProxyHooks(): void { $adviceMatcher = $this->createMock(AdviceMatcherInterface::class); @@ -546,6 +630,52 @@ public function testWeaverDemotesInterceptedPromotedProperties(): void $this->assertStringContainsString("private string \$name = 'initial' {", $actualProxyContent); } + /** + * An intercepted promoted property whose default is a new-in-initializer expression + * must not carry the default onto the proxy hook property (issue #616): `new` is legal + * in a parameter default but illegal in a property initializer. The property stays + * uninitialized in the proxy — the constructor assignment injected by the demotion + * supplies the value, and the isInitialized() guard covers the pre-construction window. + */ + public function testWeaverSkipsNewInInitializerDefaultOnProxyHookProperty(): void + { + $transformer = $this->createTransformerWithAdvices([ + AspectContainer::PROPERTY_PREFIX => [ + 'bag' => ['advisor.Go\Tests\TestProject\Application\NewInInitializerClass->bag' => true], + ], + AspectContainer::METHOD_PREFIX => [ + '__construct' => ['advisor.Go\Tests\TestProject\Application\NewInInitializerClass->__construct' => true], + 'getBagItems' => ['advisor.Go\Tests\TestProject\Application\NewInInitializerClass->getBagItems' => true], + ], + ]); + + $metadata = $this->loadTestMetadata('php81-new-in-initializer'); + $transformer->transform($metadata); + + $actual = $this->normalizeWhitespaces($metadata->source); + + // Demoted parameter keeps its new-in-initializer default in the woven trait... + $this->assertStringContainsString("\ArrayObject \$bag = new \ArrayObject(['seed'])", $actual); + // ...and the injected constructor assignment routes the value through the proxy set hook + $this->assertStringContainsString('$this->bag = $bag;', $actual); + + $matches = []; + $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $actual, $matches)); + $proxyContent = (string) file_get_contents('vfs://' . $matches[1]); + + // The hook property must NOT carry the new-in-initializer default (compile error); + // note the proxy __construct parameter legitimately keeps it (legal in param defaults) + $this->assertStringNotContainsString('private \ArrayObject $bag =', $proxyContent); + $this->assertStringContainsString('private \ArrayObject $bag {', $proxyContent); + // Uninitialized typed property must be guarded in the hooks + $this->assertStringContainsString('isInitialized($this)', $proxyContent); + + // The generated proxy must stay parseable as PHP (guards against emitting + // constructs that are syntactically invalid in property context) + $parser = (new \PhpParser\ParserFactory())->createForHostVersion(); + $this->assertNotNull($parser->parse($proxyContent)); + } + /** * A promoted property inside a single-line constructor must weave without a parse error * (issue #599). Commenting the parameter out used to swallow the closing ')' and '{'. diff --git a/tests/Instrument/Transformer/_files/php80-82-syntax-proxy.php b/tests/Instrument/Transformer/_files/php80-82-syntax-proxy.php new file mode 100644 index 00000000..43e5cb58 --- /dev/null +++ b/tests/Instrument/Transformer/_files/php80-82-syntax-proxy.php @@ -0,0 +1,31 @@ + $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod(self::class, '__construct', ['advisor.Test\ns1\TestPhp80To82SyntaxClass->__construct'], $this->__aop____construct(...)); + return $__joinPoint->__invoke($this, \array_slice([$label, $items], 0, \func_num_args())); + } + public function describe(?\ArrayObject $extra = null): string + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod(self::class, 'describe', ['advisor.Test\ns1\TestPhp80To82SyntaxClass->describe'], $this->__aop__describe(...)); + return $__joinPoint->__invoke($this, \array_slice([$extra], 0, \func_num_args())); + } +} \ No newline at end of file diff --git a/tests/Instrument/Transformer/_files/php80-82-syntax-woven.php b/tests/Instrument/Transformer/_files/php80-82-syntax-woven.php new file mode 100644 index 00000000..699b4a21 --- /dev/null +++ b/tests/Instrument/Transformer/_files/php80-82-syntax-woven.php @@ -0,0 +1,42 @@ +ratio = \round(num: 0.5, precision: 1); + } + + public function describe(?\ArrayObject $extra = null): string + { + $lengthOf = \strlen(...); + $count = $extra?->count() ?? $this->items->count(); + + return match (true) { + $count >= self::LIMIT => 'huge:' . $lengthOf($this->label), + $count >= SyntaxPriority::High->value => 'several', + default => 'few', + }; + } +} +include_once AOP_CACHE_DIR . '/Transformer/_files/php80-82-syntax.php'; diff --git a/tests/Instrument/Transformer/_files/php80-82-syntax.php b/tests/Instrument/Transformer/_files/php80-82-syntax.php new file mode 100644 index 00000000..31f85ad4 --- /dev/null +++ b/tests/Instrument/Transformer/_files/php80-82-syntax.php @@ -0,0 +1,41 @@ +ratio = \round(num: 0.5, precision: 1); + } + + public function describe(?\ArrayObject $extra = null): string + { + $lengthOf = \strlen(...); + $count = $extra?->count() ?? $this->items->count(); + + return match (true) { + $count >= self::LIMIT => 'huge:' . $lengthOf($this->label), + $count >= SyntaxPriority::High->value => 'several', + default => 'few', + }; + } +} diff --git a/tests/Instrument/Transformer/_files/php80-attribute-class-woven.php b/tests/Instrument/Transformer/_files/php80-attribute-class-woven.php new file mode 100644 index 00000000..8e237ed4 --- /dev/null +++ b/tests/Instrument/Transformer/_files/php80-attribute-class-woven.php @@ -0,0 +1,44 @@ +reason; + } +} +include_once AOP_CACHE_DIR . '/Transformer/_files/php80-attribute-class.php'; diff --git a/tests/Instrument/Transformer/_files/php80-attribute-class.php b/tests/Instrument/Transformer/_files/php80-attribute-class.php new file mode 100644 index 00000000..af55c432 --- /dev/null +++ b/tests/Instrument/Transformer/_files/php80-attribute-class.php @@ -0,0 +1,41 @@ +reason; + } +} diff --git a/tests/Instrument/Transformer/_files/php80-class-attribute-woven.php b/tests/Instrument/Transformer/_files/php80-class-attribute-woven.php index 097134b1..fa37dfd7 100644 --- a/tests/Instrument/Transformer/_files/php80-class-attribute-woven.php +++ b/tests/Instrument/Transformer/_files/php80-class-attribute-woven.php @@ -5,7 +5,9 @@ /** * PHP 8.0 — classes with class-level attributes (issue #598). * WeavingTransformer must skip the attribute groups when converting the class - * to a trait: attributes are legal on traits and must be kept untouched. + * to a trait: attributes are legal on traits and must be kept untouched — + * except #[\Attribute]/#[\AllowDynamicProperties], which are compile-time + * invalid on traits and are removed from the woven trait (issue #615). */ #[\FakeMarkerAttr] trait TestClassWithPlainAttribute__AopProxied @@ -17,7 +19,7 @@ public function doSomething(): int } include_once AOP_CACHE_DIR . '/Transformer/_files/php80-class-attribute.php'; -#[\Attribute(\Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD)] + #[\FakeMarkerAttr] trait TestClassWithArgumentAttribute__AopProxied { diff --git a/tests/Instrument/Transformer/_files/php80-class-attribute.php b/tests/Instrument/Transformer/_files/php80-class-attribute.php index bdcc9dfa..c685c9a9 100644 --- a/tests/Instrument/Transformer/_files/php80-class-attribute.php +++ b/tests/Instrument/Transformer/_files/php80-class-attribute.php @@ -5,7 +5,9 @@ /** * PHP 8.0 — classes with class-level attributes (issue #598). * WeavingTransformer must skip the attribute groups when converting the class - * to a trait: attributes are legal on traits and must be kept untouched. + * to a trait: attributes are legal on traits and must be kept untouched — + * except #[\Attribute]/#[\AllowDynamicProperties], which are compile-time + * invalid on traits and are removed from the woven trait (issue #615). */ #[\FakeMarkerAttr] class TestClassWithPlainAttribute diff --git a/tests/Instrument/Transformer/_files/php81-new-in-initializer.php b/tests/Instrument/Transformer/_files/php81-new-in-initializer.php new file mode 100644 index 00000000..723dc3cc --- /dev/null +++ b/tests/Instrument/Transformer/_files/php81-new-in-initializer.php @@ -0,0 +1,25 @@ +bag->getArrayCopy(); + } +} diff --git a/tests/Proxy/Generator/FunctionGeneratorTest.php b/tests/Proxy/Generator/FunctionGeneratorTest.php index 6a99a47d..32437ee2 100644 --- a/tests/Proxy/Generator/FunctionGeneratorTest.php +++ b/tests/Proxy/Generator/FunctionGeneratorTest.php @@ -131,17 +131,6 @@ public function testSetReturnsReference(): void $this->assertStringContainsString('function &funcGenHelper_simple', $output); } - public function testWideningMode(): void - { - $gen = FunctionGenerator::fromReflection( - new ReflectionFunction(self::STUBS_NS . '\funcGenHelper_simple'), - true - ); - $output = $gen->generate(); - $this->assertStringNotContainsString('string $name', $output); - $this->assertStringContainsString('$name', $output); - } - public function testSetBodyEmptyString(): void { $gen = FunctionGenerator::fromReflection(new ReflectionFunction(self::STUBS_NS . '\funcGenHelper_simple')); diff --git a/tests/Proxy/Generator/MethodGeneratorTest.php b/tests/Proxy/Generator/MethodGeneratorTest.php index 8ec2ba75..1b59eec7 100644 --- a/tests/Proxy/Generator/MethodGeneratorTest.php +++ b/tests/Proxy/Generator/MethodGeneratorTest.php @@ -161,15 +161,6 @@ public function testAddParameter(): void $this->assertStringContainsString('bool $extra', $output); } - public function testWideningMode(): void - { - $gen = MethodGenerator::fromReflection($this->getMethod('publicMethod'), true); - $output = $gen->generate(); - // With widening, parameter types are dropped - $this->assertStringNotContainsString('string $name', $output); - $this->assertStringContainsString('$name', $output); - } - public function testSetAbstract(): void { $gen = MethodGenerator::fromReflection($this->getMethod('publicMethod')); diff --git a/tests/Proxy/Generator/ParameterGeneratorTest.php b/tests/Proxy/Generator/ParameterGeneratorTest.php index c35b14c4..3c8b86c0 100644 --- a/tests/Proxy/Generator/ParameterGeneratorTest.php +++ b/tests/Proxy/Generator/ParameterGeneratorTest.php @@ -114,18 +114,6 @@ public function testSetDefaultValue(): void $this->assertSame("string \$myParam = 'hello'", $output); } - public function testWideningModeDropsTypeForBuiltin(): void - { - // When useWidening=true, builtin-typed params lose their type constraint - $gen = ParameterGenerator::fromReflection( - $this->getParam(self::STUBS_NS . '\paramGenHelper_simple', 0), - true - ); - $output = $gen->generate(); - // With widening, the type should be removed - $this->assertSame('$name', $output); - } - public function testFromReflectionPreservesParameterAttribute(): void { $gen = ParameterGenerator::fromReflection( diff --git a/tests/Proxy/Part/InterceptedConstructorGeneratorTest.php b/tests/Proxy/Part/InterceptedConstructorGeneratorTest.php index 6d39d170..9896905f 100644 --- a/tests/Proxy/Part/InterceptedConstructorGeneratorTest.php +++ b/tests/Proxy/Part/InterceptedConstructorGeneratorTest.php @@ -92,7 +92,6 @@ public function testGenerateWithConstructorInTrait(): void $generator = new InterceptedConstructorGenerator( $reflectionConstructor, null, - false, true // $constructorIsInTrait );