Merge pull request #222 from phpcr/phpstan-fixes

raise phpstan level and improve type safety

Author: dbu

phpstan.neon.dist

@@ -1,4 +1,16 @@
 parameters:
-    level: 2
+    level: 5
     paths:
-        - src
+        - src/
+
+    ignoreErrors:
+        # phpstan does not understand that the empty arrays are only the default
+        -
+            message: "#^Empty array passed to foreach\\.$#"
+            count: 5
+            path: src/PHPCR/Util/Console/Helper/PhpcrHelper.php
+        # only formulated in phpdoc that the return value must be countable
+        -
+            message: "#expects array|Countable, Iterator<mixed, PHPCR\\Query\\RowInterface> given\\.$#"
+            count: 1
+            path: src/PHPCR/Util/Console/Command/NodesUpdateCommand.php

phpstan.tests.neon.dist

@@ -1,4 +1,31 @@
 parameters:
-    level: 1
+    level: 7
     paths:
-        - tests
+        - tests/
+
+    excludePaths:
+        analyse:
+            - tests/*/Fixtures/*
+
+    ignoreErrors:
+        # not sure what is going on here
+        -
+            message: "#^Interface Iterator specifies template type TKey of interface Traversable as string but it's already specified as mixed\\.$#"
+            count: 1
+            path: tests/PHPCR/Tests/Stubs/MockNodeTypeManager.php
+
+        -
+            message: "#^Interface Iterator specifies template type TValue of interface Traversable as PHPCR\\\\NodeType\\\\NodeTypeInterface but it's already specified as mixed\\.$#"
+            count: 1
+            path: tests/PHPCR/Tests/Stubs/MockNodeTypeManager.php
+
+        # too pedantic for tests
+        -
+            message: "#^Parameter \\#1 \\.\\.\\.\\$arrays of function array_merge expects array, array\\<int, string\\>\\|false given\\.$#"
+            count: 1
+            path: tests/PHPCR/Tests/Util/CND/Reader/FileReaderTest.php
+
+        -
+            message: "#^Parameter \\#3 \\.\\.\\.\\$arrays of function array_merge expects array, array\\<int, string\\>\\|false given\\.$#"
+            count: 1
+            path: tests/PHPCR/Tests/Util/CND/Reader/FileReaderTest.php

src/PHPCR/Util/CND/Parser/AbstractParser.php

@@ -5,6 +5,7 @@
 namespace PHPCR\Util\CND\Parser;
 
 use PHPCR\Util\CND\Exception\ParserException;
+use PHPCR\Util\CND\Scanner\GenericToken;
 use PHPCR\Util\CND\Scanner\GenericToken as Token;
 use PHPCR\Util\CND\Scanner\TokenQueue;
 
@@ -29,12 +30,8 @@ abstract class AbstractParser
      * Check the next token without consuming it and return true if it matches the given type and data.
      * If the data is not provided (equal to null) then only the token type is checked.
      * Return false otherwise.
-     *
-     * @param int         $type       The expected token type
-     * @param string|null $data       The expected data or null
-     * @param bool        $ignoreCase whether to do string comparisons case insensitive or sensitive
      */
-    protected function checkToken($type, string $data = null, bool $ignoreCase = false): bool
+    protected function checkToken(int $type, string $data = null, bool $ignoreCase = false): bool
     {
         if ($this->tokenQueue->isEof()) {
             return false;
@@ -48,7 +45,7 @@ protected function checkToken($type, string $data = null, bool $ignoreCase = fal
 
         if ($data && $token->getData() !== $data) {
             if ($ignoreCase && is_string($data) && is_string($token->getData())) {
-                return strcasecmp($data, $token->getData());
+                return 0 !== strcasecmp($data, $token->getData());
             }
 
             return false;
@@ -89,7 +86,7 @@ protected function expectToken(int $type, string $data = null): Token
         if (!$this->checkToken($type, $data)) {
             throw new ParserException($this->tokenQueue, sprintf("Expected token [%s, '%s']", Token::getTypeName($type), $data));
         }
-
+        \assert($token instanceof GenericToken);
         $this->tokenQueue->next();
 
         return $token;

src/PHPCR/Util/CND/Parser/CndParser.php

@@ -71,7 +71,7 @@ final class CndParser extends AbstractParser
     private array $namespaces = [];
 
     /**
-     * @var string[]
+     * @var NodeTypeDefinitionInterface[]
      */
     private array $nodeTypes = [];
 

src/PHPCR/Util/CND/Scanner/GenericScanner.php

@@ -28,8 +28,7 @@ public function scan(ReaderInterface $reader): TokenQueue
         $this->resetQueue();
 
         while (!$reader->isEof()) {
-            $tokenFound = false;
-            $tokenFound = $tokenFound || $this->consumeComments($reader);
+            $tokenFound = $this->consumeComments($reader);
             $tokenFound = $tokenFound || $this->consumeNewLine($reader);
             $tokenFound = $tokenFound || $this->consumeSpaces($reader);
             $tokenFound = $tokenFound || $this->consumeString($reader);

src/PHPCR/Util/CND/Writer/CndWriter.php

@@ -119,9 +119,7 @@ protected function writeNodeType(NodeTypeDefinitionInterface $nodeType): string
         if ($nodeType->getPrimaryItemName()) {
             $attributes .= 'primaryitem '.$nodeType->getPrimaryItemName().' ';
         }
-        if ($attributes) {
-            $s .= trim($attributes)."\n";
-        }
+        $s .= trim($attributes)."\n";
 
         $s .= $this->writeProperties($nodeType->getDeclaredPropertyDefinitions());
 

src/PHPCR/Util/Console/Command/BaseCommand.php

@@ -25,16 +25,31 @@ protected function getPhpcrSession(): SessionInterface
 
     protected function getPhpcrHelper(): PhpcrHelper
     {
-        return $this->getHelper('phpcr');
+        $helper = $this->getHelper('phpcr');
+        if (!$helper instanceof PhpcrHelper) {
+            throw new \RuntimeException('phpcr must be the PhpcrHelper');
+        }
+
+        return $helper;
     }
 
     protected function getPhpcrConsoleDumperHelper(): PhpcrConsoleDumperHelper
     {
-        return $this->getHelper('phpcr_console_dumper');
+        $helper = $this->getHelper('phpcr_console_dumper');
+        if (!$helper instanceof PhpcrConsoleDumperHelper) {
+            throw new \RuntimeException('phpcr_console_dumper must be the PhpcrConsoleDumperHelper');
+        }
+
+        return $helper;
     }
 
     protected function getQuestionHelper(): QuestionHelper
     {
-        return $this->getHelper('question');
+        $helper = $this->getHelper('question');
+        if (!$helper instanceof QuestionHelper) {
+            throw new \RuntimeException('question must be the QuestionHelper');
+        }
+
+        return $helper;
     }
 }

src/PHPCR/Util/Console/Helper/TreeDumper/ConsoleDumperPropertyVisitor.php

@@ -47,7 +47,7 @@ public function __construct(OutputInterface $output, array $options = [])
     public function visit(ItemInterface $item): void
     {
         if (!$item instanceof PropertyInterface) {
-            throw new \Exception(sprintf('Internal error: did not expect to visit a non-property object: %s', is_object($item) ? $item::class : $item));
+            throw new \Exception(sprintf('Internal error: did not expect to visit a non-property object: %s', $item::class));
         }
 
         $value = $item->getString();

src/PHPCR/Util/QOM/Sql2ToQomQueryConverter.php

@@ -429,11 +429,6 @@ protected function parseNot(): NotInterface
     protected function parseComparison(): ComparisonInterface
     {
         $op1 = $this->parseDynamicOperand();
-
-        if (null === $op1) {
-            throw new InvalidQueryException("Syntax error: dynamic operator expected in '{$this->sql2}'");
-        }
-
         $operator = $this->parseOperator();
         $op2 = $this->parseStaticOperand();
 

tests/PHPCR/Tests/Stubs/MockNode.php

@@ -6,6 +6,9 @@
 
 use PHPCR\NodeInterface;
 
+/**
+ * @implements  \Iterator<string, NodeInterface>
+ */
 abstract class MockNode implements \Iterator, NodeInterface
 {
 }