From 55c1c72d4158b2b1a4158ce3c7dee1e1deb2480f Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 10 Sep 2025 12:31:52 -0500 Subject: [PATCH 1/3] remove unecessary `with()` helper calls when we have a well defined closure, there is no reason to use the `with()` helper, or even the closure at all. in most cases, but using a temporary variable we achieve the same effect with arguably cleaner easier to read code. --- src/Illuminate/Bus/Batch.php | 13 ++-- src/Illuminate/Console/Signals.php | 18 +++--- .../Foundation/Bootstrap/HandleExceptions.php | 60 +++++++++---------- .../Foundation/Console/DocsCommand.php | 38 ++++++------ .../Concerns/InteractsWithDatabase.php | 32 +++++----- src/Illuminate/Mail/Attachment.php | 18 +++--- .../Routing/RoutingServiceProvider.php | 21 +++---- .../Concerns/ValidatesAttributes.php | 9 +-- .../Bootstrap/HandleExceptionsTest.php | 16 +---- 9 files changed, 110 insertions(+), 115 deletions(-) diff --git a/src/Illuminate/Bus/Batch.php b/src/Illuminate/Bus/Batch.php index 717d1c4ab11d..81613690b272 100644 --- a/src/Illuminate/Bus/Batch.php +++ b/src/Illuminate/Bus/Batch.php @@ -168,12 +168,13 @@ public function add($jobs) if (is_array($job)) { $count += count($job); - return with($this->prepareBatchedChain($job), function ($chain) { - return $chain->first() - ->allOnQueue($this->options['queue'] ?? null) - ->allOnConnection($this->options['connection'] ?? null) - ->chain($chain->slice(1)->values()->all()); - }); + $chain = $this->prepareBatchedChain($job); + + return $chain->first() + ->allOnQueue($this->options['queue'] ?? null) + ->allOnConnection($this->options['connection'] ?? null) + ->chain($chain->slice(1)->values()->all()); + } else { $job->withBatchId($this->id); diff --git a/src/Illuminate/Console/Signals.php b/src/Illuminate/Console/Signals.php index 425352594c88..341e0de2681f 100644 --- a/src/Illuminate/Console/Signals.php +++ b/src/Illuminate/Console/Signals.php @@ -51,21 +51,21 @@ public function register($signal, $callback) { $this->previousHandlers[$signal] ??= $this->initializeSignal($signal); - with($this->getHandlers(), function ($handlers) use ($signal) { - $handlers[$signal] ??= $this->initializeSignal($signal); + $handlers = $this->getHandlers(); - $this->setHandlers($handlers); - }); + $handlers[$signal] ??= $this->initializeSignal($signal); + + $this->setHandlers($handlers); $this->registry->register($signal, $callback); - with($this->getHandlers(), function ($handlers) use ($signal) { - $lastHandlerInserted = array_pop($handlers[$signal]); + $handlers = $this->getHandlers(); + + $lastHandlerInserted = array_pop($handlers[$signal]); - array_unshift($handlers[$signal], $lastHandlerInserted); + array_unshift($handlers[$signal], $lastHandlerInserted); - $this->setHandlers($handlers); - }); + $this->setHandlers($handlers); } /** diff --git a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php index a06b655dffad..ba2b9fca08a8 100644 --- a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php +++ b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php @@ -102,15 +102,15 @@ public function handleDeprecationError($message, $file, $line, $level = E_DEPREC $options = static::$app['config']->get('logging.deprecations') ?? []; - with($logger->channel('deprecations'), function ($log) use ($message, $file, $line, $level, $options) { - if ($options['trace'] ?? false) { - $log->warning((string) new ErrorException($message, 0, $level, $file, $line)); - } else { - $log->warning(sprintf('%s in %s on line %s', - $message, $file, $line - )); - } - }); + $log = $logger->channel('deprecations'); + + if ($options['trace'] ?? false) { + $log->warning((string) new ErrorException($message, 0, $level, $file, $line)); + } else { + $log->warning(sprintf('%s in %s on line %s', + $message, $file, $line + )); + } } /** @@ -132,21 +132,21 @@ protected function shouldIgnoreDeprecationErrors() */ protected function ensureDeprecationLoggerIsConfigured() { - with(static::$app['config'], function ($config) { - if ($config->get('logging.channels.deprecations')) { - return; - } + $config = static::$app['config']; - $this->ensureNullLogDriverIsConfigured(); + if ($config->get('logging.channels.deprecations')) { + return; + } - if (is_array($options = $config->get('logging.deprecations'))) { - $driver = $options['channel'] ?? 'null'; - } else { - $driver = $options ?? 'null'; - } + $this->ensureNullLogDriverIsConfigured(); - $config->set('logging.channels.deprecations', $config->get("logging.channels.{$driver}")); - }); + if (is_array($options = $config->get('logging.deprecations'))) { + $driver = $options['channel'] ?? 'null'; + } else { + $driver = $options ?? 'null'; + } + + $config->set('logging.channels.deprecations', $config->get("logging.channels.{$driver}")); } /** @@ -156,16 +156,16 @@ protected function ensureDeprecationLoggerIsConfigured() */ protected function ensureNullLogDriverIsConfigured() { - with(static::$app['config'], function ($config) { - if ($config->get('logging.channels.null')) { - return; - } + $config = static::$app['config']; + + if ($config->get('logging.channels.null')) { + return; + } - $config->set('logging.channels.null', [ - 'driver' => 'monolog', - 'handler' => NullHandler::class, - ]); - }); + $config->set('logging.channels.null', [ + 'driver' => 'monolog', + 'handler' => NullHandler::class, + ]); } /** diff --git a/src/Illuminate/Foundation/Console/DocsCommand.php b/src/Illuminate/Foundation/Console/DocsCommand.php index 1eea49070cc5..9edca71de61c 100644 --- a/src/Illuminate/Foundation/Console/DocsCommand.php +++ b/src/Illuminate/Foundation/Console/DocsCommand.php @@ -125,11 +125,11 @@ public function handle(Http $http, Cache $cache) */ protected function openUrl() { - with($this->url(), function ($url) { - $this->components->info("Opening the docs to: {$url}"); + $url = $this->url(); - $this->open($url); - }); + $this->components->info("Opening the docs to: {$url}"); + + $this->open($url); } /** @@ -145,9 +145,9 @@ protected function url() ]); } - return with($this->page(), function ($page) { - return trim("https://laravel.com/docs/{$this->version()}/{$page}#{$this->section($page)}", '#/'); - }); + $page = $this->page(); + + return trim("https://laravel.com/docs/{$this->version()}/{$page}#{$this->section($page)}", '#/'); } /** @@ -157,15 +157,15 @@ protected function url() */ protected function page() { - return with($this->resolvePage(), function ($page) { - if ($page === null) { - $this->components->warn('Unable to determine the page you are trying to visit.'); + $page = $this->resolvePage(); - return '/'; - } + if ($page === null) { + $this->components->warn('Unable to determine the page you are trying to visit.'); - return $page; - }); + return '/'; + } + + return $page; } /** @@ -441,11 +441,11 @@ public function docs() */ protected function refreshDocs() { - with($this->fetchDocs(), function ($response) { - if ($response->successful()) { - $this->cache->put("artisan.docs.{{$this->version()}}.index", $response->collect(), CarbonInterval::months(2)); - } - }); + $response = $this->fetchDocs(); + + if ($response->successful()) { + $this->cache->put("artisan.docs.{{$this->version()}}.index", $response->collect(), CarbonInterval::months(2)); + } } /** diff --git a/src/Illuminate/Foundation/Testing/Concerns/InteractsWithDatabase.php b/src/Illuminate/Foundation/Testing/Concerns/InteractsWithDatabase.php index c23ce0de4e37..b07211835d31 100644 --- a/src/Illuminate/Foundation/Testing/Concerns/InteractsWithDatabase.php +++ b/src/Illuminate/Foundation/Testing/Concerns/InteractsWithDatabase.php @@ -225,22 +225,22 @@ protected function assertModelMissing($model) */ public function expectsDatabaseQueryCount($expected, $connection = null) { - with($this->getConnection($connection), function ($connectionInstance) use ($expected, $connection) { - $actual = 0; - - $connectionInstance->listen(function (QueryExecuted $event) use (&$actual, $connectionInstance, $connection) { - if (is_null($connection) || $connectionInstance === $event->connection) { - $actual++; - } - }); - - $this->beforeApplicationDestroyed(function () use (&$actual, $expected, $connectionInstance) { - $this->assertSame( - $expected, - $actual, - "Expected {$expected} database queries on the [{$connectionInstance->getName()}] connection. {$actual} occurred." - ); - }); + $connectionInstance = $this->getConnection($connection); + + $actual = 0; + + $connectionInstance->listen(function (QueryExecuted $event) use (&$actual, $connectionInstance, $connection) { + if (is_null($connection) || $connectionInstance === $event->connection) { + $actual++; + } + }); + + $this->beforeApplicationDestroyed(function () use (&$actual, $expected, $connectionInstance) { + $this->assertSame( + $expected, + $actual, + "Expected {$expected} database queries on the [{$connectionInstance->getName()}] connection. {$actual} occurred." + ); }); return $this; diff --git a/src/Illuminate/Mail/Attachment.php b/src/Illuminate/Mail/Attachment.php index 8e2e87ed5496..bcd3e06f9fbe 100644 --- a/src/Illuminate/Mail/Attachment.php +++ b/src/Illuminate/Mail/Attachment.php @@ -206,15 +206,17 @@ function ($data) use ($mail, $options) { */ public function isEquivalent(Attachment $attachment, $options = []) { - return with([ + $newOptions = [ 'as' => $options['as'] ?? $attachment->as, 'mime' => $options['mime'] ?? $attachment->mime, - ], fn ($options) => $this->attachWith( - fn ($path) => [$path, ['as' => $this->as, 'mime' => $this->mime]], - fn ($data) => [$data(), ['as' => $this->as, 'mime' => $this->mime]], - ) === $attachment->attachWith( - fn ($path) => [$path, $options], - fn ($data) => [$data(), $options], - )); + ]; + + return $this->attachWith( + fn ($path) => [$path, ['as' => $this->as, 'mime' => $this->mime]], + fn ($data) => [$data(), ['as' => $this->as, 'mime' => $this->mime]], + ) === $attachment->attachWith( + fn ($path) => [$path, $newOptions], + fn ($data) => [$data(), $newOptions], + ); } } diff --git a/src/Illuminate/Routing/RoutingServiceProvider.php b/src/Illuminate/Routing/RoutingServiceProvider.php index f964f5b9baa9..a8dae545ade6 100755 --- a/src/Illuminate/Routing/RoutingServiceProvider.php +++ b/src/Illuminate/Routing/RoutingServiceProvider.php @@ -134,16 +134,17 @@ protected function registerPsrRequest() { $this->app->bind(ServerRequestInterface::class, function ($app) { if (class_exists(PsrHttpFactory::class)) { - return with((new PsrHttpFactory) - ->createRequest($illuminateRequest = $app->make('request')), function (ServerRequestInterface $request) use ($illuminateRequest) { - if ($illuminateRequest->getContentTypeFormat() !== 'json' && $illuminateRequest->request->count() === 0) { - return $request; - } - - return $request->withParsedBody( - array_merge($request->getParsedBody() ?? [], $illuminateRequest->getPayload()->all()) - ); - }); + + $illuminateRequest = $app->make('request'); + $request = (new PsrHttpFactory)->createRequest($illuminateRequest); + + if ($illuminateRequest->getContentTypeFormat() !== 'json' && $illuminateRequest->request->count() === 0) { + return $request; + } + + return $request->withParsedBody( + array_merge($request->getParsedBody() ?? [], $illuminateRequest->getPayload()->all()) + ); } throw new BindingResolutionException('Unable to resolve PSR request. Please install the "symfony/psr-http-message-bridge" package.'); diff --git a/src/Illuminate/Validation/Concerns/ValidatesAttributes.php b/src/Illuminate/Validation/Concerns/ValidatesAttributes.php index 29967af83276..092197a32672 100644 --- a/src/Illuminate/Validation/Concerns/ValidatesAttributes.php +++ b/src/Illuminate/Validation/Concerns/ValidatesAttributes.php @@ -472,10 +472,11 @@ public function validateBetween($attribute, $value, $parameters) $this->requireParameterCount(2, $parameters, 'between'); try { - return with( - BigNumber::of($this->getSize($attribute, $value)), - fn ($size) => $size->isGreaterThanOrEqualTo($this->trim($parameters[0])) && $size->isLessThanOrEqualTo($this->trim($parameters[1])) - ); + + $size = BigNumber::of($this->getSize($attribute, $value)); + + return $size->isGreaterThanOrEqualTo($this->trim($parameters[0])) && $size->isLessThanOrEqualTo($this->trim($parameters[1])); + } catch (MathException) { return false; } diff --git a/tests/Foundation/Bootstrap/HandleExceptionsTest.php b/tests/Foundation/Bootstrap/HandleExceptionsTest.php index f900144068fc..d0f84de2cc72 100644 --- a/tests/Foundation/Bootstrap/HandleExceptionsTest.php +++ b/tests/Foundation/Bootstrap/HandleExceptionsTest.php @@ -29,9 +29,7 @@ protected function setUp(): void protected function handleExceptions() { return tap(new HandleExceptions(), function ($instance) { - with(new ReflectionClass($instance), function ($reflection) use ($instance) { - $reflection->getProperty('app')->setValue($instance, $this->app); - }); + (new ReflectionClass($instance))->getProperty('app')->setValue($instance, $this->app); }); } @@ -381,11 +379,7 @@ public function testForgetApp() { $instance = $this->handleExceptions(); - $appResolver = fn () => with(new ReflectionClass($instance), function ($reflection) use ($instance) { - $property = $reflection->getProperty('app'); - - return $property->getValue($instance); - }); + $appResolver = fn () => (new ReflectionClass($instance))->getProperty('app')->getValue($instance); $this->assertNotNull($appResolver()); @@ -398,11 +392,7 @@ public function testHandlerForgetsPreviousApp() { $instance = $this->handleExceptions(); - $appResolver = fn () => with(new ReflectionClass($instance), function ($reflection) use ($instance) { - $property = $reflection->getProperty('app'); - - return $property->getValue($instance); - }); + $appResolver = fn () => (new ReflectionClass($instance))->getProperty('app')->getValue($instance); $this->assertSame($this->app, $appResolver()); From d4330fe68a927b95336427b36de18e1cd3e14fae Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 10 Sep 2025 12:34:47 -0500 Subject: [PATCH 2/3] style fixes --- src/Illuminate/Bus/Batch.php | 1 - src/Illuminate/Mail/Attachment.php | 12 ++++++------ src/Illuminate/Routing/RoutingServiceProvider.php | 1 - .../Validation/Concerns/ValidatesAttributes.php | 2 -- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/Illuminate/Bus/Batch.php b/src/Illuminate/Bus/Batch.php index 81613690b272..51f84ed86822 100644 --- a/src/Illuminate/Bus/Batch.php +++ b/src/Illuminate/Bus/Batch.php @@ -174,7 +174,6 @@ public function add($jobs) ->allOnQueue($this->options['queue'] ?? null) ->allOnConnection($this->options['connection'] ?? null) ->chain($chain->slice(1)->values()->all()); - } else { $job->withBatchId($this->id); diff --git a/src/Illuminate/Mail/Attachment.php b/src/Illuminate/Mail/Attachment.php index bcd3e06f9fbe..04b40f8e997c 100644 --- a/src/Illuminate/Mail/Attachment.php +++ b/src/Illuminate/Mail/Attachment.php @@ -212,11 +212,11 @@ public function isEquivalent(Attachment $attachment, $options = []) ]; return $this->attachWith( - fn ($path) => [$path, ['as' => $this->as, 'mime' => $this->mime]], - fn ($data) => [$data(), ['as' => $this->as, 'mime' => $this->mime]], - ) === $attachment->attachWith( - fn ($path) => [$path, $newOptions], - fn ($data) => [$data(), $newOptions], - ); + fn ($path) => [$path, ['as' => $this->as, 'mime' => $this->mime]], + fn ($data) => [$data(), ['as' => $this->as, 'mime' => $this->mime]], + ) === $attachment->attachWith( + fn ($path) => [$path, $newOptions], + fn ($data) => [$data(), $newOptions], + ); } } diff --git a/src/Illuminate/Routing/RoutingServiceProvider.php b/src/Illuminate/Routing/RoutingServiceProvider.php index a8dae545ade6..bd4fb5d91274 100755 --- a/src/Illuminate/Routing/RoutingServiceProvider.php +++ b/src/Illuminate/Routing/RoutingServiceProvider.php @@ -134,7 +134,6 @@ protected function registerPsrRequest() { $this->app->bind(ServerRequestInterface::class, function ($app) { if (class_exists(PsrHttpFactory::class)) { - $illuminateRequest = $app->make('request'); $request = (new PsrHttpFactory)->createRequest($illuminateRequest); diff --git a/src/Illuminate/Validation/Concerns/ValidatesAttributes.php b/src/Illuminate/Validation/Concerns/ValidatesAttributes.php index 092197a32672..082f994efe8c 100644 --- a/src/Illuminate/Validation/Concerns/ValidatesAttributes.php +++ b/src/Illuminate/Validation/Concerns/ValidatesAttributes.php @@ -472,11 +472,9 @@ public function validateBetween($attribute, $value, $parameters) $this->requireParameterCount(2, $parameters, 'between'); try { - $size = BigNumber::of($this->getSize($attribute, $value)); return $size->isGreaterThanOrEqualTo($this->trim($parameters[0])) && $size->isLessThanOrEqualTo($this->trim($parameters[1])); - } catch (MathException) { return false; } From 9e149a175d5296c97cf0469f9269dfd25394b445 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Fri, 12 Sep 2025 12:09:54 -0500 Subject: [PATCH 3/3] revert refactor I can't figure out how to fix the tests right now, so reverting these --- .../Foundation/Bootstrap/HandleExceptions.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php index ba2b9fca08a8..a5588cf0694e 100644 --- a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php +++ b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php @@ -102,15 +102,15 @@ public function handleDeprecationError($message, $file, $line, $level = E_DEPREC $options = static::$app['config']->get('logging.deprecations') ?? []; - $log = $logger->channel('deprecations'); - - if ($options['trace'] ?? false) { - $log->warning((string) new ErrorException($message, 0, $level, $file, $line)); - } else { - $log->warning(sprintf('%s in %s on line %s', - $message, $file, $line - )); - } + with($logger->channel('deprecations'), function ($log) use ($message, $file, $line, $level, $options) { + if ($options['trace'] ?? false) { + $log->warning((string) new ErrorException($message, 0, $level, $file, $line)); + } else { + $log->warning(sprintf('%s in %s on line %s', + $message, $file, $line + )); + } + }); } /**