From b1cabc05283599aaf858679939673dace5e6b1c6 Mon Sep 17 00:00:00 2001 From: Orklah Date: Sat, 1 Feb 2020 09:22:48 +0100 Subject: [PATCH] various improvements for psalm --- psalm-baseline.xml | 21 +------ src/Reflection/ReflectionClass.php | 7 ++- src/Reflection/ReflectionFunctionAbstract.php | 61 ++++++++++--------- .../Ast/FindReflectionsInTree.php | 19 +++--- .../PhpStormStubsSourceStubber.php | 29 +++++++-- .../SourceStubber/ReflectionSourceStubber.php | 1 + .../Type/AutoloadSourceLocator.php | 5 +- ...LocatorForComposerJsonAndInstalledJson.php | 1 + .../Factory/MakeLocatorForInstalledJson.php | 1 + .../PhpStormStubsSourceStubberTest.php | 5 +- .../Type/PhpInternalSourceLocatorTest.php | 1 + 11 files changed, 87 insertions(+), 64 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 1f5038be0..9eaf1af11 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,5 +1,5 @@ - + $this->sourceStubber @@ -40,10 +40,6 @@ - - $this->node === null - self::$parser - $returnType @@ -91,18 +87,6 @@ BuilderHelpers::normalizeValue($constantReflection->getValue()) - - $extensionConstants - - - $extensionConstants[$constantName] - - - $extensionConstants[$constantName] - - - $extensionConstants[$constantName] - $classReflection->getTraitAliases() @@ -117,9 +101,6 @@ ConstantNodeChecker::assertValidDefineFunctionCall($node) - - get_defined_constants(true)['user'] - diff --git a/src/Reflection/ReflectionClass.php b/src/Reflection/ReflectionClass.php index 76b845d5b..19d345fff 100644 --- a/src/Reflection/ReflectionClass.php +++ b/src/Reflection/ReflectionClass.php @@ -186,6 +186,8 @@ public function getShortName() : string /** * Get the "full" name of the class (e.g. for A\B\Foo, this will return * "A\B\Foo"). + * + * @psalm-return class-string */ public function getName() : string { @@ -817,6 +819,8 @@ public function getParentClass() : ?ReflectionClass * Gets the parent class names. * * @return string[] A numerical array with parent class names as the values. + * + * @psalm-return list */ public function getParentClassNames() : array { @@ -1178,7 +1182,8 @@ public function isIterateable() : bool /** * @return ReflectionClass[] indexed by interface name - * @return array + * + * @psalm-return array */ private function getCurrentClassImplementedInterfacesIndexedByName() : array { diff --git a/src/Reflection/ReflectionFunctionAbstract.php b/src/Reflection/ReflectionFunctionAbstract.php index 83fd9218e..7048b39b9 100644 --- a/src/Reflection/ReflectionFunctionAbstract.php +++ b/src/Reflection/ReflectionFunctionAbstract.php @@ -49,13 +49,13 @@ abstract class ReflectionFunctionAbstract /** @var LocatedSource */ private $locatedSource; - /** @var Node\Stmt\ClassMethod|Node\Stmt\Function_|Node\Expr\Closure */ + /** @var Node\Stmt\ClassMethod|Node\Stmt\Function_|Node\Expr\Closure|null */ private $node; /** @var Reflector */ private $reflector; - /** @var Parser */ + /** @var Parser|null */ private static $parser; protected function __construct() @@ -86,10 +86,12 @@ protected function populateFunctionAbstract( /** * Get the AST node from which this function was created * - * @return Node\Stmt\ClassMethod|Node\Stmt\Function_|Node\FunctionLike + * @return Node\Expr\Closure|Node\Stmt\ClassMethod|Node\Stmt\Function_ */ protected function getNode() : Node\FunctionLike { + assert($this->node !== null); + return $this->node; } @@ -100,9 +102,9 @@ protected function getNode() : Node\FunctionLike private function setNodeOptionalFlag() : void { $overallOptionalFlag = true; - $lastParamIndex = count($this->node->params) - 1; + $lastParamIndex = count($this->getNode()->params) - 1; for ($i = $lastParamIndex; $i >= 0; $i--) { - $hasDefault = ($this->node->params[$i]->default !== null); + $hasDefault = ($this->getNode()->params[$i]->default !== null); // When we find the first parameter that does not have a default, // flip the flag as all params for this are no longer optional @@ -111,7 +113,7 @@ private function setNodeOptionalFlag() : void $overallOptionalFlag = false; } - $this->node->params[$i]->isOptional = $overallOptionalFlag; + $this->getNode()->params[$i]->isOptional = $overallOptionalFlag; } } @@ -134,11 +136,12 @@ public function getName() : string */ public function getShortName() : string { - if ($this->node instanceof Node\Expr\Closure) { + $initializedNode = $this->getNode(); + if ($initializedNode instanceof Node\Expr\Closure) { return self::CLOSURE_NAME; } - return $this->node->name->name; + return $initializedNode->name->name; } /** @@ -199,7 +202,9 @@ public function getParameters() : array { $parameters = []; - foreach ($this->node->params as $paramIndex => $paramNode) { + /** @var list $nodeParams */ + $nodeParams = $this->getNode()->params; + foreach ($nodeParams as $paramIndex => $paramNode) { $parameters[] = ReflectionParameter::createFromNode( $this->reflector, $paramNode, @@ -229,7 +234,7 @@ public function getParameter(string $parameterName) : ?ReflectionParameter public function getDocComment() : string { - return GetFirstDocComment::forNode($this->node); + return GetFirstDocComment::forNode($this->getNode()); } public function setDocCommentFromString(string $string) : void @@ -353,7 +358,7 @@ public function isGenerator() : bool */ public function getStartLine() : int { - return $this->node->getStartLine(); + return $this->getNode()->getStartLine(); } /** @@ -361,17 +366,17 @@ public function getStartLine() : int */ public function getEndLine() : int { - return $this->node->getEndLine(); + return $this->getNode()->getEndLine(); } public function getStartColumn() : int { - return CalculateReflectionColum::getStartColumn($this->locatedSource->getSource(), $this->node); + return CalculateReflectionColum::getStartColumn($this->locatedSource->getSource(), $this->getNode()); } public function getEndColumn() : int { - return CalculateReflectionColum::getEndColumn($this->locatedSource->getSource(), $this->node); + return CalculateReflectionColum::getEndColumn($this->locatedSource->getSource(), $this->getNode()); } /** @@ -379,7 +384,7 @@ public function getEndColumn() : int */ public function returnsReference() : bool { - return $this->node->byRef; + return $this->getNode()->byRef; } /** @@ -400,7 +405,7 @@ public function getDocBlockReturnTypes() : array */ public function getReturnType() : ?ReflectionType { - $returnType = $this->node->getReturnType(); + $returnType = $this->getNode()->getReturnType(); if ($returnType === null) { return null; @@ -426,7 +431,7 @@ public function hasReturnType() : bool */ public function setReturnType(string $newReturnType) : void { - $this->node->returnType = new Node\Name($newReturnType); + $this->getNode()->returnType = new Node\Name($newReturnType); } /** @@ -434,7 +439,7 @@ public function setReturnType(string $newReturnType) : void */ public function removeReturnType() : void { - $this->node->returnType = null; + $this->getNode()->returnType = null; } /** @@ -452,7 +457,7 @@ public function __clone() */ public function getBodyAst() : array { - return $this->node->stmts ?: []; + return $this->getNode()->stmts ?: []; } /** @@ -481,7 +486,7 @@ public function getBodyCode(?PrettyPrinterAbstract $printer = null) : string */ public function getAst() : Node\FunctionLike { - return $this->node; + return $this->getNode(); } /** @@ -504,7 +509,7 @@ public function setBodyFromClosure(Closure $newBody) : void $functionNode = $closureReflection->getNode(); - $this->node->stmts = $functionNode->getStmts(); + $this->getNode()->stmts = $functionNode->getStmts(); } /** @@ -516,7 +521,7 @@ public function setBodyFromClosure(Closure $newBody) : void */ public function setBodyFromString(string $newBody) : void { - $this->node->stmts = $this->loadStaticParser()->parse('getNode()->stmts = $this->loadStaticParser()->parse('node->stmts = $validator(...$nodes); + $this->getNode()->stmts = $validator(...$nodes); } /** @@ -544,7 +549,7 @@ public function setBodyFromAst(array $nodes) : void */ public function addParameter(string $parameterName) : void { - $this->node->params[] = new ParamNode(new Node\Expr\Variable($parameterName)); + $this->getNode()->params[] = new ParamNode(new Node\Expr\Variable($parameterName)); } /** @@ -554,7 +559,7 @@ public function removeParameter(string $parameterName) : void { $lowerName = strtolower($parameterName); - foreach ($this->node->params as $key => $paramNode) { + foreach ($this->getNode()->params as $key => $paramNode) { if ($paramNode->var instanceof Node\Expr\Error) { throw new LogicException('PhpParser left an "Error" node in the parameters AST, this should NOT happen'); } @@ -563,7 +568,7 @@ public function removeParameter(string $parameterName) : void continue; } - unset($this->node->params[$key]); + unset($this->getNode()->params[$key]); } } @@ -582,7 +587,7 @@ public function getReturnStatementsAst() : array $traverser = new NodeTraverser(); $traverser->addVisitor($visitor); - $traverser->traverse($this->node->getStmts()); + $traverser->traverse($this->getNode()->getStmts()); return $visitor->getReturnNodes(); } diff --git a/src/SourceLocator/Ast/FindReflectionsInTree.php b/src/SourceLocator/Ast/FindReflectionsInTree.php index e41e5c708..6204b74f8 100644 --- a/src/SourceLocator/Ast/FindReflectionsInTree.php +++ b/src/SourceLocator/Ast/FindReflectionsInTree.php @@ -6,6 +6,7 @@ use Closure; use PhpParser\Node; +use PhpParser\Node\Name; use PhpParser\Node\Stmt\Namespace_; use PhpParser\NodeTraverser; use PhpParser\NodeVisitor\NameResolver; @@ -145,13 +146,17 @@ public function enterNode(Node $node) return null; } - if ($node->name->hasAttribute('namespacedName') && count($node->name->getAttribute('namespacedName')->parts) > 1) { - try { - $this->functionReflector->reflect($node->name->getAttribute('namespacedName')->toString()); - - return null; - } catch (IdentifierNotFound $e) { - // Global define() + if ($node->name->hasAttribute('namespacedName')) { + /** @var Name $namespacedName */ + $namespacedName = $node->name->getAttribute('namespacedName'); + if(count($namespacedName->parts) > 1) { + try { + $this->functionReflector->reflect($namespacedName->toString()); + + return null; + } catch (IdentifierNotFound $e) { + // Global define() + } } } diff --git a/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php b/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php index a6df82efa..37802d3bd 100644 --- a/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php +++ b/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php @@ -7,6 +7,7 @@ use JetBrains\PHPStormStub\PhpStormStubsMap; use PhpParser\BuilderHelpers; use PhpParser\Node; +use PhpParser\NodeAbstract; use PhpParser\NodeTraverser; use PhpParser\NodeVisitor\NameResolver; use PhpParser\NodeVisitorAbstract; @@ -146,17 +147,29 @@ private function parseFile(string $filePath) : void $this->nodeTraverser->traverse($ast); - /** @psalm-suppress UndefinedMethod */ + /** + * @var string $className + * @var Node\Stmt\ClassLike $classNode + * @psalm-suppress UndefinedMethod + */ foreach ($this->cachingVisitor->getClassNodes() as $className => $classNode) { $this->classNodes[$className] = $classNode; } - /** @psalm-suppress UndefinedMethod */ + /** + * @var string $functionName + * @var Node\Stmt\Function_ $functionNode + * @psalm-suppress UndefinedMethod + */ foreach ($this->cachingVisitor->getFunctionNodes() as $functionName => $functionNode) { $this->functionNodes[$functionName] = $functionNode; } - /** @psalm-suppress UndefinedMethod */ + /** + * @var string $constantName + * @var NodeAbstract $constantNode + * @psalm-suppress UndefinedMethod + */ foreach ($this->cachingVisitor->getConstantNodes() as $constantName => $constantNode) { $this->constantNodes[$constantName] = $constantNode; } @@ -183,14 +196,16 @@ private function createCachingVisitor() : NodeVisitorAbstract public function enterNode(Node $node) : ?int { if ($node instanceof Node\Stmt\ClassLike) { - $this->classNodes[$node->namespacedName->toString()] = $node; + $nodeName = (string) $node->namespacedName->toString(); + $this->classNodes[$nodeName] = $node; return NodeTraverser::DONT_TRAVERSE_CHILDREN; } if ($node instanceof Node\Stmt\Function_) { /** @psalm-suppress UndefinedPropertyFetch */ - $this->functionNodes[$node->namespacedName->toString()] = $node; + $nodeName = (string) $node->namespacedName->toString(); + $this->functionNodes[$nodeName] = $node; return NodeTraverser::DONT_TRAVERSE_CHILDREN; } @@ -198,7 +213,8 @@ public function enterNode(Node $node) : ?int if ($node instanceof Node\Stmt\Const_) { foreach ($node->consts as $constNode) { /** @psalm-suppress UndefinedPropertyFetch */ - $this->constantNodes[$constNode->namespacedName->toString()] = $node; + $constNodeName = (string) $constNode->namespacedName->toString(); + $this->constantNodes[$constNodeName] = $node; } return NodeTraverser::DONT_TRAVERSE_CHILDREN; @@ -217,6 +233,7 @@ public function enterNode(Node $node) : ?int // Some constants has different values on different systems, some are not actual in stubs if (defined($constantName)) { + /** @psalm-var scalar|scalar[]|null $constantValue */ $constantValue = constant($constantName); $node->args[1]->value = BuilderHelpers::normalizeValue($constantValue); } diff --git a/src/SourceLocator/SourceStubber/ReflectionSourceStubber.php b/src/SourceLocator/SourceStubber/ReflectionSourceStubber.php index 16c003f76..313286abb 100644 --- a/src/SourceLocator/SourceStubber/ReflectionSourceStubber.php +++ b/src/SourceLocator/SourceStubber/ReflectionSourceStubber.php @@ -150,6 +150,7 @@ public function generateConstantStub(string $constantName) : ?StubData */ private function findConstantData(string $constantName) : ?array { + /** @psalm-var array> $constants */ $constants = get_defined_constants(true); foreach ($constants as $constantExtensionName => $extensionConstants) { diff --git a/src/SourceLocator/Type/AutoloadSourceLocator.php b/src/SourceLocator/Type/AutoloadSourceLocator.php index 84206718d..0f2dbd96f 100644 --- a/src/SourceLocator/Type/AutoloadSourceLocator.php +++ b/src/SourceLocator/Type/AutoloadSourceLocator.php @@ -210,7 +210,10 @@ private function locateConstantByName(string $constantName) : ?string return null; } - if (! array_key_exists($constantName, get_defined_constants(true)['user'])) { + /** @psalm-var array> $constants */ + $constants = get_defined_constants(true); + + if (! array_key_exists($constantName, $constants['user'])) { return null; } diff --git a/src/SourceLocator/Type/Composer/Factory/MakeLocatorForComposerJsonAndInstalledJson.php b/src/SourceLocator/Type/Composer/Factory/MakeLocatorForComposerJsonAndInstalledJson.php index 37b7078d2..68e484cef 100644 --- a/src/SourceLocator/Type/Composer/Factory/MakeLocatorForComposerJsonAndInstalledJson.php +++ b/src/SourceLocator/Type/Composer/Factory/MakeLocatorForComposerJsonAndInstalledJson.php @@ -50,6 +50,7 @@ public function __invoke(string $installationPath, Locator $astLocator) : Source /** @var array{autoload: array{classmap: array, files: array, psr-4: array>, psr-0: array>}}|null $composer */ $composer = json_decode((string) file_get_contents($composerJsonPath), true); + /** @var array{packages: list}|list|null $installedJson */ $installedJson = json_decode((string) file_get_contents($installedJsonPath), true); if (! is_array($composer)) { diff --git a/src/SourceLocator/Type/Composer/Factory/MakeLocatorForInstalledJson.php b/src/SourceLocator/Type/Composer/Factory/MakeLocatorForInstalledJson.php index be7579875..fc5696196 100644 --- a/src/SourceLocator/Type/Composer/Factory/MakeLocatorForInstalledJson.php +++ b/src/SourceLocator/Type/Composer/Factory/MakeLocatorForInstalledJson.php @@ -42,6 +42,7 @@ public function __invoke(string $installationPath, Locator $astLocator) : Source throw MissingInstalledJson::inProjectPath($installationPath); } + /** @var array{packages: list}|list|null $installedJson */ $installedJson = json_decode((string) file_get_contents($installedJsonPath), true); if (! is_array($installedJson)) { diff --git a/test/unit/SourceLocator/SourceStubber/PhpStormStubsSourceStubberTest.php b/test/unit/SourceLocator/SourceStubber/PhpStormStubsSourceStubberTest.php index 5e4ec1b73..1ccb49065 100644 --- a/test/unit/SourceLocator/SourceStubber/PhpStormStubsSourceStubberTest.php +++ b/test/unit/SourceLocator/SourceStubber/PhpStormStubsSourceStubberTest.php @@ -425,7 +425,10 @@ public function internalConstantsProvider() : array { $provider = []; - foreach (get_defined_constants(true) as $extensionName => $extensionConstants) { + /** @psalm-var array> $constants */ + $constants = get_defined_constants(true); + + foreach ($constants as $extensionName => $extensionConstants) { // Check only always enabled extensions if (! in_array($extensionName, ['Core', 'standard', 'pcre', 'SPL'], true)) { continue; diff --git a/test/unit/SourceLocator/Type/PhpInternalSourceLocatorTest.php b/test/unit/SourceLocator/Type/PhpInternalSourceLocatorTest.php index 06ea394b6..54d85d1da 100644 --- a/test/unit/SourceLocator/Type/PhpInternalSourceLocatorTest.php +++ b/test/unit/SourceLocator/Type/PhpInternalSourceLocatorTest.php @@ -169,6 +169,7 @@ public function testCanFetchInternalLocatedSourceForConstants(string $constantNa */ public function internalConstantsProvider() : array { + /** @psalm-var array> $allSymbols */ $allSymbols = get_defined_constants(true); return array_map(