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..22a837c8 100644 --- a/src/Reflection/EntityFieldsViaMagicReflectionExtension.php +++ b/src/Reflection/EntityFieldsViaMagicReflectionExtension.php @@ -2,13 +2,12 @@ namespace mglaman\PHPStanDrupal\Reflection; +use Drupal\Core\Field\FieldItemListInterface; use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\PropertiesClassReflectionExtension; 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 +51,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->is(FieldItemListInterface::class)) { return FieldItemListPropertyReflection::canHandleProperty($classReflection, $propertyName); } @@ -64,20 +63,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->is(FieldItemListInterface::class)) { 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/Classes/PluginManagerInspectionRule.php b/src/Rules/Classes/PluginManagerInspectionRule.php index 8ad311ce..4981faa8 100644 --- a/src/Rules/Classes/PluginManagerInspectionRule.php +++ b/src/Rules/Classes/PluginManagerInspectionRule.php @@ -7,62 +7,58 @@ 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 []; } - if ($node->extends === null) { + $originalNode = $node->getOriginalNode(); + if (!$originalNode instanceof Node\Stmt\Class_) { return []; } - if (str_contains($node->namespacedName->toLowerString(), 'test')) { + if ($originalNode->extends === null) { + return []; + } + 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) { - 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 []; } $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,15 @@ 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(); + if (!$classReflection->hasConstructor()) { + return $errors; + } + $constructor = $classReflection->getConstructor(); if ($constructor->getDeclaringClass()->getName() !== $fqn) { $errors[] = RuleErrorBuilder::message( 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() ]; 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()); + } } diff --git a/tests/src/Rules/PluginManagerInspectionRuleTest.php b/tests/src/Rules/PluginManagerInspectionRuleTest.php index b4df446f..fc796d22 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(); } /** @@ -36,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() {} + }; + } +}