Improve PHPStan level to 5 (#814)

* Improve PHPStan level to 3 * Improve PHPStan level to 4 * Improve PHPStan level to 5 * fix stan
atk4 · Dec 26, 2020 · 6ce8726 · 6ce8726
1 parent 1dee056
commit 6ce8726
Show file tree

Hide file tree

Showing 26 changed files with 84 additions and 121 deletions.
diff --git a/docs/expressions.rst b/docs/expressions.rst
@@ -63,7 +63,7 @@ no purpose initially, it does come in handy in some cases, when you need to
 unite a few statistical queries. Let's start by looking a at a very basic
 example::
 
-    $m = new Model($db, false);
+    $m = new Model($db, ['table' => false]);
     $m->addExpression('now', 'now()');
     $m->loadAny();
     echo $m->get('now');
@@ -79,7 +79,7 @@ actually use consistently throughout the system. The real benefit from this
 can be gained when you need to pull various statistical values from your
 database at once::
 
-    $m = new Model($db, false);
+    $m = new Model($db, ['table' => false]);
     $m->addExpression('total_orders', (new Model_Order($db))->action('count'));
     $m->addExpression('total_payments', (new Model_Payment($db))->action('count'));
     $m->addExpression('total_received', (new Model_Payment($db))->action('fx0', ['sum', 'amount']));

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -2,7 +2,7 @@ includes:
     - vendor/mahalux/atk4-hintable/phpstan-ext.neon
 
 parameters:
-    level: 2
+    level: 5
     paths:
         - ./
     excludes_analyse:
@@ -19,13 +19,14 @@ parameters:
         # TODO remove once https://github.com/atk4/ui/issues/1548 is resolved
         - '~^Class Atk4\\Ui\\(Button|Icon) not found\.$~'
 
+        # TODO do we want to allow to pass an Model instance?
+        - '~^Parameter #2 \$defaults of method Atk4\\Data\\Model::(hasOne|hasMany)\(\) expects array, .+ given\.$~'
+        - '~^Parameter #2 \$defaults of method Atk4\\Data\\Model\\Join::(hasOne|hasMany)\(\) expects array, .+ given\.$~'
+
         # TODO these rules are generated, this ignores should be fixed in the code
-        # for level = 2
         # for src-schema/PhpunitTestCase.php
         - '~^Access to an undefined property Atk4\\Data\\Persistence\:\:\$connection\.$~'
         - '~^Call to an undefined method Atk4\\Data\\Persistence\:\:dsql\(\)\.$~'
-        # for src/Action/Iterator.php
-        - '~^Access to an undefined property CallbackFilterIterator\:\:\$filterFx\.$~'
         # for src/FieldSqlExpression.php
         - '~^Call to an undefined method Atk4\\Data\\Model\:\:expr\(\)\.$~'
         # for src/Model.php

diff --git a/src-schema/Migration.php b/src-schema/Migration.php
@@ -239,8 +239,10 @@ protected function getReferenceField(Field $field): ?Field
 
             $referenceField = $referenceTheirField ?? $field->reference->getOwner()->id_field;
 
-            $referenceModelClass = $field->reference->model;
-            $referenceModel = new $referenceModelClass(new Persistence\Sql($this->connection));
+            $modelSeed = is_array($field->reference->model)
+                ? $field->reference->model
+                : [get_class($field->reference->model)];
+            $referenceModel = Model::fromSeed($modelSeed, [new Persistence\Sql($this->connection)]);
 
             return $referenceModel->getField($referenceField);
         }

diff --git a/src-schema/PhpunitTestCase.php b/src-schema/PhpunitTestCase.php
@@ -73,7 +73,7 @@ public function stopQuery(): void
 
     protected function tearDown(): void
     {
-        $this->db = null;
+        $this->db = null; // @phpstan-ignore-line
 
         parent::tearDown(); // TODO: Change the autogenerated stub
     }

diff --git a/src/Action/Iterator.php b/src/Action/Iterator.php
@@ -15,7 +15,7 @@
 class Iterator
 {
     /**
-     * @var \ArrayIterator
+     * @var \Iterator
      */
     public $generator;
 
@@ -50,7 +50,7 @@ public function filter(Model\Scope\AbstractScope $condition)
                 $this->generator = new \CallbackFilterIterator($this->generator, static function (array $row) use ($filterFxWeakRef) {
                     return $filterFxWeakRef->get()($row);
                 });
-                $this->generator->filterFx = $filterFx; // prevent filter function to be GCed
+                $this->generator->filterFx = $filterFx; // @phpstan-ignore-line prevent filter function to be GCed
             } else {
                 $this->generator = new \CallbackFilterIterator($this->generator, $filterFx);
             }

diff --git a/src/Field.php b/src/Field.php
@@ -157,7 +157,7 @@ class Field implements Expressionable
      *
      * Value can be array [$encode_callback, $decode_callback].
      *
-     * @var bool|array|null
+     * @var bool|array|string|null
      */
     public $serialize;
 
@@ -400,8 +400,6 @@ public function normalize($value)
             case 'bool':
                 throw (new Exception('Use of obsolete field type abbreviation. Use "integer", "string", "boolean" etc.'))
                     ->addMoreInfo('type', $f->type);
-
-                break;
             }
 
             return $value;

diff --git a/src/Model.php b/src/Model.php
@@ -114,7 +114,7 @@ class Model implements \IteratorAggregate
      *
      * $table = ['user', 'mysql'=>'tbl_user'];
      *
-     * @var string|array
+     * @var string|array|false
      */
     public $table;
 
@@ -135,7 +135,7 @@ class Model implements \IteratorAggregate
     /**
      * Persistence driver inherited from Atk4\Data\Persistence.
      *
-     * @var Persistence|Persistence\Sql
+     * @var Persistence|Persistence\Sql|null
      */
     public $persistence;
 
@@ -331,12 +331,12 @@ public function __construct($persistence = null, $defaults = [])
             return new Model\Scope\RootScope();
         }, null, Model\Scope\RootScope::class)();
 
-        if ((is_string($persistence) || is_array($persistence)) && func_num_args() === 1) {
+        if (is_array($persistence) && func_num_args() === 1) {
             $defaults = $persistence;
             $persistence = null;
         }
 
-        if (is_string($defaults) || $defaults === false) {
+        if (is_string($defaults)) {
             $defaults = ['table' => $defaults];
         }
 
@@ -1152,7 +1152,7 @@ public function load($id)
         if ($ret === false) {
             return $this->unload();
         } elseif (is_object($ret)) {
-            return $ret;
+            return $ret; // @phpstan-ignore-line
         }
 
         return $this;
@@ -1265,7 +1265,7 @@ public function newInstance(string $class = null, array $options = [])
         $model = (self::class)::fromSeed([$class ?? static::class], $options);
 
         if ($this->persistence) {
-            return $this->persistence->add($model);
+            return $this->persistence->add($model); // @phpstan-ignore-line
         }
 
         return $model;
@@ -1337,7 +1337,7 @@ public function tryLoad($id)
             if ($ret === false) {
                 return $this->unload();
             } elseif (is_object($ret)) {
-                return $ret;
+                return $ret; // @phpstan-ignore-line
             }
         } else {
             $this->unload();
@@ -1369,7 +1369,7 @@ public function loadAny()
         if ($ret === false) {
             return $this->unload();
         } elseif (is_object($ret)) {
-            return $ret;
+            return $ret; // @phpstan-ignore-line
         }
 
         return $this;
@@ -1401,7 +1401,7 @@ public function tryLoadAny()
             if ($ret === false) {
                 return $this->unload();
             } elseif (is_object($ret)) {
-                return $ret;
+                return $ret; // @phpstan-ignore-line
             }
         } else {
             $this->unload();
@@ -1800,9 +1800,9 @@ public function getIterator(): \Traversable
 
             if (is_object($ret)) {
                 if ($ret->id_field) {
-                    yield $ret->getId() => $ret;
+                    yield $ret->getId() => $ret; // @phpstan-ignore-line
                 } else {
-                    yield $ret;
+                    yield $ret; // @phpstan-ignore-line
                 }
             } else {
                 if ($this->id_field) {
@@ -1926,7 +1926,7 @@ public function action($mode, $args = [])
     /**
      * Add expression field.
      *
-     * @param string|array|\Closure $expression
+     * @param string|array|\Atk4\Dsql\Expression|\Closure $expression
      *
      * @return Field\Callback
      */
@@ -1939,6 +1939,7 @@ public function addExpression(string $name, $expression)
             unset($expression[0]);
         }
 
+        /** @var Field\Callback */
         $field = Field::fromSeed($this->_default_seed_addExpression, $expression);
 
         $this->addField($name, $field);
@@ -1962,7 +1963,11 @@ public function addCalculatedField(string $name, $expression)
             unset($expression[0]);
         }
 
-        return $this->addField($name, new Field\Callback($expression));
+        $field = new Field\Callback($expression);
+
+        $this->addField($name, $field);
+
+        return $field;
     }
 
     // }}}

diff --git a/src/Model/Join.php b/src/Model/Join.php
@@ -38,7 +38,7 @@ class Join
      * If $persistence is set, then it's used for loading
      * and storing the values, instead $owner->persistence.
      *
-     * @var Persistence|Persistence\Sql
+     * @var Persistence|Persistence\Sql|null
      */
     protected $persistence;
 
@@ -259,7 +259,7 @@ public function addFields($fields = [])
      *
      * @param array $defaults
      *
-     * @return static
+     * @return self
      */
     public function join(string $foreign_table, $defaults = [])
     {
@@ -276,7 +276,7 @@ public function join(string $foreign_table, $defaults = [])
      *
      * @param array $defaults
      *
-     * @return static
+     * @return self
      */
     public function leftJoin(string $foreign_table, $defaults = [])
     {
@@ -316,7 +316,7 @@ public function weakJoin($defaults = [])
     public function hasOne(string $link, $defaults = [])
     {
         if (!is_array($defaults)) {
-            $defaults = ['model' => $defaults ?: 'Model_' . $link];
+            $defaults = ['model' => $defaults];
         }
 
         $defaults['joinName'] = $this->short_name;
@@ -334,7 +334,7 @@ public function hasOne(string $link, $defaults = [])
     public function hasMany(string $link, $defaults = [])
     {
         if (!is_array($defaults)) {
-            $defaults = ['model' => $defaults ?: 'Model_' . $link];
+            $defaults = ['model' => $defaults];
         }
 
         $defaults = array_merge([

diff --git a/src/Model/JoinsTrait.php b/src/Model/JoinsTrait.php
@@ -12,7 +12,7 @@ trait JoinsTrait
     /**
      * The class used by join() method.
      *
-     * @var string|array
+     * @var array
      */
     public $_default_seed_join = [Join::class];
 
@@ -23,7 +23,7 @@ trait JoinsTrait
      * join will also query $foreignTable in order to find additional fields. When inserting
      * the record will be also added inside $foreignTable and relationship will be maintained.
      *
-     * @param array $defaults
+     * @param array|string $defaults
      */
     public function join(string $foreignTable, $defaults = []): Join
     {
@@ -44,7 +44,7 @@ public function join(string $foreignTable, $defaults = []): Join
      *
      * @see join()
      *
-     * @param array $defaults
+     * @param array|string $defaults
      */
     public function leftJoin(string $foreignTable, $defaults = []): Join
     {

diff --git a/src/Model/ReferencesTrait.php b/src/Model/ReferencesTrait.php
@@ -53,7 +53,7 @@ trait ReferencesTrait
     protected function _hasReference(array $seed, string $link, $defaults = []): Reference
     {
         if (!is_array($defaults)) {
-            $defaults = ['model' => $defaults ?: 'Model_' . $link];
+            $defaults = ['model' => $defaults];
         } elseif (isset($defaults[0])) {
             $defaults['model'] = $defaults[0];
             unset($defaults[0]);
@@ -93,7 +93,7 @@ public function addRef(string $link, $fx): Reference
      */
     public function hasOne(string $link, $defaults = []): Reference
     {
-        return $this->_hasReference($this->_default_seed_hasOne, $link, $defaults);
+        return $this->_hasReference($this->_default_seed_hasOne, $link, $defaults); // @phpstan-ignore-line
     }
 
     /**
@@ -105,7 +105,7 @@ public function hasOne(string $link, $defaults = []): Reference
      */
     public function hasMany(string $link, $defaults = []): Reference
     {
-        return $this->_hasReference($this->_default_seed_hasMany, $link, $defaults);
+        return $this->_hasReference($this->_default_seed_hasMany, $link, $defaults); // @phpstan-ignore-line
     }
 
     /**
@@ -117,7 +117,7 @@ public function hasMany(string $link, $defaults = []): Reference
      */
     public function containsOne(string $link, $defaults = []): Reference
     {
-        return $this->_hasReference($this->_default_seed_containsOne, $link, $defaults);
+        return $this->_hasReference($this->_default_seed_containsOne, $link, $defaults); // @phpstan-ignore-line
     }
 
     /**
@@ -129,7 +129,7 @@ public function containsOne(string $link, $defaults = []): Reference
      */
     public function containsMany(string $link, $defaults = []): Reference
     {
-        return $this->_hasReference($this->_default_seed_containsMany, $link, $defaults);
+        return $this->_hasReference($this->_default_seed_containsMany, $link, $defaults); // @phpstan-ignore-line
     }
 
     /**

diff --git a/src/Model/Scope.php b/src/Model/Scope.php
@@ -19,11 +19,7 @@ class Scope extends Scope\AbstractScope
     public const OR = 'OR';
     public const AND = 'AND';
 
-    /**
-     * Junction to use in case more than one element.
-     *
-     * @var self::AND|self::OR
-     */
+    /** @var self::AND|self::OR Junction to use in case more than one element. */
     protected $junction = self::AND;
 
     /**
@@ -67,7 +63,7 @@ public function __clone()
         if ($this->issetOwner()) {
             $this->unsetOwner();
         }
-        $this->short_name = null;
+        $this->short_name = null; // @phpstan-ignore-line
     }
 
     /**