Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: PHP 8.0 constructor promotion #6924

Closed
Closed
2 changes: 1 addition & 1 deletion .github/workflows/test-coding-standards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ jobs:
fail-fast: false
matrix:
php-version:
- '7.4'
- '8.0'
- '8.1'

steps:
- name: Checkout
Expand Down
6 changes: 1 addition & 5 deletions .github/workflows/test-phpunit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ jobs:
strategy:
matrix:
php-version:
- '7.4'
- '8.0'
- '8.1'
- '8.2'
Expand Down Expand Up @@ -79,7 +78,6 @@ jobs:
fail-fast: false
matrix:
php-version:
- '7.4'
- '8.0'
- '8.1'
- '8.2'
Expand All @@ -92,7 +90,7 @@ jobs:
mysql-version:
- '5.7'
include:
- php-version: '7.4'
- php-version: '8.0'
db-platform: MySQLi
mysql-version: '8.0'
- php-version: '8.2'
Expand Down Expand Up @@ -120,7 +118,6 @@ jobs:
strategy:
matrix:
php-version:
- '7.4'
- '8.0'
- '8.1'
- '8.2'
Expand Down Expand Up @@ -148,7 +145,6 @@ jobs:
strategy:
matrix:
php-version:
- '7.4'
- '8.0'
- '8.1'
- '8.2'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-rector.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ jobs:
strategy:
fail-fast: false
matrix:
php-versions: ['7.4', '8.0']
php-versions: ['8.0', '8.1']
paths:
- app
- system
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ Made with [contrib.rocks](https://contrib.rocks).

## Server Requirements

PHP version 7.4 or higher is required, with the following extensions installed:
PHP version 8.0 or higher is required, with the following extensions installed:

- [intl](http://php.net/manual/en/intl.requirements.php)
- [mbstring](http://php.net/manual/en/mbstring.installation.php)
Expand Down
2 changes: 1 addition & 1 deletion admin/framework/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Please read the [*Contributing to CodeIgniter*](https://github.com/codeigniter4/

## Server Requirements

PHP version 7.4 or higher is required, with the following extensions installed:
PHP version 8.0 or higher is required, with the following extensions installed:

- [intl](http://php.net/manual/en/intl.requirements.php)
- [mbstring](http://php.net/manual/en/mbstring.installation.php)
Expand Down
2 changes: 1 addition & 1 deletion admin/framework/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"homepage": "https://codeigniter.com",
"license": "MIT",
"require": {
"php": "^7.4 || ^8.0",
"php": "^8.0",
"ext-intl": "*",
"ext-json": "*",
"ext-mbstring": "*",
Expand Down
4 changes: 2 additions & 2 deletions admin/starter/.github/workflows/phpunit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: PHPUnit

on:
pull_request:
branches:
branches:
- develop

jobs:
Expand All @@ -11,7 +11,7 @@ jobs:

strategy:
matrix:
php-versions: ['7.4', '8.0']
php-versions: ['8.0', '8.1']

runs-on: ubuntu-latest

Expand Down
2 changes: 1 addition & 1 deletion admin/starter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ Problems with it can be raised on our forum, or as issues in the main repository

## Server Requirements

PHP version 7.4 or higher is required, with the following extensions installed:
PHP version 8.0 or higher is required, with the following extensions installed:

- [intl](http://php.net/manual/en/intl.requirements.php)
- [mbstring](http://php.net/manual/en/mbstring.installation.php)
Expand Down
2 changes: 1 addition & 1 deletion admin/starter/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"homepage": "https://codeigniter.com",
"license": "MIT",
"require": {
"php": "^7.4 || ^8.0",
"php": "^8.0",
"codeigniter4/framework": "^4.0"
},
"require-dev": {
Expand Down
2 changes: 1 addition & 1 deletion admin/userguide/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"homepage": "https://codeigniter.com",
"license": "MIT",
"require": {
"php": "^7.4 || ^8.0",
"php": "^8.0",
"codeigniter4/framework": "^4"
},
"support": {
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"homepage": "https://codeigniter.com",
"license": "MIT",
"require": {
"php": "^7.4 || ^8.0",
"php": "^8.0",
"ext-intl": "*",
"ext-json": "*",
"ext-mbstring": "*",
Expand Down
2 changes: 1 addition & 1 deletion contributing/pull_request.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ See [Contribution CSS](./css.md).

### Compatibility

CodeIgniter4 requires [PHP 7.4](https://php.net/releases/7_4_0.php).
CodeIgniter4 requires [PHP 8.0](https://php.net/releases/8_0_0.php).

### Backwards Compatibility

Expand Down
10 changes: 0 additions & 10 deletions phpstan-baseline.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,6 @@ parameters:
count: 1
path: system/HTTP/Files/UploadedFile.php

-
message: "#^Property CodeIgniter\\\\HTTP\\\\Files\\\\UploadedFile\\:\\:\\$error \\(int\\) on left side of \\?\\? is not nullable\\.$#"
count: 2
path: system/HTTP/Files/UploadedFile.php

-
message: "#^Return type \\(bool\\) of method CodeIgniter\\\\HTTP\\\\Files\\\\UploadedFile\\:\\:move\\(\\) should be compatible with return type \\(CodeIgniter\\\\Files\\\\File\\) of method CodeIgniter\\\\Files\\\\File\\:\\:move\\(\\)$#"
count: 1
Expand Down Expand Up @@ -365,11 +360,6 @@ parameters:
count: 1
path: system/Throttle/Throttler.php

-
message: "#^Property CodeIgniter\\\\View\\\\Cell\\:\\:\\$cache \\(CodeIgniter\\\\Cache\\\\CacheInterface\\) in empty\\(\\) is not falsy\\.$#"
count: 2
path: system/View/Cell.php

-
message: "#^Call to an undefined static method CodeIgniter\\\\Config\\\\Factories\\:\\:cells\\(\\)\\.$#"
count: 1
Expand Down
2 changes: 1 addition & 1 deletion public/index.php
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php

// Check PHP version.
$minPhpVersion = '7.4'; // If you update this, don't forget to update `spark`.
$minPhpVersion = '8.0'; // If you update this, don't forget to update `spark`.
if (version_compare(PHP_VERSION, $minPhpVersion, '<')) {
$message = sprintf(
'Your PHP version must be %s or higher to run CodeIgniter. Current version: %s',
Expand Down
14 changes: 13 additions & 1 deletion rector.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
use Rector\Php71\Rector\FuncCall\CountOnNullRector;
use Rector\Php73\Rector\FuncCall\JsonThrowOnErrorRector;
use Rector\Php73\Rector\FuncCall\StringifyStrNeedlesRector;
use Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector;
use Rector\Php80\Rector\FuncCall\TokenGetAllToObjectRector;
use Rector\Php80\Rector\FunctionLike\MixedTypeRector;
use Rector\Php80\Rector\FunctionLike\UnionTypesRector;
use Rector\PHPUnit\Rector\MethodCall\GetMockBuilderGetMockToCreateMockRector;
use Rector\PHPUnit\Set\PHPUnitSetList;
use Rector\Privatization\Rector\Property\PrivatizeFinalClassPropertyRector;
Expand All @@ -55,7 +59,7 @@
return static function (RectorConfig $rectorConfig): void {
$rectorConfig->sets([
SetList::DEAD_CODE,
LevelSetList::UP_TO_PHP_74,
LevelSetList::UP_TO_PHP_80,
PHPUnitSetList::PHPUNIT_SPECIFIC_METHOD,
PHPUnitSetList::PHPUNIT_80,
PHPUnitSetList::REMOVE_MOCKS,
Expand Down Expand Up @@ -125,6 +129,14 @@
GetMockBuilderGetMockToCreateMockRector::class => [
__DIR__ . '/tests/system/Email/EmailTest.php',
],

// This rule breaks code due to a bug.
TokenGetAllToObjectRector::class,

// PHP 8.0 features but cause breaking changes
UnionTypesRector::class,
ClassPropertyAssignToConstructorPromotionRector::class,
MixedTypeRector::class,
]);

// auto import fully qualified class names
Expand Down
2 changes: 1 addition & 1 deletion spark
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (strpos(PHP_SAPI, 'cgi') === 0) {
}

// Check PHP version.
$minPhpVersion = '7.4'; // If you update this, don't forget to update `public/index.php`.
$minPhpVersion = '8.0'; // If you update this, don't forget to update `public/index.php`.
if (version_compare(PHP_VERSION, $minPhpVersion, '<')) {
$message = sprintf(
'Your PHP version must be %s or higher to run CodeIgniter. Current version: %s',
Expand Down
12 changes: 4 additions & 8 deletions system/Autoloader/Autoloader.php
Original file line number Diff line number Diff line change
Expand Up @@ -275,15 +275,15 @@ public function loadClass(string $class)
*/
protected function loadInNamespace(string $class)
{
if (strpos($class, '\\') === false) {
if (! str_contains($class, '\\')) {
return false;
}

foreach ($this->prefixes as $namespace => $directories) {
foreach ($directories as $directory) {
$directory = rtrim($directory, '\\/');

if (strpos($class, $namespace) === 0) {
if (str_starts_with($class, $namespace)) {
$filePath = $directory . str_replace('\\', DIRECTORY_SEPARATOR, substr($class, strlen($namespace))) . '.php';
$filename = $this->includeFile($filePath);

Expand Down Expand Up @@ -344,11 +344,7 @@ public function sanitizeFilename(string $filename): string
);
}
if ($result === false) {
if (version_compare(PHP_VERSION, '8.0.0', '>=')) {
$message = preg_last_error_msg();
} else {
$message = 'Regex error. error code: ' . preg_last_error();
}
$message = preg_last_error_msg();

throw new RuntimeException($message . '. filename: "' . $filename . '"');
}
Expand Down Expand Up @@ -404,7 +400,7 @@ private function loadComposerNamespaces(ClassLoader $composer, array $composerPa

foreach ($srcPaths as $path) {
foreach ($installPaths as $installPath) {
if ($installPath === substr($path, 0, strlen($installPath))) {
if (str_starts_with($path, $installPath)) {
$add = true;
break 2;
}
Expand Down
24 changes: 10 additions & 14 deletions system/Autoloader/FileLocator.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,11 @@
class FileLocator
{
/**
* The Autoloader to use.
*
* @var Autoloader
* @param Autoloader $autoloader The Autoloader to use.
*/
protected $autoloader;

public function __construct(Autoloader $autoloader)
{
$this->autoloader = $autoloader;
public function __construct(
protected Autoloader $autoloader
) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, if a dev extends FileLocator and it has protected $autoloader;,
this change is a breaking change.
https://3v4l.org/NhqDb

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are possible options:

  • make a note about property re-definition possible breaking change without add type
  • wait to push to 4.4 only

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way;: make Rector extension to help user migrating to new definition.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing type in the constructor makes no errors (I don't know why),
https://3v4l.org/2A85L
but I don't want to remove already declared types in constructors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better way: make a patch to rector to:

  • skip non-typed property for protected/public property, except define INLINE_PUBLIC config that allow bc break on non-final class, same like TypedPropertyRector way

That's just quick guessing, probably there is a better way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kenjis here possibly the solution, I create PR to make it configurable INLINE_PUBLIC default false:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems the best approach to me. Glad your patch was already accepted!

}

/**
Expand All @@ -44,12 +40,12 @@ public function locateFile(string $file, ?string $folder = null, string $ext = '
$file = $this->ensureExt($file, $ext);

// Clears the folder name if it is at the beginning of the filename
if (! empty($folder) && strpos($file, $folder) === 0) {
if (! empty($folder) && str_starts_with($file, $folder)) {
$file = substr($file, strlen($folder . '/'));
}

// Is not namespaced? Try the application folder.
if (strpos($file, '\\') === false) {
if (! str_contains($file, '\\')) {
return $this->legacyLocate($file, $folder);
}

Expand All @@ -71,7 +67,7 @@ public function locateFile(string $file, ?string $folder = null, string $ext = '
$namespaces = $this->autoloader->getNamespace();

foreach (array_keys($namespaces) as $namespace) {
if (substr($file, 0, strlen($namespace)) === $namespace) {
if (str_starts_with($file, $namespace)) {
// There may be sub-namespaces of the same vendor,
// so overwrite them with namespaces found later.
$paths = $namespaces[$namespace];
Expand All @@ -94,7 +90,7 @@ public function locateFile(string $file, ?string $folder = null, string $ext = '
// If we have a folder name, then the calling function
// expects this file to be within that folder, like 'Views',
// or 'libraries'.
if (! empty($folder) && strpos($path . $filename, '/' . $folder . '/') === false) {
if (! empty($folder) && ! str_contains($path . $filename, '/' . $folder . '/')) {
$path .= trim($folder, '/') . '/';
}

Expand Down Expand Up @@ -177,7 +173,7 @@ public function search(string $path, string $ext = 'php', bool $prioritizeApp =

if ($prioritizeApp) {
$foundPaths[] = $fullPath;
} elseif (strpos($fullPath, APPPATH) === 0) {
} elseif (str_starts_with($fullPath, APPPATH)) {
$appPaths[] = $fullPath;
} else {
$foundPaths[] = $fullPath;
Expand All @@ -201,7 +197,7 @@ protected function ensureExt(string $path, string $ext): string
if ($ext) {
$ext = '.' . $ext;

if (substr($path, -strlen($ext)) !== $ext) {
if (! str_ends_with($path, $ext)) {
$path .= $ext;
}
}
Expand Down
2 changes: 1 addition & 1 deletion system/BaseModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ public function find($id = null)
*/
public function findColumn(string $columnName)
{
if (strpos($columnName, ',') !== false) {
if (str_contains($columnName, ',')) {
throw DataException::forFindColumnHaveMultipleColumns();
}

Expand Down
23 changes: 6 additions & 17 deletions system/CLI/BaseCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,24 +73,13 @@ abstract class BaseCommand
protected $arguments = [];

/**
* The Logger to use for a command
*
* @var LoggerInterface
* @param LoggerInterface $logger The Logger to use for a command
* @param Commands $commands Instance of Commands, so commands can call other commands.
*/
protected $logger;

/**
* Instance of Commands so
* commands can call other commands.
*
* @var Commands
*/
protected $commands;

public function __construct(LoggerInterface $logger, Commands $commands)
{
$this->logger = $logger;
$this->commands = $commands;
public function __construct(
protected LoggerInterface $logger,
protected Commands $commands
) {
}

/**
Expand Down
2 changes: 1 addition & 1 deletion system/CLI/CLI.php
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ public static function color(string $text, string $foreground, ?string $backgrou
$newText = '';

// Detect if color method was already in use with this text
if (strpos($text, "\033[0m") !== false) {
if (str_contains($text, "\033[0m")) {
$pattern = '/\\033\\[0;.+?\\033\\[0m/u';

preg_match_all($pattern, $text, $matches);
Expand Down
2 changes: 1 addition & 1 deletion system/CLI/Commands.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ protected function getCommandAlternatives(string $name, array $collection): arra
foreach (array_keys($collection) as $commandName) {
$lev = levenshtein($name, $commandName);

if ($lev <= strlen($commandName) / 3 || strpos($commandName, $name) !== false) {
if ($lev <= strlen($commandName) / 3 || str_contains($commandName, $name)) {
$alternatives[$commandName] = $lev;
}
}
Expand Down
Loading