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.

Sign up for GitHub

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

Jump to bottom

[8.x] Fixes for PHP 8.1 #37101

Merged
merged 9 commits into from Apr 23, 2021
Merged

[8.x] Fixes for PHP 8.1 #37101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
65e5ea4
fix PHP 8.1 deprecations (broadcasting)
jrmajor Apr 23, 2021
1e93718
fix PHP 8.1 deprecations (cache)
jrmajor Apr 23, 2021
304bc82
remove returns after never-return method call
jrmajor Apr 23, 2021
1046255
fix PHP 8.1 deprecations (bus)
jrmajor Apr 23, 2021
493876b
fix PHP 8.1 deprecations (database)
jrmajor Apr 23, 2021
4838af5
fix PHP 8.1 deprecations (validation)
jrmajor Apr 23, 2021
b3bfa4a
use strict comparison
jrmajor Apr 23, 2021
6facd57
fix PHP 8.1 deprecations (mail)
jrmajor Apr 23, 2021
173362c
fix PHP 8.1 deprecations (routing)
jrmajor Apr 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Illuminate/Broadcasting/Broadcasters/RedisBroadcaster.php
View file
Open in desktop
Expand Up @@ -20,16 +20,16 @@ class RedisBroadcaster extends Broadcaster
/**
* The Redis connection to use for broadcasting.
*
* @var string
* @var ?string
*/
protected $connection;
protected $connection = null;
Comment on lines -23 to +25
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the default in the constructor, which is bypassed by mocks. The same goes for $prefix.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it will throw a type exception when it takes a "null" value, we can fix it by converting it to a string. I am saying this from the following point of view. It doesn't seem nice to me to define default values for parameters in the class.

Copy link
Contributor

Choose a reason for hiding this comment

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

Briefly, isn't it a good solution to convert a "null" value to a string instead of defining it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't seem nice to me to define default values for parameters in the class.

It has already had the default value and it was defined in the constructor. Using mocks, however, bypasses the constructor and this property isn't set to its default. I've written that already in this comment.

Seems pretty nice to me, that's how default properties are meant to be used. It just has been unnecessary, as it was set in the constructor anyway (except for when this class was mocked, which nobody noticed).


/**
* The Redis key prefix.
*
* @var string
*/
protected $prefix;
protected $prefix = '';

/**
* Create a new broadcaster instance.
Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/Cache/Console/ClearCommand.php
View file
Open in desktop
Expand Up @@ -116,7 +116,7 @@ protected function cache()
*/
protected function tags()
{
return array_filter(explode(',', $this->option('tags')));
return array_filter(explode(',', $this->option('tags') ?? ''));
}

/**
Expand Down
4 changes: 0 additions & 4 deletions src/Illuminate/Foundation/Testing/Concerns/InteractsWithRedis.php
View file
Open in desktop
Expand Up @@ -36,14 +36,10 @@ public function setUpRedis()

if (! extension_loaded('redis')) {
$this->markTestSkipped('The redis extension is not installed. Please install the extension to enable '.__CLASS__);

return;
}

if (static::$connectionFailedOnceWithDefaultsSkip) {
$this->markTestSkipped('Trying default host/port failed, please set environment variable REDIS_HOST & REDIS_PORT to enable '.__CLASS__);

return;
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

markTestSkipped() has @psalm-return never-return annotation. There are 48 occurrences of this method in Laravel tests and only those two are followed by a return.

}

foreach ($this->redisDriverProvider() as $driver) {
Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/Mail/MailManager.php
View file
Open in desktop
Expand Up @@ -168,7 +168,7 @@ public function createTransport(array $config)
return call_user_func($this->customCreators[$transport], $config);
}

if (trim($transport) === '' || ! method_exists($this, $method = 'create'.ucfirst($transport).'Transport')) {
if (trim($transport ?? '') === '' || ! method_exists($this, $method = 'create'.ucfirst($transport).'Transport')) {
throw new InvalidArgumentException("Unsupported mail transport [{$transport}].");
}

Expand Down
7 changes: 5 additions & 2 deletions src/Illuminate/Routing/AbstractRouteCollection.php
View file
Open in desktop
Expand Up @@ -199,8 +199,11 @@ protected function addToSymfonyRoutesCollection(SymfonyRouteCollection $symfonyR
{
$name = $route->getName();

if (Str::endsWith($name, '.') &&
! is_null($symfonyRoutes->get($name))) {
if (
! is_null($name)
&& Str::endsWith($name, '.')
&& ! is_null($symfonyRoutes->get($name))
) {
$name = null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/Validation/Concerns/FormatsMessages.php
View file
Open in desktop
Expand Up @@ -54,7 +54,7 @@ protected function getMessage($attribute, $rule)
// messages out of the translator service for this validation rule.
$key = "validation.{$lowerRule}";

if ($key != ($value = $this->translator->get($key))) {
if ($key !== ($value = $this->translator->get($key))) {
return $value;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/Validation/Concerns/ReplacesAttributes.php
View file
Open in desktop
Expand Up @@ -115,7 +115,7 @@ protected function replaceMax($message, $attribute, $rule, $parameters)
*/
protected function replaceMultipleOf($message, $attribute, $rule, $parameters)
{
return str_replace(':value', $parameters[0], $message);
return str_replace(':value', $parameters[0] ?? '', $message);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/Validation/ValidationRuleParser.php
View file
Open in desktop
Expand Up @@ -214,7 +214,7 @@ public static function parse($rule)
*/
protected static function parseArrayRule(array $rule)
{
return [Str::studly(trim(Arr::get($rule, 0))), array_slice($rule, 1)];
return [Str::studly(trim(Arr::get($rule, 0, ''))), array_slice($rule, 1)];
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/Bus/BusBatchTest.php
View file
Open in desktop
Expand Up @@ -377,7 +377,7 @@ public function test_options_unserialize_on_postgres($serialize, $options)
'failed_jobs' => '',
'failed_job_ids' => '[]',
'options' => $serialize,
'created_at' => null,
'created_at' => now()->timestamp,
'cancelled_at' => null,
'finished_at' => null,
]);
Expand Down
24 changes: 18 additions & 6 deletions tests/Database/DatabaseMigrationMakeCommandTest.php
View file
Open in desktop
Expand Up @@ -27,7 +27,9 @@ public function testBasicCreateDumpsAutoload()
$app = new Application;
$app->useDatabasePath(__DIR__);
$command->setLaravel($app);
$creator->shouldReceive('create')->once()->with('create_foo', __DIR__.DIRECTORY_SEPARATOR.'migrations', 'foo', true);
$creator->shouldReceive('create')->once()
->with('create_foo', __DIR__.DIRECTORY_SEPARATOR.'migrations', 'foo', true)
->andReturn(__DIR__.'/migrations/2021_04_23_110457_create_foo.php');
$composer->shouldReceive('dumpAutoloads')->once();

$this->runCommand($command, ['name' => 'create_foo']);
Expand All @@ -42,7 +44,9 @@ public function testBasicCreateGivesCreatorProperArguments()
$app = new Application;
$app->useDatabasePath(__DIR__);
$command->setLaravel($app);
$creator->shouldReceive('create')->once()->with('create_foo', __DIR__.DIRECTORY_SEPARATOR.'migrations', 'foo', true);
$creator->shouldReceive('create')->once()
->with('create_foo', __DIR__.DIRECTORY_SEPARATOR.'migrations', 'foo', true)
->andReturn(__DIR__.'/migrations/2021_04_23_110457_create_foo.php');

$this->runCommand($command, ['name' => 'create_foo']);
}
Expand All @@ -56,7 +60,9 @@ public function testBasicCreateGivesCreatorProperArgumentsWhenNameIsStudlyCase()
$app = new Application;
$app->useDatabasePath(__DIR__);
$command->setLaravel($app);
$creator->shouldReceive('create')->once()->with('create_foo', __DIR__.DIRECTORY_SEPARATOR.'migrations', 'foo', true);
$creator->shouldReceive('create')->once()
->with('create_foo', __DIR__.DIRECTORY_SEPARATOR.'migrations', 'foo', true)
->andReturn(__DIR__.'/migrations/2021_04_23_110457_create_foo.php');

$this->runCommand($command, ['name' => 'CreateFoo']);
}
Expand All @@ -70,7 +76,9 @@ public function testBasicCreateGivesCreatorProperArgumentsWhenTableIsSet()
$app = new Application;
$app->useDatabasePath(__DIR__);
$command->setLaravel($app);
$creator->shouldReceive('create')->once()->with('create_foo', __DIR__.DIRECTORY_SEPARATOR.'migrations', 'users', true);
$creator->shouldReceive('create')->once()
->with('create_foo', __DIR__.DIRECTORY_SEPARATOR.'migrations', 'users', true)
->andReturn(__DIR__.'/migrations/2021_04_23_110457_create_foo.php');

$this->runCommand($command, ['name' => 'create_foo', '--create' => 'users']);
}
Expand All @@ -84,7 +92,9 @@ public function testBasicCreateGivesCreatorProperArgumentsWhenCreateTablePattern
$app = new Application;
$app->useDatabasePath(__DIR__);
$command->setLaravel($app);
$creator->shouldReceive('create')->once()->with('create_users_table', __DIR__.DIRECTORY_SEPARATOR.'migrations', 'users', true);
$creator->shouldReceive('create')->once()
->with('create_users_table', __DIR__.DIRECTORY_SEPARATOR.'migrations', 'users', true)
->andReturn(__DIR__.'/migrations/2021_04_23_110457_create_users_table.php');

$this->runCommand($command, ['name' => 'create_users_table']);
}
Expand All @@ -98,7 +108,9 @@ public function testCanSpecifyPathToCreateMigrationsIn()
$app = new Application;
$command->setLaravel($app);
$app->setBasePath('/home/laravel');
$creator->shouldReceive('create')->once()->with('create_foo', '/home/laravel/vendor/laravel-package/migrations', 'users', true);
$creator->shouldReceive('create')->once()
->with('create_foo', '/home/laravel/vendor/laravel-package/migrations', 'users', true)
->andReturn('/home/laravel/vendor/laravel-package/migrations/2021_04_23_110457_create_foo.php');
$this->runCommand($command, ['name' => 'create_foo', '--path' => 'vendor/laravel-package/migrations', '--create' => 'users']);
}

Expand Down
8 changes: 4 additions & 4 deletions tests/Validation/ValidationValidatorTest.php
View file
Open in desktop
Expand Up @@ -770,7 +770,7 @@ public function testValidatePassword()
$container->shouldReceive('make')->with('hash')->andReturn($hasher);

$trans = $this->getTranslator();
$trans->shouldReceive('get');
$trans->shouldReceive('get')->andReturnArg(0);

$v = new Validator($trans, ['password' => 'foo'], ['password' => 'password']);
$v->setContainer($container);
Expand All @@ -794,7 +794,7 @@ public function testValidatePassword()
$container->shouldReceive('make')->with('hash')->andReturn($hasher);

$trans = $this->getTranslator();
$trans->shouldReceive('get');
$trans->shouldReceive('get')->andReturnArg(0);

$v = new Validator($trans, ['password' => 'foo'], ['password' => 'password']);
$v->setContainer($container);
Expand All @@ -818,7 +818,7 @@ public function testValidatePassword()
$container->shouldReceive('make')->with('hash')->andReturn($hasher);

$trans = $this->getTranslator();
$trans->shouldReceive('get');
$trans->shouldReceive('get')->andReturnArg(0);

$v = new Validator($trans, ['password' => 'foo'], ['password' => 'password']);
$v->setContainer($container);
Expand All @@ -842,7 +842,7 @@ public function testValidatePassword()
$container->shouldReceive('make')->with('hash')->andReturn($hasher);

$trans = $this->getTranslator();
$trans->shouldReceive('get');
$trans->shouldReceive('get')->andReturnArg(0);

$v = new Validator($trans, ['password' => 'foo'], ['password' => 'password:custom']);
$v->setContainer($container);
Expand Down