Apply "risky" PHP migration formatter rules (#2668)

This PR continues our ongoing effort to modernize the codebase by applying stricter code formatting rules. This PR also helps us better take advantage of new PHP features. Most of the rules applied here are "risky" in cases which do not apply to CDash.
Kitware · Jan 17, 2025 · c54e150 · c54e150
1 parent 43b055a
commit c54e150
Show file tree

Hide file tree

Showing 78 changed files with 257 additions and 266 deletions.
diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php
@@ -1,6 +1,9 @@
 <?php
 
-$finder = PhpCsFixer\Finder::create()
+use PhpCsFixer\Config;
+use PhpCsFixer\Finder;
+
+$finder = Finder::create()
     ->exclude('app/cdash/config')
     ->exclude('bootstrap/cache')
     ->exclude('node_modules')
@@ -9,23 +12,25 @@
     ->exclude('_build')
     ->exclude('resources')
     ->exclude('public')
+    ->exclude('app/cdash/tests/kwtest/simpletest')
     ->notPath('app/cdash/tests/config.test.local.php')
-    ->in(__DIR__)
-;
+    ->in(__DIR__);
 
-$config = new PhpCsFixer\Config();
+$config = new Config();
 return $config->setRules([
-        '@PSR12' => true,
-        '@PHP82Migration' => true,
-        '@Symfony' => true,
-        'yoda_style' => false,
-        'blank_line_before_statement' => false,
-        'phpdoc_summary' => false,
-        'concat_space' => ['spacing' => 'one'],
-        'increment_style' => ['style' => 'post'],
-        'fully_qualified_strict_types' => ['import_symbols' => true],
-        'global_namespace_import' => ['import_classes' => true, 'import_constants' => null, 'import_functions' => null],
-        'phpdoc_align' => ['align' => 'left'],
-    ])
-    ->setFinder($finder)
-;
+    '@PSR12' => true,
+    '@PSR12:risky' => true,
+    '@PHP82Migration' => true,
+    '@PHP82Migration:risky' => true,
+    '@Symfony' => true,
+    'yoda_style' => false,
+    'blank_line_before_statement' => false,
+    'phpdoc_summary' => false,
+    'concat_space' => ['spacing' => 'one'],
+    'increment_style' => ['style' => 'post'],
+    'fully_qualified_strict_types' => ['import_symbols' => true],
+    'global_namespace_import' => ['import_classes' => true, 'import_constants' => null, 'import_functions' => null],
+    'phpdoc_align' => ['align' => 'left'],
+    'declare_strict_types' => false, // TODO: turn this back on.  Currently causes errors...
+    'void_return' => false, // TODO: turn this back on.  Currently causes errors...
+])->setFinder($finder);
diff --git a/app/Http/Controllers/BuildController.php b/app/Http/Controllers/BuildController.php
@@ -666,9 +666,7 @@ public function viewUpdatePageContent(): JsonResponse
 
         $directoryarray = array_unique($directoryarray);
 
-        usort($directoryarray, function ($a, $b) {
-            return $a > $b ? 1 : 0;
-        });
+        usort($directoryarray, fn ($a, $b) => $a > $b ? 1 : 0);
         usort($updatearray1, function ($a, $b) {
             // Extract directory
             $filenamea = $a['filename'];
@@ -1009,9 +1007,7 @@ public function apiViewBuildError(): JsonResponse
         }
 
         if ($this->build->IsParentBuild()) {
-            $response['numSubprojects'] = count(array_unique(array_map(function ($buildError) {
-                return $buildError['subprojectid'];
-            }, $response['errors'])));
+            $response['numSubprojects'] = count(array_unique(array_map(fn ($buildError) => $buildError['subprojectid'], $response['errors'])));
         }
 
         $pageTimer->end($response);

diff --git a/app/Http/Controllers/BuildPropertiesController.php b/app/Http/Controllers/BuildPropertiesController.php
@@ -116,9 +116,7 @@ public function apiBuildProperties(): JsonResponse
         }
         $response['defecttypes'] = $defect_types;
 
-        $defect_types = array_filter($defect_types, function ($defect_type) {
-            return $defect_type['selected'];
-        });
+        $defect_types = array_filter($defect_types, fn ($defect_type) => $defect_type['selected']);
 
         // Construct an SQL SELECT clause for the requested types of defects.
         $defect_keys = [];

diff --git a/app/Http/Controllers/CoverageController.php b/app/Http/Controllers/CoverageController.php
@@ -1794,9 +1794,7 @@ public function apiCompareCoverage(): JsonResponse
 
         if (!empty($subproject_groups)) {
             // At this point it is safe to remove any empty $coveragegroups from our response.
-            $coveragegroups_response = array_filter($coveragegroups, function ($group) {
-                return $group['label'] === 'Total' || !empty($group['coverages']);
-            });
+            $coveragegroups_response = array_filter($coveragegroups, fn ($group) => $group['label'] === 'Total' || !empty($group['coverages']));
 
             // Report coveragegroups as a list, not an associative array.
             $coveragegroups_response = array_values($coveragegroups_response);

diff --git a/app/Http/Controllers/RemoteProcessingController.php b/app/Http/Controllers/RemoteProcessingController.php
@@ -111,7 +111,7 @@ public function requeueSubmissionFile(): Response
 
         // Requeue the file with exponential backoff.
         PendingSubmissions::IncrementForBuildId($buildid);
-        $delay = (int) pow(config('cdash.retry_base'), $retry_handler->Retries);
+        $delay = ((int) config('cdash.retry_base')) ** $retry_handler->Retries;
         ProcessSubmission::dispatch($filename, $projectid, $buildid, md5_file(Storage::path("inbox/{$filename}")))->delay(now()->addSeconds($delay));
         return response('OK', Response::HTTP_OK);
     }

diff --git a/app/Jobs/ProcessSubmission.php b/app/Jobs/ProcessSubmission.php
@@ -116,7 +116,7 @@ private function requeueSubmissionFile($buildid): bool
 
             // Requeue the file with exponential backoff.
             PendingSubmissions::IncrementForBuildId($this->buildid);
-            $delay = pow(config('cdash.retry_base'), $retry_handler->Retries);
+            $delay = ((int) config('cdash.retry_base')) ** $retry_handler->Retries;
             if (config('queue.default') === 'sqs-fifo') {
                 // Special handling for sqs-fifo, which does not support per-message delays.
                 sleep(10);

diff --git a/app/Utils/RepositoryUtils.php b/app/Utils/RepositoryUtils.php
@@ -709,7 +709,7 @@ public static function linkify_compiler_output($projecturl, $source_dir, $revisi
         }
 
         // Make sure we specify a protocol so this isn't interpreted as a relative path.
-        if (strpos($projecturl, '//') === false) {
+        if (!str_contains($projecturl, '//')) {
             $projecturl = '//' . $projecturl;
         }
         $repo_link = "<a class='cdash-link' href='$projecturl/blob/$revision";
@@ -776,7 +776,7 @@ public static function post_github_pull_request_comment(Project $project, $pull_
         $repo = null;
         $repositories = $project->GetRepositories();
         foreach ($repositories as $repository) {
-            if (strpos($repository['url'], 'github.com') !== false) {
+            if (str_contains($repository['url'], 'github.com')) {
                 $repo = $repository;
                 break;
             }

diff --git a/app/cdash/app/Controller/Api/Index.php b/app/cdash/app/Controller/Api/Index.php
@@ -189,9 +189,7 @@ public function getDailyBuilds(): array
         $sql .= $this->limitSQL;
 
         $builds = DB::select($sql, $query_params);
-        return array_map(function ($item) {
-            return (array) $item;
-        }, $builds);
+        return array_map(fn ($item) => (array) $item, $builds);
     }
 
     public function getDynamicBuilds(): array

diff --git a/app/cdash/app/Controller/Api/QueryTests.php b/app/cdash/app/Controller/Api/QueryTests.php
@@ -86,7 +86,7 @@ private function rowSurvivesTestOutputFilter($row, &$build)
         $match_length = 0;
 
         foreach ($this->testOutputExclude as $exclude) {
-            if (strpos($test_output, $exclude) !== false) {
+            if (str_contains($test_output, $exclude)) {
                 return false;
             }
         }

diff --git a/app/cdash/app/Controller/Api/TestOverview.php b/app/cdash/app/Controller/Api/TestOverview.php
@@ -163,7 +163,7 @@ public function getResponse()
 
             if ($status === 'passed') {
                 $all_tests[$test_name]['passed']++;
-            } elseif (strpos($row['details'], 'Timeout') !== false) {
+            } elseif (str_contains($row['details'], 'Timeout')) {
                 $all_tests[$test_name]['timeout']++;
             } else {
                 $all_tests[$test_name]['failed']++;

diff --git a/app/cdash/app/Lib/Repository/GitHub.php b/app/cdash/app/Lib/Repository/GitHub.php
@@ -75,7 +75,7 @@ public function __construct(Project $project)
 
         $repositories = $this->project->GetRepositories();
         foreach ($repositories as $repo) {
-            if (strpos($repo['url'], 'github.com') !== false) {
+            if (str_contains($repo['url'], 'github.com')) {
                 $this->installationId = $repo['username'];
                 break;
             }
@@ -182,9 +182,7 @@ public function setStatus($options): void
         }
 
         $commitHash = $options['commit_hash'];
-        $params = array_filter($options, function ($key) {
-            return in_array($key, ['state', 'context', 'description', 'target_url'], true);
-        }, ARRAY_FILTER_USE_KEY);
+        $params = array_filter($options, fn ($key) => in_array($key, ['state', 'context', 'description', 'target_url'], true), ARRAY_FILTER_USE_KEY);
 
         $statuses = $this->apiClient
             ->api('repo')
@@ -256,9 +254,7 @@ public function dedupeAndSortBuildRows($rows)
             }
             $build_names[$build_name][] = $row;
         }
-        $build_names = array_filter($build_names, function ($k, $v) {
-            return count($k) > 1;
-        }, ARRAY_FILTER_USE_BOTH);
+        $build_names = array_filter($build_names, fn ($k, $v) => count($k) > 1, ARRAY_FILTER_USE_BOTH);
 
         // Find the ids of all the older builds that should not be included
         // in our report.
@@ -293,9 +289,7 @@ public function dedupeAndSortBuildRows($rows)
         }
 
         // Alphabetize this array by build name.
-        usort($output_rows, function ($a, $b) {
-            return strcmp((string) $a['name'], (string) $b['name']);
-        });
+        usort($output_rows, fn ($a, $b) => strcmp((string) $a['name'], (string) $b['name']));
 
         return $output_rows;
     }

diff --git a/app/cdash/app/Model/BuildErrorFilter.php b/app/cdash/app/Model/BuildErrorFilter.php
@@ -120,7 +120,7 @@ private function FilterText($subject, $filterString)
     {
         if ($filterString) {
             foreach (preg_split("/\R/", $filterString) as $filter) {
-                if (strpos($subject, $filter) !== false) {
+                if (str_contains($subject, $filter)) {
                     return true;
                 }
             }

diff --git a/app/cdash/app/Model/BuildUpdate.php b/app/cdash/app/Model/BuildUpdate.php
@@ -425,9 +425,7 @@ public function FillFromBuildId(): bool
             $this->AddFile($file);
         }
 
-        usort($this->Files, function ($file1, $file2) {
-            return Str::afterLast('/', $file1->Filename) <=> Str::afterLast('/', $file2->Filename);
-        });
+        usort($this->Files, fn ($file1, $file2) => Str::afterLast('/', $file1->Filename) <=> Str::afterLast('/', $file2->Filename));
 
         return true;
     }

diff --git a/app/cdash/app/Model/Repository.php b/app/cdash/app/Model/Repository.php
@@ -54,7 +54,7 @@ public static function getViewers()
         $self = new ReflectionClass(__CLASS__);
         $viewers = [];
         foreach ($self->getConstants() as $key => $text) {
-            if (strpos($key, 'VIEWER_') === 0) {
+            if (str_starts_with($key, 'VIEWER_')) {
                 $value = strtolower(substr($key, strlen('VIEWER_')));
                 $viewers[$text] = $value;
             }

diff --git a/app/cdash/include/Collection/CollectionCollection.php b/app/cdash/include/Collection/CollectionCollection.php
@@ -24,7 +24,7 @@ class CollectionCollection extends Collection
      */
     public function add(Collection $collection)
     {
-        $this->addItem($collection, get_class($collection));
+        $this->addItem($collection, $collection::class);
         return $this;
     }
 }
diff --git a/app/cdash/include/Collection/SubscriptionBuilderCollection.php b/app/cdash/include/Collection/SubscriptionBuilderCollection.php
@@ -23,7 +23,7 @@ class SubscriptionBuilderCollection extends Collection
 {
     public function add(SubscriptionBuilderInterface $builder)
     {
-        $this->addItem($builder, get_class($builder));
+        $this->addItem($builder, $builder::class);
         return $this;
     }
 }
diff --git a/app/cdash/include/Test/UseCase/DynamicAnalysisUseCase.php b/app/cdash/include/Test/UseCase/DynamicAnalysisUseCase.php
@@ -202,7 +202,7 @@ private function createResults(array &$properties): void
             $faker = $this->getFaker();
             $num_defects = $faker->randomDigit;
             for ($i = 0; $i < $num_defects; $i++) {
-                $random = rand(0, 4);
+                $random = random_int(0, 4);
                 $properties['Results'][] = [
                     'type' => $posibilities[$random],
                     'value' => $faker->randomNumber(1),

diff --git a/app/cdash/include/Test/UseCase/UpdateUseCase.php b/app/cdash/include/Test/UseCase/UpdateUseCase.php
@@ -289,7 +289,7 @@ public function createRevisionHash(): string
 
     public function randomizeCheckinDate(): string
     {
-        $random = rand(1, 9 * 60 * 60) + (9 * 60 * 60); // seconds between 8am and 5pm
+        $random = random_int(1, 9 * 60 * 60) + (9 * 60 * 60); // seconds between 8am and 5pm
         $time = strtotime("yesterday +{$random} seconds");
         return date('Y-m-d H:i:s -0500', $time);
     }

diff --git a/app/cdash/include/Test/UseCase/UseCase.php b/app/cdash/include/Test/UseCase/UseCase.php
@@ -255,9 +255,7 @@ public function createChildElementsFromKeys(DOMElement $parent, array $attribute
     {
         $subset = array_filter(
             $attributes,
-            function ($key) use ($keys) {
-                return empty($keys) || in_array($key, $keys);
-            },
+            fn ($key) => empty($keys) || in_array($key, $keys),
             ARRAY_FILTER_USE_KEY
         );
 

diff --git a/app/cdash/include/common.php b/app/cdash/include/common.php
@@ -935,9 +935,7 @@ function extract_tar(string $filename, string $dirName): bool
  */
 function deepEncodeHTMLEntities(&$structure): void
 {
-    $encode = function ($string) {
-        return htmlspecialchars($string, ENT_QUOTES, 'UTF-8', false);
-    };
+    $encode = fn ($string) => htmlspecialchars($string, ENT_QUOTES, 'UTF-8', false);
 
     if (is_object($structure)) {
         $properties = get_object_vars($structure);

diff --git a/app/cdash/include/dailyupdates.php b/app/cdash/include/dailyupdates.php
@@ -482,7 +482,7 @@ function get_svn_repository_commits($svnroot, $dates, $username = '', $password
                         $previous_revision = '';
 
                         // Look if we have a A or a M
-                        if (strpos(substr($ff, 0, 5), 'A') !== false) {
+                        if (str_contains(substr($ff, 0, 5), 'A')) {
                             $previous_revision = '-1'; // newly added file so we marked it as no prior revision
                         }
 

diff --git a/app/cdash/include/filterdataFunctions.php b/app/cdash/include/filterdataFunctions.php
@@ -1160,7 +1160,7 @@ function generate_filter_sql($filter, $pageSpecificFilters)
 
     // Time durations can either be specified as a number of seconds,
     // or as a string representing a time interval.
-    if (strpos($field, 'duration') !== false) {
+    if (str_contains($field, 'duration')) {
         $input_value = trim($sql_value, "'");
         $sql_value = get_seconds_from_interval($input_value);
         if ($input_value !== $sql_value && $field === 'updateduration') {

diff --git a/app/cdash/tests/bootstrap.php b/app/cdash/tests/bootstrap.php
@@ -1,6 +1,6 @@
 <?php
 
-$cdash_root = dirname(dirname(__FILE__));
+$cdash_root = dirname(__FILE__, 2);
 $cdash_root = str_replace('\\', '/', $cdash_root);
 set_include_path(get_include_path() . PATH_SEPARATOR . $cdash_root);
 

diff --git a/app/cdash/tests/case/CDash/Model/RepositoryTest.php b/app/cdash/tests/case/CDash/Model/RepositoryTest.php
@@ -38,9 +38,7 @@ public function setUp(): void
         $this->project->CvsUrl = 'https://github.com/foo/bar';
         $this->project->expects($this->once())
             ->method('GetRepositories')
-            ->willReturnCallback(function () {
-                return $this->repo;
-            });
+            ->willReturnCallback(fn () => $this->repo);
     }
 
     public function testGetRepositoryInterfaceReturnsGitHubService()

diff --git a/app/cdash/tests/cdash_test_case.php b/app/cdash/tests/cdash_test_case.php
@@ -13,7 +13,7 @@
 // of getcwd() value:
 //
 global $cdashpath;
-$cdashpath = str_replace('\\', '/', dirname(dirname(__FILE__)));
+$cdashpath = str_replace('\\', '/', dirname(__FILE__, 2));
 set_include_path($cdashpath . PATH_SEPARATOR . get_include_path());
 
 require_once 'tests/kwtest/kw_web_tester.php'; // KWWebTestCase

diff --git a/app/cdash/tests/kwtest/kw_web_tester.php b/app/cdash/tests/kwtest/kw_web_tester.php
@@ -105,7 +105,7 @@ public function tearDown()
         $this->stopCodeCoverage();
         unset($_SERVER['Authorization']);
         foreach (array_keys($_SERVER) as $key) {
-            if (strpos($key, 'HTTP_') === 0) {
+            if (str_starts_with($key, 'HTTP_')) {
                 unset($_SERVER[$key]);
             }
         }
@@ -135,7 +135,7 @@ public function stopCodeCoverage()
             $file = config('cdash.coverage_dir') . DIRECTORY_SEPARATOR .
                 md5($_SERVER['SCRIPT_FILENAME']);
             file_put_contents(
-                $file . '.' . md5(uniqid(rand(), true)) . '.' . get_class(),
+                $file . '.' . md5(uniqid(random_int(0, getrandmax()), true)) . '.' . get_class(),
                 serialize($data)
             );
         }
@@ -151,7 +151,7 @@ public function stopCodeCoverage()
      */
     public function findString($mystring, $findme)
     {
-        if (strpos($mystring, $findme) === false) {
+        if (!str_contains($mystring, $findme)) {
             return false;
         }
         return true;
@@ -288,9 +288,7 @@ public function loginActingAs()
         $user = $this->getUser($this->actingAs['email']);
         Auth::shouldReceive('check')->andReturn(true);
         Auth::shouldReceive('user')->andReturn($user);
-        Auth::shouldReceive('userResolver')->andReturn(function () use ($user) {
-            return $user;
-        });
+        Auth::shouldReceive('userResolver')->andReturn(fn () => $user);
         Auth::shouldReceive('id')->andReturn($user->id);
         Auth::shouldReceive('guard')->andReturnSelf();
         Auth::shouldReceive('shouldUse')->andReturn('web');
@@ -582,7 +580,7 @@ public function expectsPageRequiresLogin($page)
         $this->logout();
         $content = $this->get("{$this->url}{$page}");
 
-        if (strpos($content, '<form method="POST" action="login" name="loginform" id="loginform">') === false) {
+        if (!str_contains($content, '<form method="POST" action="login" name="loginform" id="loginform">')) {
             $this->fail('Login not found when expected');
             return false;
         }
@@ -718,7 +716,7 @@ private function setQueryParameters($url)
 
         if (!empty($query)) {
             foreach (explode('&', $query) as $parameter) {
-                if (strpos($parameter, '=') !== false) {
+                if (str_contains($parameter, '=')) {
                     [$key, $value] = explode('=', $parameter);
                     $this->setRequestKeyValuePair($parameters, $key, $value);
                 } else {