codeigniter4 / devkit

Development toolkit for CodeIgniter libraries and projects
MIT License
58 stars 11 forks source link

TypedPropertyFromAssignsRector is trying to update App/Config files and ruin the app #103

Closed jozefrebjak closed 1 year ago

jozefrebjak commented 1 year ago

If we copy rector.php from Templates to new project, and run rector process it's trying to update App/Config files like:

./vendor/bin/rector process
 11/11 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
11 files with changes
=====================

1) tests/_support/Models/ExampleModel.php:5

    ---------- begin diff ----------
@@ @@

 class ExampleModel extends Model
 {
-    protected $table          = 'factories';
-    protected $primaryKey     = 'id';
-    protected $returnType     = 'object';
-    protected $useSoftDeletes = false;
-    protected $allowedFields  = [
+    protected string $table          = 'factories';
+    protected string $primaryKey     = 'id';
+    protected string $returnType     = 'object';
+    protected bool $useSoftDeletes = false;
+    protected array $allowedFields  = [
         'name',
         'uid',
         'class',
@@ @@
         'icon',
         'summary',
     ];
-    protected $useTimestamps      = true;
-    protected $validationRules    = [];
-    protected $validationMessages = [];
-    protected $skipValidation     = false;
+    protected bool $useTimestamps      = true;
+    protected array $validationRules    = [];
+    protected array $validationMessages = [];
+    protected bool $skipValidation     = false;
 }
    ----------- end diff -----------

Applied rules:
 * TypedPropertyFromAssignsRector

2) tests/_support/Libraries/ConfigReader.php:1

    ---------- begin diff ----------
@@ @@

 namespace Tests\Support\Libraries;

+use Config\App;
 /**
  * Class ConfigReader
  *
@@ @@
  * loading external values. Used to read actual local values from
  * a config file.
  */
-class ConfigReader extends \Config\App
+class ConfigReader extends App
 {
     public function __construct()
     {
    ----------- end diff -----------

3) tests/_support/Database/Migrations/2020-02-22-222222_example_migration.php:5

    ---------- begin diff ----------
@@ @@

 class ExampleMigration extends Migration
 {
-    protected $DBGroup = 'tests';
+    protected string $DBGroup = 'tests';

     public function up()
     {
    ----------- end diff -----------

Applied rules:
 * TypedPropertyFromAssignsRector

4) app/Controllers/BaseController.php:31

    ---------- begin diff ----------
@@ @@
      * An array of helpers to be loaded automatically upon
      * class instantiation. These helpers will be available
      * to all other controllers that extend BaseController.
-     *
-     * @var array
      */
-    protected $helpers = [];
+    protected array $helpers = [];

     /**
      * Be sure to declare properties for any property fetch you initialized.
    ----------- end diff -----------

Applied rules:
 * RemoveUselessVarTagRector
 * TypedPropertyFromAssignsRector

5) app/Config/View.php:13

    ---------- begin diff ----------
@@ @@
      * to each view. You might prefer to have the data stick around between
      * calls so that it is available to all views. If that is the case,
      * set $saveData to true.
-     *
-     * @var bool
      */
-    public $saveData = true;
+    public bool $saveData = true;

     /**
      * Parser Filters map a filter name with any PHP callable. When the
@@ @@
      * Examples:
      *  { title|esc(js) }
      *  { created_on|date(Y-m-d)|esc(attr) }
-     *
-     * @var array
      */
-    public $filters = [];
+    public array $filters = [];

     /**
      * Parser Plugins provide a way to extend the functionality provided
      * by the core Parser by creating aliases that will be replaced with
      * any callable. Can be single or tag pair.
-     *
-     * @var array
      */
-    public $plugins = [];
+    public array $plugins = [];

     /**
      * View Decorators are class methods that will be run in sequence to
    ----------- end diff -----------

Applied rules:
 * RemoveUselessVarTagRector
 * TypedPropertyFromAssignsRector

6) app/Config/Publisher.php:20

    ---------- begin diff ----------
@@ @@
      *
      * @var array<string,string>
      */
-    public $restrictions = [
+    public array $restrictions = [
         ROOTPATH => '*',
         FCPATH   => '#\.(s?css|js|map|html?|xml|json|webmanifest|ttf|eot|woff2?|gif|jpe?g|tiff?|png|webp|bmp|ico|svg)$#i',
     ];
    ----------- end diff -----------

Applied rules:
 * TypedPropertyFromAssignsRector

7) app/Config/Modules.php:13

    ---------- begin diff ----------
@@ @@
      * If true, then auto-discovery will happen across all elements listed in
      * $aliases below. If false, no auto-discovery will happen at all,
      * giving a slight performance boost.
-     *
-     * @var bool
      */
-    public $enabled = true;
+    public bool $enabled = true;

     /**
      * --------------------------------------------------------------------------
@@ @@
      *
      * If true, then auto-discovery will happen across all namespaces loaded
      * by Composer, as well as the namespaces configured locally.
-     *
-     * @var bool
      */
-    public $discoverInComposer = true;
+    public bool $discoverInComposer = true;

     /**
      * The Composer package list for Auto-Discovery
@@ @@
      *           'pestphp/pest',
      *       ],
      *   ]
-     *
-     * @var array
      */
-    public $composerPackages = [];
+    public array $composerPackages = [];

     /**
      * --------------------------------------------------------------------------
@@ @@
      *
      * @var string[]
      */
-    public $aliases = [
+    public array $aliases = [
         'events',
         'filters',
         'registrars',
    ----------- end diff -----------

Applied rules:
 * RemoveUselessVarTagRector
 * TypedPropertyFromAssignsRector

8) app/Config/Cookie.php:26

    ---------- begin diff ----------
@@ @@
      *
      * @var DateTimeInterface|int|string
      */
-    public $expires = 0;
+    public int $expires = 0;

     /**
      * --------------------------------------------------------------------------
    ----------- end diff -----------

Applied rules:
 * TypedPropertyFromAssignsRector

9) app/Config/ContentSecurityPolicy.php:53

    ---------- begin diff ----------
@@ @@
      *
      * @var string|string[]
      */
-    public $scriptSrc = 'self';
+    public string $scriptSrc = 'self';

     /**
      * Lists allowed stylesheets' URLs.
@@ @@
      *
      * @var string|string[]
      */
-    public $styleSrc = 'self';
+    public string $styleSrc = 'self';

     /**
      * Defines the origins from which images can be loaded.
@@ @@
      *
      * @var string|string[]
      */
-    public $imageSrc = 'self';
+    public string $imageSrc = 'self';

     /**
      * Restricts the URLs that can appear in a page's `<base>` element.
@@ @@
      *
      * @var string|string[]
      */
-    public $childSrc = 'self';
+    public string $childSrc = 'self';

     /**
      * Limits the origins that you can connect to (via XHR,
@@ @@
      *
      * @var string|string[]
      */
-    public $connectSrc = 'self';
+    public string $connectSrc = 'self';

     /**
      * Specifies the origins that can serve web fonts.
@@ @@
      *
      * @var string|string[]
      */
-    public $formAction = 'self';
+    public string $formAction = 'self';

     /**
      * Specifies the sources that can embed the current page.
@@ @@
      *
      * @var string|string[]
      */
-    public $objectSrc = 'self';
+    public string $objectSrc = 'self';

     /**
      * @var string|string[]|null
    ----------- end diff -----------

Applied rules:
 * TypedPropertyFromAssignsRector

10) app/Config/Cache.php:61

    ---------- begin diff ----------
@@ @@
      *
      * @var bool|string[]
      */
-    public $cacheQueryString = false;
+    public bool $cacheQueryString = false;

     /**
      * --------------------------------------------------------------------------
    ----------- end diff -----------

Applied rules:
 * TypedPropertyFromAssignsRector

11) app/Config/Autoload.php:60

    ---------- begin diff ----------
@@ @@
      *
      * @var array<string, string>
      */
-    public $classmap = [];
+    public array $classmap = [];

     /**
      * -------------------------------------------------------------------
@@ @@
      * @var string[]
      * @phpstan-var list<string>
      */
-    public $files = [];
+    public array $files = [];

     /**
      * -------------------------------------------------------------------
@@ @@
      * @var string[]
      * @phpstan-var list<string>
      */
-    public $helpers = [];
+    public array $helpers = [];
 }
    ----------- end diff -----------

Applied rules:
 * TypedPropertyFromAssignsRector

 [OK] 11 files have been changed by Rector                                                                              

and after that php spark serve is not wokring:

hp spark serve            
PHP Fatal error:  Type of Config\Autoload::$classmap must not be defined (as in class CodeIgniter\Config\AutoloadConfig) in /Users/jrebjak/CodeIgniter4/crm/app/Config/Autoload.php on line 18

Fatal error: Type of Config\Autoload::$classmap must not be defined (as in class CodeIgniter\Config\AutoloadConfig) in /Users/jrebjak/CodeIgniter4/crm/app/Config/Autoload.php on line 18

I believe devkit should fix this mainly because of newcomers.

Easy fix is to set false in line:

TypedPropertyFromAssignsRector::INLINE_PUBLIC => false
kenjis commented 1 year ago

Yes, it should be false by default. It cannot be used without skip list for CI4 app. PR is welcome.

MGatner commented 1 year ago

I have a number of exceptions I put in place for projects with Rector; the template definitely shouldn't be used without consideration. I think I'm this case updating the template is a good idea.

kenjis commented 1 year ago

Yes, templates should be used only after careful consideration.

This setting should be true. However, the CI4 code is not properly typed yet, so it will break the app. Some classes should be added to the skip list.