From bd537473abdfc80b2ef90451db0d06d7c71f8bd0 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Wed, 5 Aug 2026 08:39:41 -0500 Subject: [PATCH 1/4] Migrate PluginManagerInspectionRule to InClassNode InClassNode hands the rule its ClassReflection directly, which drops the ReflectionProvider constructor dependency, the anonymous-class namespacedName juggling, and the manual FQN round-trip through ObjectType. Behavior is unchanged; the rule now simply never fires for classes PHPStan cannot reflect. Co-Authored-By: Claude Fable 5 --- .../Classes/PluginManagerInspectionRule.php | 50 ++++++++----------- .../Rules/PluginManagerInspectionRuleTest.php | 4 +- 2 files changed, 23 insertions(+), 31 deletions(-) diff --git a/src/Rules/Classes/PluginManagerInspectionRule.php b/src/Rules/Classes/PluginManagerInspectionRule.php index 8ad311ce..406a7b57 100644 --- a/src/Rules/Classes/PluginManagerInspectionRule.php +++ b/src/Rules/Classes/PluginManagerInspectionRule.php @@ -7,53 +7,49 @@ use PhpParser\Node; use PhpParser\NodeFinder; use PHPStan\Analyser\Scope; -use PHPStan\Reflection\ReflectionProvider; +use PHPStan\Node\InClassNode; +use PHPStan\Reflection\ClassReflection; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use PHPStan\Type\ObjectType; use function sprintf; +use function str_contains; +use function strtolower; /** - * @implements \PHPStan\Rules\Rule<\PhpParser\Node\Stmt\Class_> + * @implements Rule */ class PluginManagerInspectionRule implements Rule { - /** @var ReflectionProvider */ - private $reflectionProvider; - public function __construct(ReflectionProvider $reflectionProvider) - { - $this->reflectionProvider = $reflectionProvider; - } - public function getNodeType(): string { - return Node\Stmt\Class_::class; + return InClassNode::class; } public function processNode(Node $node, Scope $scope): array { - if ($node->namespacedName === null) { - // anonymous class + $classReflection = $node->getClassReflection(); + if ($classReflection->isAnonymous()) { + return []; + } + $originalNode = $node->getOriginalNode(); + if (!$originalNode instanceof Node\Stmt\Class_) { return []; } - if ($node->extends === null) { + if ($originalNode->extends === null) { return []; } - if (str_contains($node->namespacedName->toLowerString(), 'test')) { + if (str_contains(strtolower($classReflection->getName()), 'test')) { return []; } - $pluginManagerType = $scope->resolveTypeByName($node->namespacedName); - $pluginManagerInterfaceType = new ObjectType(PluginManagerInterface::class); - if (!$pluginManagerInterfaceType->isSuperTypeOf($pluginManagerType)->yes()) { + if (!$classReflection->is(PluginManagerInterface::class)) { return []; } - $defaultPluginManager = new ObjectType(DefaultPluginManager::class); - if ($defaultPluginManager->equals($pluginManagerType)) { + if ($classReflection->getName() === DefaultPluginManager::class) { return []; } - $constructorMethodNode = (new NodeFinder())->findFirst($node->stmts, static function (Node $node) { + $constructorMethodNode = (new NodeFinder())->findFirst($originalNode->stmts, static function (Node $node) { return $node instanceof Node\Stmt\ClassMethod && $node->name->toString() === '__construct'; }); if (!$constructorMethodNode instanceof Node\Stmt\ClassMethod) { @@ -61,8 +57,8 @@ public function processNode(Node $node, Scope $scope): array } $errors = []; - if ($this->isYamlDiscovery($node)) { - $errors = $this->inspectYamlPluginManager($node, $constructorMethodNode); + if ($this->isYamlDiscovery($originalNode)) { + $errors = $this->inspectYamlPluginManager($classReflection, $constructorMethodNode); } else { // @todo inspect annotated plugin managers. } @@ -79,7 +75,6 @@ public function processNode(Node $node, Scope $scope): array 'Plugin managers should call alterInfo to allow plugin definitions to be altered.' ) ->tip('For example, to invoke hook_mymodule_data_alter() call alterInfo with "mymodule_data".') - ->line($node->getStartLine()) ->identifier('pluginManagerInspection.alterInfoMissing') ->build(); } @@ -113,13 +108,12 @@ private function isYamlDiscovery(Node\Stmt\Class_ $class): bool /** * @return list<\PHPStan\Rules\IdentifierRuleError> */ - private function inspectYamlPluginManager(Node\Stmt\Class_ $class, Node\Stmt\ClassMethod $constructorMethodNode): array + private function inspectYamlPluginManager(ClassReflection $classReflection, Node\Stmt\ClassMethod $constructorMethodNode): array { $errors = []; - $fqn = (string) $class->namespacedName; - $reflection = $this->reflectionProvider->getClass($fqn); - $constructor = $reflection->getConstructor(); + $fqn = $classReflection->getName(); + $constructor = $classReflection->getConstructor(); if ($constructor->getDeclaringClass()->getName() !== $fqn) { $errors[] = RuleErrorBuilder::message( diff --git a/tests/src/Rules/PluginManagerInspectionRuleTest.php b/tests/src/Rules/PluginManagerInspectionRuleTest.php index b4df446f..4fafb99b 100644 --- a/tests/src/Rules/PluginManagerInspectionRuleTest.php +++ b/tests/src/Rules/PluginManagerInspectionRuleTest.php @@ -11,9 +11,7 @@ final class PluginManagerInspectionRuleTest extends DrupalRuleTestCase protected function getRule(): Rule { - return new PluginManagerInspectionRule( - self::createReflectionProvider() - ); + return new PluginManagerInspectionRule(); } /** From 7940a085f6150f32c5479d6c585783d0e6c3b23d Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Wed, 5 Aug 2026 08:39:41 -0500 Subject: [PATCH 2/4] Align legacy rules and reflections with current PHPStan API conventions - GlobalDrupalDependencyInjectionRule: resolve the called class through $scope->resolveName() instead of requiring a FullyQualified node, and type-check against the MethodReflection interface instead of ExtendedMethodReflection, which is marked api-do-not-implement. - EntityFieldReflection: guard ReflectionProvider::getClass() with hasClass() so a missing Drupal interface degrades instead of throwing. - EntityFieldsViaMagicReflectionExtension: check interfaces via ClassReflection::implementsInterface() instead of rebuilding an ObjectType from the class name, which discarded generics and allocated on every property lookup; drop the now-unused static helpers. - FieldItemListPropertyReflection: build nullable types with TypeCombinator::addNull() instead of new UnionType. - Drop ->line($node->getStartLine()) calls that restate the default. Co-Authored-By: Claude Fable 5 --- src/Reflection/EntityFieldReflection.php | 6 ++++++ .../EntityFieldsViaMagicReflectionExtension.php | 16 ++-------------- .../FieldItemListPropertyReflection.php | 7 +++---- .../GlobalDrupalDependencyInjectionRule.php | 6 +++--- src/Rules/Drupal/LoadIncludes.php | 3 --- src/Rules/Drupal/ModuleLoadInclude.php | 3 --- 6 files changed, 14 insertions(+), 27 deletions(-) diff --git a/src/Reflection/EntityFieldReflection.php b/src/Reflection/EntityFieldReflection.php index 0c087dab..f12a10de 100644 --- a/src/Reflection/EntityFieldReflection.php +++ b/src/Reflection/EntityFieldReflection.php @@ -60,11 +60,17 @@ public function getReadableType(): Type private function isContentEntityType(): bool { + if (!$this->reflectionProvider->hasClass(ContentEntityInterface::class)) { + return false; + } return $this->declaringClass->isSubclassOfClass($this->reflectionProvider->getClass(ContentEntityInterface::class)); } private function isConfigEntityType(): bool { + if (!$this->reflectionProvider->hasClass(ConfigEntityInterface::class)) { + return false; + } return $this->declaringClass->isSubclassOfClass($this->reflectionProvider->getClass(ConfigEntityInterface::class)); } diff --git a/src/Reflection/EntityFieldsViaMagicReflectionExtension.php b/src/Reflection/EntityFieldsViaMagicReflectionExtension.php index 09c264e7..e9c74359 100644 --- a/src/Reflection/EntityFieldsViaMagicReflectionExtension.php +++ b/src/Reflection/EntityFieldsViaMagicReflectionExtension.php @@ -7,8 +7,6 @@ use PHPStan\Reflection\PropertyReflection; use PHPStan\Reflection\ReflectionProvider; use PHPStan\ShouldNotHappenException; -use PHPStan\Type\IsSuperTypeOfResult; -use PHPStan\Type\ObjectType; use function array_key_exists; /** @@ -52,7 +50,7 @@ public function hasProperty(ClassReflection $classReflection, string $propertyNa // Content entities have magical __get... so it is kind of true. return true; } - if (self::classObjectIsSuperOfInterface($classReflection->getName(), self::getFieldItemListInterfaceObject())->yes()) { + if ($classReflection->implementsInterface('Drupal\Core\Field\FieldItemListInterface')) { return FieldItemListPropertyReflection::canHandleProperty($classReflection, $propertyName); } @@ -64,20 +62,10 @@ public function getProperty(ClassReflection $classReflection, string $propertyNa if ($classReflection->implementsInterface('Drupal\Core\Entity\EntityInterface')) { return new EntityFieldReflection($classReflection, $propertyName, $this->reflectionProvider); } - if (self::classObjectIsSuperOfInterface($classReflection->getName(), self::getFieldItemListInterfaceObject())->yes()) { + if ($classReflection->implementsInterface('Drupal\Core\Field\FieldItemListInterface')) { return new FieldItemListPropertyReflection($classReflection, $propertyName); } throw new ShouldNotHappenException($classReflection->getName() . "::$propertyName should be handled earlier."); } - - public static function classObjectIsSuperOfInterface(string $name, ObjectType $interfaceObject) : IsSuperTypeOfResult - { - return $interfaceObject->isSuperTypeOf(new ObjectType($name)); - } - - protected static function getFieldItemListInterfaceObject() : ObjectType - { - return new ObjectType('Drupal\Core\Field\FieldItemListInterface'); - } } diff --git a/src/Reflection/FieldItemListPropertyReflection.php b/src/Reflection/FieldItemListPropertyReflection.php index d83362ba..2b1670d3 100644 --- a/src/Reflection/FieldItemListPropertyReflection.php +++ b/src/Reflection/FieldItemListPropertyReflection.php @@ -6,11 +6,10 @@ use PHPStan\Reflection\PropertyReflection; use PHPStan\TrinaryLogic; use PHPStan\Type\MixedType; -use PHPStan\Type\NullType; use PHPStan\Type\ObjectType; use PHPStan\Type\StringType; use PHPStan\Type\Type; -use PHPStan\Type\UnionType; +use PHPStan\Type\TypeCombinator; /** * Allows field access via magic methods @@ -42,7 +41,7 @@ public static function canHandleProperty(ClassReflection $classReflection, strin public function getReadableType(): Type { if ($this->propertyName === 'entity') { - return new UnionType([new ObjectType('Drupal\Core\Entity\EntityInterface'), new NullType()]); + return TypeCombinator::addNull(new ObjectType('Drupal\Core\Entity\EntityInterface')); } if ($this->propertyName === 'target_id') { // @todo needs to be union type. @@ -60,7 +59,7 @@ public function getReadableType(): Type public function getWritableType(): Type { if ($this->propertyName === 'entity') { - return new UnionType([new ObjectType('Drupal\Core\Entity\EntityInterface'), new NullType()]); + return TypeCombinator::addNull(new ObjectType('Drupal\Core\Entity\EntityInterface')); } if ($this->propertyName === 'target_id') { return new StringType(); diff --git a/src/Rules/Drupal/GlobalDrupalDependencyInjectionRule.php b/src/Rules/Drupal/GlobalDrupalDependencyInjectionRule.php index 0b80ba6b..a5e0b6aa 100644 --- a/src/Rules/Drupal/GlobalDrupalDependencyInjectionRule.php +++ b/src/Rules/Drupal/GlobalDrupalDependencyInjectionRule.php @@ -4,7 +4,7 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; -use PHPStan\Reflection\ExtendedMethodReflection; +use PHPStan\Reflection\MethodReflection; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -21,7 +21,7 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { // Only check static calls to \Drupal - if (!($node->class instanceof Node\Name\FullyQualified) || (string) $node->class !== 'Drupal') { + if (!$node->class instanceof Node\Name || $scope->resolveName($node->class) !== 'Drupal') { return []; } // Do not raise if called inside a trait. @@ -61,7 +61,7 @@ public function processNode(Node $node, Scope $scope): array if ($scopeFunction === null) { return []; } - if (!$scopeFunction instanceof ExtendedMethodReflection) { + if (!$scopeFunction instanceof MethodReflection) { return []; } if ($scopeFunction->isStatic()) { diff --git a/src/Rules/Drupal/LoadIncludes.php b/src/Rules/Drupal/LoadIncludes.php index d12c3974..eb48e47c 100644 --- a/src/Rules/Drupal/LoadIncludes.php +++ b/src/Rules/Drupal/LoadIncludes.php @@ -58,7 +58,6 @@ public function processNode(Node $node, Scope $scope): array ModuleHandlerInterface::class, $moduleName )) - ->line($node->getStartLine()) ->identifier('loadIncludes.moduleNotFound') ->build() ]; @@ -79,7 +78,6 @@ public function processNode(Node $node, Scope $scope): array 'A file could not be loaded from %s::loadInclude', ModuleHandlerInterface::class )) - ->line($node->getStartLine()) ->identifier('loadIncludes.fileNotLoadable') ->build() ]; @@ -92,7 +90,6 @@ public function processNode(Node $node, Scope $scope): array $module->getPath() . '/' . $filename, ModuleHandlerInterface::class )) - ->line($node->getStartLine()) ->identifier('loadIncludes.fileNotLoadable') ->build() ]; diff --git a/src/Rules/Drupal/ModuleLoadInclude.php b/src/Rules/Drupal/ModuleLoadInclude.php index ae04f48d..7f70831d 100644 --- a/src/Rules/Drupal/ModuleLoadInclude.php +++ b/src/Rules/Drupal/ModuleLoadInclude.php @@ -56,7 +56,6 @@ public function processNode(Node $node, Scope $scope): array $filename, $moduleName )) - ->line($node->getStartLine()) ->identifier('moduleLoadInclude.moduleNotFound') ->build() ]; @@ -73,7 +72,6 @@ public function processNode(Node $node, Scope $scope): array } catch (Throwable $e) { return [ RuleErrorBuilder::message('A file could not be loaded from module_load_include') - ->line($node->getStartLine()) ->identifier('moduleLoadInclude.moduleNotLoadable') ->build() ]; @@ -85,7 +83,6 @@ public function processNode(Node $node, Scope $scope): array 'File %s could not be loaded from module_load_include.', $module->getPath() . '/' . $filename )) - ->line($node->getStartLine()) ->identifier('moduleLoadInclude.moduleNotLoadable') ->build() ]; From 089c9d8f2856a2ed4006ff9a32ced5f3279514c3 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Tue, 8 Sep 2026 14:45:51 -0500 Subject: [PATCH 3/4] Use ClassReflection::is() for the FieldItemListInterface check implementsInterface() returns false when asked about the interface itself, so values typed as FieldItemListInterface, which is what $node->uid is, lost their magic entity and target_id properties. is() covers the interface and every implementation. Co-Authored-By: Claude Fable 5.1 --- ...ntityFieldsViaMagicReflectionExtension.php | 5 ++-- ...yFieldsViaMagicReflectionExtensionTest.php | 23 ++++++++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/Reflection/EntityFieldsViaMagicReflectionExtension.php b/src/Reflection/EntityFieldsViaMagicReflectionExtension.php index e9c74359..22a837c8 100644 --- a/src/Reflection/EntityFieldsViaMagicReflectionExtension.php +++ b/src/Reflection/EntityFieldsViaMagicReflectionExtension.php @@ -2,6 +2,7 @@ namespace mglaman\PHPStanDrupal\Reflection; +use Drupal\Core\Field\FieldItemListInterface; use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\PropertiesClassReflectionExtension; use PHPStan\Reflection\PropertyReflection; @@ -50,7 +51,7 @@ public function hasProperty(ClassReflection $classReflection, string $propertyNa // Content entities have magical __get... so it is kind of true. return true; } - if ($classReflection->implementsInterface('Drupal\Core\Field\FieldItemListInterface')) { + if ($classReflection->is(FieldItemListInterface::class)) { return FieldItemListPropertyReflection::canHandleProperty($classReflection, $propertyName); } @@ -62,7 +63,7 @@ public function getProperty(ClassReflection $classReflection, string $propertyNa if ($classReflection->implementsInterface('Drupal\Core\Entity\EntityInterface')) { return new EntityFieldReflection($classReflection, $propertyName, $this->reflectionProvider); } - if ($classReflection->implementsInterface('Drupal\Core\Field\FieldItemListInterface')) { + if ($classReflection->is(FieldItemListInterface::class)) { return new FieldItemListPropertyReflection($classReflection, $propertyName); } diff --git a/tests/src/Reflection/EntityFieldsViaMagicReflectionExtensionTest.php b/tests/src/Reflection/EntityFieldsViaMagicReflectionExtensionTest.php index 143dd9fb..d4e1d7b2 100644 --- a/tests/src/Reflection/EntityFieldsViaMagicReflectionExtensionTest.php +++ b/tests/src/Reflection/EntityFieldsViaMagicReflectionExtensionTest.php @@ -84,6 +84,18 @@ public static function dataHasProperty(): \Generator 'value', false, ]; + // Values typed as the interface itself, such as $node->uid, are the + // common case and must be handled like the concrete class. + yield 'field item list interface: entity' => [ + \Drupal\Core\Field\FieldItemListInterface::class, + 'entity', + true, + ]; + yield 'field item list interface: target_id' => [ + \Drupal\Core\Field\FieldItemListInterface::class, + 'target_id', + true, + ]; yield 'field item list: format' => [ \Drupal\Core\Field\FieldItemList::class, 'format', @@ -125,6 +137,15 @@ public function testGetPropertyFieldItemList(): void $readableType = $propertyReflection->getReadableType(); self::assertInstanceOf(MixedType::class, $readableType); } - + + public function testGetPropertyFieldItemListInterface(): void + { + $classReflection = $this->createReflectionProvider()->getClass(FieldItemListInterface::class); + $propertyReflection = $this->extension->getProperty($classReflection, 'entity'); + $readableType = $propertyReflection->getReadableType(); + self::assertSame('Drupal\Core\Entity\EntityInterface|null', $readableType->describe(VerbosityLevel::typeOnly())); + $propertyReflection = $this->extension->getProperty($classReflection, 'target_id'); + self::assertInstanceOf(StringType::class, $propertyReflection->getReadableType()); + } } From dbca0c762eb134fdb1ea3a15532e403acb9338d0 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Tue, 8 Sep 2026 15:22:56 -0500 Subject: [PATCH 4/4] Do not crash PluginManagerInspectionRule on a nested anonymous class constructor The constructor lookup searched the whole class body recursively, so a constructor declared by an anonymous class inside a method was taken as the plugin manager's own. With YAML discovery that reached ClassReflection::getConstructor() on a hierarchy with no constructor, which throws. Look up the class's own method and guard with hasConstructor(). Co-Authored-By: Claude Fable 5.1 --- .../Classes/PluginManagerInspectionRule.php | 11 +++++--- .../Rules/PluginManagerInspectionRuleTest.php | 4 +++ .../plugin-manager-nested-constructor.php | 26 +++++++++++++++++++ 3 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 tests/src/Rules/data/plugin-manager-nested-constructor.php diff --git a/src/Rules/Classes/PluginManagerInspectionRule.php b/src/Rules/Classes/PluginManagerInspectionRule.php index 406a7b57..4981faa8 100644 --- a/src/Rules/Classes/PluginManagerInspectionRule.php +++ b/src/Rules/Classes/PluginManagerInspectionRule.php @@ -49,10 +49,10 @@ public function processNode(Node $node, Scope $scope): array return []; } - $constructorMethodNode = (new NodeFinder())->findFirst($originalNode->stmts, static function (Node $node) { - return $node instanceof Node\Stmt\ClassMethod && $node->name->toString() === '__construct'; - }); - if (!$constructorMethodNode instanceof Node\Stmt\ClassMethod) { + // Only look at the class's own methods. A recursive search would also + // match a constructor declared by an anonymous class nested in a method. + $constructorMethodNode = $originalNode->getMethod('__construct'); + if ($constructorMethodNode === null) { return []; } @@ -113,6 +113,9 @@ private function inspectYamlPluginManager(ClassReflection $classReflection, Node $errors = []; $fqn = $classReflection->getName(); + if (!$classReflection->hasConstructor()) { + return $errors; + } $constructor = $classReflection->getConstructor(); if ($constructor->getDeclaringClass()->getName() !== $fqn) { diff --git a/tests/src/Rules/PluginManagerInspectionRuleTest.php b/tests/src/Rules/PluginManagerInspectionRuleTest.php index 4fafb99b..fc796d22 100644 --- a/tests/src/Rules/PluginManagerInspectionRuleTest.php +++ b/tests/src/Rules/PluginManagerInspectionRuleTest.php @@ -34,6 +34,10 @@ public static function pluginManagerData(): \Generator __DIR__ . '/data/plugin-manager-valid.php', [] ]; + yield 'nested anonymous class constructor does not crash' => [ + __DIR__ . '/data/plugin-manager-nested-constructor.php', + [] + ]; yield [ __DIR__ . '/data/plugin-manager-alter-info.php', [ diff --git a/tests/src/Rules/data/plugin-manager-nested-constructor.php b/tests/src/Rules/data/plugin-manager-nested-constructor.php new file mode 100644 index 00000000..83eeccf3 --- /dev/null +++ b/tests/src/Rules/data/plugin-manager-nested-constructor.php @@ -0,0 +1,26 @@ +discovery = new YamlDiscovery('foo', []); + return $this->discovery; + } + + public function helper(): object { + return new class { + public function __construct() {} + }; + } +}