From 25c779159e528dfabde191731e6472a123dfecba Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 26 May 2020 11:00:02 +0200 Subject: [PATCH] AutoloadSourceLocator - give all found identifiers in an autoloaded file --- src/Analyser/NodeScopeResolver.php | 26 ++-- .../SourceLocator/AutoloadSourceLocator.php | 134 ++++++++++++++---- .../OptimizedDirectorySourceLocator.php | 2 +- src/Testing/TestCase.php | 3 +- .../Analyser/AnalyserIntegrationTest.php | 14 +- .../Analyser/NodeScopeResolverTest.php | 1 - .../Analyser/data/two-same-classes.php | 14 +- .../AutoloadSourceLocatorTest.php | 20 ++- .../BetterReflection/SourceLocator/data/a.php | 4 + .../ExistingClassInTraitUseRuleTest.php | 1 - .../Generics/ClassTemplateTypeRuleTest.php | 3 +- .../FunctionSignatureVarianceRuleTest.php | 1 - 12 files changed, 168 insertions(+), 55 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 2fefae24cb..a19adb13d8 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -531,20 +531,20 @@ private function processStmtNode( if (!$betterReflectionClass instanceof \Roave\BetterReflection\Reflection\ReflectionClass) { throw new \PHPStan\ShouldNotHappenException(); } - $classScope = $scope->enterClass( - new ClassReflection( - $this->reflectionProvider, - $this->fileTypeMapper, - $this->classReflectionExtensionRegistryProvider->getRegistry()->getPropertiesClassReflectionExtensions(), - $this->classReflectionExtensionRegistryProvider->getRegistry()->getMethodsClassReflectionExtensions(), - $betterReflectionClass->getName(), - new ReflectionClass($betterReflectionClass), - null, - null, - null, - sprintf('%s:%d', $scope->getFile(), $stmt->getStartLine()) - ) + $classReflection = new ClassReflection( + $this->reflectionProvider, + $this->fileTypeMapper, + $this->classReflectionExtensionRegistryProvider->getRegistry()->getPropertiesClassReflectionExtensions(), + $this->classReflectionExtensionRegistryProvider->getRegistry()->getMethodsClassReflectionExtensions(), + $betterReflectionClass->getName(), + new ReflectionClass($betterReflectionClass), + null, + null, + null, + sprintf('%s:%d', $scope->getFile(), $stmt->getStartLine()) ); + $this->reflectionProvider->hasClass($classReflection->getName()); + $classScope = $scope->enterClass($classReflection); } elseif ($stmt instanceof Class_) { if ($stmt->name === null) { throw new \PHPStan\ShouldNotHappenException(); diff --git a/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php index 10fcfb2d9c..dee6a00353 100644 --- a/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php @@ -20,6 +20,7 @@ use Roave\BetterReflection\SourceLocator\Ast\Strategy\NodeToReflection; use Roave\BetterReflection\SourceLocator\Located\LocatedSource; use Roave\BetterReflection\SourceLocator\Type\SourceLocator; +use function array_key_exists; use function file_exists; use function restore_error_handler; @@ -40,6 +41,18 @@ class AutoloadSourceLocator implements SourceLocator private static ?FileNodesFetcher $currentFileNodesFetcher = null; + /** @var array> */ + private array $classNodes = []; + + /** @var array> */ + private array $functionNodes = []; + + /** @var array> */ + private array $constantNodes = []; + + /** @var array */ + private array $locatedSourcesByFile = []; + /** * Note: the constructor has been made a 0-argument constructor because `\stream_wrapper_register` * is a piece of trash, and doesn't accept instances, just class names. @@ -63,6 +76,16 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): { if ($identifier->isFunction()) { $functionName = $identifier->getName(); + $loweredFunctionName = strtolower($functionName); + if (array_key_exists($loweredFunctionName, $this->functionNodes)) { + $nodeToReflection = new NodeToReflection(); + return $nodeToReflection->__invoke( + $reflector, + $this->functionNodes[$loweredFunctionName]->getNode(), + $this->locatedSourcesByFile[$this->functionNodes[$loweredFunctionName]->getFileName()], + $this->functionNodes[$loweredFunctionName]->getNamespace() + ); + } if (!function_exists($functionName)) { return null; } @@ -79,6 +102,51 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): if ($identifier->isConstant()) { $constantName = $identifier->getName(); + $nodeToReflection = new NodeToReflection(); + foreach ($this->constantNodes as $stmtConst) { + if ($stmtConst->getNode() instanceof FuncCall) { + $constantReflection = $nodeToReflection->__invoke( + $reflector, + $stmtConst->getNode(), + $this->locatedSourcesByFile[$stmtConst->getFileName()], + $stmtConst->getNamespace() + ); + if ($constantReflection === null) { + continue; + } + if (!$constantReflection instanceof ReflectionConstant) { + throw new \PHPStan\ShouldNotHappenException(); + } + if ($constantReflection->getName() !== $identifier->getName()) { + continue; + } + + return $constantReflection; + } + + foreach (array_keys($stmtConst->getNode()->consts) as $i) { + $constantReflection = $nodeToReflection->__invoke( + $reflector, + $stmtConst->getNode(), + $this->locatedSourcesByFile[$stmtConst->getFileName()], + $stmtConst->getNamespace(), + $i + ); + if ($constantReflection === null) { + continue; + } + if (!$constantReflection instanceof ReflectionConstant) { + throw new \PHPStan\ShouldNotHappenException(); + } + if ($constantReflection->getName() !== $identifier->getName()) { + continue; + } + + return $constantReflection; + } + } + + if (!defined($constantName)) { return null; } @@ -102,6 +170,17 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): return null; } + $loweredClassName = strtolower($identifier->getName()); + if (array_key_exists($loweredClassName, $this->classNodes)) { + $nodeToReflection = new NodeToReflection(); + return $nodeToReflection->__invoke( + $reflector, + $this->classNodes[$loweredClassName]->getNode(), + $this->locatedSourcesByFile[$this->classNodes[$loweredClassName]->getFileName()], + $this->classNodes[$loweredClassName]->getNamespace() + ); + } + $locateResult = $this->locateClassByName($identifier->getName()); if ($locateResult === null) { return null; @@ -114,36 +193,43 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): private function findReflection(Reflector $reflector, string $file, Identifier $identifier): ?Reflection { $result = $this->fileNodesFetcher->fetchNodes($file); + $this->locatedSourcesByFile[$file] = $result->getLocatedSource(); + foreach ($result->getClassNodes() as $className => $fetchedClassNode) { + $this->classNodes[$className] = $fetchedClassNode; + } + foreach ($result->getFunctionNodes() as $functionName => $fetchedFunctionNode) { + $this->functionNodes[$functionName] = $fetchedFunctionNode; + } + foreach ($result->getConstantNodes() as $fetchedConstantNode) { + $this->constantNodes[] = $fetchedConstantNode; + } + $nodeToReflection = new NodeToReflection(); if ($identifier->isClass()) { - foreach ($result->getClassNodes() as $fetchedFunctionNode) { - $reflection = $nodeToReflection->__invoke( - $reflector, - $fetchedFunctionNode->getNode(), - $result->getLocatedSource(), - $fetchedFunctionNode->getNamespace() - ); - if ($reflection === null) { - continue; - } - - return $reflection; + $identifierName = strtolower($identifier->getName()); + if (!array_key_exists($identifierName, $this->classNodes)) { + return null; } + + return $nodeToReflection->__invoke( + $reflector, + $this->classNodes[$identifierName]->getNode(), + $result->getLocatedSource(), + $this->classNodes[$identifierName]->getNamespace() + ); } if ($identifier->isFunction()) { - foreach ($result->getFunctionNodes() as $fetchedFunctionNode) { - $reflection = $nodeToReflection->__invoke( - $reflector, - $fetchedFunctionNode->getNode(), - $result->getLocatedSource(), - $fetchedFunctionNode->getNamespace() - ); - if ($reflection === null) { - continue; - } - - return $reflection; + $identifierName = strtolower($identifier->getName()); + if (!array_key_exists($identifierName, $this->functionNodes)) { + return null; } + + return $nodeToReflection->__invoke( + $reflector, + $this->functionNodes[$identifierName]->getNode(), + $result->getLocatedSource(), + $this->functionNodes[$identifierName]->getNamespace() + ); } return null; diff --git a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php index c648cd510a..3f91f5031e 100644 --- a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php @@ -33,7 +33,7 @@ class OptimizedDirectorySourceLocator implements SourceLocator private array $functionNodes = []; /** @var array */ - private array $locatedSourcesByFile; + private array $locatedSourcesByFile = []; public function __construct( FileNodesFetcher $fileNodesFetcher, diff --git a/src/Testing/TestCase.php b/src/Testing/TestCase.php index 1a6a90e00d..8c9c85c040 100644 --- a/src/Testing/TestCase.php +++ b/src/Testing/TestCase.php @@ -38,6 +38,7 @@ use PHPStan\Reflection\BetterReflection\Reflector\MemoizingFunctionReflector; use PHPStan\Reflection\BetterReflection\SourceLocator\AutoloadSourceLocator; use PHPStan\Reflection\BetterReflection\SourceLocator\ComposerJsonAndInstalledJsonSourceLocatorMaker; +use PHPStan\Reflection\BetterReflection\SourceLocator\FileNodesFetcher; use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\FunctionReflectionFactory; use PHPStan\Reflection\Mixin\MixinMethodsClassReflectionExtension; @@ -429,7 +430,7 @@ public static function getReflectors(): array }); $reflectionSourceStubber = new ReflectionSourceStubber(); $locators[] = new PhpInternalSourceLocator($astLocator, new PhpStormStubsSourceStubber($phpParser)); - $locators[] = new AutoloadSourceLocator($astLocator); + $locators[] = new AutoloadSourceLocator(self::getContainer()->getByType(FileNodesFetcher::class)); $locators[] = new PhpInternalSourceLocator($astLocator, $reflectionSourceStubber); $locators[] = new EvaledCodeSourceLocator($astLocator, $reflectionSourceStubber); $sourceLocator = new MemoizingSourceLocator(new AggregateSourceLocator($locators)); diff --git a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php index 2cbbc29326..5d01ec4cd9 100644 --- a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php +++ b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php @@ -64,8 +64,14 @@ public function testExtendingUnknownClass(): void { $errors = $this->runAnalyse(__DIR__ . '/data/extending-unknown-class.php'); $this->assertCount(1, $errors); - $this->assertSame(5, $errors[0]->getLine()); - $this->assertSame('Class ExtendingUnknownClass\Foo extends unknown class ExtendingUnknownClass\Bar.', $errors[0]->getMessage()); + + if (self::$useStaticReflectionProvider) { + $this->assertSame(5, $errors[0]->getLine()); + $this->assertSame('Class ExtendingUnknownClass\Foo extends unknown class ExtendingUnknownClass\Bar.', $errors[0]->getMessage()); + } else { + $this->assertNull($errors[0]->getLine()); + $this->assertSame('Class ExtendingUnknownClass\Bar not found and could not be autoloaded.', $errors[0]->getMessage()); + } } public function testExtendingKnownClassWithCheck(): void @@ -201,11 +207,11 @@ public function testTwoSameClassesInSingleFile(): void $error = $errors[1]; $this->assertSame('Property TwoSame\Foo::$prop (int) does not accept default value of type string.', $error->getMessage()); - $this->assertSame(17, $error->getLine()); + $this->assertSame(18, $error->getLine()); $error = $errors[2]; $this->assertSame('Property TwoSame\Foo::$prop2 (int) does not accept default value of type string.', $error->getMessage()); - $this->assertSame(20, $error->getLine()); + $this->assertSame(21, $error->getLine()); } /** diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index c09b326e47..ed4ba2b729 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -9018,7 +9018,6 @@ public function testDynamicConstants( string $expression ): void { - require_once __DIR__ . '/data/dynamic-constant.php'; $this->assertTypes( __DIR__ . '/data/dynamic-constant.php', $description, diff --git a/tests/PHPStan/Analyser/data/two-same-classes.php b/tests/PHPStan/Analyser/data/two-same-classes.php index d369b7f38b..bd5043fcc5 100644 --- a/tests/PHPStan/Analyser/data/two-same-classes.php +++ b/tests/PHPStan/Analyser/data/two-same-classes.php @@ -10,13 +10,15 @@ class Foo } -class Foo -{ +if (rand(0, 0)) { + class Foo + { - /** @var int */ - private $prop = 'str'; + /** @var int */ + private $prop = 'str'; - /** @var int */ - private $prop2 = 'str'; + /** @var int */ + private $prop2 = 'str'; + } } diff --git a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocatorTest.php b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocatorTest.php index f993091011..cd6d66cd78 100644 --- a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocatorTest.php +++ b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocatorTest.php @@ -4,10 +4,11 @@ use PHPStan\Testing\TestCase; use Roave\BetterReflection\Reflector\ClassReflector; +use Roave\BetterReflection\Reflector\ConstantReflector; use Roave\BetterReflection\Reflector\FunctionReflector; use TestSingleFileSourceLocator\AFoo; -function testFunctionForLocator() +function testFunctionForLocator(): void // phpcs:disable { } @@ -22,11 +23,28 @@ public function testAutoloadEverythingInFile(): void $locator = new AutoloadSourceLocator(self::getContainer()->getByType(FileNodesFetcher::class)); $classReflector = new ClassReflector($locator); $functionReflector = new FunctionReflector($locator, $classReflector); + $constantReflector = new ConstantReflector($locator, $classReflector); $aFoo = $classReflector->reflect(AFoo::class); + $this->assertNotNull($aFoo->getFileName()); $this->assertSame('a.php', basename($aFoo->getFileName())); $testFunctionReflection = $functionReflector->reflect('PHPStan\\Reflection\\BetterReflection\\SourceLocator\testFunctionForLocator'); $this->assertSame(__FILE__, $testFunctionReflection->getFileName()); + + $someConstant = $constantReflector->reflect('TestSingleFileSourceLocator\\SOME_CONSTANT'); + $this->assertNotNull($someConstant->getFileName()); + $this->assertSame('a.php', basename($someConstant->getFileName())); + $this->assertSame(1, $someConstant->getValue()); + + $anotherConstant = $constantReflector->reflect('TestSingleFileSourceLocator\\ANOTHER_CONSTANT'); + $this->assertNotNull($anotherConstant->getFileName()); + $this->assertSame('a.php', basename($anotherConstant->getFileName())); + $this->assertSame(2, $anotherConstant->getValue()); + + $doFooFunctionReflection = $functionReflector->reflect('TestSingleFileSourceLocator\\doFoo'); + $this->assertSame('TestSingleFileSourceLocator\\doFoo', $doFooFunctionReflection->getName()); + $this->assertNotNull($doFooFunctionReflection->getFileName()); + $this->assertSame('a.php', basename($doFooFunctionReflection->getFileName())); } } diff --git a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/a.php b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/a.php index 17f63b35c7..ad3461a804 100644 --- a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/a.php +++ b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/a.php @@ -11,3 +11,7 @@ function doFoo() { } + +define('TestSingleFileSourceLocator\\SOME_CONSTANT', 1); + +const ANOTHER_CONSTANT = 2; diff --git a/tests/PHPStan/Rules/Classes/ExistingClassInTraitUseRuleTest.php b/tests/PHPStan/Rules/Classes/ExistingClassInTraitUseRuleTest.php index 7f822fc66f..42f73d58ef 100644 --- a/tests/PHPStan/Rules/Classes/ExistingClassInTraitUseRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ExistingClassInTraitUseRuleTest.php @@ -22,7 +22,6 @@ protected function getRule(): Rule public function testClassWithWrongCase(): void { - require_once __DIR__ . '/data/trait-use.php'; $this->analyse([__DIR__ . '/data/trait-use.php'], [ [ 'Trait TraitUseCase\FooTrait referenced with incorrect case: TraitUseCase\FOOTrait.', diff --git a/tests/PHPStan/Rules/Generics/ClassTemplateTypeRuleTest.php b/tests/PHPStan/Rules/Generics/ClassTemplateTypeRuleTest.php index c705753661..b17ce01b1d 100644 --- a/tests/PHPStan/Rules/Generics/ClassTemplateTypeRuleTest.php +++ b/tests/PHPStan/Rules/Generics/ClassTemplateTypeRuleTest.php @@ -39,8 +39,7 @@ public function testRule(): void 24, ], [ - //'Class ClassTemplateType\Baz referenced with incorrect case: ClassTemplateType\baz.', - 'PHPDoc tag @template T for class ClassTemplateType\Lorem has invalid bound type ClassTemplateType\baz.', + 'Class ClassTemplateType\Baz referenced with incorrect case: ClassTemplateType\baz.', 32, ], [ diff --git a/tests/PHPStan/Rules/Generics/FunctionSignatureVarianceRuleTest.php b/tests/PHPStan/Rules/Generics/FunctionSignatureVarianceRuleTest.php index 194c3ab9ce..b693fd4a96 100644 --- a/tests/PHPStan/Rules/Generics/FunctionSignatureVarianceRuleTest.php +++ b/tests/PHPStan/Rules/Generics/FunctionSignatureVarianceRuleTest.php @@ -22,7 +22,6 @@ protected function getRule(): Rule public function testRule(): void { - require_once __DIR__ . '/data/function-signature-variance.php'; $this->analyse([__DIR__ . '/data/function-signature-variance.php'], [ [ 'Template type T is declared as covariant, but occurs in contravariant position in parameter a of function FunctionSignatureVariance\f().',