Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement RequireExtendsPropertiesClassReflectionExtension #2856

Merged
merged 1 commit into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,12 @@ services:
-
class: PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedSingleFileSourceLocatorRepository

-
class: PHPStan\Reflection\RequireExtension\RequireExtendsMethodsClassReflectionExtension

-
class: PHPStan\Reflection\RequireExtension\RequireExtendsPropertiesClassReflectionExtension

-
class: PHPStan\Reflection\Mixin\MixinMethodsClassReflectionExtension
tags:
Expand Down
2 changes: 2 additions & 0 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -1693,6 +1693,8 @@ private function createAstClassReflection(Node\Stmt\ClassLike $stmt, string $cla
$this->classReflectionExtensionRegistryProvider->getRegistry()->getPropertiesClassReflectionExtensions(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getMethodsClassReflectionExtensions(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getAllowedSubTypesClassReflectionExtensions(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getRequireExtendsPropertyClassReflectionExtension(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getRequireExtendsMethodsClassReflectionExtension(),
$betterReflectionClass->getName(),
$betterReflectionClass instanceof ReflectionEnum && PHP_VERSION_ID >= 80000 ? new $enumAdapter($betterReflectionClass) : new ReflectionClass($betterReflectionClass),
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
use PHPStan\Reflection\Annotations\AnnotationsPropertiesClassReflectionExtension;
use PHPStan\Reflection\ClassReflectionExtensionRegistry;
use PHPStan\Reflection\Php\PhpClassReflectionExtension;
use PHPStan\Reflection\RequireExtension\RequireExtendsMethodsClassReflectionExtension;
use PHPStan\Reflection\RequireExtension\RequireExtendsPropertiesClassReflectionExtension;
use function array_merge;

class LazyClassReflectionExtensionRegistryProvider implements ClassReflectionExtensionRegistryProvider
Expand All @@ -32,6 +34,8 @@ public function getRegistry(): ClassReflectionExtensionRegistry
array_merge([$phpClassReflectionExtension], $this->container->getServicesByTag(BrokerFactory::PROPERTIES_CLASS_REFLECTION_EXTENSION_TAG), [$annotationsPropertiesClassReflectionExtension]),
array_merge([$phpClassReflectionExtension], $this->container->getServicesByTag(BrokerFactory::METHODS_CLASS_REFLECTION_EXTENSION_TAG), [$annotationsMethodsClassReflectionExtension]),
$this->container->getServicesByTag(BrokerFactory::ALLOWED_SUB_TYPES_CLASS_REFLECTION_EXTENSION_TAG),
$this->container->getByType(RequireExtendsPropertiesClassReflectionExtension::class),
$this->container->getByType(RequireExtendsMethodsClassReflectionExtension::class),
);
}

Expand Down
4 changes: 4 additions & 0 deletions src/Reflection/BetterReflection/BetterReflectionProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ public function getClass(string $className): ClassReflection
$this->classReflectionExtensionRegistryProvider->getRegistry()->getPropertiesClassReflectionExtensions(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getMethodsClassReflectionExtensions(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getAllowedSubTypesClassReflectionExtensions(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getRequireExtendsPropertyClassReflectionExtension(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getRequireExtendsMethodsClassReflectionExtension(),
$reflectionClass->getName(),
$reflectionClass instanceof ReflectionEnum && PHP_VERSION_ID >= 80000 ? new $enumAdapter($reflectionClass) : new ReflectionClass($reflectionClass),
null,
Expand Down Expand Up @@ -221,6 +223,8 @@ public function getAnonymousClassReflection(Node\Stmt\Class_ $classNode, Scope $
$this->classReflectionExtensionRegistryProvider->getRegistry()->getPropertiesClassReflectionExtensions(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getMethodsClassReflectionExtensions(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getAllowedSubTypesClassReflectionExtensions(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getRequireExtendsPropertyClassReflectionExtension(),
$this->classReflectionExtensionRegistryProvider->getRegistry()->getRequireExtendsMethodsClassReflectionExtension(),
sprintf('class@anonymous/%s:%s', $filename, $classNode->getLine()),
new ReflectionClass($reflectionClass),
$scopeFile,
Expand Down
33 changes: 33 additions & 0 deletions src/Reflection/ClassReflection.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
use PHPStan\Reflection\Php\PhpClassReflectionExtension;
use PHPStan\Reflection\Php\PhpPropertyReflection;
use PHPStan\Reflection\Php\UniversalObjectCratesClassReflectionExtension;
use PHPStan\Reflection\RequireExtension\RequireExtendsMethodsClassReflectionExtension;
use PHPStan\Reflection\RequireExtension\RequireExtendsPropertiesClassReflectionExtension;
use PHPStan\Reflection\SignatureMap\SignatureMapProvider;
use PHPStan\ShouldNotHappenException;
use PHPStan\Type\CircularTypeAliasDefinitionException;
Expand Down Expand Up @@ -146,6 +148,8 @@ public function __construct(
private array $propertiesClassReflectionExtensions,
private array $methodsClassReflectionExtensions,
private array $allowedSubTypesClassReflectionExtensions,
private RequireExtendsPropertiesClassReflectionExtension $requireExtendsPropertiesClassReflectionExtension,
private RequireExtendsMethodsClassReflectionExtension $requireExtendsMethodsClassReflectionExtension,
private string $displayName,
private ReflectionClass|ReflectionEnum $reflection,
private ?string $anonymousFilename,
Expand Down Expand Up @@ -435,6 +439,10 @@ public function hasProperty(string $propertyName): bool
}
}

if ($this->requireExtendsPropertiesClassReflectionExtension->hasProperty($this, $propertyName)) {
return true;
}

return false;
}

Expand All @@ -446,6 +454,10 @@ public function hasMethod(string $methodName): bool
}
}

if ($this->requireExtendsMethodsClassReflectionExtension->hasMethod($this, $methodName)) {
return true;
}

return false;
}

Expand All @@ -455,6 +467,7 @@ public function getMethod(string $methodName, ClassMemberAccessAnswerer $scope):
if ($scope->isInClass()) {
$key = sprintf('%s-%s', $key, $scope->getClassReflection()->getCacheKey());
}

if (!isset($this->methods[$key])) {
foreach ($this->methodsClassReflectionExtensions as $extension) {
if (!$extension->hasMethod($this, $methodName)) {
Expand All @@ -469,6 +482,13 @@ public function getMethod(string $methodName, ClassMemberAccessAnswerer $scope):
}
}

if (!isset($this->methods[$key])) {
if ($this->requireExtendsMethodsClassReflectionExtension->hasMethod($this, $methodName)) {
$method = $this->requireExtendsMethodsClassReflectionExtension->getMethod($this, $methodName);
$this->methods[$key] = $method;
staabm marked this conversation as resolved.
Show resolved Hide resolved
}
}

if (!isset($this->methods[$key])) {
throw new MissingMethodFromReflectionException($this->getName(), $methodName);
}
Expand Down Expand Up @@ -577,11 +597,13 @@ public function getProperty(string $propertyName, ClassMemberAccessAnswerer $sco
if ($scope->isInClass()) {
$key = sprintf('%s-%s', $key, $scope->getClassReflection()->getCacheKey());
}

if (!isset($this->properties[$key])) {
foreach ($this->propertiesClassReflectionExtensions as $i => $extension) {
if ($i > 0 && !$this->allowsDynamicPropertiesExtensions()) {
break;
}

if (!$extension->hasProperty($this, $propertyName)) {
continue;
}
Expand All @@ -594,6 +616,13 @@ public function getProperty(string $propertyName, ClassMemberAccessAnswerer $sco
}
}

if (!isset($this->properties[$key])) {
if ($this->requireExtendsPropertiesClassReflectionExtension->hasProperty($this, $propertyName)) {
$property = $this->requireExtendsPropertiesClassReflectionExtension->getProperty($this, $propertyName);
$this->properties[$key] = $property;
staabm marked this conversation as resolved.
Show resolved Hide resolved
}
}

if (!isset($this->properties[$key])) {
throw new MissingPropertyFromReflectionException($this->getName(), $propertyName);
}
Expand Down Expand Up @@ -1410,6 +1439,8 @@ public function withTypes(array $types): self
$this->propertiesClassReflectionExtensions,
$this->methodsClassReflectionExtensions,
$this->allowedSubTypesClassReflectionExtensions,
$this->requireExtendsPropertiesClassReflectionExtension,
$this->requireExtendsMethodsClassReflectionExtension,
$this->displayName,
$this->reflection,
$this->anonymousFilename,
Expand Down Expand Up @@ -1437,6 +1468,8 @@ public function withVariances(array $variances): self
$this->propertiesClassReflectionExtensions,
$this->methodsClassReflectionExtensions,
$this->allowedSubTypesClassReflectionExtensions,
$this->requireExtendsPropertiesClassReflectionExtension,
$this->requireExtendsMethodsClassReflectionExtension,
$this->displayName,
$this->reflection,
$this->anonymousFilename,
Expand Down
14 changes: 14 additions & 0 deletions src/Reflection/ClassReflectionExtensionRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
namespace PHPStan\Reflection;

use PHPStan\Broker\Broker;
use PHPStan\Reflection\RequireExtension\RequireExtendsMethodsClassReflectionExtension;
use PHPStan\Reflection\RequireExtension\RequireExtendsPropertiesClassReflectionExtension;
use function array_merge;

class ClassReflectionExtensionRegistry
Expand All @@ -18,6 +20,8 @@ public function __construct(
private array $propertiesClassReflectionExtensions,
private array $methodsClassReflectionExtensions,
private array $allowedSubTypesClassReflectionExtensions,
private RequireExtendsPropertiesClassReflectionExtension $requireExtendsPropertiesClassReflectionExtension,
private RequireExtendsMethodsClassReflectionExtension $requireExtendsMethodsClassReflectionExtension,
)
{
foreach (array_merge($propertiesClassReflectionExtensions, $methodsClassReflectionExtensions, $allowedSubTypesClassReflectionExtensions) as $extension) {
Expand Down Expand Up @@ -53,4 +57,14 @@ public function getAllowedSubTypesClassReflectionExtensions(): array
return $this->allowedSubTypesClassReflectionExtensions;
}

public function getRequireExtendsPropertyClassReflectionExtension(): RequireExtendsPropertiesClassReflectionExtension
{
return $this->requireExtendsPropertiesClassReflectionExtension;
}

public function getRequireExtendsMethodsClassReflectionExtension(): RequireExtendsMethodsClassReflectionExtension
{
return $this->requireExtendsMethodsClassReflectionExtension;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php declare(strict_types = 1);

namespace PHPStan\Reflection\RequireExtension;

use PHPStan\Analyser\OutOfClassScope;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\ExtendedMethodReflection;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Reflection\MethodsClassReflectionExtension;
use PHPStan\ShouldNotHappenException;

class RequireExtendsMethodsClassReflectionExtension implements MethodsClassReflectionExtension
{

public function hasMethod(ClassReflection $classReflection, string $methodName): bool
{
return $this->findMethod($classReflection, $methodName) !== null;
}

/**
* @return ExtendedMethodReflection
*/
public function getMethod(ClassReflection $classReflection, string $methodName): MethodReflection
staabm marked this conversation as resolved.
Show resolved Hide resolved
{
$method = $this->findMethod($classReflection, $methodName);
if ($method === null) {
throw new ShouldNotHappenException();
}

return $method;
}

/**
* @return ExtendedMethodReflection|null
*/
private function findMethod(ClassReflection $classReflection, string $methodName): ?MethodReflection
{
if (!$classReflection->isInterface()) {
return null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a point for the rules - @...-require-extends can only be above an interface.

Copy link
Contributor Author

@staabm staabm Jan 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so do you mean we should still try to infer the property across the type hierarchy, even though the require-extends is placed on a invalid location?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't get it. This comment was just a reminder for you to check whether require-extends is always above an interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rules will be topic for a followup PR. thanks for noticing.

}

$extendsTags = $classReflection->getRequireExtendsTags();
foreach ($extendsTags as $extendsTag) {
$type = $extendsTag->getType();

if (!$type->hasMethod($methodName)->yes()) {
continue;
}

return $type->getMethod($methodName, new OutOfClassScope());
}

$interfaces = $classReflection->getInterfaces();
foreach ($interfaces as $interface) {
$method = $this->findMethod($interface, $methodName);
if ($method !== null) {
return $method;
}
}

return null;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?php declare(strict_types = 1);

namespace PHPStan\Reflection\RequireExtension;

use PHPStan\Analyser\OutOfClassScope;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\PropertiesClassReflectionExtension;
use PHPStan\Reflection\PropertyReflection;
use PHPStan\ShouldNotHappenException;

class RequireExtendsPropertiesClassReflectionExtension implements PropertiesClassReflectionExtension
staabm marked this conversation as resolved.
Show resolved Hide resolved
{

public function hasProperty(ClassReflection $classReflection, string $propertyName): bool
{
return $this->findProperty($classReflection, $propertyName) !== null;
}

public function getProperty(ClassReflection $classReflection, string $propertyName): PropertyReflection
{
$property = $this->findProperty($classReflection, $propertyName);
if ($property === null) {
throw new ShouldNotHappenException();
}

return $property;
}

private function findProperty(ClassReflection $classReflection, string $propertyName): ?PropertyReflection
{
if (!$classReflection->isInterface()) {
return null;
}

$requireExtendsTags = $classReflection->getRequireExtendsTags();
foreach ($requireExtendsTags as $requireExtendsTag) {
$type = $requireExtendsTag->getType();

if (!$type->hasProperty($propertyName)->yes()) {
continue;
}

return $type->getProperty($propertyName, new OutOfClassScope());
}

$interfaces = $classReflection->getInterfaces();
foreach ($interfaces as $interface) {
$property = $this->findProperty($interface, $propertyName);
if ($property !== null) {
return $property;
}
}

return null;
}

}
2 changes: 2 additions & 0 deletions src/Rules/PhpDoc/InvalidPHPStanDocTagRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ class InvalidPHPStanDocTagRule implements Rule
'@phpstan-allow-private-mutation',
'@phpstan-readonly',
'@phpstan-readonly-allow-private-mutation',
'@phpstan-require-extends',
'@phpstan-require-implements',
];

public function __construct(
Expand Down
3 changes: 3 additions & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1419,6 +1419,9 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-5961.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-10189.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-10317.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-10302-interface-extends.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-10302-trait-extends.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-10302-trait-implements.php');
}

/**
Expand Down
53 changes: 53 additions & 0 deletions tests/PHPStan/Analyser/data/bug-10302-interface-extends.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

namespace Bug10302InterfaceExtends;

use function PHPStan\Testing\assertType;

/**
* @phpstan-require-extends SomeClass
*/
interface SampleInterface
{
}

class SomeClass {
public int $x;
protected string $y;
private array $z = [];

public function doFoo():int
{
return 1;
}
}

function test(SampleInterface $test): void
{
assertType('int', $test->x);
assertType('string', $test->y);
assertType('array', $test->z);

assertType('int', $test->doFoo());
}

function testExtendedInterface(AnotherInterface $test): void
{
assertType('int', $test->x);
assertType('string', $test->y);
assertType('array', $test->z);

assertType('int', $test->doFoo());
}

interface AnotherInterface extends SampleInterface
{
}

class SomeSubClass extends SomeClass {}

class ValidClass extends SomeClass implements SampleInterface {}

class ValidSubClass extends SomeSubClass implements SampleInterface {}

class InvalidClass implements SampleInterface {}