Merge pull request #222 from phpcr/phpstan-fixes

raise phpstan level and improve type safety

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
 {
 }

tests/PHPCR/Tests/Stubs/MockNodeTypeManager.php

@@ -4,8 +4,12 @@
 
 namespace PHPCR\Tests\Stubs;
 
+use PHPCR\NodeType\NodeTypeInterface;
 use PHPCR\NodeType\NodeTypeManagerInterface;
 
+/**
+ * @implements \Iterator<string, NodeTypeInterface>
+ */
 abstract class MockNodeTypeManager implements \Iterator, NodeTypeManagerInterface
 {
 }

tests/PHPCR/Tests/Stubs/MockRow.php

@@ -6,6 +6,9 @@
 
 use PHPCR\Query\RowInterface;
 
+/**
+ * @implements \Iterator<string, mixed>
+ */
 abstract class MockRow implements \Iterator, RowInterface
 {
 }

tests/PHPCR/Tests/Util/CND/Reader/FileReaderTest.php

@@ -20,7 +20,7 @@ class FileReaderTest extends TestCase
     private $reader;
 
     /**
-     * @var array
+     * @var string[]
      */
     private $lines;
 

tests/PHPCR/Tests/Util/CND/Scanner/GenericScannerTest.php

@@ -15,7 +15,7 @@
 class GenericScannerTest extends TestCase
 {
     /**
-     * @var array<Token[]>
+     * @var array<array{0: int, 1: string}>
      */
     private array $expectedTokens = [
         // <opening php tag>
@@ -108,7 +108,7 @@ class GenericScannerTest extends TestCase
     ];
 
     /**
-     * @var Token[]
+     * @var array<array{0: int, 1: string}>
      */
     protected array $expectedTokensNoEmptyToken;
 
@@ -146,7 +146,10 @@ public function testFilteredScan(): void
         $this->assertTokens($this->expectedTokensNoEmptyToken, $queue);
     }
 
-    protected function assertTokens($tokens, TokenQueue $queue): void
+    /**
+     * @param array<array{0: int, 1: string}> $tokens
+     */
+    protected function assertTokens(array $tokens, TokenQueue $queue): void
     {
         $queue->reset();
 
@@ -155,6 +158,8 @@ protected function assertTokens($tokens, TokenQueue $queue): void
         $token = $queue->peek();
 
         while ($it->valid()) {
+            $this->assertInstanceOf(Token::class, $token);
+
             $expectedToken = $it->current();
 
             $this->assertFalse($queue->isEof(), 'There is no more tokens, expected = '.$expectedToken[1]);
@@ -168,7 +173,7 @@ protected function assertTokens($tokens, TokenQueue $queue): void
         $this->assertTrue($queue->isEof(), 'There are more unexpected tokens.');
     }
 
-    protected function assertToken($type, $data, Token|false $token): void
+    protected function assertToken(int $type, string $data, Token $token): void
     {
         $this->assertEquals(
             $type,

tests/PHPCR/Tests/Util/Console/Command/BaseCommandTest.php

@@ -27,37 +27,37 @@
 abstract class BaseCommandTest extends TestCase
 {
     /**
-     * @var SessionInterface|MockObject
+     * @var SessionInterface&MockObject
      * */
     public $session;
 
     /**
-     * @var WorkspaceInterface|MockObject
+     * @var WorkspaceInterface&MockObject
      */
     public $workspace;
 
     /**
-     * @var RepositoryInterface|MockObject
+     * @var RepositoryInterface&MockObject
      */
     public $repository;
 
     /**
-     * @var PhpcrConsoleDumperHelper|MockObject
+     * @var PhpcrConsoleDumperHelper&MockObject
      */
     public $dumperHelper;
 
     /**
-     * @var NodeInterface|MockObject
+     * @var NodeInterface&MockObject
      */
     public $node1;
 
     /**
-     * @var RowInterface|MockObject
+     * @var RowInterface&MockObject
      */
     public $row1;
 
     /**
-     * @var QueryManagerInterface|MockObject
+     * @var QueryManagerInterface&MockObject
      */
     public $queryManager;
 
@@ -113,18 +113,14 @@ public function setUp(): void
     /**
      * Build and execute the command tester.
      *
-     * @param string $name   command name
-     * @param array  $args   command arguments
-     * @param int    $status expected return status
-     *
-     * @return CommandTester
+     * @param mixed[] $arguments
      */
-    public function executeCommand($name, $args, $status = 0)
+    public function executeCommand(string $commandName, array $arguments, int $expectedReturnStatus = 0): CommandTester
     {
-        $command = $this->application->find($name);
+        $command = $this->application->find($commandName);
         $commandTester = new CommandTester($command);
-        $args = array_merge(['command' => $command->getName()], $args);
-        $this->assertEquals($status, $commandTester->execute($args));
+        $arguments = array_merge(['command' => $command->getName()], $arguments);
+        $this->assertEquals($expectedReturnStatus, $commandTester->execute($arguments));
 
         return $commandTester;
     }

tests/PHPCR/Tests/Util/Console/Command/NodeDumpCommandTest.php

@@ -12,7 +12,7 @@
 
 class NodeDumpCommandTest extends BaseCommandTest
 {
-    /** @var TreeWalker|MockObject */
+    /** @var TreeWalker&MockObject */
     protected $treeWalker;
 
     public function setUp(): void

tests/PHPCR/Tests/Util/Console/Command/NodeMoveCommandTest.php

@@ -8,15 +8,27 @@
 
 class NodeMoveCommandTest extends BaseCommandTest
 {
-    public function provideCommand()
+    /**
+     * @return array<array<mixed[]>>
+     */
+    public function provideCommand(): array
     {
-        return [[['source' => '/foo', 'destination' => '/bar']]];
+        return [
+            [
+                [
+                    'source' => '/foo',
+                    'destination' => '/bar',
+                ],
+            ],
+        ];
     }
 
     /**
      * @dataProvider provideCommand
+     *
+     * @param array<mixed[]> $args
      */
-    public function testCommand($args): void
+    public function testCommand(array $args): void
     {
         $this->session->expects($this->once())
             ->method('move')

tests/PHPCR/Tests/Util/Console/Command/NodeTouchCommandTest.php

@@ -17,7 +17,7 @@
 class NodeTouchCommandTest extends BaseCommandTest
 {
     /**
-     * @var PhpcrHelper|MockObject
+     * @var PhpcrHelper&MockObject
      */
     public $phpcrHelper;
 

tests/PHPCR/Tests/Util/Console/Command/NodeTypeListCommandTest.php

@@ -11,7 +11,7 @@
 class NodeTypeListCommandTest extends BaseCommandTest
 {
     /**
-     * @var MockNodeTypeManager|MockObject
+     * @var MockNodeTypeManager&MockObject
      */
     private $nodeTypeManager;