Upgrade to phpstan 0.12 (#592)

* wip phpstan 0.12

* Fix some errors, ignore others

* improve generics

* add required generic "casts"

* Clean up phpstan.neon.dist ignoreErrors

* Comment out failing type hint in NodeList.php

* Strongly type Parser.php

* Upgrade to phpstan 0.12.1 and remove ignored errors

* Elaborate on NodeList/Visitor strict typing TODO

* Revert new self -> new static

* Add rationale for ignoring the new static() warning

* Bump to phpstan 0.12.2 and adapt ignoreErrors

Author: spawnia

composer.json

@@ -15,10 +15,10 @@
   },
   "require-dev": {
     "doctrine/coding-standard": "^6.0",
-    "phpbench/phpbench": "^0.14.0",
-    "phpstan/phpstan": "0.11.16",
-    "phpstan/phpstan-phpunit": "0.11.2",
-    "phpstan/phpstan-strict-rules": "0.11.1",
+    "phpbench/phpbench": "^0.14",
+    "phpstan/phpstan": "0.12.2",
+    "phpstan/phpstan-phpunit": "0.12.1",
+    "phpstan/phpstan-strict-rules": "0.12.0",
     "phpunit/phpcov": "^5.0",
     "phpunit/phpunit": "^7.2",
     "psr/http-message": "^1.0",
@@ -51,7 +51,7 @@
     "test": "phpunit",
     "lint" : "phpcs",
     "fix" : "phpcbf",
-    "stan": "phpstan analyse --ansi --memory-limit 256M",
+    "stan": "phpstan analyse --ansi --memory-limit 2048M",
     "check": "composer lint && composer stan && composer test"
   }
 }

phpstan.neon.dist

@@ -1,31 +1,40 @@
 parameters:
     level: 3
 
-    reportUnmatchedIgnoredErrors: false
-
     inferPrivatePropertyTypeFromConstructor: true
 
     paths:
         - %currentWorkingDirectory%/src
         - %currentWorkingDirectory%/tests
 
     ignoreErrors:
-        - "~Construct empty\\(\\) is not allowed\\. Use more strict comparison~"
-        - "~(Method|Property) .+::.+(\\(\\))? (has parameter \\$\\w+ with no|has no return|has no) typehint specified~"
-        - "~Variable property access on .+~"
+        # Since this is a library that is supposed to be flexible, we don't
+        # want to lock down every possible extension point.
+        - "~Unsafe usage of new static\\(\\)~"
+
+        # This class uses magic methods to reduce a whole lot of boilerplate required to
+        # allow partial parsing of language fragments.
         - "~Variable method call on GraphQL\\\\Language\\\\Parser\\.~"
-        - "~Variable method call on static\\(GraphQL\\\\Server\\\\ServerConfig\\)~" # TODO get rid of
-        - "~Only booleans are allowed in .*~" # TODO https://github.com/phpstan/phpstan-strict-rules/issues/2
-        # A whole class of errors in PHPStan is a result of PHP's lack of union types.
-        # This commonly happens in the parts of the code that deal with the GraphQL
-        # type system where we can currently use interfaces and lose type safety.
-        # Until we find a better way, we can list related error's here.
-        - "~Access to an undefined property GraphQL\\\\Type\\\\Definition\\\\NamedType::\\$name~"
+
         # Those come from graphql-php\tests\Language\VisitorTest.php
         - "~Access to an undefined property GraphQL\\\\Language\\\\AST\\\\.+::\\$didEnter~"
         - "~Access to an undefined property GraphQL\\\\Language\\\\AST\\\\.+::\\$didLeave~"
         - "~Access to an undefined property GraphQL\\\\Language\\\\AST\\\\Node::\\$value~"
 
+        # TODO fix those errors to improve type safety
+        - "~Construct empty\\(\\) is not allowed\\. Use more strict comparison~"
+        - "~Variable property access on .+~"
+        - "~Anonymous function should have native return typehint~"
+        - "~Anonymous function should return .* but return statement is missing.~"
+        - "~Anonymous function sometimes return something but return statement at the end is missing.~"
+        - "~Short ternary operator is not allowed. Use null coalesce operator if applicable or consider using long ternary.~"
+
+        # TODO convert to less magical code
+        - "~Variable method call on static\\(GraphQL\\\\Server\\\\ServerConfig\\)~"
+
+        # TODO cast booleans explicitly https://github.com/phpstan/phpstan-strict-rules/issues/2
+        - "~Only booleans are allowed in .*~"
+
 includes:
     - vendor/phpstan/phpstan-phpunit/extension.neon
     - vendor/phpstan/phpstan-phpunit/rules.neon

src/Experimental/Executor/Collector.php

@@ -6,15 +6,24 @@
 
 use Generator;
 use GraphQL\Error\Error;
+use GraphQL\Language\AST\BooleanValueNode;
 use GraphQL\Language\AST\DefinitionNode;
 use GraphQL\Language\AST\DocumentNode;
+use GraphQL\Language\AST\EnumValueNode;
 use GraphQL\Language\AST\FieldNode;
+use GraphQL\Language\AST\FloatValueNode;
 use GraphQL\Language\AST\FragmentDefinitionNode;
 use GraphQL\Language\AST\FragmentSpreadNode;
 use GraphQL\Language\AST\InlineFragmentNode;
+use GraphQL\Language\AST\IntValueNode;
+use GraphQL\Language\AST\ListValueNode;
 use GraphQL\Language\AST\Node;
+use GraphQL\Language\AST\NullValueNode;
+use GraphQL\Language\AST\ObjectValueNode;
 use GraphQL\Language\AST\OperationDefinitionNode;
 use GraphQL\Language\AST\SelectionSetNode;
+use GraphQL\Language\AST\StringValueNode;
+use GraphQL\Language\AST\VariableNode;
 use GraphQL\Type\Definition\AbstractType;
 use GraphQL\Type\Definition\Directive;
 use GraphQL\Type\Definition\ObjectType;

src/Language/AST/DocumentNode.php

@@ -9,6 +9,6 @@ class DocumentNode extends Node
     /** @var string */
     public $kind = NodeKind::DOCUMENT;
 
-    /** @var NodeList|array<DefinitionNode> */
+    /** @var NodeList<DefinitionNode&Node> */
     public $definitions;
 }

src/Language/AST/EnumTypeDefinitionNode.php

@@ -15,7 +15,7 @@ class EnumTypeDefinitionNode extends Node implements TypeDefinitionNode
     /** @var DirectiveNode[] */
     public $directives;
 
-    /** @var EnumValueDefinitionNode[]|NodeList|null */
+    /** @var NodeList<EnumValueDefinitionNode>|null */
     public $values;
 
     /** @var StringValueNode|null */

src/Language/AST/FieldDefinitionNode.php

@@ -12,13 +12,13 @@ class FieldDefinitionNode extends Node
     /** @var NameNode */
     public $name;
 
-    /** @var InputValueDefinitionNode[]|NodeList */
+    /** @var NodeList<InputValueDefinitionNode> */
     public $arguments;
 
     /** @var NamedTypeNode|ListTypeNode|NonNullTypeNode */
     public $type;
 
-    /** @var DirectiveNode[]|NodeList */
+    /** @var NodeList<DirectiveNode> */
     public $directives;
 
     /** @var StringValueNode|null */

src/Language/AST/FragmentDefinitionNode.php

@@ -16,14 +16,14 @@ class FragmentDefinitionNode extends Node implements ExecutableDefinitionNode, H
      * Note: fragment variable definitions are experimental and may be changed
      * or removed in the future.
      *
-     * @var VariableDefinitionNode[]|NodeList
+     * @var NodeList<VariableDefinitionNode>
      */
     public $variableDefinitions;
 
     /** @var NamedTypeNode */
     public $typeCondition;
 
-    /** @var DirectiveNode[]|NodeList */
+    /** @var NodeList<DirectiveNode> */
     public $directives;
 
     /** @var SelectionSetNode */

src/Language/AST/ListValueNode.php

@@ -9,6 +9,6 @@ class ListValueNode extends Node implements ValueNode
     /** @var string */
     public $kind = NodeKind::LST;
 
-    /** @var ValueNode[]|NodeList */
+    /** @var NodeList<ValueNode&Node> */
     public $values;
 }

src/Language/AST/NodeList.php

@@ -6,101 +6,132 @@
 
 use ArrayAccess;
 use Countable;
-use Generator;
 use GraphQL\Utils\AST;
+use InvalidArgumentException;
 use IteratorAggregate;
+use Traversable;
 use function array_merge;
 use function array_splice;
 use function count;
 use function is_array;
 
+/**
+ * @template T of Node
+ * @phpstan-implements ArrayAccess<int|string, T>
+ * @phpstan-implements IteratorAggregate<T>
+ */
 class NodeList implements ArrayAccess, IteratorAggregate, Countable
 {
-    /** @var Node[]|mixed[] */
+    /**
+     * @var Node[]
+     * @phpstan-var array<T>
+     */
     private $nodes;
 
     /**
-     * @param Node[]|mixed[] $nodes
+     * @param Node[] $nodes
      *
-     * @return static
+     * @phpstan-param array<T> $nodes
+     * @phpstan-return self<T>
      */
-    public static function create(array $nodes)
+    public static function create(array $nodes) : self
     {
         return new static($nodes);
     }
 
     /**
-     * @param Node[]|mixed[] $nodes
+     * @param Node[] $nodes
+     *
+     * @phpstan-param array<T> $nodes
      */
     public function __construct(array $nodes)
     {
         $this->nodes = $nodes;
     }
 
     /**
-     * @param mixed $offset
-     *
-     * @return bool
+     * @param int|string $offset
      */
-    public function offsetExists($offset)
+    public function offsetExists($offset) : bool
     {
         return isset($this->nodes[$offset]);
     }
 
     /**
-     * @param mixed $offset
+     * TODO enable strict typing by changing how the Visitor deals with NodeList.
+     * Ideally, this function should always return a Node instance.
+     * However, the Visitor currently allows mutation of the NodeList
+     * and puts arbitrary values in the NodeList, such as strings.
+     * We will have to switch to using an array or a less strict
+     * type instead so we can enable strict typing in this class.
+     *
+     * @param int|string $offset
      *
-     * @return mixed
+     * @phpstan-return T
      */
-    public function offsetGet($offset)
+    public function offsetGet($offset)// : Node
     {
         $item = $this->nodes[$offset];
 
         if (is_array($item) && isset($item['kind'])) {
-            $this->nodes[$offset] = $item = AST::fromArray($item);
+            /** @phpstan-var T $node */
+            $node                 = AST::fromArray($item);
+            $this->nodes[$offset] = $node;
         }
 
-        return $item;
+        return $this->nodes[$offset];
     }
 
     /**
-     * @param mixed $offset
-     * @param mixed $value
+     * @param int|string   $offset
+     * @param Node|mixed[] $value
+     *
+     * @phpstan-param T|mixed[] $value
      */
-    public function offsetSet($offset, $value)
+    public function offsetSet($offset, $value) : void
     {
-        if (is_array($value) && isset($value['kind'])) {
-            $value = AST::fromArray($value);
+        if (is_array($value)) {
+            if (isset($value['kind'])) {
+                /** @phpstan-var T $node */
+                $node                 = AST::fromArray($value);
+                $this->nodes[$offset] = $node;
+
+                return;
+            }
+
+            throw new InvalidArgumentException(
+                'Expected array value to be valid node data structure, missing key "kind"'
+            );
         }
+
         $this->nodes[$offset] = $value;
     }
 
     /**
-     * @param mixed $offset
+     * @param int|string $offset
      */
-    public function offsetUnset($offset)
+    public function offsetUnset($offset) : void
     {
         unset($this->nodes[$offset]);
     }
 
     /**
-     * @param int   $offset
-     * @param int   $length
      * @param mixed $replacement
      *
-     * @return NodeList
+     * @phpstan-return NodeList<T>
      */
-    public function splice($offset, $length, $replacement = null)
+    public function splice(int $offset, int $length, $replacement = null) : NodeList
     {
         return new NodeList(array_splice($this->nodes, $offset, $length, $replacement));
     }
 
     /**
      * @param NodeList|Node[] $list
      *
-     * @return NodeList
+     * @phpstan-param NodeList<T>|array<T> $list
+     * @phpstan-return NodeList<T>
      */
-    public function merge($list)
+    public function merge($list) : NodeList
     {
         if ($list instanceof self) {
             $list = $list->nodes;
@@ -109,20 +140,14 @@ public function merge($list)
         return new NodeList(array_merge($this->nodes, $list));
     }
 
-    /**
-     * @return Generator
-     */
-    public function getIterator()
+    public function getIterator() : Traversable
     {
         foreach ($this->nodes as $key => $_) {
             yield $this->offsetGet($key);
         }
     }
 
-    /**
-     * @return int
-     */
-    public function count()
+    public function count() : int
     {
         return count($this->nodes);
     }

src/Language/AST/ObjectValueNode.php

@@ -9,6 +9,6 @@ class ObjectValueNode extends Node implements ValueNode
     /** @var string */
     public $kind = NodeKind::OBJECT;
 
-    /** @var ObjectFieldNode[]|NodeList */
+    /** @var NodeList<ObjectFieldNode> */
     public $fields;
 }