refactor: enable instanceof and strictBooleans rector set (#9339)

* refactor: enable instanceof and strictBooleans rector set * refactor: bump to rector 2.0.4 * refactor: clean condition check * refactor: revert ThirdParty change * refactor: re-run phpstan baseline * refactor: fix never empty array on Image * refactor: avoid repetitive call pg_last_error() * refactor: pg_last_error() always returns string * refactor: use return empty string on pg_last_error() got empty "0" * refactor: various compare !== 1 on preg_match() * refactor: use falsy check on getenv() * refactor: more !== 1 compare on preg_match() * refactor: use empty string check on guessExtension * refactor: run cs fix * refactor: more !== 1 compare on preg_match() * use direct result of pg_last_error() * refactor: use str_contains() over strpos on Forge * refactor: fix unused variable * refactor: check empty string on ResponseTrait * refactor: more preg_match() and empty string only check * refactor: compare to 0 on preg_match_all() * refactor: re-run rector * refactor: preg_match_all() likely less to return false
codeigniter4 · Dec 28, 2024 · ee4ee98 · ee4ee98
1 parent 8cff099
commit ee4ee98
Show file tree

Hide file tree

Showing 37 changed files with 63 additions and 99 deletions.
diff --git a/app/Views/errors/cli/error_exception.php b/app/Views/errors/cli/error_exception.php
@@ -52,7 +52,7 @@
 
         $args = implode(', ', array_map(static fn ($value) => match (true) {
             is_object($value) => 'Object(' . $value::class . ')',
-            is_array($value)  => count($value) ? '[...]' : '[]',
+            is_array($value)  => $value !== [] ? '[...]' : '[]',
             $value === null   => 'null', // return the lowercased version
             default           => var_export($value, true),
         }, array_values($error['args'] ?? [])));

diff --git a/composer.json b/composer.json
@@ -28,7 +28,7 @@
         "phpunit/phpcov": "^9.0.2 || ^10.0",
         "phpunit/phpunit": "^10.5.16 || ^11.2",
         "predis/predis": "^1.1 || ^2.0",
-        "rector/rector": "2.0.3",
+        "rector/rector": "2.0.4",
         "shipmonk/phpstan-baseline-per-identifier": "^2.0"
     },
     "replace": {

diff --git a/rector.php b/rector.php
@@ -38,7 +38,6 @@
 use Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector;
 use Rector\Php81\Rector\FuncCall\NullToStrictStringFuncCallArgRector;
 use Rector\PHPUnit\CodeQuality\Rector\Class_\YieldDataProviderRector;
-use Rector\PHPUnit\Set\PHPUnitSetList;
 use Rector\Privatization\Rector\Property\PrivatizeFinalClassPropertyRector;
 use Rector\Strict\Rector\Empty_\DisallowedEmptyRuleFixerRector;
 use Rector\Strict\Rector\If_\BooleanInIfConditionRuleFixerRector;
@@ -56,11 +55,8 @@
 
 return RectorConfig::configure()
     ->withPhpSets(php81: true)
-    ->withPreparedSets(deadCode: true)
-    ->withSets([
-        PHPUnitSetList::PHPUNIT_CODE_QUALITY,
-        PHPUnitSetList::PHPUNIT_100,
-    ])
+    ->withPreparedSets(deadCode: true, instanceOf: true, strictBooleans: true, phpunitCodeQuality: true)
+    ->withComposerBased(phpunit: true)
     ->withParallel(120, 8, 10)
     ->withCache(
         // Github action cache or local

diff --git a/system/CLI/CLI.php b/system/CLI/CLI.php
@@ -348,7 +348,7 @@ public static function promptByMultipleKeys(string $text, array $options): array
             // return the prompt again if $input contain(s) non-numeric character, except a comma.
             // And if max from $options less than max from input,
             // it means user tried to access null value in $options
-            if (! $pattern || $maxOptions < $maxInput) {
+            if ($pattern === 0 || $maxOptions < $maxInput) {
                 static::error('Please select correctly.');
                 CLI::newLine();
 

diff --git a/system/Cache/Handlers/FileHandler.php b/system/Cache/Handlers/FileHandler.php
@@ -308,7 +308,7 @@ protected function deleteFiles(string $path, bool $delDir = false, bool $htdocs
             if ($filename !== '.' && $filename !== '..') {
                 if (is_dir($path . DIRECTORY_SEPARATOR . $filename) && $filename[0] !== '.') {
                     $this->deleteFiles($path . DIRECTORY_SEPARATOR . $filename, $delDir, $htdocs, $_level + 1);
-                } elseif (! $htdocs || ! preg_match('/^(\.htaccess|index\.(html|htm|php)|web\.config)$/i', $filename)) {
+                } elseif (! $htdocs || preg_match('/^(\.htaccess|index\.(html|htm|php)|web\.config)$/i', $filename) !== 1) {
                     @unlink($path . DIRECTORY_SEPARATOR . $filename);
                 }
             }

diff --git a/system/Commands/Utilities/Routes/AutoRouterImproved/ControllerMethodReader.php b/system/Commands/Utilities/Routes/AutoRouterImproved/ControllerMethodReader.php
@@ -221,7 +221,7 @@ private function getRouteForDefaultController(
         if ($classShortname === $defaultController) {
             $pattern                = '#' . preg_quote(lcfirst($defaultController), '#') . '\z#';
             $routeWithoutController = rtrim(preg_replace($pattern, '', $uriByClass), '/');
-            $routeWithoutController = $routeWithoutController ?: '/';
+            $routeWithoutController = $routeWithoutController !== '' && $routeWithoutController !== '0' ? $routeWithoutController : '/';
 
             [$params, $routeParams] = $this->getParameters($method);
 

diff --git a/system/Commands/Utilities/Routes/ControllerMethodReader.php b/system/Commands/Utilities/Routes/ControllerMethodReader.php
@@ -161,7 +161,7 @@ private function getRouteWithoutController(
 
         $pattern                = '#' . preg_quote(lcfirst($defaultController), '#') . '\z#';
         $routeWithoutController = rtrim(preg_replace($pattern, '', $uriByClass), '/');
-        $routeWithoutController = $routeWithoutController ?: '/';
+        $routeWithoutController = $routeWithoutController !== '' && $routeWithoutController !== '0' ? $routeWithoutController : '/';
 
         return [[
             'route'   => $routeWithoutController,

diff --git a/system/Config/DotEnv.php b/system/Config/DotEnv.php
@@ -94,7 +94,7 @@ public function parse(): ?array
      */
     protected function setVariable(string $name, string $value = '')
     {
-        if (! getenv($name, true)) {
+        if (getenv($name, true) === false) {
             putenv("{$name}={$value}");
         }
 

diff --git a/system/Config/Services.php b/system/Config/Services.php
@@ -345,7 +345,7 @@ public static function image(?string $handler = null, ?Images $config = null, bo
         $config ??= config(Images::class);
         assert($config instanceof Images);
 
-        $handler = $handler ?: $config->defaultHandler;
+        $handler = $handler !== null && $handler !== '' && $handler !== '0' ? $handler : $config->defaultHandler;
         $class   = $config->handlers[$handler];
 
         return new $class($config);
@@ -385,7 +385,7 @@ public static function language(?string $locale = null, bool $getShared = true)
         }
 
         // Use '?:' for empty string check
-        $locale = $locale ?: $requestLocale;
+        $locale = $locale !== null && $locale !== '' && $locale !== '0' ? $locale : $requestLocale;
 
         return new Language($locale);
     }
@@ -484,7 +484,7 @@ public static function parser(?string $viewPath = null, ?ViewConfig $config = nu
             return static::getSharedInstance('parser', $viewPath, $config);
         }
 
-        $viewPath = $viewPath ?: (new Paths())->viewDirectory;
+        $viewPath = $viewPath !== null && $viewPath !== '' && $viewPath !== '0' ? $viewPath : (new Paths())->viewDirectory;
         $config ??= config(ViewConfig::class);
 
         return new Parser($config, $viewPath, AppServices::get('locator'), CI_DEBUG, AppServices::get('logger'));
@@ -503,7 +503,7 @@ public static function renderer(?string $viewPath = null, ?ViewConfig $config =
             return static::getSharedInstance('renderer', $viewPath, $config);
         }
 
-        $viewPath = $viewPath ?: (new Paths())->viewDirectory;
+        $viewPath = $viewPath !== null && $viewPath !== '' && $viewPath !== '0' ? $viewPath : (new Paths())->viewDirectory;
         $config ??= config(ViewConfig::class);
 
         return new View($config, $viewPath, AppServices::get('locator'), CI_DEBUG, AppServices::get('logger'));

diff --git a/system/Cookie/Cookie.php b/system/Cookie/Cookie.php
@@ -482,7 +482,7 @@ public function withNeverExpiring()
      */
     public function withPath(?string $path)
     {
-        $path = $path ?: self::$defaults['path'];
+        $path = $path !== null && $path !== '' && $path !== '0' ? $path : self::$defaults['path'];
         $this->validatePrefix($this->prefix, $this->secure, $path, $this->domain);
 
         $cookie = clone $this;

diff --git a/system/Database/BaseBuilder.php b/system/Database/BaseBuilder.php
@@ -1736,7 +1736,7 @@ public function countAllResults(bool $reset = true)
         // Restore the LIMIT setting
         $this->QBLimit = $limit;
 
-        $row = ! $result instanceof ResultInterface ? null : $result->getRow();
+        $row = $result instanceof ResultInterface ? $result->getRow() : null;
 
         if (empty($row)) {
             return 0;
@@ -3167,11 +3167,11 @@ protected function compileWhereHaving(string $qbKey): string
                     $op = $this->getOperator($condition);
                     if (
                         $op === false
-                        || ! preg_match(
+                        || preg_match(
                             '/^(\(?)(.*)(' . preg_quote($op, '/') . ')\s*(.*(?<!\)))?(\)?)$/i',
                             $condition,
                             $matches
-                        )
+                        ) !== 1
                     ) {
                         continue;
                     }

diff --git a/system/Database/BaseConnection.php b/system/Database/BaseConnection.php
@@ -610,7 +610,7 @@ abstract protected function execute(string $sql);
      */
     public function query(string $sql, $binds = null, bool $setEscapeFlags = true, string $queryClass = '')
     {
-        $queryClass = $queryClass ?: $this->queryClass;
+        $queryClass = $queryClass !== '' && $queryClass !== '0' ? $queryClass : $this->queryClass;
 
         if (empty($this->connID)) {
             $this->initialize();

diff --git a/system/Database/Database.php b/system/Database/Database.php
@@ -96,7 +96,7 @@ protected function parseDSN(array $params): array
     {
         $dsn = parse_url($params['DSN']);
 
-        if (! $dsn) {
+        if ($dsn === 0 || $dsn === '' || $dsn === '0' || $dsn === [] || $dsn === false || $dsn === null) {
             throw new InvalidArgumentException('Your DSN connection string is invalid.');
         }
 

diff --git a/system/Database/MigrationRunner.php b/system/Database/MigrationRunner.php
@@ -450,7 +450,7 @@ protected function migrationFromFile(string $path, string $namespace)
 
         $filename = basename($path, '.php');
 
-        if (! preg_match($this->regex, $filename)) {
+        if (preg_match($this->regex, $filename) !== 1) {
             return false;
         }
 

diff --git a/system/Database/MySQLi/Forge.php b/system/Database/MySQLi/Forge.php
@@ -116,11 +116,11 @@ protected function _createTableAttributes(array $attributes): string
             }
         }
 
-        if ($this->db->charset !== '' && ! strpos($sql, 'CHARACTER SET') && ! strpos($sql, 'CHARSET')) {
+        if ($this->db->charset !== '' && ! str_contains($sql, 'CHARACTER SET') && ! str_contains($sql, 'CHARSET')) {
             $sql .= ' DEFAULT CHARACTER SET = ' . $this->db->escapeString($this->db->charset);
         }
 
-        if ($this->db->DBCollat !== '' && ! strpos($sql, 'COLLATE')) {
+        if ($this->db->DBCollat !== '' && ! str_contains($sql, 'COLLATE')) {
             $sql .= ' COLLATE = ' . $this->db->escapeString($this->db->DBCollat);
         }
 

diff --git a/system/Database/Postgre/Connection.php b/system/Database/Postgre/Connection.php
@@ -463,7 +463,7 @@ public function error(): array
     {
         return [
             'code'    => '',
-            'message' => pg_last_error($this->connID) ?: '',
+            'message' => pg_last_error($this->connID),
         ];
     }
 

diff --git a/system/Email/Email.php b/system/Email/Email.php
@@ -495,7 +495,7 @@ public function setFrom($from, $name = '', $returnPath = null)
 
         if ($name !== '') {
             // only use Q encoding if there are characters that would require it
-            if (! preg_match('/[\200-\377]/', $name)) {
+            if (preg_match('/[\200-\377]/', $name) !== 1) {
                 $name = '"' . addcslashes($name, "\0..\37\177'\"\\") . '"';
             } else {
                 $name = $this->prepQEncoding($name);
@@ -532,7 +532,7 @@ public function setReplyTo($replyto, $name = '')
             $this->tmpArchive['replyName'] = $name;
 
             // only use Q encoding if there are characters that would require it
-            if (! preg_match('/[\200-\377]/', $name)) {
+            if (preg_match('/[\200-\377]/', $name) !== 1) {
                 $name = '"' . addcslashes($name, "\0..\37\177'\"\\") . '"';
             } else {
                 $name = $this->prepQEncoding($name);

diff --git a/system/HTTP/Files/UploadedFile.php b/system/HTTP/Files/UploadedFile.php
@@ -302,7 +302,9 @@ public function getTempName(): string
      */
     public function getExtension(): string
     {
-        return $this->guessExtension() ?: $this->getClientExtension();
+        $guessExtension = $this->guessExtension();
+
+        return $guessExtension !== '' ? $guessExtension : $this->getClientExtension();
     }
 
     /**

diff --git a/system/HTTP/ResponseTrait.php b/system/HTTP/ResponseTrait.php
@@ -569,7 +569,7 @@ public function getCookieStore()
      */
     public function hasCookie(string $name, ?string $value = null, string $prefix = ''): bool
     {
-        $prefix = $prefix ?: Cookie::setDefaults()['prefix']; // to retain BC
+        $prefix = $prefix !== '' ? $prefix : Cookie::setDefaults()['prefix']; // to retain BC
 
         return $this->cookieStore->has($name, $prefix, $value);
     }
@@ -589,7 +589,7 @@ public function getCookie(?string $name = null, string $prefix = '')
         }
 
         try {
-            $prefix = $prefix ?: Cookie::setDefaults()['prefix']; // to retain BC
+            $prefix = $prefix !== '' ? $prefix : Cookie::setDefaults()['prefix']; // to retain BC
 
             return $this->cookieStore->get($name, $prefix);
         } catch (CookieException $e) {
@@ -610,7 +610,7 @@ public function deleteCookie(string $name = '', string $domain = '', string $pat
             return $this;
         }
 
-        $prefix = $prefix ?: Cookie::setDefaults()['prefix']; // to retain BC
+        $prefix = $prefix !== '' ? $prefix : Cookie::setDefaults()['prefix']; // to retain BC
 
         $prefixed = $prefix . $name;
         $store    = $this->cookieStore;

diff --git a/system/HTTP/SiteURI.php b/system/HTTP/SiteURI.php
@@ -421,7 +421,7 @@ public function siteUrl($relativePath = '', ?string $scheme = null, ?App $config
         $relativePath = $this->stringifyRelativePath($relativePath);
 
         // Check current host.
-        $host = ! $config instanceof App ? $this->getHost() : null;
+        $host = $config instanceof App ? null : $this->getHost();
 
         $config ??= config(App::class);
 

diff --git a/system/Helpers/filesystem_helper.php b/system/Helpers/filesystem_helper.php
@@ -166,7 +166,7 @@ function delete_files(string $path, bool $delDir = false, bool $htdocs = false,
                     continue;
                 }
 
-                if (! $htdocs || ! preg_match('/^(\.htaccess|index\.(html|htm|php)|web\.config)$/i', $filename)) {
+                if (! $htdocs || preg_match('/^(\.htaccess|index\.(html|htm|php)|web\.config)$/i', $filename) !== 1) {
                     $isDir = $object->isDir();
                     if ($isDir && $delDir) {
                         rmdir($object->getPathname());

diff --git a/system/Helpers/form_helper.php b/system/Helpers/form_helper.php
@@ -62,7 +62,7 @@ function form_open(string $action = '', $attributes = [], array $hidden = []): s
         // Add CSRF field if enabled, but leave it out for GET requests and requests to external websites
         $before = service('filters')->getFilters()['before'];
 
-        if ((in_array('csrf', $before, true) || array_key_exists('csrf', $before)) && str_contains($action, base_url()) && ! stripos($form, 'method="get"')) {
+        if ((in_array('csrf', $before, true) || array_key_exists('csrf', $before)) && str_contains($action, base_url()) && stripos($form, 'method="get"') === false) {
             $form .= csrf_field($csrfId ?? null);
         }
 

diff --git a/system/Helpers/html_helper.php b/system/Helpers/html_helper.php
@@ -112,7 +112,7 @@ function img($src = '', bool $indexPage = false, $attributes = ''): string
         $img = '<img';
 
         // Check for a relative URI
-        if (! preg_match('#^([a-z]+:)?//#i', $src['src']) && ! str_starts_with($src['src'], 'data:')) {
+        if (preg_match('#^([a-z]+:)?//#i', $src['src']) !== 1 && ! str_starts_with($src['src'], 'data:')) {
             if ($indexPage) {
                 $img .= ' src="' . site_url($src['src']) . '"';
             } else {
@@ -206,7 +206,7 @@ function script_tag($src = '', bool $indexPage = false): string
         }
 
         foreach ($src as $k => $v) {
-            if ($k === 'src' && ! preg_match('#^([a-z]+:)?//#i', $v)) {
+            if ($k === 'src' && preg_match('#^([a-z]+:)?//#i', $v) !== 1) {
                 if ($indexPage) {
                     $script .= 'src="' . site_url($v) . '" ';
                 } else {
@@ -252,7 +252,7 @@ function link_tag(
             $href      = $href['href'] ?? '';
         }
 
-        if (! preg_match('#^([a-z]+:)?//#i', $href)) {
+        if (preg_match('#^([a-z]+:)?//#i', $href) !== 1) {
             $attributes['href'] = $indexPage ? site_url($href) : slash_item('baseURL') . $href;
         } else {
             $attributes['href'] = $href;

diff --git a/system/I18n/TimeTrait.php b/system/I18n/TimeTrait.php
@@ -73,7 +73,7 @@ trait TimeTrait
      */
     public function __construct(?string $time = null, $timezone = null, ?string $locale = null)
     {
-        $this->locale = $locale ?: Locale::getDefault();
+        $this->locale = $locale !== null && $locale !== '' && $locale !== '0' ? $locale : Locale::getDefault();
 
         $time ??= '';
 
@@ -958,7 +958,7 @@ public function sameAs($testTime, ?string $timezone = null): bool
         if ($testTime instanceof DateTimeInterface) {
             $testTime = $testTime->format('Y-m-d H:i:s');
         } elseif (is_string($testTime)) {
-            $timezone = $timezone ?: $this->timezone;
+            $timezone = $timezone !== null && $timezone !== '' && $timezone !== '0' ? $timezone : $this->timezone;
             $timezone = $timezone instanceof DateTimeZone ? $timezone : new DateTimeZone($timezone);
             $testTime = new DateTime($testTime, $timezone);
             $testTime = $testTime->format('Y-m-d H:i:s');
@@ -1108,7 +1108,7 @@ public function getUTCObject($time, ?string $timezone = null)
         if ($time instanceof self) {
             $time = $time->toDateTime();
         } elseif (is_string($time)) {
-            $timezone = $timezone ?: $this->timezone;
+            $timezone = $timezone !== null && $timezone !== '' && $timezone !== '0' ? $timezone : $this->timezone;
             $timezone = $timezone instanceof DateTimeZone ? $timezone : new DateTimeZone($timezone);
             $time     = new DateTime($time, $timezone);
         }

diff --git a/system/Images/Image.php b/system/Images/Image.php
@@ -102,8 +102,9 @@ public function copy(string $targetPath, ?string $targetName = null, int $perms
     public function getProperties(bool $return = false)
     {
         $path = $this->getPathname();
+        $vals = getimagesize($path);
 
-        if (! $vals = getimagesize($path)) {
+        if ($vals === false) {
             throw ImageException::forFileNotSupported();
         }
 

diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php
@@ -1329,7 +1329,7 @@ protected function fillRouteParams(string $from, ?array $params = null): string
         $patterns = $matches[0];
 
         foreach ($patterns as $index => $pattern) {
-            if (! preg_match('#^' . $pattern . '$#u', $params[$index])) {
+            if (preg_match('#^' . $pattern . '$#u', $params[$index]) !== 1) {
                 throw RouterException::forInvalidParameterType();
             }
 
@@ -1391,7 +1391,7 @@ protected function buildReverseRoute(string $from, array $params): string
             // or maybe $placeholder is not a placeholder, but a regex.
             $pattern = $this->placeholders[$placeholderName] ?? $placeholder;
 
-            if (! preg_match('#^' . $pattern . '$#u', (string) $params[$index])) {
+            if (preg_match('#^' . $pattern . '$#u', (string) $params[$index]) !== 1) {
                 throw RouterException::forInvalidParameterType();
             }
 

diff --git a/system/Session/Handlers/FileHandler.php b/system/Session/Handlers/FileHandler.php
@@ -290,7 +290,7 @@ public function gc($max_lifetime)
 
         while (($file = readdir($directory)) !== false) {
             // If the filename doesn't match this pattern, it's either not a session file or is not ours
-            if (! preg_match($pattern, $file)
+            if (preg_match($pattern, $file) !== 1
                 || ! is_file($this->savePath . DIRECTORY_SEPARATOR . $file)
                 || ($mtime = filemtime($this->savePath . DIRECTORY_SEPARATOR . $file)) === false
                 || $mtime > $ts

diff --git a/system/Session/Handlers/MemcachedHandler.php b/system/Session/Handlers/MemcachedHandler.php
@@ -93,12 +93,12 @@ public function open($path, $name): bool
         }
 
         if (
-            ! preg_match_all(
+            preg_match_all(
                 '#,?([^,:]+)\:(\d{1,5})(?:\:(\d+))?#',
                 $this->savePath,
                 $matches,
                 PREG_SET_ORDER
-            )
+            ) === 0
         ) {
             $this->memcached = null;
             $this->logger->error('Session: Invalid Memcached save path format: ' . $this->savePath);