From a12c3ae7758db306897b9f0ac3d2dcade59e8c78 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 2 Nov 2026 18:17:00 +0000 Subject: [PATCH 1/5] Extract attribute service registration into AttributeServicesRegistrar AttributeServicesRegistrar owns turning attribute targets into service definitions, including the Conditional handling. AttributeTargetsProvider is the single view over vendor/attributes.php and the targets that will be discovered in attributeServicesDirectories. No behavior change. Amp-Thread-ID: https://ampcode.com/threads/T-01a19cd4-be31-75b4-bd15-67786eff5983 --- phpstan-baseline.neon | 4 +- .../AttributeServicesRegistrar.php | 322 ++++++++++++++++++ .../AttributeTargetsProvider.php | 93 +++++ .../DiscoveredAttributeTargets.php | 31 ++ .../AutowiredAttributeServicesExtension.php | 297 +--------------- .../StubValidatorRuleServicesExtension.php | 23 +- .../AttributeServicesRegistrarTest.php} | 8 +- 7 files changed, 465 insertions(+), 313 deletions(-) create mode 100644 src/DependencyInjection/AttributeServices/AttributeServicesRegistrar.php create mode 100644 src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php create mode 100644 src/DependencyInjection/AttributeServices/DiscoveredAttributeTargets.php rename tests/PHPStan/DependencyInjection/{AutowiredAttributeServicesExtensionTest.php => AttributeServices/AttributeServicesRegistrarTest.php} (82%) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 1c46e0a5187..311f356272b 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -239,13 +239,13 @@ parameters: rawMessage: 'Call to static method escape() of internal class Nette\DI\Helpers from outside its root namespace Nette.' identifier: staticMethod.internalClass count: 1 - path: src/DependencyInjection/AutowiredAttributeServicesExtension.php + path: src/DependencyInjection/AttributeServices/AttributeServicesRegistrar.php - rawMessage: 'Call to static method expand() of internal class Nette\DI\Helpers from outside its root namespace Nette.' identifier: staticMethod.internalClass count: 1 - path: src/DependencyInjection/AutowiredAttributeServicesExtension.php + path: src/DependencyInjection/AttributeServices/AttributeServicesRegistrar.php - rawMessage: 'Call to static method expand() of internal class Nette\DI\Helpers from outside its root namespace Nette.' diff --git a/src/DependencyInjection/AttributeServices/AttributeServicesRegistrar.php b/src/DependencyInjection/AttributeServices/AttributeServicesRegistrar.php new file mode 100644 index 00000000000..a94d27433a7 --- /dev/null +++ b/src/DependencyInjection/AttributeServices/AttributeServicesRegistrar.php @@ -0,0 +1,322 @@ +findTargetClasses(Conditional::class) as $class) { + $conditionalTargets[] = [ + new TargetClass($class->attribute->attribute, $class->name), + self::isConditionMet($builder, $class->attribute->if), + ]; + } + + foreach (self::findTargetClasses($targets, AutowiredService::class, $conditionalTargets) as [$class, $tagged]) { + $reflection = new ReflectionClass($class->name); + $attribute = $class->attribute; + + $definition = $builder->addDefinition($attribute->name) + ->setType($class->name) + ->setAutowired($attribute->as); + + if ($attribute->factory !== null) { + [$ref, $method] = explode('::', $attribute->factory); + $definition->setFactory(new Statement([new Reference(substr($ref, 1)), $method])); + } + + self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); + + if (!$attribute->autoTag || !$tagged) { + continue; + } + + foreach (ValidateServiceTagsExtension::getInterfaceTagMapping() as $interface => $tag) { + if (!$reflection->implementsInterface($interface)) { + continue; + } + + $definition->addTag($tag); + } + } + + foreach ($targets->findTargetClasses(NonAutowiredService::class) as $class) { + $attribute = $class->attribute; + + $definition = $builder->addDefinition($attribute->name) + ->setType($class->name) + ->setAutowired(false); + + if ($attribute->factory !== null) { + [$ref, $method] = explode('::', $attribute->factory); + $definition->setFactory(new Statement([new Reference(substr($ref, 1)), $method])); + } + + self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); + } + + foreach ($targets->findTargetClasses(GenerateFactory::class) as $class) { + $attribute = $class->attribute; + $definition = $builder->addFactoryDefinition(null) + ->setImplement($attribute->interface); + + if ($attribute->resultType !== null) { + $definition->getResultDefinition()->setType($attribute->resultType); + } + + $resultDefinition = $definition->getResultDefinition(); + self::processConstructorParameters($builder, $class->name, $resultDefinition, $constructorParameters); + } + + if ($level === null) { + return; + } + + foreach (self::findTargetClasses($targets, RegisteredRule::class, $conditionalTargets) as [$class, $tagged]) { + $attribute = $class->attribute; + if ($attribute->level > $level) { + continue; + } + + $definition = $builder->addDefinition(null) + ->setFactory($class->name) + ->setAutowired($class->name); + if ($tagged) { + $definition->addTag(LazyRegistry::RULE_TAG); + } + + self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); + } + + foreach (self::findTargetClasses($targets, RegisteredCollector::class, $conditionalTargets) as [$class, $tagged]) { + $attribute = $class->attribute; + if ($attribute->level > $level) { + continue; + } + + $definition = $builder->addDefinition(null) + ->setFactory($class->name) + ->setAutowired($class->name); + if ($tagged) { + $definition->addTag(RegistryFactory::COLLECTOR_TAG); + } + + self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); + } + } + + /** + * @return array>> + */ + public static function collectConstructorParameters(AttributeTargetsProvider $targets): array + { + $constructorParameters = []; + foreach ($targets->findTargetMethodParameters(AutowiredParameter::class) as $parameter) { + if (strcasecmp($parameter->method, '__construct') !== 0) { + continue; + } + $lowerClass = strtolower($parameter->class); + $constructorParameters[$lowerClass] ??= []; + $constructorParameters[$lowerClass][] = $parameter; + } + + return $constructorParameters; + } + + /** + * @template T of object + * @param class-string $attributeClass + * @param list, bool}> $conditionalTargets attributes wrapped in #[Conditional] and whether the condition is met + * @return list, bool}> targets and whether they get their tags + */ + private static function findTargetClasses(AttributeTargetsProvider $targetsProvider, string $attributeClass, array $conditionalTargets): array + { + $targets = []; + foreach ($targetsProvider->findTargetClasses($attributeClass) as $target) { + $targets[] = [$target, true]; + } + foreach ($conditionalTargets as [$target, $isConditionMet]) { + if (!$target->attribute instanceof $attributeClass) { + continue; + } + + $targets[] = [new TargetClass($target->attribute, $target->name), $isConditionMet]; + } + + return $targets; + } + + /** + * The strings are NEON values like in the conditionalTags config section, so 'not(%foo%)' is supported too. + * + * @param string|non-empty-list $if + */ + public static function isConditionMet(ContainerBuilder $builder, string|array $if): bool + { + $condition = []; + foreach ((array) $if as $item) { + $value = Neon::decode($item); + if ($value instanceof Entity) { + $value = new Statement($value->value, $value->attributes); + } + + $condition[] = Helpers::expand($value, $builder->parameters, true); + } + + return ConditionalTagsExtension::isConditionMet($condition); + } + + /** + * @param class-string $className + * @param array>> $constructorParameters + */ + public static function processConstructorParameters(ContainerBuilder $builder, string $className, ServiceDefinition $definition, array $constructorParameters): void + { + foreach ($constructorParameters[strtolower($className)] ?? [] as $autowiredParameter) { + $ref = $autowiredParameter->attribute->ref; + if ($ref === null) { + $argument = self::createDeferredParameter($builder, '%' . Helpers::escape($autowiredParameter->name) . '%'); + } elseif (Strings::match($ref, '#^@[\w\\\\]+$#D') !== null) { + $argument = new Reference(substr($ref, 1)); + } else { + $argument = self::createDeferredParameter($builder, $ref); + } + $definition->setArgument($autowiredParameter->name, $argument); + } + } + + /** + * Turns a `%foo%` reference into a deferred `$this->getParameter('foo')` lookup instead of reading + * ContainerBuilder::$parameters right now. Extensions registered after this one still rewrite the + * parameters during loadConfiguration() - ValidateExcludePathsExtension unwraps OptionalPath objects + * in `excludePaths` - and a value snapshotted here would keep the pre-rewrite contents. + * + * `%foo.bar%` becomes `$this->getParameter('foo')['bar']` and `%foo%/suffix` becomes a concatenation, + * mirroring how Nette itself compiles references to dynamic parameters. + * + * @return PhpLiteral|string + * @throws ShouldNotHappenException when the reference points at a parameter that does not exist + */ + private static function createDeferredParameter(ContainerBuilder $builder, string $ref) + { + $parts = preg_split('#%([\w.-]*)%#', $ref, flags: PREG_SPLIT_DELIM_CAPTURE); + if ($parts === false) { + throw new ShouldNotHappenException(); + } + + $dumper = new Dumper(); + $lookups = []; + $pieces = []; + $withoutReferences = ''; + foreach ($parts as $i => $part) { + if ($i % 2 === 0) { + if ($part !== '') { + $pieces[] = $dumper->dump($part); + $withoutReferences .= $part; + } + continue; + } + + if ($part === '') { + // '%%' is an escaped percent sign + $pieces[] = $dumper->dump('%'); + $withoutReferences .= '%'; + continue; + } + + $keys = explode('.', $part); + self::checkParameterExists($builder, $part, $keys); + + $code = $dumper->format('$this->getParameter(?)', $keys[0]); + foreach (array_slice($keys, 1) as $key) { + $code .= sprintf('[%s]', $dumper->dump($key)); + } + + $lookups[] = $code; + $pieces[] = sprintf('(%s)', $code); + } + + if (count($lookups) === 0) { + return $withoutReferences; + } + + if (count($pieces) === 1) { + // the reference is the whole value, no string coercion + return ContainerBuilder::literal($lookups[0]); + } + + return ContainerBuilder::literal(implode(' . ', $pieces)); + } + + /** + * The values are resolved at runtime but the keys never change after loadConfiguration(), + * so a typo in a reference is still caught while compiling the container. + * + * @param non-empty-list $keys + * @throws ShouldNotHappenException + */ + private static function checkParameterExists(ContainerBuilder $builder, string $ref, array $keys): void + { + $value = $builder->parameters; + foreach ($keys as $key) { + if (!is_array($value)) { + // a dynamic parameter - cannot be traversed while compiling + return; + } + if (!array_key_exists($key, $value)) { + throw new ShouldNotHappenException(sprintf("Missing parameter '%s'.", $ref)); + } + + $value = $value[$key]; + } + } + +} diff --git a/src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php b/src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php new file mode 100644 index 00000000000..ca3289ae8bc --- /dev/null +++ b/src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php @@ -0,0 +1,93 @@ + $attributeClass + * @return list> + */ + public function findTargetClasses(string $attributeClass): array + { + $targets = array_values(Attributes::findTargetClasses($attributeClass)); + $knownClasses = []; + foreach ($targets as $target) { + $knownClasses[strtolower($target->name)] = true; + } + + foreach ($this->discoveredTargets->targetClasses[$attributeClass] ?? [] as $target) { + if (array_key_exists(strtolower($target->name), $knownClasses)) { + continue; + } + + $targets[] = $target; + } + + /** @var list> */ + return $targets; + } + + /** + * @template T of object + * @param class-string $attributeClass + * @return list> + */ + public function findTargetMethodParameters(string $attributeClass): array + { + $targets = array_values(Attributes::findTargetMethodParameters($attributeClass)); + $knownParameters = []; + foreach ($targets as $target) { + $knownParameters[self::getParameterKey($target)] = true; + } + + foreach ($this->discoveredTargets->targetMethodParameters[$attributeClass] ?? [] as $target) { + if (array_key_exists(self::getParameterKey($target), $knownParameters)) { + continue; + } + + $targets[] = $target; + } + + /** @var list> */ + return $targets; + } + + /** + * @param TargetMethodParameter $target + */ + private static function getParameterKey(TargetMethodParameter $target): string + { + return sprintf('%s::%s $%s', strtolower($target->class), strtolower($target->method), $target->name); + } + +} diff --git a/src/DependencyInjection/AttributeServices/DiscoveredAttributeTargets.php b/src/DependencyInjection/AttributeServices/DiscoveredAttributeTargets.php new file mode 100644 index 00000000000..98e9b97377d --- /dev/null +++ b/src/DependencyInjection/AttributeServices/DiscoveredAttributeTargets.php @@ -0,0 +1,31 @@ +>> $targetClasses + * @param array>> $targetMethodParameters + */ + public function __construct( + public array $targetClasses, + public array $targetMethodParameters, + ) + { + } + + public static function createEmpty(): self + { + return new self([], []); + } + +} diff --git a/src/DependencyInjection/AutowiredAttributeServicesExtension.php b/src/DependencyInjection/AutowiredAttributeServicesExtension.php index 20078d42291..6e17df6e622 100644 --- a/src/DependencyInjection/AutowiredAttributeServicesExtension.php +++ b/src/DependencyInjection/AutowiredAttributeServicesExtension.php @@ -3,39 +3,12 @@ namespace PHPStan\DependencyInjection; use Nette\DI\CompilerExtension; -use Nette\DI\ContainerBuilder; -use Nette\DI\Definitions\Reference; -use Nette\DI\Definitions\ServiceDefinition; -use Nette\DI\Definitions\Statement; -use Nette\DI\Helpers; -use Nette\Neon\Entity; -use Nette\Neon\Neon; -use Nette\PhpGenerator\Dumper; -use Nette\PhpGenerator\PhpLiteral; use Nette\Schema\Expect; use Nette\Schema\Schema; -use Nette\Utils\Strings; -use olvlvl\ComposerAttributeCollector\Attributes; -use olvlvl\ComposerAttributeCollector\TargetClass; -use olvlvl\ComposerAttributeCollector\TargetMethodParameter; use Override; -use PHPStan\Collectors\RegistryFactory; -use PHPStan\Rules\LazyRegistry; -use PHPStan\ShouldNotHappenException; -use ReflectionClass; +use PHPStan\DependencyInjection\AttributeServices\AttributeServicesRegistrar; +use PHPStan\DependencyInjection\AttributeServices\AttributeTargetsProvider; use stdClass; -use function array_key_exists; -use function array_slice; -use function count; -use function explode; -use function implode; -use function is_array; -use function preg_split; -use function sprintf; -use function strcasecmp; -use function strtolower; -use function substr; -use const PREG_SPLIT_DELIM_CAPTURE; #[ContainerExtension(name: 'autowiredAttributeServices')] final class AutowiredAttributeServicesExtension extends CompilerExtension @@ -52,269 +25,13 @@ public function getConfigSchema(): Schema #[Override] public function loadConfiguration(): void { - require_once __DIR__ . '/../../vendor/attributes.php'; - $builder = $this->getContainerBuilder(); - - $autowiredParameters = Attributes::findTargetMethodParameters(AutowiredParameter::class); - $constructorParameters = []; - foreach ($autowiredParameters as $parameter) { - if (strcasecmp($parameter->method, '__construct') !== 0) { - continue; - } - $lowerClass = strtolower($parameter->class); - $constructorParameters[$lowerClass] ??= []; - $constructorParameters[$lowerClass][] = $parameter; - } - - $conditionalTargets = []; - foreach (Attributes::findTargetClasses(Conditional::class) as $class) { - $conditionalTargets[] = [ - new TargetClass($class->attribute->attribute, $class->name), - self::isConditionMet($builder, $class->attribute->if), - ]; - } - - foreach (self::findTargetClasses(AutowiredService::class, $conditionalTargets) as [$class, $tagged]) { - $reflection = new ReflectionClass($class->name); - $attribute = $class->attribute; - - $definition = $builder->addDefinition($attribute->name) - ->setType($class->name) - ->setAutowired($attribute->as); - - if ($attribute->factory !== null) { - [$ref, $method] = explode('::', $attribute->factory); - $definition->setFactory(new Statement([new Reference(substr($ref, 1)), $method])); - } - - self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); - - if (!$attribute->autoTag || !$tagged) { - continue; - } - - foreach (ValidateServiceTagsExtension::getInterfaceTagMapping() as $interface => $tag) { - if (!$reflection->implementsInterface($interface)) { - continue; - } - - $definition->addTag($tag); - } - } - - foreach (Attributes::findTargetClasses(NonAutowiredService::class) as $class) { - $attribute = $class->attribute; - - $definition = $builder->addDefinition($attribute->name) - ->setType($class->name) - ->setAutowired(false); - - if ($attribute->factory !== null) { - [$ref, $method] = explode('::', $attribute->factory); - $definition->setFactory(new Statement([new Reference(substr($ref, 1)), $method])); - } - - self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); - } - - foreach (Attributes::findTargetClasses(GenerateFactory::class) as $class) { - $attribute = $class->attribute; - $definition = $builder->addFactoryDefinition(null) - ->setImplement($attribute->interface); - - if ($attribute->resultType !== null) { - $definition->getResultDefinition()->setType($attribute->resultType); - } - - $resultDefinition = $definition->getResultDefinition(); - self::processConstructorParameters($builder, $class->name, $resultDefinition, $constructorParameters); - } - /** @var stdClass&object{level: int|null} $config */ $config = $this->getConfig(); - if ($config->level === null) { - return; - } - - foreach (self::findTargetClasses(RegisteredRule::class, $conditionalTargets) as [$class, $tagged]) { - $attribute = $class->attribute; - if ($attribute->level > $config->level) { - continue; - } - - $definition = $builder->addDefinition(null) - ->setFactory($class->name) - ->setAutowired($class->name); - if ($tagged) { - $definition->addTag(LazyRegistry::RULE_TAG); - } - - self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); - } - - foreach (self::findTargetClasses(RegisteredCollector::class, $conditionalTargets) as [$class, $tagged]) { - $attribute = $class->attribute; - if ($attribute->level > $config->level) { - continue; - } - - $definition = $builder->addDefinition(null) - ->setFactory($class->name) - ->setAutowired($class->name); - if ($tagged) { - $definition->addTag(RegistryFactory::COLLECTOR_TAG); - } - - self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); - } - } - - /** - * @template T of object - * @param class-string $attributeClass - * @param list, bool}> $conditionalTargets attributes wrapped in #[Conditional] and whether the condition is met - * @return list, bool}> targets and whether they get their tags - */ - private static function findTargetClasses(string $attributeClass, array $conditionalTargets): array - { - $targets = []; - foreach (Attributes::findTargetClasses($attributeClass) as $target) { - $targets[] = [$target, true]; - } - foreach ($conditionalTargets as [$target, $isConditionMet]) { - if (!$target->attribute instanceof $attributeClass) { - continue; - } - - $targets[] = [new TargetClass($target->attribute, $target->name), $isConditionMet]; - } - - return $targets; - } - - /** - * The strings are NEON values like in the conditionalTags config section, so 'not(%foo%)' is supported too. - * - * @param string|non-empty-list $if - */ - public static function isConditionMet(ContainerBuilder $builder, string|array $if): bool - { - $condition = []; - foreach ((array) $if as $item) { - $value = Neon::decode($item); - if ($value instanceof Entity) { - $value = new Statement($value->value, $value->attributes); - } - - $condition[] = Helpers::expand($value, $builder->parameters, true); - } - - return ConditionalTagsExtension::isConditionMet($condition); - } - - /** - * @param class-string $className - * @param array>> $constructorParameters - */ - public static function processConstructorParameters(ContainerBuilder $builder, string $className, ServiceDefinition $definition, array $constructorParameters): void - { - foreach ($constructorParameters[strtolower($className)] ?? [] as $autowiredParameter) { - $ref = $autowiredParameter->attribute->ref; - if ($ref === null) { - $argument = self::createDeferredParameter($builder, '%' . Helpers::escape($autowiredParameter->name) . '%'); - } elseif (Strings::match($ref, '#^@[\w\\\\]+$#D') !== null) { - $argument = new Reference(substr($ref, 1)); - } else { - $argument = self::createDeferredParameter($builder, $ref); - } - $definition->setArgument($autowiredParameter->name, $argument); - } - } - - /** - * Turns a `%foo%` reference into a deferred `$this->getParameter('foo')` lookup instead of reading - * ContainerBuilder::$parameters right now. Extensions registered after this one still rewrite the - * parameters during loadConfiguration() - ValidateExcludePathsExtension unwraps OptionalPath objects - * in `excludePaths` - and a value snapshotted here would keep the pre-rewrite contents. - * - * `%foo.bar%` becomes `$this->getParameter('foo')['bar']` and `%foo%/suffix` becomes a concatenation, - * mirroring how Nette itself compiles references to dynamic parameters. - * - * @return PhpLiteral|string - * @throws ShouldNotHappenException when the reference points at a parameter that does not exist - */ - private static function createDeferredParameter(ContainerBuilder $builder, string $ref) - { - $parts = preg_split('#%([\w.-]*)%#', $ref, flags: PREG_SPLIT_DELIM_CAPTURE); - if ($parts === false) { - throw new ShouldNotHappenException(); - } - - $dumper = new Dumper(); - $lookups = []; - $pieces = []; - $withoutReferences = ''; - foreach ($parts as $i => $part) { - if ($i % 2 === 0) { - if ($part !== '') { - $pieces[] = $dumper->dump($part); - $withoutReferences .= $part; - } - continue; - } - - if ($part === '') { - // '%%' is an escaped percent sign - $pieces[] = $dumper->dump('%'); - $withoutReferences .= '%'; - continue; - } - - $keys = explode('.', $part); - self::checkParameterExists($builder, $part, $keys); - - $code = $dumper->format('$this->getParameter(?)', $keys[0]); - foreach (array_slice($keys, 1) as $key) { - $code .= sprintf('[%s]', $dumper->dump($key)); - } - - $lookups[] = $code; - $pieces[] = sprintf('(%s)', $code); - } - - if (count($lookups) === 0) { - return $withoutReferences; - } - - if (count($pieces) === 1) { - // the reference is the whole value, no string coercion - return ContainerBuilder::literal($lookups[0]); - } - - return ContainerBuilder::literal(implode(' . ', $pieces)); - } - - /** - * The values are resolved at runtime but the keys never change after loadConfiguration(), - * so a typo in a reference is still caught while compiling the container. - * - * @param non-empty-list $keys - * @throws ShouldNotHappenException - */ - private static function checkParameterExists(ContainerBuilder $builder, string $ref, array $keys): void - { - $value = $builder->parameters; - foreach ($keys as $key) { - if (!is_array($value)) { - // a dynamic parameter - cannot be traversed while compiling - return; - } - if (!array_key_exists($key, $value)) { - throw new ShouldNotHappenException(sprintf("Missing parameter '%s'.", $ref)); - } - - $value = $value[$key]; - } + AttributeServicesRegistrar::registerServices( + $this->getContainerBuilder(), + AttributeTargetsProvider::create(), + $config->level, + ); } } diff --git a/src/DependencyInjection/StubValidatorRuleServicesExtension.php b/src/DependencyInjection/StubValidatorRuleServicesExtension.php index f737d202dfd..3922a7e8358 100644 --- a/src/DependencyInjection/StubValidatorRuleServicesExtension.php +++ b/src/DependencyInjection/StubValidatorRuleServicesExtension.php @@ -3,11 +3,10 @@ namespace PHPStan\DependencyInjection; use Nette\DI\CompilerExtension; -use olvlvl\ComposerAttributeCollector\Attributes; use Override; +use PHPStan\DependencyInjection\AttributeServices\AttributeServicesRegistrar; +use PHPStan\DependencyInjection\AttributeServices\AttributeTargetsProvider; use PHPStan\PhpDoc\StubValidator; -use function strcasecmp; -use function strtolower; final class StubValidatorRuleServicesExtension extends CompilerExtension { @@ -15,27 +14,17 @@ final class StubValidatorRuleServicesExtension extends CompilerExtension #[Override] public function loadConfiguration(): void { - require_once __DIR__ . '/../../vendor/attributes.php'; $builder = $this->getContainerBuilder(); + $targets = AttributeTargetsProvider::create(); + $constructorParameters = AttributeServicesRegistrar::collectConstructorParameters($targets); - $autowiredParameters = Attributes::findTargetMethodParameters(AutowiredParameter::class); - $constructorParameters = []; - foreach ($autowiredParameters as $parameter) { - if (strcasecmp($parameter->method, '__construct') !== 0) { - continue; - } - $lowerClass = strtolower($parameter->class); - $constructorParameters[$lowerClass] ??= []; - $constructorParameters[$lowerClass][] = $parameter; - } - - foreach (Attributes::findTargetClasses(ValidatesStubFiles::class) as $class) { + foreach ($targets->findTargetClasses(ValidatesStubFiles::class) as $class) { $definition = $builder->addDefinition(null) ->setFactory($class->name) ->setAutowired(false) ->addTag(StubValidator::SERVICE_RULE_TAG); - AutowiredAttributeServicesExtension::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); + AttributeServicesRegistrar::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); } } diff --git a/tests/PHPStan/DependencyInjection/AutowiredAttributeServicesExtensionTest.php b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesRegistrarTest.php similarity index 82% rename from tests/PHPStan/DependencyInjection/AutowiredAttributeServicesExtensionTest.php rename to tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesRegistrarTest.php index dcc7a5e83b1..b6ac3670e1a 100644 --- a/tests/PHPStan/DependencyInjection/AutowiredAttributeServicesExtensionTest.php +++ b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesRegistrarTest.php @@ -1,13 +1,13 @@ assertSame($expected, AutowiredAttributeServicesExtension::isConditionMet($this->createBuilder(), $if)); + $this->assertSame($expected, AttributeServicesRegistrar::isConditionMet($this->createBuilder(), $if)); } public function testMissingParameter(): void { $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage("Missing parameter 'toggles.missing'."); - AutowiredAttributeServicesExtension::isConditionMet($this->createBuilder(), 'not(%toggles.missing%)'); + AttributeServicesRegistrar::isConditionMet($this->createBuilder(), 'not(%toggles.missing%)'); } private function createBuilder(): ContainerBuilder From 4b4721b65919153984e0ad087867930c071576da Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 2 Nov 2026 18:18:09 +0000 Subject: [PATCH 2/5] Add the attributeServicesDirectories section and include it in the container cache key The top-level section lists directories whose classes may carry PHPStan's DI attributes. Paths are relative to the declaring config file. The resolver checks each directory against the psr-4 and classmap autoload rules of the owning Composer package or the root project and requires PHP 8.0+. It also computes the directory's part of the container cache key: the package version for installed vendor packages, file hashes for the project's own directories and path repositories. Amp-Thread-ID: https://ampcode.com/threads/T-01a19cd4-be31-75b4-bd15-67786eff5983 --- conf/parametersSchema.neon | 2 + .../AttributeServicesDirectoriesResolver.php | 382 ++++++++++++++++++ .../AttributeServices/AutoloadRules.php | 45 +++ .../AttributeServices/ComposerPackage.php | 24 ++ .../AttributeServices/ComposerProject.php | 51 +++ .../ComposerProjectFactory.php | 259 ++++++++++++ ...dAttributeServicesDirectoriesException.php | 27 ++ .../ResolvedAttributeServicesDirectories.php | 51 +++ .../ResolvedAttributeServicesDirectory.php | 32 ++ .../AttributeServicesDirectoriesExtension.php | 26 ++ src/DependencyInjection/Configurator.php | 12 + src/DependencyInjection/ContainerFactory.php | 10 +- src/DependencyInjection/NeonAdapter.php | 2 +- ...tributeServicesDirectoriesResolverTest.php | 169 ++++++++ 14 files changed, 1090 insertions(+), 2 deletions(-) create mode 100644 src/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolver.php create mode 100644 src/DependencyInjection/AttributeServices/AutoloadRules.php create mode 100644 src/DependencyInjection/AttributeServices/ComposerPackage.php create mode 100644 src/DependencyInjection/AttributeServices/ComposerProject.php create mode 100644 src/DependencyInjection/AttributeServices/ComposerProjectFactory.php create mode 100644 src/DependencyInjection/AttributeServices/InvalidAttributeServicesDirectoriesException.php create mode 100644 src/DependencyInjection/AttributeServices/ResolvedAttributeServicesDirectories.php create mode 100644 src/DependencyInjection/AttributeServices/ResolvedAttributeServicesDirectory.php create mode 100644 src/DependencyInjection/AttributeServicesDirectoriesExtension.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 62dab9d9c3b..a0f58e0c5ad 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -218,12 +218,14 @@ parametersSchema: analysedPathsFromConfig: listOf(string()) usedLevel: string() cliAutoloadFile: schema(string(), nullable()) + attributeServicesDirectories: listOf(string()) # internal - editor mode singleReflectionFile: schema(string(), nullable()) singleReflectionInsteadOfFile: schema(string(), nullable()) expandRelativePaths: + - '[attributeServicesDirectories][]' - '[parameters][paths][]' - '[parameters][excludePaths][]' - '[parameters][excludePaths][analyse][]' diff --git a/src/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolver.php b/src/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolver.php new file mode 100644 index 00000000000..821a4a3733b --- /dev/null +++ b/src/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolver.php @@ -0,0 +1,382 @@ +|null */ + private ?array $composerProjects = null; + + /** + * @param string[] $composerAutoloaderProjectPaths + */ + public function __construct( + private FileHelper $fileHelper, + private array $composerAutoloaderProjectPaths, + private int $runtimePhpVersionId = PHP_VERSION_ID, + ) + { + $this->composerProjectFactory = new ComposerProjectFactory($fileHelper); + } + + /** + * @param mixed $rawSectionValue + * @throws InvalidAttributeServicesDirectoriesException + */ + public function resolve($rawSectionValue): ResolvedAttributeServicesDirectories + { + if ($rawSectionValue === null || $rawSectionValue === []) { + return ResolvedAttributeServicesDirectories::createEmpty(); + } + + if (!is_array($rawSectionValue)) { + throw new InvalidAttributeServicesDirectoriesException([ + 'The attributeServicesDirectories section must contain a list of directory paths.', + ]); + } + + $errors = []; + $directories = []; + foreach ($rawSectionValue as $entry) { + if (!is_string($entry)) { + $errors[] = 'The attributeServicesDirectories section must contain a list of directory paths.'; + continue; + } + if (str_contains($entry, '%')) { + $errors[] = sprintf('Entry %s in the attributeServicesDirectories section must be a plain path - %% parameters are not supported.', $entry); + continue; + } + if (str_starts_with($entry, '*')) { + $errors[] = sprintf('Entry %s in the attributeServicesDirectories section must be a plain path - wildcards are not supported.', $entry); + continue; + } + + $directories[] = rtrim($this->fileHelper->normalizePath($this->fileHelper->absolutizePath($entry), '/'), '/'); + } + + $directories = $this->deduplicateNested($directories); + + if (count($directories) > 0 && $this->runtimePhpVersionId < 80000) { + $errors[] = sprintf( + 'The attributeServicesDirectories section requires PHP 8.0 or later, PHPStan is running on PHP %d.%d.%d.', + intdiv($this->runtimePhpVersionId, 10000), + intdiv($this->runtimePhpVersionId, 100) % 100, + $this->runtimePhpVersionId % 100, + ); + throw new InvalidAttributeServicesDirectoriesException($errors); + } + + $resolved = []; + foreach ($directories as $directory) { + if (!is_dir($directory)) { + $errors[] = sprintf('Directory %s from the attributeServicesDirectories section does not exist.', $directory); + continue; + } + + $resolvedDirectory = $this->resolveDirectory($directory, $errors); + if ($resolvedDirectory === null) { + continue; + } + + $resolved[] = $resolvedDirectory; + } + + if (count($errors) > 0) { + throw new InvalidAttributeServicesDirectoriesException($errors); + } + + return new ResolvedAttributeServicesDirectories($resolved); + } + + /** + * @param list $errors + */ + private function resolveDirectory(string $directory, array &$errors): ?ResolvedAttributeServicesDirectory + { + $ownership = $this->findOwnership($directory); + if ($ownership === null) { + $errors[] = sprintf('Directory %s from the attributeServicesDirectories section is not inside any project with Composer metadata known to PHPStan.', $directory); + return null; + } + + [$project, $package] = $ownership; + + if ($package !== null) { + // autoload-dev of a dependency is never installed by Composer + $rules = $package->autoload; + } elseif ($project->devInstalled) { + $rules = $project->rootAutoload->union($project->rootAutoloadDev); + } else { + $rules = $project->rootAutoload; + } + + $psr4 = []; + foreach ($rules->psr4 as $namespacePrefix => $baseDirectories) { + foreach ($baseDirectories as $baseDirectory) { + if (!$this->pathsIntersect($directory, $baseDirectory)) { + continue; + } + + $psr4[$namespacePrefix][] = $baseDirectory; + } + } + + $classmapPaths = []; + foreach ($rules->classmapPaths as $classmapPath) { + if (!$this->pathsIntersect($directory, $classmapPath)) { + continue; + } + + $classmapPaths[] = $classmapPath; + } + + if (count($psr4) === 0 && count($classmapPaths) === 0) { + $errors[] = $this->describeUncoveredDirectory($directory, $project, $package); + return null; + } + + if ($package !== null && $package->cacheToken !== null) { + $cacheKeyComponent = [$directory => sprintf('package:%s:%s', $package->name, $package->cacheToken)]; + } else { + $cacheKeyComponent = $this->hashDirectory($directory, $errors); + } + + return new ResolvedAttributeServicesDirectory( + $directory, + $package === null ? null : $package->name, + $psr4, + $classmapPaths, + $project->getAutoloadClassmapPath(), + $cacheKeyComponent, + ); + } + + private function describeUncoveredDirectory(string $directory, ComposerProject $project, ?ComposerPackage $package): string + { + $subject = $package === null + ? sprintf('the autoload section of %s/composer.json', $project->rootPath) + : sprintf('the autoload section of the Composer package %s', $package->name); + + if ($package === null) { + foreach ($this->collectPsrAndFilePaths($project->rootAutoloadDev) as $path) { + if (!$this->pathsIntersect($directory, $path)) { + continue; + } + + if (!$project->devInstalled) { + return sprintf( + 'Directory %s from the attributeServicesDirectories section is only covered by the autoload-dev section of %s/composer.json but Composer dependencies were installed with --no-dev.', + $directory, + $project->rootPath, + ); + } + } + } + + $unsupportedRules = $package === null + ? ($project->devInstalled ? $project->rootAutoload->union($project->rootAutoloadDev) : $project->rootAutoload) + : $package->autoload; + foreach ($unsupportedRules->psr0 as $baseDirectories) { + foreach ($baseDirectories as $baseDirectory) { + if (!$this->pathsIntersect($directory, $baseDirectory)) { + continue; + } + + return sprintf( + 'Directory %s from the attributeServicesDirectories section is only covered by a psr-0 autoload rule of %s. Only psr-4 and classmap rules are supported.', + $directory, + $subject, + ); + } + } + + return sprintf( + 'Directory %s from the attributeServicesDirectories section is not covered by %s. Add it to autoload.psr-4 or autoload.classmap in composer.json and run composer dump-autoload.', + $directory, + $subject, + ); + } + + /** + * @return array{ComposerProject, ComposerPackage|null}|null + */ + private function findOwnership(string $directory): ?array + { + foreach ($this->getComposerProjects() as $project) { + $package = $project->findPackageOfDirectory($directory); + if ($package !== null) { + return [$project, $package]; + } + } + + $containing = null; + foreach ($this->getComposerProjects() as $project) { + if (!$project->containsDirectory($directory)) { + continue; + } + + if ($containing !== null && strlen($containing->rootPath) >= strlen($project->rootPath)) { + continue; + } + + $containing = $project; + } + + if ($containing === null) { + return null; + } + + return [$containing, null]; + } + + /** + * @return list + */ + private function getComposerProjects(): array + { + if ($this->composerProjects !== null) { + return $this->composerProjects; + } + + $projects = []; + foreach ($this->composerAutoloaderProjectPaths as $projectPath) { + $project = $this->composerProjectFactory->create($projectPath); + if ($project === null) { + continue; + } + + $projects[] = $project; + } + + return $this->composerProjects = $projects; + } + + /** + * @return list + */ + private function collectPsrAndFilePaths(AutoloadRules $rules): array + { + $paths = []; + foreach ($rules->psr4 as $baseDirectories) { + foreach ($baseDirectories as $baseDirectory) { + $paths[] = $baseDirectory; + } + } + foreach ($rules->classmapPaths as $path) { + $paths[] = $path; + } + + return $paths; + } + + private function pathsIntersect(string $a, string $b): bool + { + return $a === $b || str_starts_with($a, $b . '/') || str_starts_with($b, $a . '/'); + } + + /** + * Content hashes of all PHP files under the directory - the cache-key fallback when + * a package version cannot stand in for the directory contents. Mirrors how config + * files invalidate the container in Configurator::getAllConfigFilesHashes(). + * + * @param list $errors + * @return array + */ + private function hashDirectory(string $directory, array &$errors): array + { + $files = []; + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($directory, RecursiveDirectoryIterator::SKIP_DOTS), + ); + foreach ($iterator as $fileInfo) { + if (!$fileInfo instanceof SplFileInfo || !$fileInfo->isFile()) { + continue; + } + if (!str_ends_with($fileInfo->getFilename(), '.php')) { + continue; + } + + $files[] = $this->fileHelper->normalizePath($fileInfo->getPathname(), '/'); + } + + sort($files); + + $hashes = []; + foreach ($files as $file) { + $hash = hash_file('sha256', $file); + if ($hash === false) { + $errors[] = (new CouldNotReadFileException($file))->getMessage(); + continue; + } + + $hashes[$file] = $hash; + } + + ksort($hashes); + + return $hashes; + } + + /** + * Unique directories with entries nested inside another configured directory dropped - + * the outer directory already covers them for both discovery and hashing. + * + * @param list $directories + * @return list + */ + private function deduplicateNested(array $directories): array + { + usort($directories, static fn (string $a, string $b): int => strlen($a) <=> strlen($b)); + + $result = []; + foreach ($directories as $directory) { + foreach ($result as $kept) { + if ($directory === $kept || str_starts_with($directory, $kept . '/')) { + continue 2; + } + } + + $result[] = $directory; + } + + sort($result); + + return $result; + } + +} diff --git a/src/DependencyInjection/AttributeServices/AutoloadRules.php b/src/DependencyInjection/AttributeServices/AutoloadRules.php new file mode 100644 index 00000000000..1190e43f1d7 --- /dev/null +++ b/src/DependencyInjection/AttributeServices/AutoloadRules.php @@ -0,0 +1,45 @@ +> $psr4 namespace prefix => base directories + * @param list $classmapPaths + * @param array> $psr0 + * @param list $files + */ + public function __construct( + public array $psr4, + public array $classmapPaths, + public array $psr0, + public array $files, + ) + { + } + + public static function createEmpty(): self + { + return new self([], [], [], []); + } + + public function union(self $other): self + { + return new self( + array_merge_recursive($this->psr4, $other->psr4), + array_merge($this->classmapPaths, $other->classmapPaths), + array_merge_recursive($this->psr0, $other->psr0), + array_merge($this->files, $other->files), + ); + } + +} diff --git a/src/DependencyInjection/AttributeServices/ComposerPackage.php b/src/DependencyInjection/AttributeServices/ComposerPackage.php new file mode 100644 index 00000000000..bbce4ce6ac4 --- /dev/null +++ b/src/DependencyInjection/AttributeServices/ComposerPackage.php @@ -0,0 +1,24 @@ + $packagesByInstallPath install path => package, longest path first + */ + public function __construct( + public string $rootPath, + public string $vendorDirectory, + public bool $devInstalled, + public AutoloadRules $rootAutoload, + public AutoloadRules $rootAutoloadDev, + public array $packagesByInstallPath, + ) + { + } + + public function findPackageOfDirectory(string $directory): ?ComposerPackage + { + foreach ($this->packagesByInstallPath as $installPath => $package) { + if ($directory === $installPath || str_starts_with($directory, $installPath . '/')) { + return $package; + } + } + + return null; + } + + public function containsDirectory(string $directory): bool + { + return $directory === $this->rootPath || str_starts_with($directory, $this->rootPath . '/'); + } + + public function getAutoloadClassmapPath(): string + { + return $this->vendorDirectory . '/composer/autoload_classmap.php'; + } + +} diff --git a/src/DependencyInjection/AttributeServices/ComposerProjectFactory.php b/src/DependencyInjection/AttributeServices/ComposerProjectFactory.php new file mode 100644 index 00000000000..ee93d118fe9 --- /dev/null +++ b/src/DependencyInjection/AttributeServices/ComposerProjectFactory.php @@ -0,0 +1,259 @@ +fileHelper->normalizePath($projectPath, '/'), '/'); + $vendorDirectory = rtrim($this->fileHelper->normalizePath( + ComposerHelper::getVendorDirFromComposerConfig($projectPath, $composer), + '/', + ), '/'); + + $installedJson = $this->loadInstalledJson($vendorDirectory); + $installedPackages = []; + $devInstalled = true; + if ($installedJson !== null) { + $installedPackages = $installedJson['packages'] ?? $installedJson; + $devInstalled = (bool) ($installedJson['dev'] ?? true); + } + + $versions = $this->loadInstalledVersions($vendorDirectory); + + $packagesByInstallPath = []; + if (is_array($installedPackages)) { + foreach ($installedPackages as $package) { + if (!is_array($package) || !isset($package['name']) || !is_string($package['name'])) { + continue; + } + + if (isset($package['install-path']) && is_string($package['install-path'])) { + $installPath = $vendorDirectory . '/composer/' . $package['install-path']; + } else { + $installPath = $vendorDirectory . '/' . $package['name']; + } + $installPath = rtrim($this->fileHelper->normalizePath($installPath, '/'), '/'); + + $packagesByInstallPath[$installPath] = new ComposerPackage( + $package['name'], + $this->createCacheToken($package['name'], $installPath, $vendorDirectory, $versions), + $this->extractAutoloadRules($package, 'autoload', $installPath), + ); + } + } + + // Longest install path first, so a package nested under another package's directory matches first. + uksort($packagesByInstallPath, static fn (string $a, string $b): int => strlen($b) <=> strlen($a)); + + return new ComposerProject( + $rootPath, + $vendorDirectory, + $devInstalled, + $this->extractAutoloadRules($composer, 'autoload', $rootPath), + $this->extractAutoloadRules($composer, 'autoload-dev', $rootPath), + $packagesByInstallPath, + ); + } + + /** + * @return array|null + */ + private function loadInstalledJson(string $vendorDirectory): ?array + { + $installedJsonPath = $vendorDirectory . '/composer/installed.json'; + if (!is_file($installedJsonPath)) { + return null; + } + + try { + $installedJson = Json::decode(FileReader::read($installedJsonPath), Json::FORCE_ARRAY); + } catch (CouldNotReadFileException | JsonException) { + return null; + } + + if (!is_array($installedJson)) { + return null; + } + + return $installedJson; + } + + /** + * @return array> + */ + private function loadInstalledVersions(string $vendorDirectory): array + { + $installedPhp = $vendorDirectory . '/composer/installed.php'; + if (!is_file($installedPhp)) { + return []; + } + + $installed = require $installedPhp; + if (!is_array($installed) || !isset($installed['versions']) || !is_array($installed['versions'])) { + return []; + } + + $versions = []; + foreach ($installed['versions'] as $package => $info) { + if (!is_string($package) || !is_array($info)) { + continue; + } + + $versions[$package] = $info; + } + + return $versions; + } + + /** + * Version identity of the package usable as a container cache key, or null when the installed + * files can change without the recorded version changing - a path repository (the install path + * escapes the vendor directory or is a symlink) or a missing reference. Null makes the resolver + * fall back to hashing the directory contents. + * + * @param array> $versions + */ + private function createCacheToken(string $packageName, string $installPath, string $vendorDirectory, array $versions): ?string + { + if (!str_starts_with($installPath, $vendorDirectory . '/')) { + return null; + } + + $realInstallPath = realpath($installPath); + if ($realInstallPath === false || rtrim($this->fileHelper->normalizePath($realInstallPath, '/'), '/') !== $installPath) { + return null; + } + + $info = $versions[$packageName] ?? null; + if ($info === null || !isset($info['pretty_version']) || !is_string($info['pretty_version'])) { + return null; + } + + if (preg_match('/[^v\d.]/', $info['pretty_version']) === 0) { + // a tagged version, see ComposerHelper::processPackageVersion() + return $info['pretty_version']; + } + + if (isset($info['reference']) && is_string($info['reference']) && $info['reference'] !== '') { + return $info['pretty_version'] . '@' . $info['reference']; + } + + return null; + } + + /** + * @param array $package + */ + private function extractAutoloadRules(array $package, string $autoloadSection, string $basePath): AutoloadRules + { + $section = $package[$autoloadSection] ?? []; + if (!is_array($section)) { + return AutoloadRules::createEmpty(); + } + + return new AutoloadRules( + $this->extractPsrRules($section, 'psr-4', $basePath), + $this->extractPathList($section, 'classmap', $basePath), + $this->extractPsrRules($section, 'psr-0', $basePath), + $this->extractPathList($section, 'files', $basePath), + ); + } + + /** + * @param array $section + * @return array> + */ + private function extractPsrRules(array $section, string $key, string $basePath): array + { + $rules = $section[$key] ?? []; + if (!is_array($rules)) { + return []; + } + + $result = []; + foreach ($rules as $namespacePrefix => $paths) { + if (!is_string($namespacePrefix)) { + continue; + } + + $absolutePaths = []; + foreach (is_array($paths) ? $paths : [$paths] as $path) { + if (!is_string($path)) { + continue; + } + + $absolutePaths[] = $this->absolutizeRulePath($basePath, $path); + } + + if ($absolutePaths === []) { + continue; + } + + $result[$namespacePrefix] = $absolutePaths; + } + + return $result; + } + + /** + * @param array $section + * @return list + */ + private function extractPathList(array $section, string $key, string $basePath): array + { + $paths = $section[$key] ?? []; + if (!is_array($paths)) { + return []; + } + + $result = []; + foreach ($paths as $path) { + if (!is_string($path)) { + continue; + } + + $result[] = $this->absolutizeRulePath($basePath, $path); + } + + return $result; + } + + private function absolutizeRulePath(string $basePath, string $path): string + { + return rtrim($this->fileHelper->normalizePath($basePath . '/' . $path, '/'), '/'); + } + +} diff --git a/src/DependencyInjection/AttributeServices/InvalidAttributeServicesDirectoriesException.php b/src/DependencyInjection/AttributeServices/InvalidAttributeServicesDirectoriesException.php new file mode 100644 index 00000000000..f1e9f4da529 --- /dev/null +++ b/src/DependencyInjection/AttributeServices/InvalidAttributeServicesDirectoriesException.php @@ -0,0 +1,27 @@ + $errors + */ + public function __construct(private array $errors) + { + parent::__construct(implode("\n\n", $errors)); + } + + /** + * @return non-empty-list + */ + public function getErrors(): array + { + return $this->errors; + } + +} diff --git a/src/DependencyInjection/AttributeServices/ResolvedAttributeServicesDirectories.php b/src/DependencyInjection/AttributeServices/ResolvedAttributeServicesDirectories.php new file mode 100644 index 00000000000..74d4459ec6b --- /dev/null +++ b/src/DependencyInjection/AttributeServices/ResolvedAttributeServicesDirectories.php @@ -0,0 +1,51 @@ + $directories + */ + public function __construct(public array $directories) + { + } + + public static function createEmpty(): self + { + return new self([]); + } + + /** + * @return list + */ + public function getDirectoryPaths(): array + { + $paths = []; + foreach ($this->directories as $directory) { + $paths[] = $directory->directory; + } + + return $paths; + } + + /** + * @return array + */ + public function getCacheKeyComponent(): array + { + $component = []; + foreach ($this->directories as $directory) { + foreach ($directory->cacheKeyComponent as $key => $value) { + $component[$key] = $value; + } + } + + return $component; + } + +} diff --git a/src/DependencyInjection/AttributeServices/ResolvedAttributeServicesDirectory.php b/src/DependencyInjection/AttributeServices/ResolvedAttributeServicesDirectory.php new file mode 100644 index 00000000000..0af89999861 --- /dev/null +++ b/src/DependencyInjection/AttributeServices/ResolvedAttributeServicesDirectory.php @@ -0,0 +1,32 @@ +> $psr4 namespace prefix => base directories intersecting the directory + * @param list $classmapPaths classmap rule paths intersecting the directory + * @param string $autoloadClassmapPath the owning project's vendor/composer/autoload_classmap.php + * @param array $cacheKeyComponent the directory's contribution to the container cache key - + * a package version token, or per-file content hashes + */ + public function __construct( + public string $directory, + public ?string $packageName, + public array $psr4, + public array $classmapPaths, + public string $autoloadClassmapPath, + public array $cacheKeyComponent, + ) + { + } + +} diff --git a/src/DependencyInjection/AttributeServicesDirectoriesExtension.php b/src/DependencyInjection/AttributeServicesDirectoriesExtension.php new file mode 100644 index 00000000000..517eb633d9a --- /dev/null +++ b/src/DependencyInjection/AttributeServicesDirectoriesExtension.php @@ -0,0 +1,26 @@ + */ + private array $attributeServicesDirectoriesCacheKey = []; + public function __construct(private LoaderFactory $loaderFactory, private bool $journalContainer) { parent::__construct(); @@ -58,6 +61,14 @@ public function setAllConfigFiles(array $allConfigFiles): void $this->allConfigFiles = $allConfigFiles; } + /** + * @param array $cacheKeyComponent + */ + public function setAttributeServicesDirectoriesCacheKey(array $cacheKeyComponent): void + { + $this->attributeServicesDirectoriesCacheKey = $cacheKeyComponent; + } + /** * @return mixed[] */ @@ -106,6 +117,7 @@ public function loadContainer(): string is_file($attributesPhp) ? hash_file('sha256', $attributesPhp) : 'attributes-missing', NeonAdapter::CACHE_KEY, $this->getAllConfigFilesHashes(), + $this->attributeServicesDirectoriesCacheKey, ]; $className = $loader->load( diff --git a/src/DependencyInjection/ContainerFactory.php b/src/DependencyInjection/ContainerFactory.php index 653e493b906..995419e7de4 100644 --- a/src/DependencyInjection/ContainerFactory.php +++ b/src/DependencyInjection/ContainerFactory.php @@ -25,6 +25,7 @@ use PHPStan\BetterReflection\SourceLocator\Type\SourceLocator; use PHPStan\Command\CommandHelper; use PHPStan\Command\Environment; +use PHPStan\DependencyInjection\AttributeServices\AttributeServicesDirectoriesResolver; use PHPStan\File\FileHelper; use PHPStan\Node\Printer\Printer; use PHPStan\Php\PhpVersion; @@ -125,6 +126,9 @@ public function create( ], ); + $directoriesResolver = new AttributeServicesDirectoriesResolver($this->fileHelper, $composerAutoloaderProjectPaths); + $attributeServicesDirectories = $directoriesResolver->resolve($projectConfig['attributeServicesDirectories'] ?? []); + $configurator = new Configurator(new LoaderFactory( $this->fileHelper, $this->rootDirectory, @@ -150,6 +154,7 @@ public function create( 'generateBaselineFile' => $generateBaselineFile, 'usedLevel' => $usedLevel, 'cliAutoloadFile' => $cliAutoloadFile, + 'attributeServicesDirectories' => $attributeServicesDirectories->getDirectoryPaths(), 'env' => Environment::getCleanedArray(), ], $additionalParameters)); $configurator->addDynamicParameters([ @@ -164,6 +169,7 @@ public function create( } $configurator->setAllConfigFiles($allConfigFiles); + $configurator->setAttributeServicesDirectoriesCacheKey($attributeServicesDirectories->getCacheKeyComponent()); $container = $configurator->createContainer()->getByType(Container::class); $this->validateParameters($container->getParameters(), $projectConfig['parametersSchema']); @@ -253,7 +259,9 @@ private function detectDuplicateIncludedFiles( array $loaderParameters, ): array { - $neonAdapter = new NeonCachedFileReader([]); + // attributeServicesDirectories must come out absolutized relative to the declaring file + // even in this pre-pass - ContainerFactory reads the section before the container compiles + $neonAdapter = new NeonCachedFileReader(['[attributeServicesDirectories][]']); $phpAdapter = new PhpAdapter(); $allConfigFiles = []; $configArray = []; diff --git a/src/DependencyInjection/NeonAdapter.php b/src/DependencyInjection/NeonAdapter.php index 6d76e7bcad3..47991449a1a 100644 --- a/src/DependencyInjection/NeonAdapter.php +++ b/src/DependencyInjection/NeonAdapter.php @@ -33,7 +33,7 @@ final class NeonAdapter implements Adapter { - public const CACHE_KEY = 'v33-known-placeholders-expanded'; + public const CACHE_KEY = 'v34-attribute-services-directories'; private const PREVENT_MERGING_SUFFIX = '!'; diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php new file mode 100644 index 00000000000..d48470b9356 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php @@ -0,0 +1,169 @@ +createResolver(); + $this->assertSame([], $resolver->resolve(null)->directories); + $this->assertSame([], $resolver->resolve([])->directories); + } + + public function testPhpVersionGate(): void + { + $resolver = $this->createResolver(70428); + $this->expectException(InvalidAttributeServicesDirectoriesException::class); + $this->expectExceptionMessage('The attributeServicesDirectories section requires PHP 8.0 or later, PHPStan is running on PHP 7.4.28.'); + $resolver->resolve([__DIR__]); + } + + public function testNonListSection(): void + { + $resolver = $this->createResolver(); + $this->expectException(InvalidAttributeServicesDirectoriesException::class); + $this->expectExceptionMessage('The attributeServicesDirectories section must contain a list of directory paths.'); + $resolver->resolve('src'); + } + + public function testNonStringEntry(): void + { + $resolver = $this->createResolver(); + $this->expectException(InvalidAttributeServicesDirectoriesException::class); + $this->expectExceptionMessage('The attributeServicesDirectories section must contain a list of directory paths.'); + $resolver->resolve([['src']]); + } + + public function testParameterEntryRejected(): void + { + $resolver = $this->createResolver(); + $this->expectException(InvalidAttributeServicesDirectoriesException::class); + $this->expectExceptionMessage('Entry %rootDir%/src in the attributeServicesDirectories section must be a plain path - % parameters are not supported.'); + $resolver->resolve(['%rootDir%/src']); + } + + public function testWildcardEntryRejected(): void + { + $resolver = $this->createResolver(); + $this->expectException(InvalidAttributeServicesDirectoriesException::class); + $this->expectExceptionMessage('Entry */src in the attributeServicesDirectories section must be a plain path - wildcards are not supported.'); + $resolver->resolve(['*/src']); + } + + public function testMissingDirectory(): void + { + $resolver = $this->createResolver(); + $this->expectException(InvalidAttributeServicesDirectoriesException::class); + $this->expectExceptionMessage('does not exist'); + $resolver->resolve([__DIR__ . '/does-not-exist']); + } + + public function testDirectoryOutsideComposerProjects(): void + { + $resolver = new AttributeServicesDirectoriesResolver(new FileHelper(self::getRepoRoot()), []); + $this->expectException(InvalidAttributeServicesDirectoriesException::class); + $this->expectExceptionMessage('is not inside any project with Composer metadata known to PHPStan'); + $resolver->resolve([__DIR__]); + } + + public function testProjectOwnPsr4Directory(): void + { + $repoRoot = self::getRepoRoot(); + $directory = $repoRoot . '/src/DependencyInjection/AttributeServices'; + $resolved = $this->createResolver()->resolve([$directory]); + + $this->assertCount(1, $resolved->directories); + $resolvedDirectory = $resolved->directories[0]; + $this->assertNull($resolvedDirectory->packageName); + $this->assertArrayHasKey('PHPStan\\', $resolvedDirectory->psr4); + $this->assertContains($repoRoot . '/src', $resolvedDirectory->psr4['PHPStan\\']); + + $expectedFile = $directory . '/AttributeServicesDirectoriesResolver.php'; + $this->assertArrayHasKey($expectedFile, $resolvedDirectory->cacheKeyComponent); + $this->assertSame(hash_file('sha256', $expectedFile), $resolvedDirectory->cacheKeyComponent[$expectedFile]); + } + + public function testProjectOwnClassmapDirectoryFromAutoloadDev(): void + { + $repoRoot = self::getRepoRoot(); + $resolved = $this->createResolver()->resolve([__DIR__]); + + $this->assertCount(1, $resolved->directories); + $resolvedDirectory = $resolved->directories[0]; + $this->assertNull($resolvedDirectory->packageName); + $this->assertContains($repoRoot . '/tests/PHPStan', $resolvedDirectory->classmapPaths); + $this->assertSame($repoRoot . '/vendor/composer/autoload_classmap.php', $resolvedDirectory->autoloadClassmapPath); + } + + public function testVendorPackageDirectory(): void + { + $repoRoot = self::getRepoRoot(); + $directory = $repoRoot . '/vendor/nikic/php-parser/lib/PhpParser'; + $resolved = $this->createResolver()->resolve([$directory]); + + $this->assertCount(1, $resolved->directories); + $resolvedDirectory = $resolved->directories[0]; + $this->assertSame('nikic/php-parser', $resolvedDirectory->packageName); + + $this->assertSame([$directory], array_keys($resolvedDirectory->cacheKeyComponent)); + $this->assertTrue(str_starts_with(array_values($resolvedDirectory->cacheKeyComponent)[0], 'package:nikic/php-parser:')); + } + + public function testUncoveredDirectory(): void + { + $resolver = $this->createResolver(); + $this->expectException(InvalidAttributeServicesDirectoriesException::class); + $this->expectExceptionMessage('is not covered by the autoload section of'); + $resolver->resolve([self::getRepoRoot() . '/bin']); + } + + public function testNestedDirectoriesDeduplicated(): void + { + $repoRoot = self::getRepoRoot(); + $resolved = $this->createResolver()->resolve([ + $repoRoot . '/src/DependencyInjection/AttributeServices', + $repoRoot . '/src/DependencyInjection', + ]); + + $this->assertCount(1, $resolved->directories); + $this->assertSame($repoRoot . '/src/DependencyInjection', $resolved->directories[0]->directory); + } + + public function testAllErrorsAreCollected(): void + { + $resolver = $this->createResolver(); + try { + $resolver->resolve([ + __DIR__ . '/does-not-exist', + self::getRepoRoot() . '/bin', + ]); + $this->fail('Expected InvalidAttributeServicesDirectoriesException.'); + } catch (InvalidAttributeServicesDirectoriesException $e) { + $this->assertCount(2, $e->getErrors()); + } + } + +} From 96fab8857491456d86b974645587c7ac010aa1c7 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 2 Nov 2026 18:23:53 +0000 Subject: [PATCH 3/5] Discover DI attributes in attributeServicesDirectories AttributeServicesDiscoverer finds candidate classes through the psr-4 path contract and vendor/composer/autoload_classmap.php and reads the @api attributes with native reflection: AutowiredService, NonAutowiredService, RegisteredRule, RegisteredCollector, GenerateFactory, Conditional, and AutowiredParameter on constructor parameters. No file is parsed. A content prefilter keeps classes without PHPStan attributes from being autoloaded. PHPStan-internal attributes, and files that mention a DI attribute but yield no autoloadable class, are errors. PHP 8.0 has no new in attribute arguments, so Conditional there takes [NewExpression marker => [Class::class, [arguments]]], the same array simple-downgrader writes and composer-attribute-collector understands. Discovery runs lazily while the container compiles, so a container cache hit loads no third-party class. The container cache key of the project's own directories hashes only the files the same prefilter accepts, so editing a class without PHPStan's attributes does not recompile the container. Amp-Thread-ID: https://ampcode.com/threads/T-01a19cd4-be31-75b4-bd15-67786eff5983 --- build/baseline-pre-8.0.neon | 17 + .../foo.patch | 11 + .../rule.patch | 11 + .../src/MyRule.php | 28 + .../AttributeServicesDirectoriesResolver.php | 22 +- .../AttributeServicesDiscoverer.php | 555 ++++++++++++++++++ .../AttributeServicesDiscoveryContext.php | 47 ++ .../AttributeTargetsProvider.php | 2 +- src/DependencyInjection/Conditional.php | 5 +- src/DependencyInjection/ContainerFactory.php | 2 + ...tributeServicesDirectoriesResolverTest.php | 4 +- .../AttributeServicesIntegrationTest.php | 166 ++++++ .../AttributeServices/attributeServices.neon | 5 + .../autowiredExtensionsParam.neon | 2 + .../AttributeServices/containerExtension.neon | 2 + .../BadAutowiredExtensionsService.php | 24 + .../BadCompilerExtension.php | 11 + .../BadExtensionInterface.php | 11 + .../UsesValidatesStubFiles.php | 11 + .../data/services/DiscoveredCollector.php | 27 + .../services/DiscoveredConditionalRule.php | 28 + .../DiscoveredConditionalRuleNotMet.php | 31 + .../services/DiscoveredConditionalService.php | 32 + .../data/services/DiscoveredNamedService.php | 11 + .../DiscoveredNonAutowiredService.php | 11 + .../services/DiscoveredRuleLevelEight.php | 27 + .../services/DiscoveredRuleLevelThree.php | 27 + .../data/services/DiscoveredService.php | 48 ++ .../data/services/DiscoveredValue.php | 20 + .../data/services/DiscoveredValueFactory.php | 10 + .../data/unloadable/UnloadableService.php | 11 + .../AttributeServices/extensionInterface.neon | 2 + .../AttributeServices/internalAttribute.neon | 2 + .../AttributeServices/unloadable.neon | 2 + 34 files changed, 1214 insertions(+), 11 deletions(-) create mode 100644 e2e/result-cache-extension-attribute-services/foo.patch create mode 100644 e2e/result-cache-extension-attribute-services/rule.patch create mode 100644 e2e/result-cache-extension-attribute-services/src/MyRule.php create mode 100644 src/DependencyInjection/AttributeServices/AttributeServicesDiscoverer.php create mode 100644 src/DependencyInjection/AttributeServices/AttributeServicesDiscoveryContext.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/attributeServices.neon create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/autowiredExtensionsParam.neon create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/containerExtension.neon create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/autowiredExtensions/BadAutowiredExtensionsService.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/containerExtension/BadCompilerExtension.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/extensionInterface/BadExtensionInterface.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/internalAttribute/UsesValidatesStubFiles.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredCollector.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalRule.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalRuleNotMet.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalService.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredNamedService.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredNonAutowiredService.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredRuleLevelEight.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredRuleLevelThree.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredService.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredValue.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredValueFactory.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/data/unloadable/UnloadableService.php create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/extensionInterface.neon create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/internalAttribute.neon create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/unloadable.neon diff --git a/build/baseline-pre-8.0.neon b/build/baseline-pre-8.0.neon index 203679a0de9..ec8ec032c1f 100644 --- a/build/baseline-pre-8.0.neon +++ b/build/baseline-pre-8.0.neon @@ -24,6 +24,23 @@ parameters: count: 2 path: PHPStan/Build/TurboAttributeCollector.php + - + rawMessage: 'Call to an undefined method ReflectionClass::getAttributes().' + identifier: method.notFound + count: 1 + path: ../src/DependencyInjection/AttributeServices/AttributeServicesDiscoverer.php + + - + rawMessage: 'Call to an undefined method ReflectionParameter::getAttributes().' + identifier: method.notFound + count: 1 + path: ../src/DependencyInjection/AttributeServices/AttributeServicesDiscoverer.php + + - + message: '#^(Parameter \$attribute of method .+ has invalid type|Call to method \w+\(\) on an unknown class) ReflectionAttribute\.$#' + count: 5 + path: ../src/DependencyInjection/AttributeServices/AttributeServicesDiscoverer.php + - rawMessage: 'Call to an undefined method ReflectionClass::isReadOnly().' identifier: method.notFound diff --git a/e2e/result-cache-extension-attribute-services/foo.patch b/e2e/result-cache-extension-attribute-services/foo.patch new file mode 100644 index 00000000000..84c3909f15c --- /dev/null +++ b/e2e/result-cache-extension-attribute-services/foo.patch @@ -0,0 +1,11 @@ +--- src/Foo.php ++++ src/Foo.php +@@ -5,4 +5,8 @@ + class Foo + { + ++ public function bar(): void ++ { ++ } ++ + } diff --git a/e2e/result-cache-extension-attribute-services/rule.patch b/e2e/result-cache-extension-attribute-services/rule.patch new file mode 100644 index 00000000000..976bac16869 --- /dev/null +++ b/e2e/result-cache-extension-attribute-services/rule.patch @@ -0,0 +1,11 @@ +--- src/MyRule.php ++++ src/MyRule.php +@@ -22,7 +22,7 @@ + + public function processNode(Node $node, Scope $scope): array + { +- return [RuleErrorBuilder::message('the rule says one thing')->identifier('test.myRule')->build()]; ++ return [RuleErrorBuilder::message('the rule says another thing')->identifier('test.myRule')->build()]; + } + + } diff --git a/e2e/result-cache-extension-attribute-services/src/MyRule.php b/e2e/result-cache-extension-attribute-services/src/MyRule.php new file mode 100644 index 00000000000..19dda0b56b6 --- /dev/null +++ b/e2e/result-cache-extension-attribute-services/src/MyRule.php @@ -0,0 +1,28 @@ + + */ +#[RegisteredRule(level: 0)] +class MyRule implements Rule +{ + + public function getNodeType(): string + { + return Node\Stmt\Class_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + return [RuleErrorBuilder::message('the rule says one thing')->identifier('test.myRule')->build()]; + } + +} diff --git a/src/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolver.php b/src/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolver.php index 821a4a3733b..f8c66fe2ed1 100644 --- a/src/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolver.php +++ b/src/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolver.php @@ -4,11 +4,12 @@ use PHPStan\File\CouldNotReadFileException; use PHPStan\File\FileHelper; +use PHPStan\File\FileReader; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; use SplFileInfo; use function count; -use function hash_file; +use function hash; use function intdiv; use function is_array; use function is_dir; @@ -310,9 +311,9 @@ private function pathsIntersect(string $a, string $b): bool } /** - * Content hashes of all PHP files under the directory - the cache-key fallback when - * a package version cannot stand in for the directory contents. Mirrors how config - * files invalidate the container in Configurator::getAllConfigFilesHashes(). + * Content hashes of the PHP files under the directory that can carry PHPStan's DI attributes - + * the cache-key fallback when a package version cannot stand in for the directory contents. + * Mirrors how config files invalidate the container in Configurator::getAllConfigFilesHashes(). * * @param list $errors * @return array @@ -338,13 +339,18 @@ private function hashDirectory(string $directory, array &$errors): array $hashes = []; foreach ($files as $file) { - $hash = hash_file('sha256', $file); - if ($hash === false) { - $errors[] = (new CouldNotReadFileException($file))->getMessage(); + try { + $contents = FileReader::read($file); + } catch (CouldNotReadFileException $e) { + $errors[] = $e->getMessage(); continue; } - $hashes[$file] = $hash; + if (!AttributeServicesDiscoverer::mayAffectDiscovery($contents)) { + continue; + } + + $hashes[$file] = hash('sha256', $contents); } ksort($hashes); diff --git a/src/DependencyInjection/AttributeServices/AttributeServicesDiscoverer.php b/src/DependencyInjection/AttributeServices/AttributeServicesDiscoverer.php new file mode 100644 index 00000000000..aad07a25f81 --- /dev/null +++ b/src/DependencyInjection/AttributeServices/AttributeServicesDiscoverer.php @@ -0,0 +1,555 @@ + [Foo::class, [arguments]]]`, + * like simple-downgrader does and composer-attribute-collector understands. + */ + private const NEW_EXPRESSION_CLASSES = [ + AutowiredService::class, + RegisteredRule::class, + RegisteredCollector::class, + ]; + + private const NEW_EXPRESSION_MARKER = 'olvlvl\\ComposerAttributeCollector\\NewExpression'; + + public const PUBLIC_PARAMETER_ATTRIBUTES = [ + AutowiredParameter::class, + ]; + + private const ATTRIBUTE_SHORT_NAMES = [ + 'AutowiredService', + 'NonAutowiredService', + 'RegisteredRule', + 'RegisteredCollector', + 'GenerateFactory', + 'Conditional', + 'AutowiredParameter', + 'AutowiredExtensions', + 'ContainerExtension', + 'ExtensionInterface', + 'ValidatesStubFiles', + ]; + + /** @var array>> autoload_classmap.php path => file => class names */ + private array $reverseClassmaps = []; + + /** @var list */ + private array $errors = []; + + /** + * @throws InvalidAttributeServicesDirectoriesException + */ + public function discover(ResolvedAttributeServicesDirectories $resolvedDirectories): DiscoveredAttributeTargets + { + $this->errors = []; + $targetClasses = []; + $targetMethodParameters = []; + $seenClasses = []; + + foreach ($resolvedDirectories->directories as $directory) { + foreach ($this->findCandidateClasses($directory) as $className => $file) { + if (array_key_exists(strtolower($className), $seenClasses)) { + continue; + } + $seenClasses[strtolower($className)] = true; + + $this->collectClass($className, $file, $targetClasses, $targetMethodParameters); + } + } + + if (count($this->errors) > 0) { + throw new InvalidAttributeServicesDirectoriesException($this->errors); + } + + foreach ($targetClasses as $attributeClass => $targets) { + usort($targets, static fn (TargetClass $a, TargetClass $b): int => $a->name <=> $b->name); + $targetClasses[$attributeClass] = $targets; + } + foreach ($targetMethodParameters as $attributeClass => $targets) { + usort($targets, static fn (TargetMethodParameter $a, TargetMethodParameter $b): int => [$a->class, $a->name] <=> [$b->class, $b->name]); + $targetMethodParameters[$attributeClass] = $targets; + } + + return new DiscoveredAttributeTargets($targetClasses, $targetMethodParameters); + } + + /** + * Candidate classes of one directory, derived from Composer's autoload data. Files that + * cannot yield an autoloadable class are only an error when their contents suggest one + * of PHPStan's DI attributes - anything else in the directory is none of our business. + * + * @return array class name => file + */ + private function findCandidateClasses(ResolvedAttributeServicesDirectory $directory): array + { + $candidates = []; + foreach ($this->listPhpFiles($directory->directory) as $file) { + $className = $this->derivePsr4ClassName($directory, $file); + if ($className !== null) { + $candidates[$className] = $file; + continue; + } + + $classmapClasses = $this->findClassmapClasses($directory, $file); + if ($classmapClasses !== null) { + if (count($classmapClasses) === 0) { + if (self::suggestsDiAttributes(FileReader::read($file))) { + $this->errors[] = sprintf( + 'File %s in a directory from the attributeServicesDirectories section is not present in Composer\'s class map. Run `composer dump-autoload` and try again.', + $file, + ); + } + continue; + } + + foreach ($classmapClasses as $classmapClass) { + $candidates[$classmapClass] = $file; + } + continue; + } + + if (!self::suggestsDiAttributes(FileReader::read($file))) { + continue; + } + + $this->errors[] = sprintf( + 'File %s in a directory from the attributeServicesDirectories section is not covered by the Composer autoload rules of the directory, so its class cannot be autoloaded.', + $file, + ); + } + + ksort($candidates); + + return $candidates; + } + + /** + * @param array>> $targetClasses + * @param array>> $targetMethodParameters + */ + private function collectClass(string $className, string $file, array &$targetClasses, array &$targetMethodParameters): void + { + if (!self::isPrefilterPositive(FileReader::read($file))) { + return; + } + + if (!$this->classCanBeLoaded($className)) { + if (self::suggestsDiAttributes(FileReader::read($file))) { + $this->errors[] = sprintf( + 'Class %s expected in %s (through a directory from the attributeServicesDirectories section) cannot be autoloaded.', + $className, + $file, + ); + } + + return; + } + + /** @var class-string $className */ + $reflection = new ReflectionClass($className); + + foreach ($reflection->getAttributes() as $attribute) { + $attributeClass = $this->resolveKnownAttribute($attribute->getName(), self::PUBLIC_CLASS_ATTRIBUTES); + if ($attributeClass !== null) { + try { + $attributeInstance = $this->instantiateAttribute($attribute); + } catch (Throwable $e) { + $this->errors[] = sprintf('Cannot instantiate attribute #[%s] on class %s: %s', $this->getShortName($attribute->getName()), $className, $e->getMessage()); + continue; + } + + $targetClasses[$attributeClass][] = new TargetClass($attributeInstance, $className); + continue; + } + + $this->checkDisallowedAttribute($attribute->getName(), $className); + } + + $constructor = $reflection->getConstructor(); + if ($constructor === null) { + return; + } + + foreach ($constructor->getParameters() as $parameter) { + foreach ($parameter->getAttributes() as $attribute) { + $attributeClass = $this->resolveKnownAttribute($attribute->getName(), self::PUBLIC_PARAMETER_ATTRIBUTES); + if ($attributeClass !== null) { + try { + $attributeInstance = $attribute->newInstance(); + } catch (Throwable $e) { + $this->errors[] = sprintf('Cannot instantiate attribute #[%s] on a constructor parameter of class %s: %s', $this->getShortName($attribute->getName()), $className, $e->getMessage()); + continue; + } + + $targetMethodParameters[$attributeClass][] = new TargetMethodParameter($attributeInstance, $className, $parameter->getName(), '__construct'); + continue; + } + + $this->checkDisallowedAttribute($attribute->getName(), $className); + } + } + } + + /** + * @param ReflectionAttribute $attribute + */ + private function instantiateAttribute(ReflectionAttribute $attribute): object + { + $arguments = $attribute->getArguments(); + $hasNewExpression = false; + foreach ($arguments as $key => $argument) { + $object = $this->instantiateNewExpression($argument); + if ($object === null) { + continue; + } + + $arguments[$key] = $object; + $hasNewExpression = true; + } + + if (!$hasNewExpression) { + return $attribute->newInstance(); + } + + // newInstanceArgs() instead of spreading because PHP 8.0 cannot spread string keys + return (new ReflectionClass($attribute->getName()))->newInstanceArgs($arguments); + } + + /** + * @throws ShouldNotHappenException + */ + private function instantiateNewExpression(mixed $value): ?object + { + if (!is_array($value) || count($value) !== 1 || !array_key_exists(self::NEW_EXPRESSION_MARKER, $value)) { + return null; + } + + $new = $value[self::NEW_EXPRESSION_MARKER]; + if (!is_array($new) || !isset($new[0], $new[1]) || !is_string($new[0]) || !is_array($new[1])) { + throw new ShouldNotHappenException(sprintf('%s must be followed by [ClassName::class, [arguments]].', self::NEW_EXPRESSION_MARKER)); + } + + $class = $this->resolveKnownAttribute($new[0], self::NEW_EXPRESSION_CLASSES); + if ($class === null) { + throw new ShouldNotHappenException(sprintf('Class %s is not supported in %s.', $new[0], self::NEW_EXPRESSION_MARKER)); + } + + return (new ReflectionClass($class))->newInstanceArgs($new[1]); + } + + private function checkDisallowedAttribute(string $attributeName, string $className): void + { + if (!$this->isPhpStanAttribute($attributeName)) { + return; + } + + $lowerAttributeName = strtolower($attributeName); + if ($lowerAttributeName === strtolower(ContainerExtension::class)) { + $this->errors[] = sprintf( + 'Attribute #[ContainerExtension] on class %s is not supported in directories from the attributeServicesDirectories section - the list of compiler extensions is fixed before the section is processed. Register the class in the `extensions` section of the configuration file instead.', + $className, + ); + return; + } + + if ($lowerAttributeName === strtolower(ExtensionInterface::class)) { + $this->errors[] = sprintf( + 'Attribute #[ExtensionInterface] on %s is not supported in directories from the attributeServicesDirectories section - third-party extension interfaces are not supported.', + $className, + ); + return; + } + + if ($lowerAttributeName === strtolower(AutowiredExtensions::class)) { + $this->errors[] = sprintf( + 'Attribute #[AutowiredExtensions] on a constructor parameter of class %s is not supported in directories from the attributeServicesDirectories section.', + $className, + ); + return; + } + + $this->errors[] = sprintf( + 'Attribute #[%s] on class %s is only supported on classes shipped with PHPStan itself, not on classes discovered through the attributeServicesDirectories section.', + $this->getShortName($attributeName), + $className, + ); + } + + /** + * Canonical attribute class name when $attributeName is one of $allowedAttributes, null otherwise. + * + * @param list $allowedAttributes + * @return class-string|null + */ + private function resolveKnownAttribute(string $attributeName, array $allowedAttributes): ?string + { + foreach ($allowedAttributes as $allowedAttribute) { + if (strtolower($attributeName) === strtolower($allowedAttribute)) { + return $allowedAttribute; + } + } + + return null; + } + + private function isPhpStanAttribute(string $attributeName): bool + { + if (!class_exists($attributeName)) { + return false; + } + + $file = (new ReflectionClass($attributeName))->getFileName(); + if ($file === false) { + return false; + } + + $phpstanRoot = strtr(dirname(__DIR__, 3), '\\', '/'); + + return str_starts_with(strtr($file, '\\', '/'), $phpstanRoot . '/'); + } + + private function classCanBeLoaded(string $className): bool + { + try { + if (class_exists($className) || interface_exists($className) || trait_exists($className)) { + return true; + } + + return function_exists('enum_exists') && enum_exists($className); + } catch (Throwable) { + return false; + } + } + + /** + * @return list normalized with forward slashes, sorted + */ + private function listPhpFiles(string $directory): array + { + $files = []; + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($directory, RecursiveDirectoryIterator::SKIP_DOTS), + ); + foreach ($iterator as $fileInfo) { + if (!$fileInfo instanceof SplFileInfo || !$fileInfo->isFile()) { + continue; + } + if (!str_ends_with($fileInfo->getFilename(), '.php')) { + continue; + } + + $files[] = strtr($fileInfo->getPathname(), '\\', '/'); + } + + sort($files); + + return $files; + } + + /** + * FQCN of the file per the PSR-4 path contract, or null when no PSR-4 rule of the + * directory covers the file (or a path segment cannot be a PHP name). + */ + private function derivePsr4ClassName(ResolvedAttributeServicesDirectory $directory, string $file): ?string + { + $bestBaseDirectory = null; + $bestPrefix = null; + foreach ($directory->psr4 as $namespacePrefix => $baseDirectories) { + foreach ($baseDirectories as $baseDirectory) { + if (!str_starts_with($file, $baseDirectory . '/')) { + continue; + } + if ($bestBaseDirectory !== null && strlen($baseDirectory) <= strlen($bestBaseDirectory)) { + continue; + } + + $bestBaseDirectory = $baseDirectory; + $bestPrefix = $namespacePrefix; + } + } + + if ($bestBaseDirectory === null || $bestPrefix === null) { + return null; + } + + $relativePath = substr($file, strlen($bestBaseDirectory) + 1, -4); + $segments = explode('/', $relativePath); + foreach ($segments as $segment) { + if (preg_match('/^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$/', $segment) !== 1) { + return null; + } + } + + return $bestPrefix . strtr($relativePath, '/', '\\'); + } + + /** + * Class names Composer's class map records for the file, an empty list when the file + * sits under a classmap rule but the map does not know it (a stale dump), or null + * when no classmap rule of the directory covers the file. + * + * @return list|null + */ + private function findClassmapClasses(ResolvedAttributeServicesDirectory $directory, string $file): ?array + { + $covered = false; + foreach ($directory->classmapPaths as $classmapPath) { + if ($file === $classmapPath || str_starts_with($file, $classmapPath . '/')) { + $covered = true; + break; + } + } + + if (!$covered) { + return null; + } + + $reverseClassmap = $this->getReverseClassmap($directory->autoloadClassmapPath); + + return $reverseClassmap[$file] ?? []; + } + + /** + * @return array> file => class names + */ + private function getReverseClassmap(string $autoloadClassmapPath): array + { + if (array_key_exists($autoloadClassmapPath, $this->reverseClassmaps)) { + return $this->reverseClassmaps[$autoloadClassmapPath]; + } + + $reverse = []; + if (is_file($autoloadClassmapPath)) { + $classmap = require $autoloadClassmapPath; + if (is_array($classmap)) { + foreach ($classmap as $className => $file) { + if (!is_string($className) || !is_string($file)) { + continue; + } + + $reverse[strtr($file, '\\', '/')][] = $className; + } + } + } + + return $this->reverseClassmaps[$autoloadClassmapPath] = $reverse; + } + + /** + * Whether the file contents can change what discovery finds or reports. The container + * cache key of a directory hashes only these files, so editing any other file does not + * recompile the container. + */ + public static function mayAffectDiscovery(string $contents): bool + { + return self::isPrefilterPositive($contents) || self::suggestsDiAttributes($contents); + } + + /** + * Cheap gate before a class is autoloaded: any syntactically possible reference to one of + * PHPStan's attributes - a `use` import, a fully qualified name, an alias of either - + * contains the string "phpstan" case-insensitively, so a file without it cannot use them. + */ + private static function isPrefilterPositive(string $contents): bool + { + return str_contains($contents, '#[') && stripos($contents, 'phpstan') !== false; + } + + /** + * Stricter check used only to decide whether a file that yielded no autoloadable class + * deserves an error: does it mention any of the DI attributes by name? + */ + private static function suggestsDiAttributes(string $contents): bool + { + if (!str_contains($contents, '#[')) { + return false; + } + + foreach (self::ATTRIBUTE_SHORT_NAMES as $shortName) { + if (stripos($contents, $shortName) !== false) { + return true; + } + } + + return false; + } + + private function getShortName(string $className): string + { + $parts = explode('\\', $className); + + return $parts[count($parts) - 1]; + } + +} diff --git a/src/DependencyInjection/AttributeServices/AttributeServicesDiscoveryContext.php b/src/DependencyInjection/AttributeServices/AttributeServicesDiscoveryContext.php new file mode 100644 index 00000000000..1f28ed2f114 --- /dev/null +++ b/src/DependencyInjection/AttributeServices/AttributeServicesDiscoveryContext.php @@ -0,0 +1,47 @@ +directories) === 0) { + return self::$targets = DiscoveredAttributeTargets::createEmpty(); + } + + return self::$targets = (new AttributeServicesDiscoverer())->discover(self::$currentDirectories); + } + +} diff --git a/src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php b/src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php index ca3289ae8bc..8d486bb1162 100644 --- a/src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php +++ b/src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php @@ -29,7 +29,7 @@ public static function create(): self { require_once __DIR__ . '/../../../vendor/attributes.php'; - return new self(DiscoveredAttributeTargets::createEmpty()); + return new self(AttributeServicesDiscoveryContext::getTargets()); } /** diff --git a/src/DependencyInjection/Conditional.php b/src/DependencyInjection/Conditional.php index fa195d0d47c..cace67b3ec3 100644 --- a/src/DependencyInjection/Conditional.php +++ b/src/DependencyInjection/Conditional.php @@ -17,7 +17,8 @@ * or a list of them that all must be met. * * Works thanks to https://github.com/ondrejmirtes/composer-attribute-collector - * and AutowiredAttributeServicesExtension. + * and AutowiredAttributeServicesExtension, and in the attributeServicesDirectories + * config section thanks to AttributeServicesDiscoverer. * * @api */ @@ -30,6 +31,8 @@ final class Conditional /** * On PHP < 8.1, simple-downgrader writes `new` in attribute arguments as the array, * and composer-attribute-collector turns it back into the object in vendor/attributes.php. + * Code running on PHP 8.0 in the attributeServicesDirectories section can write the array by hand, + * AttributeServicesDiscoverer turns it into the object. * * @api * @param AutowiredService|RegisteredCollector|RegisteredRule|array{ diff --git a/src/DependencyInjection/ContainerFactory.php b/src/DependencyInjection/ContainerFactory.php index 995419e7de4..60b82077464 100644 --- a/src/DependencyInjection/ContainerFactory.php +++ b/src/DependencyInjection/ContainerFactory.php @@ -26,6 +26,7 @@ use PHPStan\Command\CommandHelper; use PHPStan\Command\Environment; use PHPStan\DependencyInjection\AttributeServices\AttributeServicesDirectoriesResolver; +use PHPStan\DependencyInjection\AttributeServices\AttributeServicesDiscoveryContext; use PHPStan\File\FileHelper; use PHPStan\Node\Printer\Printer; use PHPStan\Php\PhpVersion; @@ -128,6 +129,7 @@ public function create( $directoriesResolver = new AttributeServicesDirectoriesResolver($this->fileHelper, $composerAutoloaderProjectPaths); $attributeServicesDirectories = $directoriesResolver->resolve($projectConfig['attributeServicesDirectories'] ?? []); + AttributeServicesDiscoveryContext::set($attributeServicesDirectories); $configurator = new Configurator(new LoaderFactory( $this->fileHelper, diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php index d48470b9356..c4db4b7ce94 100644 --- a/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php +++ b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php @@ -101,9 +101,11 @@ public function testProjectOwnPsr4Directory(): void $this->assertArrayHasKey('PHPStan\\', $resolvedDirectory->psr4); $this->assertContains($repoRoot . '/src', $resolvedDirectory->psr4['PHPStan\\']); - $expectedFile = $directory . '/AttributeServicesDirectoriesResolver.php'; + // only files that can carry PHPStan's DI attributes are part of the key + $expectedFile = $directory . '/AttributeServicesDiscoverer.php'; $this->assertArrayHasKey($expectedFile, $resolvedDirectory->cacheKeyComponent); $this->assertSame(hash_file('sha256', $expectedFile), $resolvedDirectory->cacheKeyComponent[$expectedFile]); + $this->assertArrayNotHasKey($directory . '/AttributeServicesDirectoriesResolver.php', $resolvedDirectory->cacheKeyComponent); } public function testProjectOwnClassmapDirectoryFromAutoloadDev(): void diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php new file mode 100644 index 00000000000..f8343209c5e --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php @@ -0,0 +1,166 @@ +create(self::$tmpDir, [__DIR__ . '/attributeServices.neon'], [], [dirname(__DIR__, 4)]); + } + + private static function createTmpDir(): string + { + $tmpDir = sys_get_temp_dir() . '/phpstan-attribute-services-' . md5(uniqid(more_entropy: true)); + mkdir($tmpDir, 0777, true); + + return $tmpDir; + } + + public function testDiscoveredServiceWithAutowiredParameters(): void + { + $container = self::createContainer(); + $service = $container->getByType(DiscoveredService::class); + $this->assertSame(__DIR__, $service->getCurrentWorkingDirectory()); + $this->assertSame(self::$tmpDir, $service->getTmpDir()); + } + + public function testDiscoveredServiceIsAutoTagged(): void + { + $container = self::createContainer(); + $extensions = $container->getExtensionsCollection(ReadWritePropertiesExtension::class)->getAll(); + $this->assertCount(1, $extensions); + $this->assertInstanceOf(DiscoveredService::class, $extensions[0]); + } + + public function testDiscoveredNamedService(): void + { + $container = self::createContainer(); + $this->assertInstanceOf(DiscoveredNamedService::class, $container->getService('attributeServicesFixtures.named')); + } + + public function testDiscoveredNonAutowiredService(): void + { + $container = self::createContainer(); + $this->assertInstanceOf(DiscoveredNonAutowiredService::class, $container->getService('attributeServicesFixtures.nonAutowired')); + } + + public function testDiscoveredRulesFollowTheLevel(): void + { + $container = self::createContainer(); + $rules = $container->getServicesByTag(LazyRegistry::RULE_TAG); + $this->assertCount(1, array_filter($rules, static fn ($rule): bool => $rule instanceof DiscoveredRuleLevelThree)); + $this->assertCount(0, array_filter($rules, static fn ($rule): bool => $rule instanceof DiscoveredRuleLevelEight)); + } + + public function testDiscoveredConditional(): void + { + $container = self::createContainer(); + $rules = $container->getServicesByTag(LazyRegistry::RULE_TAG); + $this->assertCount(1, array_filter($rules, static fn ($rule): bool => $rule instanceof DiscoveredConditionalRule)); + $this->assertCount(0, array_filter($rules, static fn ($rule): bool => $rule instanceof DiscoveredConditionalRuleNotMet)); + + // the services are registered even when the condition is not met, only their tags are skipped + $this->assertCount(1, $container->findServiceNamesByType(DiscoveredConditionalRuleNotMet::class)); + $this->assertCount(1, $container->findServiceNamesByType(DiscoveredConditionalService::class)); + } + + public function testDiscoveredCollector(): void + { + $container = self::createContainer(); + $collectors = $container->getServicesByTag(RegistryFactory::COLLECTOR_TAG); + $this->assertCount(1, array_filter($collectors, static fn ($collector): bool => $collector instanceof DiscoveredCollector)); + } + + public function testDiscoveredGeneratedFactory(): void + { + $container = self::createContainer(); + $factory = $container->getByType(DiscoveredValueFactory::class); + $value = $factory->create('hello'); + $this->assertSame('hello', $value->name); + $this->assertSame(self::$tmpDir, $value->tmpDir); + } + + public function testDirectoriesParameterIsAbsolutized(): void + { + $container = self::createContainer(); + $this->assertSame([__DIR__ . '/data/services'], $container->getParameter('attributeServicesDirectories')); + } + + #[DataProvider('dataErrors')] + public function testErrors(string $neonFile, string $expectedMessage): void + { + $containerFactory = new ContainerFactory(__DIR__); + $this->expectException(InvalidAttributeServicesDirectoriesException::class); + $this->expectExceptionMessage($expectedMessage); + $containerFactory->create(self::createTmpDir(), [__DIR__ . '/' . $neonFile], [], [dirname(__DIR__, 4)]); + } + + /** + * @return iterable + */ + public static function dataErrors(): iterable + { + yield [ + 'containerExtension.neon', + 'Attribute #[ContainerExtension] on class AttributeServicesFixtures\ContainerExtension\BadCompilerExtension is not supported in directories from the attributeServicesDirectories section - the list of compiler extensions is fixed before the section is processed. Register the class in the `extensions` section of the configuration file instead.', + ]; + + yield [ + 'extensionInterface.neon', + 'Attribute #[ExtensionInterface] on AttributeServicesFixtures\ExtensionInterface\BadExtensionInterface is not supported in directories from the attributeServicesDirectories section - third-party extension interfaces are not supported.', + ]; + + yield [ + 'autowiredExtensionsParam.neon', + 'Attribute #[AutowiredExtensions] on a constructor parameter of class AttributeServicesFixtures\AutowiredExtensions\BadAutowiredExtensionsService is not supported in directories from the attributeServicesDirectories section.', + ]; + + yield [ + 'internalAttribute.neon', + 'Attribute #[ValidatesStubFiles] on class AttributeServicesFixtures\InternalAttribute\UsesValidatesStubFiles is only supported on classes shipped with PHPStan itself, not on classes discovered through the attributeServicesDirectories section.', + ]; + + yield [ + 'unloadable.neon', + 'cannot be autoloaded.', + ]; + } + +} diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/attributeServices.neon b/tests/PHPStan/DependencyInjection/AttributeServices/attributeServices.neon new file mode 100644 index 00000000000..0400db39a29 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/attributeServices.neon @@ -0,0 +1,5 @@ +attributeServicesDirectories: + - data/services + +autowiredAttributeServices: + level: 5 diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/autowiredExtensionsParam.neon b/tests/PHPStan/DependencyInjection/AttributeServices/autowiredExtensionsParam.neon new file mode 100644 index 00000000000..69d81e5be55 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/autowiredExtensionsParam.neon @@ -0,0 +1,2 @@ +attributeServicesDirectories: + - data/autowiredExtensions diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/containerExtension.neon b/tests/PHPStan/DependencyInjection/AttributeServices/containerExtension.neon new file mode 100644 index 00000000000..3a734568639 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/containerExtension.neon @@ -0,0 +1,2 @@ +attributeServicesDirectories: + - data/containerExtension diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/data/autowiredExtensions/BadAutowiredExtensionsService.php b/tests/PHPStan/DependencyInjection/AttributeServices/data/autowiredExtensions/BadAutowiredExtensionsService.php new file mode 100644 index 00000000000..5e5c3197088 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/data/autowiredExtensions/BadAutowiredExtensionsService.php @@ -0,0 +1,24 @@ + $rules + */ + public function __construct( + #[AutowiredExtensions(of: Rule::class)] + private ExtensionsCollection $rules, + ) + { + } + +} diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/data/containerExtension/BadCompilerExtension.php b/tests/PHPStan/DependencyInjection/AttributeServices/data/containerExtension/BadCompilerExtension.php new file mode 100644 index 00000000000..bfde8258ce0 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/data/containerExtension/BadCompilerExtension.php @@ -0,0 +1,11 @@ + + */ +#[RegisteredCollector(level: 3)] +final class DiscoveredCollector implements Collector +{ + + public function getNodeType(): string + { + return Node\Expr\New_::class; + } + + public function processNode(Node $node, Scope $scope): ?string + { + return null; + } + +} diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalRule.php b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalRule.php new file mode 100644 index 00000000000..f8f662b88b4 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalRule.php @@ -0,0 +1,28 @@ + + */ +#[Conditional(attribute: new RegisteredRule(level: 3), if: '%reportUnmatchedIgnoredErrors%')] +final class DiscoveredConditionalRule implements Rule +{ + + public function getNodeType(): string + { + return Node\Expr\New_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + return []; + } + +} diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalRuleNotMet.php b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalRuleNotMet.php new file mode 100644 index 00000000000..39d9f3fc057 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalRuleNotMet.php @@ -0,0 +1,31 @@ + + */ +#[Conditional( + attribute: ['olvlvl\ComposerAttributeCollector\NewExpression' => [RegisteredRule::class, ['level' => 3]]], + if: ['%reportUnmatchedIgnoredErrors%', 'not(%reportUnmatchedIgnoredErrors%)'], +)] +final class DiscoveredConditionalRuleNotMet implements Rule +{ + + public function getNodeType(): string + { + return Node\Expr\New_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + return []; + } + +} diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalService.php b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalService.php new file mode 100644 index 00000000000..5c4464d5df4 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalService.php @@ -0,0 +1,32 @@ + [AutowiredService::class, []]], + if: 'not(%reportUnmatchedIgnoredErrors%)', +)] +final class DiscoveredConditionalService implements ReadWritePropertiesExtension +{ + + public function isAlwaysRead(PropertyReflection $property, string $propertyName): bool + { + return false; + } + + public function isAlwaysWritten(PropertyReflection $property, string $propertyName): bool + { + return false; + } + + public function isInitialized(PropertyReflection $property, string $propertyName): bool + { + return false; + } + +} diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredNamedService.php b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredNamedService.php new file mode 100644 index 00000000000..ba1796092cf --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredNamedService.php @@ -0,0 +1,11 @@ + + */ +#[RegisteredRule(level: 8)] +final class DiscoveredRuleLevelEight implements Rule +{ + + public function getNodeType(): string + { + return Node\Expr\New_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + return []; + } + +} diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredRuleLevelThree.php b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredRuleLevelThree.php new file mode 100644 index 00000000000..f9f8ec86818 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredRuleLevelThree.php @@ -0,0 +1,27 @@ + + */ +#[RegisteredRule(level: 3)] +final class DiscoveredRuleLevelThree implements Rule +{ + + public function getNodeType(): string + { + return Node\Expr\New_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + return []; + } + +} diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredService.php b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredService.php new file mode 100644 index 00000000000..7442a401cda --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredService.php @@ -0,0 +1,48 @@ +currentWorkingDirectory; + } + + public function getTmpDir(): string + { + return $this->tmpDir; + } + + public function isAlwaysRead(PropertyReflection $property, string $propertyName): bool + { + return false; + } + + public function isAlwaysWritten(PropertyReflection $property, string $propertyName): bool + { + return false; + } + + public function isInitialized(PropertyReflection $property, string $propertyName): bool + { + return false; + } + +} diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredValue.php b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredValue.php new file mode 100644 index 00000000000..dcc51755879 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredValue.php @@ -0,0 +1,20 @@ + Date: Mon, 2 Nov 2026 18:49:14 +0000 Subject: [PATCH 4/5] Invalidate the result cache when attribute services change Classes registered from attributeServicesDirectories are services, so a change in them can affect the analysis of every file. AutowiredAttributeServicesExtension stores the registered discovered classes in the attributeServicesClasses parameter. ResultCacheManager treats them like the classes from the services section: editing one of them, or one of their dependencies, invalidates the result cache. Other classes in the same directories do not. A Composer update of a package owning one of the directories invalidates the whole result cache, like it does for packages with services in the config. PackageDependencyResolver::resolveDirectoryPackage() also matches a directory that is a package's install path. Amp-Thread-ID: https://ampcode.com/threads/T-01a19cd4-be31-75b4-bd15-67786eff5983 --- .github/workflows/e2e-tests.yml | 18 +++++ conf/parametersSchema.neon | 1 + .../.gitignore | 2 + .../composer.json | 8 +++ .../phpstan.neon | 8 +++ .../src/Foo.php | 8 +++ .../tmp/.gitignore | 2 + .../ResultCache/ResultCacheManager.php | 68 ++++++++++++------- src/Dependency/PackageDependencyResolver.php | 16 +++++ .../AttributeServicesRegistrar.php | 15 +++- .../AttributeTargetsProvider.php | 18 +++++ .../AutowiredAttributeServicesExtension.php | 20 ++++-- src/DependencyInjection/ContainerFactory.php | 2 + .../ResultCacheManagerCollectedDataTest.php | 2 + .../PackageDependencyResolverTest.php | 14 ++++ .../AttributeServicesIntegrationTest.php | 23 +++++++ 16 files changed, 192 insertions(+), 33 deletions(-) create mode 100644 e2e/result-cache-extension-attribute-services/.gitignore create mode 100644 e2e/result-cache-extension-attribute-services/composer.json create mode 100644 e2e/result-cache-extension-attribute-services/phpstan.neon create mode 100644 e2e/result-cache-extension-attribute-services/src/Foo.php create mode 100644 e2e/result-cache-extension-attribute-services/tmp/.gitignore diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 1775a710833..c957a105144 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -1668,6 +1668,24 @@ jobs: ../bashunit assert contains 'Result cache not used because extension file' "$OUTPUT" ../bashunit assert contains 'the rule says another thing' "$OUTPUT" ../bashunit assert not_contains 'the rule says one thing' "$OUTPUT" + - script: | + cd e2e/result-cache-extension-attribute-services + composer install + # The rule is registered through the attributeServicesDirectories section, not in services:. + # Editing it has to invalidate the cache, editing another class in the same directory does not. + OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse -vv --error-format raw") + echo "$OUTPUT" + ../bashunit assert contains 'the rule says one thing' "$OUTPUT" + patch -p0 < foo.patch + OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse -vv --error-format raw") + echo "$OUTPUT" + ../bashunit assert contains 'Result cache restored. 1 file will be reanalysed.' "$OUTPUT" + patch -p0 < rule.patch + OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse -vv --error-format raw") + echo "$OUTPUT" + ../bashunit assert contains 'Result cache not used because extension file' "$OUTPUT" + ../bashunit assert contains 'the rule says another thing' "$OUTPUT" + ../bashunit assert not_contains 'the rule says one thing' "$OUTPUT" - script: | cd e2e/result-cache-extension-copied-package composer install diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index a0f58e0c5ad..f7a62b602a6 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -219,6 +219,7 @@ parametersSchema: usedLevel: string() cliAutoloadFile: schema(string(), nullable()) attributeServicesDirectories: listOf(string()) + attributeServicesClasses: listOf(string()) # internal - editor mode singleReflectionFile: schema(string(), nullable()) diff --git a/e2e/result-cache-extension-attribute-services/.gitignore b/e2e/result-cache-extension-attribute-services/.gitignore new file mode 100644 index 00000000000..de4a392c331 --- /dev/null +++ b/e2e/result-cache-extension-attribute-services/.gitignore @@ -0,0 +1,2 @@ +/vendor +/composer.lock diff --git a/e2e/result-cache-extension-attribute-services/composer.json b/e2e/result-cache-extension-attribute-services/composer.json new file mode 100644 index 00000000000..3568906bcd4 --- /dev/null +++ b/e2e/result-cache-extension-attribute-services/composer.json @@ -0,0 +1,8 @@ +{ + "name": "phpstan/result-cache-extension-attribute-services-e2e", + "autoload": { + "psr-4": { + "ResultCacheE2EAttributeServices\\": "src/" + } + } +} diff --git a/e2e/result-cache-extension-attribute-services/phpstan.neon b/e2e/result-cache-extension-attribute-services/phpstan.neon new file mode 100644 index 00000000000..23f041e550b --- /dev/null +++ b/e2e/result-cache-extension-attribute-services/phpstan.neon @@ -0,0 +1,8 @@ +attributeServicesDirectories: + - src + +parameters: + level: 0 + tmpDir: tmp + paths: + - src diff --git a/e2e/result-cache-extension-attribute-services/src/Foo.php b/e2e/result-cache-extension-attribute-services/src/Foo.php new file mode 100644 index 00000000000..e74b0b323a8 --- /dev/null +++ b/e2e/result-cache-extension-attribute-services/src/Foo.php @@ -0,0 +1,8 @@ + $attributeServicesClasses classes registered from attributeServicesDirectories * @param string[] $bootstrapFiles * @param string[] $scanFiles * @param string[] $scanDirectories @@ -213,6 +215,10 @@ public function __construct( #[AutowiredParameter] private array $composerAutoloaderProjectPaths, #[AutowiredParameter] + private array $attributeServicesDirectories, + #[AutowiredParameter] + private array $attributeServicesClasses, + #[AutowiredParameter] private string $usedLevel, #[AutowiredParameter] private ?string $cliAutoloadFile, @@ -2805,6 +2811,15 @@ private function readFrame($handle, int $length): mixed */ private function changedPackagesProvideContainerClass(?array $projectConfig, array $changedPackagesLookup): bool { + // Directories from the attributeServicesDirectories section register services straight from + // their classes, so a changed package owning one of them can affect the analysis of every file. + foreach ($this->attributeServicesDirectories as $attributeServicesDirectory) { + $package = $this->packageDependencyResolver->resolveDirectoryPackage($attributeServicesDirectory); + if ($package !== null && array_key_exists($package, $changedPackagesLookup)) { + return true; + } + } + // Extensions registered directly in the project config (services:/rules:) or via an included // extension neon file: resolve each service class to the package that owns its file. if ($projectConfig !== null) { @@ -2849,42 +2864,43 @@ private function getProjectExtensionFiles(?array $projectConfig, array $dependen { $this->alreadyProcessed = []; $projectExtensionFiles = []; + $classes = $this->attributeServicesClasses; if ($projectConfig !== null) { - $classes = ProjectConfigHelper::getServiceClassNames($projectConfig); - foreach ($classes as $class) { - try { - // does not use static reflection to reduce file-parsing - $classReflection = new ReflectionClass($class); /** @phpstan-ignore argument.type */ - } catch (ReflectionException) { - continue; - } + $classes = array_merge(ProjectConfigHelper::getServiceClassNames($projectConfig), $classes); + } + foreach ($classes as $class) { + try { + // does not use static reflection to reduce file-parsing + $classReflection = new ReflectionClass($class); /** @phpstan-ignore argument.type */ + } catch (ReflectionException) { + continue; + } - $fileName = $classReflection->getFileName(); - if ($fileName === false) { - continue; - } + $fileName = $classReflection->getFileName(); + if ($fileName === false) { + continue; + } - if (str_starts_with($fileName, 'phar://')) { + if (str_starts_with($fileName, 'phar://')) { + continue; + } + + $allServiceFiles = $this->getAllDependencies($fileName, $dependencies); + if (count($allServiceFiles) === 0) { + if ($this->packageDependencyResolver->isFileOfInstalledPackage($fileName)) { continue; } - $allServiceFiles = $this->getAllDependencies($fileName, $dependencies); - if (count($allServiceFiles) === 0) { - if ($this->packageDependencyResolver->isFileOfInstalledPackage($fileName)) { - continue; - } + $projectExtensionFiles[$fileName] = [$this->getFileHash($fileName), false, $class]; + continue; + } - $projectExtensionFiles[$fileName] = [$this->getFileHash($fileName), false, $class]; + foreach ($allServiceFiles as $serviceFile) { + if (array_key_exists($serviceFile, $projectExtensionFiles)) { continue; } - foreach ($allServiceFiles as $serviceFile) { - if (array_key_exists($serviceFile, $projectExtensionFiles)) { - continue; - } - - $projectExtensionFiles[$serviceFile] = [$this->getFileHash($serviceFile), true, $class]; - } + $projectExtensionFiles[$serviceFile] = [$this->getFileHash($serviceFile), true, $class]; } } diff --git a/src/Dependency/PackageDependencyResolver.php b/src/Dependency/PackageDependencyResolver.php index e3d85c23f84..b7074040651 100644 --- a/src/Dependency/PackageDependencyResolver.php +++ b/src/Dependency/PackageDependencyResolver.php @@ -20,6 +20,7 @@ use function is_file; use function is_string; use function realpath; +use function rtrim; use function str_starts_with; use function strlen; use function strtolower; @@ -113,6 +114,21 @@ public function isFileOfInstalledPackage(string $file): bool return false; } + /** + * Unlike resolvePackage(), also matches a directory that IS a package's install path. + */ + public function resolveDirectoryPackage(string $directory): ?string + { + $normalizedDirectory = rtrim($this->fileHelper->normalizePath($directory, '/'), '/'); + foreach ($this->getInstallPathToPackage() as $installPath => $package) { + if ($normalizedDirectory === $installPath || str_starts_with($normalizedDirectory, $installPath . '/')) { + return $package; + } + } + + return null; + } + private function doResolvePackage(string $file): ?string { // Normalize with a forward slash regardless of platform: normalizePath() defaults to diff --git a/src/DependencyInjection/AttributeServices/AttributeServicesRegistrar.php b/src/DependencyInjection/AttributeServices/AttributeServicesRegistrar.php index a94d27433a7..0b668e44718 100644 --- a/src/DependencyInjection/AttributeServices/AttributeServicesRegistrar.php +++ b/src/DependencyInjection/AttributeServices/AttributeServicesRegistrar.php @@ -48,8 +48,12 @@ final class AttributeServicesRegistrar { - public static function registerServices(ContainerBuilder $builder, AttributeTargetsProvider $targets, ?int $level): void + /** + * @return list the registered classes + */ + public static function registerServices(ContainerBuilder $builder, AttributeTargetsProvider $targets, ?int $level): array { + $registeredClasses = []; $constructorParameters = self::collectConstructorParameters($targets); $conditionalTargets = []; @@ -73,6 +77,7 @@ public static function registerServices(ContainerBuilder $builder, AttributeTarg $definition->setFactory(new Statement([new Reference(substr($ref, 1)), $method])); } + $registeredClasses[] = $class->name; self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); if (!$attribute->autoTag || !$tagged) { @@ -100,6 +105,7 @@ public static function registerServices(ContainerBuilder $builder, AttributeTarg $definition->setFactory(new Statement([new Reference(substr($ref, 1)), $method])); } + $registeredClasses[] = $class->name; self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); } @@ -113,11 +119,12 @@ public static function registerServices(ContainerBuilder $builder, AttributeTarg } $resultDefinition = $definition->getResultDefinition(); + $registeredClasses[] = $class->name; self::processConstructorParameters($builder, $class->name, $resultDefinition, $constructorParameters); } if ($level === null) { - return; + return $registeredClasses; } foreach (self::findTargetClasses($targets, RegisteredRule::class, $conditionalTargets) as [$class, $tagged]) { @@ -133,6 +140,7 @@ public static function registerServices(ContainerBuilder $builder, AttributeTarg $definition->addTag(LazyRegistry::RULE_TAG); } + $registeredClasses[] = $class->name; self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); } @@ -149,8 +157,11 @@ public static function registerServices(ContainerBuilder $builder, AttributeTarg $definition->addTag(RegistryFactory::COLLECTOR_TAG); } + $registeredClasses[] = $class->name; self::processConstructorParameters($builder, $class->name, $definition, $constructorParameters); } + + return $registeredClasses; } /** diff --git a/src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php b/src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php index 8d486bb1162..f485ae3f4cc 100644 --- a/src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php +++ b/src/DependencyInjection/AttributeServices/AttributeTargetsProvider.php @@ -82,6 +82,24 @@ public function findTargetMethodParameters(string $attributeClass): array return $targets; } + /** + * Whether the class was discovered in the attributeServicesDirectories section + * rather than coming from PHPStan's own vendor/attributes.php. + */ + public function isDiscoveredClass(string $className): bool + { + $lowerClassName = strtolower($className); + foreach ($this->discoveredTargets->targetClasses as $targets) { + foreach ($targets as $target) { + if (strtolower($target->name) === $lowerClassName) { + return true; + } + } + } + + return false; + } + /** * @param TargetMethodParameter $target */ diff --git a/src/DependencyInjection/AutowiredAttributeServicesExtension.php b/src/DependencyInjection/AutowiredAttributeServicesExtension.php index 6e17df6e622..20843285a44 100644 --- a/src/DependencyInjection/AutowiredAttributeServicesExtension.php +++ b/src/DependencyInjection/AutowiredAttributeServicesExtension.php @@ -9,6 +9,7 @@ use PHPStan\DependencyInjection\AttributeServices\AttributeServicesRegistrar; use PHPStan\DependencyInjection\AttributeServices\AttributeTargetsProvider; use stdClass; +use function array_keys; #[ContainerExtension(name: 'autowiredAttributeServices')] final class AutowiredAttributeServicesExtension extends CompilerExtension @@ -27,11 +28,20 @@ public function loadConfiguration(): void { /** @var stdClass&object{level: int|null} $config */ $config = $this->getConfig(); - AttributeServicesRegistrar::registerServices( - $this->getContainerBuilder(), - AttributeTargetsProvider::create(), - $config->level, - ); + $builder = $this->getContainerBuilder(); + $targets = AttributeTargetsProvider::create(); + $registeredClasses = AttributeServicesRegistrar::registerServices($builder, $targets, $config->level); + + // ResultCacheManager treats these like the classes from the services section of the config + $discoveredClasses = []; + foreach ($registeredClasses as $registeredClass) { + if (!$targets->isDiscoveredClass($registeredClass)) { + continue; + } + + $discoveredClasses[$registeredClass] = true; + } + $builder->parameters['attributeServicesClasses'] = array_keys($discoveredClasses); } } diff --git a/src/DependencyInjection/ContainerFactory.php b/src/DependencyInjection/ContainerFactory.php index 60b82077464..0a069d9ca7b 100644 --- a/src/DependencyInjection/ContainerFactory.php +++ b/src/DependencyInjection/ContainerFactory.php @@ -157,6 +157,8 @@ public function create( 'usedLevel' => $usedLevel, 'cliAutoloadFile' => $cliAutoloadFile, 'attributeServicesDirectories' => $attributeServicesDirectories->getDirectoryPaths(), + // filled by AutowiredAttributeServicesExtension + 'attributeServicesClasses' => [], 'env' => Environment::getCleanedArray(), ], $additionalParameters)); $configurator->addDynamicParameters([ diff --git a/tests/PHPStan/Analyser/ResultCache/ResultCacheManagerCollectedDataTest.php b/tests/PHPStan/Analyser/ResultCache/ResultCacheManagerCollectedDataTest.php index 33a477d968b..fd37082cae0 100644 --- a/tests/PHPStan/Analyser/ResultCache/ResultCacheManagerCollectedDataTest.php +++ b/tests/PHPStan/Analyser/ResultCache/ResultCacheManagerCollectedDataTest.php @@ -307,6 +307,8 @@ private function createManager(): ResultCacheManager analysedPaths: [$this->directory . '/src'], analysedPathsFromConfig: [], composerAutoloaderProjectPaths: [], + attributeServicesDirectories: [], + attributeServicesClasses: [], usedLevel: '8', cliAutoloadFile: null, bootstrapFiles: [], diff --git a/tests/PHPStan/Dependency/PackageDependencyResolverTest.php b/tests/PHPStan/Dependency/PackageDependencyResolverTest.php index 5928940c684..d8adb233c6b 100644 --- a/tests/PHPStan/Dependency/PackageDependencyResolverTest.php +++ b/tests/PHPStan/Dependency/PackageDependencyResolverTest.php @@ -25,6 +25,20 @@ public function testResolvePackage(): void $this->assertNull($resolver->resolvePackage('/outside/the/project/File.php')); } + public function testResolveDirectoryPackage(): void + { + $fixtureRoot = __DIR__ . '/data/package-resolver'; + $fileHelper = self::getContainer()->getByType(FileHelper::class); + $resolver = new PackageDependencyResolver([$fixtureRoot], $fileHelper); + + // Unlike resolvePackage(), the install path itself matches too. + $this->assertSame('acme/widget', $resolver->resolveDirectoryPackage($fixtureRoot . '/vendor/acme/widget')); + $this->assertSame('acme/widget', $resolver->resolveDirectoryPackage($fixtureRoot . '/vendor/acme/widget/src')); + + $this->assertNull($resolver->resolveDirectoryPackage($fixtureRoot . '/src')); + $this->assertNull($resolver->resolveDirectoryPackage('/outside/the/project')); + } + public function testExtractComposerPackageVersions(): void { $resolver = new PackageDependencyResolver([], self::getContainer()->getByType(FileHelper::class)); diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php index f8343209c5e..6b44722f5e1 100644 --- a/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php +++ b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php @@ -11,10 +11,12 @@ use AttributeServicesFixtures\DiscoveredRuleLevelEight; use AttributeServicesFixtures\DiscoveredRuleLevelThree; use AttributeServicesFixtures\DiscoveredService; +use AttributeServicesFixtures\DiscoveredValue; use AttributeServicesFixtures\DiscoveredValueFactory; use PHPStan\Collectors\RegistryFactory; use PHPStan\DependencyInjection\Container; use PHPStan\DependencyInjection\ContainerFactory; +use PHPStan\DependencyInjection\DerivativeContainerFactory; use PHPStan\Rules\LazyRegistry; use PHPStan\Rules\Properties\ReadWritePropertiesExtension; use PHPUnit\Framework\Attributes\DataProvider; @@ -123,6 +125,27 @@ public function testDirectoriesParameterIsAbsolutized(): void $this->assertSame([__DIR__ . '/data/services'], $container->getParameter('attributeServicesDirectories')); } + public function testRegisteredDiscoveredClassesParameter(): void + { + $container = self::createContainer(); + $classes = $container->getParameter('attributeServicesClasses'); + $this->assertIsArray($classes); + $this->assertContains(DiscoveredService::class, $classes); + $this->assertContains(DiscoveredRuleLevelThree::class, $classes); + $this->assertContains(DiscoveredConditionalRuleNotMet::class, $classes); + $this->assertNotContains(DiscoveredRuleLevelEight::class, $classes); + $this->assertContains(DiscoveredValue::class, $classes); + } + + public function testDerivativeContainerSeesDiscoveredServices(): void + { + $container = self::createContainer(); + $derivativeContainer = $container->getByType(DerivativeContainerFactory::class) + ->create([dirname(__DIR__, 4) . '/conf/config.stubValidator.neon'], ['allStubFiles' => []]); + $service = $derivativeContainer->getByType(DiscoveredService::class); + $this->assertSame(__DIR__, $service->getCurrentWorkingDirectory()); + } + #[DataProvider('dataErrors')] public function testErrors(string $neonFile, string $expectedMessage): void { From a9340114c212e67b3851d7de300d1a8f313d69a4 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 2 Nov 2026 19:45:48 +0000 Subject: [PATCH 5/5] Fix attribute services tests on PHP < 8.1 and Windows The fixtures with attributes are not valid PHP 7.4 and a class with new in attribute arguments cannot be loaded on PHP 8.0, so lint skips the fixtures, the integration test requires PHP 8.0 and the new fixture lives in its own directory discovered only on PHP 8.1+. The resolver tests pass PHP 8.0 to the resolver so that they run everywhere. Expected paths use forward slashes like the resolver does. Amp-Thread-ID: https://ampcode.com/threads/T-01a19cd4-be31-75b4-bd15-67786eff5983 --- Makefile | 1 + build/baseline-pre-8.0.neon | 13 +++++++--- ...tributeServicesDirectoriesResolverTest.php | 14 +++++++---- .../AttributeServicesIntegrationTest.php | 24 ++++++++++++++++--- .../attributeServicesNewInInitializers.neon | 5 ++++ .../DiscoveredConditionalRule.php | 0 6 files changed, 47 insertions(+), 10 deletions(-) create mode 100644 tests/PHPStan/DependencyInjection/AttributeServices/attributeServicesNewInInitializers.neon rename tests/PHPStan/DependencyInjection/AttributeServices/data/{services => servicesNewInInitializers}/DiscoveredConditionalRule.php (100%) diff --git a/Makefile b/Makefile index d996514c8b7..9fc4d04913a 100644 --- a/Makefile +++ b/Makefile @@ -40,6 +40,7 @@ lint: --exclude tests/PHPStan/Rules/Operators/data/invalid-assign-var.php \ --exclude tests/PHPStan/Rules/Functions/data/arrow-function-nullsafe-by-ref.php \ --exclude tests/PHPStan/Levels/data/namedArguments.php \ + --exclude tests/PHPStan/DependencyInjection/AttributeServices/data \ --exclude tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/symbol-finder/class-name-before-global-constant.php \ --exclude tests/PHPStan/Rules/Keywords/data/continue-break.php \ --exclude tests/PHPStan/Rules/Keywords/data/continue-break-property-hook.php \ diff --git a/build/baseline-pre-8.0.neon b/build/baseline-pre-8.0.neon index ec8ec032c1f..d24f1f80b9b 100644 --- a/build/baseline-pre-8.0.neon +++ b/build/baseline-pre-8.0.neon @@ -37,9 +37,16 @@ parameters: path: ../src/DependencyInjection/AttributeServices/AttributeServicesDiscoverer.php - - message: '#^(Parameter \$attribute of method .+ has invalid type|Call to method \w+\(\) on an unknown class) ReflectionAttribute\.$#' - count: 5 - path: ../src/DependencyInjection/AttributeServices/AttributeServicesDiscoverer.php + rawMessage: 'Class AttributeServicesFixtures\DiscoveredConditionalRuleNotMet not found.' + identifier: class.notFound + count: 3 + path: ../tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php + + - + rawMessage: 'Class AttributeServicesFixtures\DiscoveredConditionalService not found.' + identifier: class.notFound + count: 1 + path: ../tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php - rawMessage: 'Call to an undefined method ReflectionClass::isReadOnly().' diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php index c4db4b7ce94..a52747ca94f 100644 --- a/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php +++ b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesDirectoriesResolverTest.php @@ -9,17 +9,23 @@ use function dirname; use function hash_file; use function str_starts_with; -use const PHP_VERSION_ID; +use function strtr; class AttributeServicesDirectoriesResolverTest extends TestCase { + /** + * With forward slashes like the paths the resolver returns, also on Windows. + */ private static function getRepoRoot(): string { - return dirname(__DIR__, 4); + return strtr(dirname(__DIR__, 4), '\\', '/'); } - private function createResolver(int $phpVersionId = PHP_VERSION_ID): AttributeServicesDirectoriesResolver + /** + * PHP 8.0 by default so that the tests run on every PHP version - resolving needs no runtime attributes. + */ + private function createResolver(int $phpVersionId = 80000): AttributeServicesDirectoriesResolver { $repoRoot = self::getRepoRoot(); @@ -83,7 +89,7 @@ public function testMissingDirectory(): void public function testDirectoryOutsideComposerProjects(): void { - $resolver = new AttributeServicesDirectoriesResolver(new FileHelper(self::getRepoRoot()), []); + $resolver = new AttributeServicesDirectoriesResolver(new FileHelper(self::getRepoRoot()), [], 80000); $this->expectException(InvalidAttributeServicesDirectoriesException::class); $this->expectExceptionMessage('is not inside any project with Composer metadata known to PHPStan'); $resolver->resolve([__DIR__]); diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php index 6b44722f5e1..c707e950bd1 100644 --- a/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php +++ b/tests/PHPStan/DependencyInjection/AttributeServices/AttributeServicesIntegrationTest.php @@ -20,14 +20,18 @@ use PHPStan\Rules\LazyRegistry; use PHPStan\Rules\Properties\ReadWritePropertiesExtension; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\RequiresPhp; use PHPUnit\Framework\TestCase; use function array_filter; use function dirname; use function md5; use function mkdir; +use function strtr; use function sys_get_temp_dir; use function uniqid; +use const PHP_VERSION_ID; +#[RequiresPhp('>= 8.0.0')] class AttributeServicesIntegrationTest extends TestCase { @@ -44,7 +48,10 @@ private static function createContainer(): Container self::$tmpDir = self::createTmpDir(); $containerFactory = new ContainerFactory(__DIR__); - return self::$container = $containerFactory->create(self::$tmpDir, [__DIR__ . '/attributeServices.neon'], [], [dirname(__DIR__, 4)]); + // PHP 8.0 cannot even load a class with `new` in attribute arguments + $configFile = PHP_VERSION_ID >= 80100 ? 'attributeServicesNewInInitializers.neon' : 'attributeServices.neon'; + + return self::$container = $containerFactory->create(self::$tmpDir, [__DIR__ . '/' . $configFile], [], [dirname(__DIR__, 4)]); } private static function createTmpDir(): string @@ -91,11 +98,18 @@ public function testDiscoveredRulesFollowTheLevel(): void $this->assertCount(0, array_filter($rules, static fn ($rule): bool => $rule instanceof DiscoveredRuleLevelEight)); } - public function testDiscoveredConditional(): void + #[RequiresPhp('>= 8.1.0')] + public function testDiscoveredConditionalWithNewInAttributeArguments(): void { $container = self::createContainer(); $rules = $container->getServicesByTag(LazyRegistry::RULE_TAG); $this->assertCount(1, array_filter($rules, static fn ($rule): bool => $rule instanceof DiscoveredConditionalRule)); + } + + public function testDiscoveredConditional(): void + { + $container = self::createContainer(); + $rules = $container->getServicesByTag(LazyRegistry::RULE_TAG); $this->assertCount(0, array_filter($rules, static fn ($rule): bool => $rule instanceof DiscoveredConditionalRuleNotMet)); // the services are registered even when the condition is not met, only their tags are skipped @@ -122,7 +136,11 @@ public function testDiscoveredGeneratedFactory(): void public function testDirectoriesParameterIsAbsolutized(): void { $container = self::createContainer(); - $this->assertSame([__DIR__ . '/data/services'], $container->getParameter('attributeServicesDirectories')); + $expected = [strtr(__DIR__, '\\', '/') . '/data/services']; + if (PHP_VERSION_ID >= 80100) { + $expected[] = strtr(__DIR__, '\\', '/') . '/data/servicesNewInInitializers'; + } + $this->assertSame($expected, $container->getParameter('attributeServicesDirectories')); } public function testRegisteredDiscoveredClassesParameter(): void diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/attributeServicesNewInInitializers.neon b/tests/PHPStan/DependencyInjection/AttributeServices/attributeServicesNewInInitializers.neon new file mode 100644 index 00000000000..a5e3d5b26d7 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/AttributeServices/attributeServicesNewInInitializers.neon @@ -0,0 +1,5 @@ +includes: + - attributeServices.neon + +attributeServicesDirectories: + - data/servicesNewInInitializers diff --git a/tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalRule.php b/tests/PHPStan/DependencyInjection/AttributeServices/data/servicesNewInInitializers/DiscoveredConditionalRule.php similarity index 100% rename from tests/PHPStan/DependencyInjection/AttributeServices/data/services/DiscoveredConditionalRule.php rename to tests/PHPStan/DependencyInjection/AttributeServices/data/servicesNewInInitializers/DiscoveredConditionalRule.php