Apply conservative Rector cleanup (#500)

dereuromark · web-flow · commit d01d1a4f9008 · 2026-05-25T14:02:33.000+02:00
* Apply conservative Rector cleanup

* Run PHPCS cleanup

* Restore safe typed property checks

* Restore compact() for matching view vars

* Address queue review follow-ups

* Restore compact() for matching view vars

* Fix queue cleanup regressions

* Fix queue static analysis types
diff --git a/src/Command/AddCommand.php b/src/Command/AddCommand.php
@@ -59,7 +59,7 @@ public function execute(Arguments $args, ConsoleIo $io) {
 		$taskName = $args->getArgument('task');
 		if (!$taskName) {
 			$io->out(count($tasks) . ' tasks available:');
-			foreach ($tasks as $task => $className) {
+			foreach (array_keys($tasks) as $task) {
 				$io->out(' - ' . $task);
 			}
 
diff --git a/src/Command/BakeQueueTaskCommand.php b/src/Command/BakeQueueTaskCommand.php
@@ -86,12 +86,12 @@ protected function generateTaskTestContent(string $name, string $namespace): str
 		}
 		$taskClassNamespace = $namespace . '\Queue\\Task\\' . str_replace(DS, '\\', $name);
 
-		if (strpos($name, '/') !== false) {
+		if (str_contains($name, '/')) {
 			$parts = explode('/', $name);
 			$name = array_pop($parts);
 		}
 
-		$content = <<<TXT
+		return <<<TXT
 <?php
 
 namespace $namespace\Test\TestCase\Queue\Task$subNamespace;
@@ -122,8 +122,6 @@ public function testRun(): void {
 }
 
 TXT;
-
-		return $content;
 	}
 
 	/**
@@ -148,7 +146,7 @@ public function templateData(Arguments $arguments): array {
 		$namespace .= '\\Queue\\Task';
 
 		$namespacePart = null;
-		if (strpos($name, '/') !== false) {
+		if (str_contains($name, '/')) {
 			$parts = explode('/', $name);
 			$name = array_pop($parts);
 			$namespacePart = implode('\\', $parts);
diff --git a/src/Command/InfoCommand.php b/src/Command/InfoCommand.php
@@ -59,7 +59,7 @@ public function execute(Arguments $args, ConsoleIo $io) {
 		$addableTasks = $this->getAddableTasks();
 
 		$io->out(count($tasks) . ' tasks available:');
-		foreach ($tasks as $task => $className) {
+		foreach (array_keys($tasks) as $task) {
 			if (array_key_exists($task, $addableTasks)) {
 				$task .= ' <info>[addable via CLI]</info>';
 			}
diff --git a/src/Controller/Admin/LoadHelperTrait.php b/src/Controller/Admin/LoadHelperTrait.php
@@ -28,11 +28,7 @@ protected function loadHelpers(): void {
 		}
 
 		// Configure helper: prefer Shim, fallback to Queue's own
-		if (Plugin::isLoaded('Shim')) {
-			$helpers[] = 'Shim.Configure';
-		} else {
-			$helpers[] = 'Queue.Configure';
-		}
+		$helpers[] = Plugin::isLoaded('Shim') ? 'Shim.Configure' : 'Queue.Configure';
 
 		if (Configure::read('Icon.sets') && class_exists(IconHelper::class)) {
 			$helpers[] = 'Templating.Icon';
diff --git a/src/Controller/Admin/QueueController.php b/src/Controller/Admin/QueueController.php
@@ -307,7 +307,7 @@ protected function refererRedirect(array|string $default) {
 		if (is_array($url)) {
 			throw new NotFoundException('Invalid array in query string');
 		}
-		if ($url && (mb_substr($url, 0, 1) !== '/' || mb_substr($url, 0, 2) === '//')) {
+		if ($url && (mb_substr((string)$url, 0, 1) !== '/' || mb_substr((string)$url, 0, 2) === '//')) {
 			$url = null;
 		}
 
diff --git a/src/Controller/Admin/QueueProcessesController.php b/src/Controller/Admin/QueueProcessesController.php
@@ -100,7 +100,7 @@ public function terminate(?int $id = null) {
 			$queueProcess->terminate = true;
 			$this->QueueProcesses->saveOrFail($queueProcess);
 			$this->Flash->success(__d('queue', 'The queue process has been deleted.'));
-		} catch (Exception $exception) {
+		} catch (Exception) {
 			$this->Flash->error(__d('queue', 'The queue process could not be deleted. Please, try again.'));
 		}
 
diff --git a/src/Controller/Admin/QueuedJobsController.php b/src/Controller/Admin/QueuedJobsController.php
@@ -407,7 +407,7 @@ public function test() {
 		$taskFinder = new TaskFinder();
 		$allTasks = $taskFinder->all();
 		$tasks = [];
-		foreach ($allTasks as $task => $className) {
+		foreach (array_keys($allTasks) as $task) {
 			if (!str_starts_with($task, 'Queue.')) {
 				continue;
 			}
@@ -458,7 +458,7 @@ public function migrate() {
 			->toArray();
 
 		$tasks = [];
-		foreach ($allTasks as $task => $className) {
+		foreach (array_keys($allTasks) as $task) {
 			if (!str_starts_with($task, 'Queue.')) {
 				continue;
 			}
diff --git a/src/Generator/Task/QueuedJobTask.php b/src/Generator/Task/QueuedJobTask.php
@@ -24,7 +24,7 @@ public function collect(): array {
 		$list = [];
 
 		$names = $this->collectQueuedJobTasks();
-		foreach ($names as $name => $className) {
+		foreach (array_keys($names) as $name) {
 			$list[$name] = "'$name'";
 		}
 
@@ -44,9 +44,8 @@ public function collect(): array {
 	 */
 	protected function collectQueuedJobTasks(): array {
 		$taskFinder = new TaskFinder();
-		$tasks = $taskFinder->all();
 
-		return $tasks;
+		return $taskFinder->all();
 	}
 
 }
diff --git a/src/Mailer/Transport/SimpleQueueTransport.php b/src/Mailer/Transport/SimpleQueueTransport.php
@@ -57,8 +57,7 @@ public function send(Message $message): array {
 		];
 
 		foreach ($settings as $setting => $value) {
-			/** @phpstan-ignore-next-line */
-			if (array_key_exists(0, $value) && ($value[0] === null || $value[0] === [])) {
+			if ($value[0] === null || $value[0] === []) {
 				unset($settings[$setting]);
 			}
 		}
diff --git a/src/Model/Behavior/JsonableBehavior.php b/src/Model/Behavior/JsonableBehavior.php
@@ -172,13 +172,11 @@ protected function _getMappedFields() {
 	 * @return string|null
 	 */
 	public function _encode($val) {
-		if (!empty($this->_config['fields'])) {
-			if ($this->_config['input'] === 'json') {
-				if (!is_string($val)) {
-					throw new InvalidArgumentException('Only accepts JSON string for input type `json`');
-				}
-				$val = $this->_fromJson($val);
+		if (!empty($this->_config['fields']) && $this->_config['input'] === 'json') {
+			if (!is_string($val)) {
+				throw new InvalidArgumentException('Only accepts JSON string for input type `json`');
 			}
+			$val = $this->_fromJson($val);
 		}
 		if (!is_array($val)) {
 			return null;
diff --git a/src/Model/Table/QueueProcessesTable.php b/src/Model/Table/QueueProcessesTable.php
@@ -242,6 +242,7 @@ public function status(): array {
 			->enableHydration(false)
 			->all()
 			->toArray();
+		/** @var array<array{modified: \Cake\I18n\DateTime}> $results */
 
 		if (!$results) {
 			return [];
diff --git a/src/Model/Table/QueuedJobsTable.php b/src/Model/Table/QueuedJobsTable.php
@@ -221,6 +221,7 @@ public function createJob(string $jobTask, array|object|null $data = null, array
 				throw new InvalidArgumentException('createJob() with `unique` requires a `reference` to dedupe on.');
 			}
 
+			/** @var \Queue\Model\Entity\QueuedJob|null $existing */
 			$existing = $this->find()
 				->where([
 					'reference' => $config->getReferenceOrFail(),
@@ -267,7 +268,7 @@ public function createJob(string $jobTask, array|object|null $data = null, array
 	 * @return string
 	 */
 	protected function jobTask(string $jobType): string {
-		if ($this->taskFinder === null) {
+		if (!($this->taskFinder instanceof TaskFinder)) {
 			$this->taskFinder = new TaskFinder();
 		}
 
@@ -451,6 +452,7 @@ public function getFullStats(?string $jobTask = null): array {
 			->limit(static::STATS_LIMIT)
 			->all()
 			->toArray();
+		/** @var array<array{created: \DateTime, duration: int|float|null, job_task: string}> $jobs */
 
 		$result = [];
 
@@ -981,7 +983,7 @@ public function getHeatmapData(string $field = 'created', int $days = 30, ?strin
 
 			// MySQL DAYOFWEEK returns 1=Sunday, 2=Monday, etc. Adjust to 0=Sunday
 			if ($driverName === static::DRIVER_MYSQL) {
-				$day = $day - 1;
+				$day -= 1;
 			}
 
 			// Ensure day is in valid range (0-6)
@@ -1260,10 +1262,9 @@ public function truncate(): void {
 	 * @return string
 	 */
 	protected function getDriverName(): string {
-		$className = explode('\\', $this->getConnection()->config()['driver']);
-		$name = end($className) ?: '';
+		$className = explode('\\', (string)$this->getConnection()->config()['driver']);
 
-		return $name;
+		return end($className) ?: '';
 	}
 
 	/**
@@ -1277,7 +1278,7 @@ protected function addFilter(array $conditions, string $key, array $values): arr
 		$include = [];
 		$exclude = [];
 		foreach ($values as $value) {
-			if (substr($value, 0, 1) === '-') {
+			if (str_starts_with($value, '-')) {
 				$exclude[] = substr($value, 1);
 			} else {
 				$include[] = $value;
diff --git a/src/Queue/Config.php b/src/Queue/Config.php
@@ -27,7 +27,7 @@ public static function defaultworkertimeout(): int {
 				);
 			}
 		}
-		$timeout = $timeout ?? 600; // 10min default
+		$timeout ??= 600; // 10min default
 
 		if ($timeout <= 0) {
 			throw new InvalidArgumentException('Queue.defaultRequeueTimeout (or deprecated defaultworkertimeout) is less or equal than zero. Indefinite running of jobs is not supported.');
diff --git a/src/Queue/Processor.php b/src/Queue/Processor.php
@@ -198,10 +198,10 @@ public function run(array $args): int {
 			pcntl_async_signals(true);
 		}
 		if (function_exists('pcntl_signal')) {
-			pcntl_signal(SIGTERM, [&$this, 'exit']);
-			pcntl_signal(SIGINT, [&$this, 'abort']);
-			pcntl_signal(SIGTSTP, [&$this, 'abort']);
-			pcntl_signal(SIGQUIT, [&$this, 'abort']);
+			pcntl_signal(SIGTERM, $this->exit(...));
+			pcntl_signal(SIGINT, $this->abort(...));
+			pcntl_signal(SIGTSTP, $this->abort(...));
+			pcntl_signal(SIGQUIT, $this->abort(...));
 			if (Configure::read('Queue.canInterruptSleep')) {
 				// Defining a signal handler here will make the worker wake up
 				// from its sleep() when SIGUSR1 is received. Since waking it
@@ -222,12 +222,12 @@ public function run(array $args): int {
 
 			try {
 				$this->updatePid($pid);
-			} catch (RecordNotFoundException $exception) {
+			} catch (RecordNotFoundException) {
 				// Manually killed
 				$this->exit = true;
 
 				continue;
-			} catch (ProcessEndingException $exception) {
+			} catch (ProcessEndingException) {
 				// Soft killed, e.g. during deploy update
 				$this->exit = true;
 
@@ -533,13 +533,7 @@ protected function initPid(): string {
 	 * @return string
 	 */
 	protected function retrievePid(): string {
-		if (function_exists('posix_getpid')) {
-			$pid = (string)posix_getpid();
-		} else {
-			$pid = $this->QueuedJobs->key();
-		}
-
-		return $pid;
+		return function_exists('posix_getpid') ? (string)posix_getpid() : $this->QueuedJobs->key();
 	}
 
 	/**
diff --git a/src/Queue/Task/EmailTask.php b/src/Queue/Task/EmailTask.php
@@ -205,7 +205,7 @@ public function run(array $data, int $jobId): void {
 		}
 
 		foreach ($settings as $method => $setting) {
-			$setter = 'set' . ucfirst($method);
+			$setter = 'set' . ucfirst((string)$method);
 			if (in_array($method, ['theme', 'template', 'layout'], true)) {
 				call_user_func_array([$this->mailer->viewBuilder(), $setter], (array)$setting);
 
diff --git a/src/Queue/Task/ExecuteTask.php b/src/Queue/Task/ExecuteTask.php
@@ -106,11 +106,7 @@ public function run(array $data, int $jobId): void {
 			}
 		}
 
-		if ($data['escape']) {
-			$command = escapeshellarg($rawCommand);
-		} else {
-			$command = $rawCommand;
-		}
+		$command = $data['escape'] ? escapeshellarg($rawCommand) : $rawCommand;
 
 		if ($data['params']) {
 			$params = $data['params'];
diff --git a/src/Queue/Task/MonitorExampleTask.php b/src/Queue/Task/MonitorExampleTask.php
@@ -102,7 +102,7 @@ protected function getSystemMemInfo(): array {
 		$data = explode("\n", file_get_contents('/proc/meminfo') ?: '');
 		$meminfo = [];
 		foreach ($data as $line) {
-			if (strpos($line, ':') === false) {
+			if (!str_contains($line, ':')) {
 				continue;
 			}
 			[$key, $val] = explode(':', $line);
diff --git a/src/Queue/Task/ProgressExampleTask.php b/src/Queue/Task/ProgressExampleTask.php
@@ -75,7 +75,7 @@ public function description(): ?string {
 	public function run(array $data, int $jobId): void {
 		$this->io->hr();
 		$this->io->out('CakePHP Queue ProgressExample task.');
-		$seconds = !empty($data['duration']) ? (int)$data['duration'] : 2 * static::MINUTE;
+		$seconds = empty($data['duration']) ? 2 * static::MINUTE : (int)$data['duration'];
 
 		$this->io->out('A total of ' . $seconds . ' seconds need to pass...');
 		for ($i = 0; $i < $seconds; $i++) {
diff --git a/src/Queue/TaskFinder.php b/src/Queue/TaskFinder.php
@@ -145,7 +145,7 @@ public function resolve(string $jobTask): string {
 
 		if (!str_contains($jobTask, '\\')) {
 			// Let's try matching without plugin prefix
-			foreach ($all as $name => $className) {
+			foreach ($all as $name => $_) {
 				if (!str_contains($name, '.')) {
 					continue;
 				}
diff --git a/src/View/Helper/QueueHelper.php b/src/View/Helper/QueueHelper.php
@@ -36,11 +36,8 @@ public function hasFailed(QueuedJob $queuedJob): bool {
 
 		// Requeued
 		$taskConfig = $this->taskConfig($queuedJob->job_task);
-		if ($taskConfig && $queuedJob->attempts <= $taskConfig['retries']) {
-			return false;
-		}
 
-		return true;
+		return !($taskConfig && $queuedJob->attempts <= $taskConfig['retries']);
 	}
 
 	/**
@@ -85,11 +82,8 @@ public function isRequeued(QueuedJob $queuedJob): bool {
 		}
 
 		$taskConfig = $this->taskConfig($queuedJob->job_task);
-		if ($taskConfig && $queuedJob->attempts <= $taskConfig['retries']) {
-			return true;
-		}
 
-		return false;
+		return $taskConfig && $queuedJob->attempts <= $taskConfig['retries'];
 	}
 
 	/**
@@ -113,11 +107,7 @@ public function isRestarted(QueuedJob $queuedJob): bool {
 		}
 
 		// Must NOT have a failure_message (it was cleared by reset)
-		if ($queuedJob->failure_message) {
-			return false;
-		}
-
-		return true;
+		return !$queuedJob->failure_message;
 	}
 
 	/**
diff --git a/tests/TestCase/Command/RunCommandTest.php b/tests/TestCase/Command/RunCommandTest.php
@@ -78,7 +78,7 @@ public function testServiceInjection(): void {
 	 */
 	protected function _needsConnection() {
 		$config = ConnectionManager::getConfig('test');
-		$skip = strpos($config['driver'], 'Mysql') === false && strpos($config['driver'], 'Postgres') === false;
+		$skip = !str_contains((string)$config['driver'], 'Mysql') && !str_contains((string)$config['driver'], 'Postgres');
 		$this->skipIf($skip, 'Only Mysql/Postgres is working yet for this.');
 	}
 
diff --git a/tests/TestCase/Controller/Admin/QueueControllerTest.php b/tests/TestCase/Controller/Admin/QueueControllerTest.php
@@ -349,7 +349,7 @@ public function testIndexStandalone(): void {
 	 */
 	protected function _needsConnection() {
 		$config = ConnectionManager::getConfig('test');
-		$skip = strpos($config['driver'], 'Mysql') === false && strpos($config['driver'], 'Postgres') === false;
+		$skip = !str_contains((string)$config['driver'], 'Mysql') && !str_contains((string)$config['driver'], 'Postgres');
 		$this->skipIf($skip, 'Only Mysql/Postgres is working yet for this.');
 	}
 
diff --git a/tests/TestCase/Controller/Admin/QueuedJobsControllerTest.php b/tests/TestCase/Controller/Admin/QueuedJobsControllerTest.php
@@ -433,7 +433,7 @@ public function testImport() {
 	 */
 	protected function _needsConnection() {
 		$config = ConnectionManager::getConfig('test');
-		$skip = strpos($config['driver'], 'Mysql') === false && strpos($config['driver'], 'Postgres') === false;
+		$skip = !str_contains((string)$config['driver'], 'Mysql') && !str_contains((string)$config['driver'], 'Postgres');
 		$this->skipIf($skip, 'Only Mysql/Postgres is working yet for this.');
 	}
 
diff --git a/tests/TestCase/Model/Table/QueuedJobsTableTest.php b/tests/TestCase/Model/Table/QueuedJobsTableTest.php
@@ -324,7 +324,7 @@ public function testSequence() {
 	 * Test creating Jobs to run close to a specified time, and strtotime parsing.
 	 * Using toUnixString() function to convert Time object to timestamp, instead of strtotime
 	 *
-	 * @return null
+	 * @return void
 	 */
 	public function testNotBefore() {
 		$this->assertTrue((bool)$this->QueuedJobs->createJob('Foo', null, ['notBefore' => '+ 1 Min']));
@@ -945,7 +945,7 @@ public function testJobCreatedEventFired(): void {
 	 */
 	protected function _needsConnection() {
 		$config = ConnectionManager::getConfig('test');
-		$skip = strpos($config['driver'], 'Mysql') === false && strpos($config['driver'], 'Postgres') === false;
+		$skip = !str_contains((string)$config['driver'], 'Mysql') && !str_contains((string)$config['driver'], 'Postgres');
 		$this->skipIf($skip, 'Only Mysql/Postgres is working yet for this.');
 	}
 
diff --git a/tests/TestCase/Queue/ProcessorTest.php b/tests/TestCase/Queue/ProcessorTest.php
diff --git a/tests/TestCase/Queue/Task/EmailTaskTest.php b/tests/TestCase/Queue/Task/EmailTaskTest.php
diff --git a/tests/TestCase/View/Helper/QueueProgressHelperTest.php b/tests/TestCase/View/Helper/QueueProgressHelperTest.php
diff --git a/tests/bootstrap.php b/tests/bootstrap.php
diff --git a/tests/schema.php b/tests/schema.php

Original file line number	Diff line number	Diff line change
`@@ -59,7 +59,7 @@ public function execute(Arguments $args, ConsoleIo $io) {`
`59`	`59`	`$taskName = $args->getArgument('task');`
`60`	`60`	`if (!$taskName) {`
`61`	`61`	`$io->out(count($tasks) . ' tasks available:');`
`62`		`- foreach ($tasks as $task => $className) {`
	`62`	`+ foreach (array_keys($tasks) as $task) {`
`63`	`63`	`$io->out(' - ' . $task);`
`64`	`64`	`}`
`65`	`65`
Original file line number	Diff line number	Diff line change
`@@ -307,7 +307,7 @@ protected function refererRedirect(array\|string $default) {`
`307`	`307`	`if (is_array($url)) {`
`308`	`308`	`throw new NotFoundException('Invalid array in query string');`
`309`	`309`	`}`
`310`		`- if ($url && (mb_substr($url, 0, 1) !== '/' \|\| mb_substr($url, 0, 2) === '//')) {`
	`310`	`+ if ($url && (mb_substr((string)$url, 0, 1) !== '/' \|\| mb_substr((string)$url, 0, 2) === '//')) {`
`311`	`311`	`$url = null;`
`312`	`312`	`}`
`313`	`313`
Original file line number	Diff line number	Diff line change
`@@ -100,7 +100,7 @@ public function terminate(?int $id = null) {`
`100`	`100`	`$queueProcess->terminate = true;`
`101`	`101`	`$this->QueueProcesses->saveOrFail($queueProcess);`
`102`	`102`	`$this->Flash->success(__d('queue', 'The queue process has been deleted.'));`
`103`		`- } catch (Exception $exception) {`
	`103`	`+ } catch (Exception) {`
`104`	`104`	`$this->Flash->error(__d('queue', 'The queue process could not be deleted. Please, try again.'));`
`105`	`105`	`}`
`106`	`106`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ public function collect(): array {`
`24`	`24`	`$list = [];`
`25`	`25`
`26`	`26`	`$names = $this->collectQueuedJobTasks();`
`27`		`- foreach ($names as $name => $className) {`
	`27`	`+ foreach (array_keys($names) as $name) {`
`28`	`28`	`$list[$name] = "'$name'";`
`29`	`29`	`}`
`30`	`30`
`@@ -44,9 +44,8 @@ public function collect(): array {`
`44`	`44`	`*/`
`45`	`45`	`protected function collectQueuedJobTasks(): array {`
`46`	`46`	`$taskFinder = new TaskFinder();`
`47`		`- $tasks = $taskFinder->all();`
`48`	`47`
`49`		`- return $tasks;`
	`48`	`+ return $taskFinder->all();`
`50`	`49`	`}`
`51`	`50`
`52`	`51`	`}`
Original file line number	Diff line number	Diff line change
`@@ -57,8 +57,7 @@ public function send(Message $message): array {`
`57`	`57`	`];`
`58`	`58`
`59`	`59`	`foreach ($settings as $setting => $value) {`
`60`		`- /** @phpstan-ignore-next-line */`
`61`		`- if (array_key_exists(0, $value) && ($value[0] === null \|\| $value[0] === [])) {`
	`60`	`+ if ($value[0] === null \|\| $value[0] === []) {`
`62`	`61`	`unset($settings[$setting]);`
`63`	`62`	`}`
`64`	`63`	`}`