Merge pull request #537 from orklah/psalm-improvements

various improvements for psalm
Roave · Feb 1, 2020 · 43afb69 · 43afb69
2 parents 4a6623a + b1cabc0
commit 43afb69
Show file tree

Hide file tree

Showing 11 changed files with 87 additions and 64 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="3.8.2@90d6b73fd8062432030ef39b7b6694b3902daa31">
+<files psalm-version="3.8.3@389af1bfc739bfdff3f9e3dc7bd6499aee51a831">
   <file src="src/BetterReflection.php">
     <DocblockTypeContradiction occurrences="1">
       <code>$this-&gt;sourceStubber</code>
@@ -40,10 +40,6 @@
     </PossiblyInvalidArgument>
   </file>
   <file src="src/Reflection/ReflectionFunctionAbstract.php">
-    <DocblockTypeContradiction occurrences="2">
-      <code>$this-&gt;node === null</code>
-      <code>self::$parser</code>
-    </DocblockTypeContradiction>
     <PossiblyInvalidCast occurrences="1">
       <code>$returnType</code>
     </PossiblyInvalidCast>
@@ -91,18 +87,6 @@
     <InternalMethod occurrences="1">
       <code>BuilderHelpers::normalizeValue($constantReflection-&gt;getValue())</code>
     </InternalMethod>
-    <PossiblyInvalidArgument occurrences="1">
-      <code>$extensionConstants</code>
-    </PossiblyInvalidArgument>
-    <PossiblyInvalidArrayAccess occurrences="1">
-      <code>$extensionConstants[$constantName]</code>
-    </PossiblyInvalidArrayAccess>
-    <PossiblyInvalidArrayOffset occurrences="1">
-      <code>$extensionConstants[$constantName]</code>
-    </PossiblyInvalidArrayOffset>
-    <PossiblyNullArrayAccess occurrences="1">
-      <code>$extensionConstants[$constantName]</code>
-    </PossiblyNullArrayAccess>
     <PossiblyNullIterator occurrences="1">
       <code>$classReflection-&gt;getTraitAliases()</code>
     </PossiblyNullIterator>
@@ -117,9 +101,6 @@
     <InternalMethod occurrences="1">
       <code>ConstantNodeChecker::assertValidDefineFunctionCall($node)</code>
     </InternalMethod>
-    <PossiblyInvalidArgument occurrences="1">
-      <code>get_defined_constants(true)['user']</code>
-    </PossiblyInvalidArgument>
   </file>
   <file src="src/SourceLocator/Type/ClosureSourceLocator.php">
     <PossiblyNullReference occurrences="1">

diff --git 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<class-string>
      */
     public function getParentClassNames() : array
     {
@@ -1178,7 +1182,8 @@ public function isIterateable() : bool
 
     /**
      * @return ReflectionClass[] indexed by interface name
-     * @return array<string, ReflectionClass>
+     *
+     * @psalm-return array<string, ReflectionClass>
      */
     private function getCurrentClassImplementedInterfacesIndexedByName() : array
     {

diff --git 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<Node\Param> $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,33 +358,33 @@ public function isGenerator() : bool
      */
     public function getStartLine() : int
     {
-        return $this->node->getStartLine();
+        return $this->getNode()->getStartLine();
     }
 
     /**
      * Get the line number that this function ends on.
      */
     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());
     }
 
     /**
      * Is this function declared as a reference.
      */
     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,15 +431,15 @@ public function hasReturnType() : bool
      */
     public function setReturnType(string $newReturnType) : void
     {
-        $this->node->returnType = new Node\Name($newReturnType);
+        $this->getNode()->returnType = new Node\Name($newReturnType);
     }
 
     /**
      * Remove the return type declaration completely.
      */
     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('<?php ' . $newBody);
+        $this->getNode()->stmts = $this->loadStaticParser()->parse('<?php ' . $newBody);
     }
 
     /**
@@ -533,18 +538,18 @@ public function setBodyFromAst(array $nodes) : void
     {
         // This slightly confusing code simply type-checks the $sourceLocators
         // array by unpacking them and splatting them in the closure.
-        $validator         = static function (Node ...$node) : array {
+        $validator              = static function (Node ...$node) : array {
             return $node;
         };
-        $this->node->stmts = $validator(...$nodes);
+        $this->getNode()->stmts = $validator(...$nodes);
     }
 
     /**
      * Add a new parameter to the method/function.
      */
     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
@@ -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()
+                            }
                         }
                     }