Upgrade phpstan to level 6 (#301)

atk4 · May 11, 2021 · a1dfc07 · a1dfc07
1 parent c72b10f
commit a1dfc07
Show file tree

Hide file tree

Showing 18 changed files with 350 additions and 319 deletions.
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -1,5 +1,5 @@
 parameters:
-    level: 5
+    level: 6
     paths:
         - ./
     excludes_analyse:
@@ -10,5 +10,17 @@ parameters:
     # TODO review once we drop PHP 7.x support
     treatPhpDocTypesAsCertain: false
 
+    # some extra rules
+    checkAlwaysTrueCheckTypeFunctionCall: true
+    checkAlwaysTrueInstanceof: true
+    checkAlwaysTrueStrictComparison: true
+    checkExplicitMixedMissingReturn: true
+    checkFunctionNameCase: true
+    # TODO checkMissingClosureNativeReturnTypehintRule: true
+    reportMaybesInMethodSignatures: true
+    reportStaticMethodSignatures: true
+    checkTooWideReturnTypesInProtectedAndPublicMethods: true
+    checkMissingIterableValueType: false # TODO
+
     ignoreErrors:
         - '~^Unsafe usage of new static\(\)\.$~'
diff --git a/src/Connection.php b/src/Connection.php
@@ -160,7 +160,7 @@ public static function connect($dsn, $user = null, $password = null, $args = [])
      * @param string $connectionClass
      * @param string $driverSchema
      */
-    public static function registerConnectionClass($connectionClass = null, $driverSchema = null)
+    public static function registerConnectionClass($connectionClass = null, $driverSchema = null): void
     {
         if ($connectionClass === null) {
             $connectionClass = static::class;
@@ -508,6 +508,10 @@ public function execute(Expression $expr): object
      * Atomic executes operations within one begin/end transaction, so if
      * the code inside callback will fail, then all of the transaction
      * will be also rolled back.
+     *
+     * @param mixed ...$args
+     *
+     * @return mixed
      */
     public function atomic(\Closure $fx, ...$args)
     {

diff --git a/src/Expression.php b/src/Expression.php
@@ -9,6 +9,9 @@
 use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\Result as DbalResult;
 
+/**
+ * @phpstan-implements \ArrayAccess<int|string, mixed>
+ */
 class Expression implements Expressionable, \ArrayAccess
 {
     /** @const string "[]" in template, escape as parameter */
@@ -31,7 +34,7 @@ class Expression implements Expressionable, \ArrayAccess
      * This property is made public to ease customization and make it accessible
      * from Connection class for example.
      *
-     * @var array
+     * @var array<int|string, mixed>
      */
     public $args = ['custom' => []];
 
@@ -56,7 +59,7 @@ class Expression implements Expressionable, \ArrayAccess
     /** @var array Populated with actual values by escapeParam() */
     public $params = [];
 
-    /** @var Connection */
+    /** @var Connection|null */
     public $connection;
 
     /** @var bool Wrap the expression in parentheses when consumed by another expression or not. */
@@ -125,7 +128,7 @@ public function getDsqlExpression(self $expression): self
     /**
      * Whether or not an offset exists.
      *
-     * @param string $offset
+     * @param int|string $offset
      */
     public function offsetExists($offset): bool
     {
@@ -135,7 +138,7 @@ public function offsetExists($offset): bool
     /**
      * Returns the value at specified offset.
      *
-     * @param string $offset
+     * @param int|string $offset
      *
      * @return mixed
      */
@@ -147,8 +150,8 @@ public function offsetGet($offset)
     /**
      * Assigns a value to the specified offset.
      *
-     * @param string|null $offset
-     * @param mixed       $value  The value to set
+     * @param int|string|null $offset
+     * @param mixed           $value  The value to set
      */
     public function offsetSet($offset, $value): void
     {
@@ -162,7 +165,7 @@ public function offsetSet($offset, $value): void
     /**
      * Unsets an offset.
      *
-     * @param string $offset
+     * @param int|string $offset
      */
     public function offsetUnset($offset): void
     {
@@ -180,37 +183,30 @@ public function offsetUnset($offset): void
      */
     public function expr($properties = [], $arguments = null)
     {
-        // If we use DSQL Connection, then we should call expr() from there.
-        // Connection->expr() will return correct, connection specific Expression class.
-        if ($this->connection instanceof Connection) {
+        if ($this->connection !== null) {
             // TODO - condition above always satisfied when connection is set - adjust tests,
             // so connection is always set and remove the code below
             return $this->connection->expr($properties, $arguments);
         }
 
-        // Otherwise, connection is probably PDO and we don't know which Expression
-        // class to use, so we make a smart guess :)
-        // @phpstan-ignore-next-line
+        // make a smart guess :) when connection is not set
         if ($this instanceof Query) {
             $e = new self($properties, $arguments);
         } else {
             $e = new static($properties, $arguments);
         }
 
         $e->escape_char = $this->escape_char;
-        $e->connection = $this->connection;
 
         return $e;
     }
 
     /**
      * Resets arguments.
      *
-     * @param string $tag
-     *
      * @return $this
      */
-    public function reset($tag = null)
+    public function reset(string $tag = null)
     {
         // unset all arguments
         if ($tag === null) {
@@ -219,11 +215,6 @@ public function reset($tag = null)
             return $this;
         }
 
-        if (!is_string($tag)) {
-            throw (new Exception('Tag should be string'))
-                ->addMoreInfo('tag', $tag);
-        }
-
         // unset custom/argument or argument if such exists
         if ($this->offsetExists($tag)) {
             $this->offsetUnset($tag);
@@ -240,7 +231,7 @@ public function reset($tag = null)
      * @param mixed  $expression Expression
      * @param string $escapeMode Fall-back escaping mode - using one of the Expression::ESCAPE_* constants
      *
-     * @return string|array Quoted expression or array of param names
+     * @return string Quoted expression
      */
     protected function consume($expression, string $escapeMode = self::ESCAPE_PARAM)
     {
@@ -281,12 +272,6 @@ protected function consume($expression, string $escapeMode = self::ESCAPE_PARAM)
             $expression->_paramBase = null;
         }
 
-        if (isset($expression->allowToWrapInParenthesis)) {
-            'trigger_error'('Usage of Query::$allowToWrapInParenthesis is deprecated, use $wrapInParentheses instead - will be removed in version 2.5', E_USER_DEPRECATED);
-
-            $expression->wrapInParentheses = $expression->allowToWrapInParenthesis;
-        }
-
         // Wrap in parentheses if expression requires so
         if ($expression->wrapInParentheses === true) {
             $ret = '(' . $ret . ')';
@@ -369,8 +354,10 @@ protected function escapeIdentifierSoft(string $value): string
      * soft escaping, such as "*" or "(".
      * Those will typically indicate that expression is passed and shouldn't
      * be escaped.
+     *
+     * @param self|string $value
      */
-    protected function isUnescapablePattern($value)
+    protected function isUnescapablePattern($value): bool
     {
         return is_object($value)
         || $value === '*'
@@ -380,10 +367,8 @@ protected function isUnescapablePattern($value)
 
     /**
      * Render expression and return it as string.
-     *
-     * @return string Rendered query
      */
-    public function render()
+    public function render(): string
     {
         $hadUnderscoreParamBase = $this->_paramBase !== null;
         if (!$hadUnderscoreParamBase) {
@@ -438,8 +423,6 @@ function ($matches) use (&$nameless_count) {
                 }
                 $fx = '_render_' . $identifier;
 
-                // [foo] will attempt to call $this->_render_foo()
-
                 if (array_key_exists($identifier, $this->args['custom'])) {
                     $value = $this->consume($this->args['custom'][$identifier], $escaping);
                 } elseif (method_exists($this, $fx)) {
@@ -449,7 +432,7 @@ function ($matches) use (&$nameless_count) {
                         ->addMoreInfo('tag', $identifier);
                 }
 
-                return is_array($value) ? '(' . implode(',', $value) . ')' : $value;
+                return $value;
             },
             $this->template
         );
@@ -495,7 +478,7 @@ public function getDebugQuery(): string
         return $result;
     }
 
-    public function __debugInfo()
+    public function __debugInfo(): array
     {
         $arr = [
             'R' => false,
@@ -591,6 +574,8 @@ public function execute(object $connection = null): object
 
     /**
      * TODO drop method once we support DBAL 3.x only.
+     *
+     * @return \Traversable<array<mixed>>
      */
     public function getIterator(): \Traversable
     {

diff --git a/src/Mssql/ExpressionTrait.php b/src/Mssql/ExpressionTrait.php
@@ -25,7 +25,9 @@ private function fixOpenEscapeChar(string $v): string
 
     // {{{ MSSQL does not support named parameters, so convert them to numerical inside execute
 
+    /** @var array|null */
     private $numQueryParamsBackup;
+    /** @var string|null */
     private $numQueryRender;
 
     /**
@@ -61,7 +63,7 @@ function ($matches) use (&$numParams, &$i, &$j) {
         }
     }
 
-    public function render()
+    public function render(): string
     {
         if ($this->numQueryParamsBackup !== null) {
             return $this->numQueryRender;

diff --git a/src/Mssql/Query.php b/src/Mssql/Query.php
@@ -22,16 +22,18 @@ class Query extends BaseQuery
         . "\n" . 'set IDENTITY_INSERT [table_noalias] off'
         . "\n" . 'end end catch';
 
-    public function _render_limit()
+    public function _render_limit(): ?string
     {
-        if (isset($this->args['limit'])) {
-            $cnt = (int) $this->args['limit']['cnt'];
-            $shift = (int) $this->args['limit']['shift'];
-
-            return (!isset($this->args['order']) ? ' order by (select null)' : '')
-                . ' offset ' . $shift . ' rows'
-                . ' fetch next ' . $cnt . ' rows only';
+        if (!isset($this->args['limit'])) {
+            return null;
         }
+
+        $cnt = (int) $this->args['limit']['cnt'];
+        $shift = (int) $this->args['limit']['shift'];
+
+        return (!isset($this->args['order']) ? ' order by (select null)' : '')
+            . ' offset ' . $shift . ' rows'
+            . ' fetch next ' . $cnt . ' rows only';
     }
 
     public function groupConcat($field, string $delimiter = ',')

diff --git a/src/Oracle/AbstractQuery.php b/src/Oracle/AbstractQuery.php
@@ -13,7 +13,7 @@ abstract class AbstractQuery extends BaseQuery
     /** @var string */
     protected $template_seq_nextval = '[sequence].NEXTVAL';
 
-    public function render()
+    public function render(): string
     {
         if ($this->mode === 'select' && $this->main_table === null) {
             $this->table('DUAL');
@@ -36,7 +36,7 @@ public function sequence($sequence)
         return $this;
     }
 
-    public function _render_sequence()
+    public function _render_sequence(): ?string
     {
         return $this->args['sequence'];
     }

diff --git a/src/Oracle/Query.php b/src/Oracle/Query.php
@@ -9,6 +9,7 @@ class Query extends AbstractQuery
     // {{{ for Oracle 11 and lower to support LIMIT with OFFSET
 
     protected $template_select = '[with]select[option] [field] [from] [table][join][where][group][having][order]';
+    /** @var string */
     protected $template_select_limit = 'select * from (select "__t".*, rownum "__dsql_rownum" [from] ([with]select[option] [field] [from] [table][join][where][group][having][order]) "__t") where "__dsql_rownum" > [limit_start][and_limit_end]';
 
     public function limit($cnt, $shift = null)
@@ -19,12 +20,12 @@ public function limit($cnt, $shift = null)
         return parent::limit($cnt, $shift);
     }
 
-    public function _render_limit_start()
+    public function _render_limit_start(): string
     {
-        return (int) $this->args['limit']['shift'];
+        return $this->args['limit']['shift'] ?? '0';
     }
 
-    public function _render_and_limit_end()
+    public function _render_and_limit_end(): ?string
     {
         if (!$this->args['limit']['cnt']) {
             return '';

diff --git a/src/Oracle/Version12/Query.php b/src/Oracle/Version12/Query.php
@@ -8,19 +8,21 @@
 
 class Query extends AbstractQuery
 {
-    public function _render_limit()
+    public function _render_limit(): ?string
     {
-        if (isset($this->args['limit'])) {
-            $cnt = (int) $this->args['limit']['cnt'];
-            $shift = (int) $this->args['limit']['shift'];
-
-            return ' ' . trim(
-                ($shift ? 'OFFSET ' . $shift . ' ROWS' : '') .
-                ' ' .
-                // as per spec 'NEXT' is synonymous to 'FIRST', so not bothering with it.
-                // https://docs.oracle.com/javadb/10.8.3.0/ref/rrefsqljoffsetfetch.html
-                ($cnt ? 'FETCH NEXT ' . $cnt . ' ROWS ONLY' : '')
-            );
+        if (!isset($this->args['limit'])) {
+            return null;
         }
+
+        $cnt = (int) $this->args['limit']['cnt'];
+        $shift = (int) $this->args['limit']['shift'];
+
+        return ' ' . trim(
+            ($shift ? 'OFFSET ' . $shift . ' ROWS' : '') .
+            ' ' .
+            // as per spec 'NEXT' is synonymous to 'FIRST', so not bothering with it.
+            // https://docs.oracle.com/javadb/10.8.3.0/ref/rrefsqljoffsetfetch.html
+            ($cnt ? 'FETCH NEXT ' . $cnt . ' ROWS ONLY' : '')
+        );
     }
 }