Add stricter PHP code style rules (#2649)

CDash currently uses the relatively weak PSR-2 code style standard. This
PR enables the PSR-12 and Symfony rulesets for much stricter code
formatting guidelines. I took the liberty to adjust some of the styles
based on my opinion of the styles and their effect on the CDash
codebase. Feel free to leave review comments about any or all of these
changes.

.php-cs-fixer.dist.php

@@ -8,16 +8,24 @@
     ->exclude('vendor')
     ->exclude('_build')
     ->exclude('resources')
+    ->exclude('public')
     ->notPath('app/cdash/tests/config.test.local.php')
     ->in(__DIR__)
 ;
 
 $config = new PhpCsFixer\Config();
 return $config->setRules([
-        '@PSR2' => true,
+        '@PSR12' => true,
         '@PHP82Migration' => true,
-        'method_argument_space' => ['on_multiline' => 'ignore'],
-        'no_unused_imports' => 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)
 ;

app/Console/Commands/AuditDependencies.php

@@ -43,14 +43,14 @@ public function handle()
     {
         //  PHP auditing via composer
         $output = null;
-        print("\n\nComposer Report:\n\n");
-        exec("HOME=" . base_path() . " composer audit --no-interaction -d" . base_path(), $output);
-        print(implode("\n", $output));
+        echo "\n\nComposer Report:\n\n";
+        exec('HOME=' . base_path() . ' composer audit --no-interaction -d' . base_path(), $output);
+        echo implode("\n", $output);
 
         //  NPM audit too
-        print("\n\nNPM Report:\n\n");
+        echo "\n\nNPM Report:\n\n";
         $output = null;
-        exec("/usr/bin/npm audit", $output);
-        print(implode("\n", $output));
+        exec('/usr/bin/npm audit', $output);
+        echo implode("\n", $output);
     }
 }

app/Console/Commands/AutoRemoveBuilds.php

@@ -36,8 +36,6 @@ public function __construct()
 
     /**
      * Execute the console command.
-     *
-     * @return mixed
      */
     public function handle()
     {

app/Console/Commands/CheckKey.php

@@ -31,13 +31,10 @@ public function __construct()
         parent::__construct();
     }
 
-    /**
-     * @return mixed
-     */
     public function handle()
     {
         if (trim(config('app.key')) === '') {
-            echo "Error: APP_KEY environment variable is not set.  You can use the following randomly generated key:" . PHP_EOL;
+            echo 'Error: APP_KEY environment variable is not set.  You can use the following randomly generated key:' . PHP_EOL;
             // Print a new key to the screen.  Note: we can't use Artisan's key:generate command if there is no .env,
             // so we generate a random key ourselves.
             echo Str::password(32, true, true, false) . PHP_EOL;

app/Console/Commands/CleanDatabase.php

@@ -27,9 +27,9 @@ public function handle(): void
         // Reconfigure laravel to log to stderr for the rest of this command.
         config(['logging.default' => 'stderr']);
 
-        Log::info("Deleting unused rows from `banner`");
-        $num_deleted = DB::delete("DELETE FROM banner WHERE projectid != 0 AND
-                NOT EXISTS (SELECT 1 FROM project WHERE project.id = banner.projectid)");
+        Log::info('Deleting unused rows from `banner`');
+        $num_deleted = DB::delete('DELETE FROM banner WHERE projectid != 0 AND
+                NOT EXISTS (SELECT 1 FROM project WHERE project.id = banner.projectid)');
         Log::info("{$num_deleted} rows deleted from `banner`");
 
         DatabaseCleanupUtils::deleteUnusedRows('dailyupdate', 'projectid', 'project');
@@ -42,10 +42,10 @@ public function handle(): void
         DatabaseCleanupUtils::deleteUnusedRows('testoutput', 'id', 'build2test', 'outputid');
         DatabaseCleanupUtils::deleteUnusedRows('uploadfile', 'id', 'build2uploadfile', 'fileid');
 
-        Log::info("Deleting unused rows from `image`");
-        $num_deleted = DB::delete("DELETE FROM image WHERE
+        Log::info('Deleting unused rows from `image`');
+        $num_deleted = DB::delete('DELETE FROM image WHERE
                 NOT EXISTS (SELECT 1 FROM project WHERE project.imageid = image.id) AND
-                NOT EXISTS (SELECT 1 FROM test2image WHERE test2image.imgid = image.id)");
+                NOT EXISTS (SELECT 1 FROM test2image WHERE test2image.imgid = image.id)');
         Log::info("{$num_deleted} rows deleted from `image`");
     }
 }

app/Console/Commands/QueueSubmissions.php

@@ -5,8 +5,8 @@
 use App\Jobs\ProcessSubmission;
 use App\Utils\AuthTokenUtil;
 use CDash\Model\Project;
-
 use Illuminate\Console\Command;
+use Storage;
 
 class QueueSubmissions extends Command
 {
@@ -36,22 +36,20 @@ public function __construct()
 
     /**
      * Execute the console command.
-     *
-     * @return mixed
      */
     public function handle()
     {
         // Queue the "build metadata" JSON files first, so they have a chance
         // to get parsed before the subsequent payload files.
-        foreach (\Storage::files('inbox') as $inboxFile) {
+        foreach (Storage::files('inbox') as $inboxFile) {
             if (!str_contains($inboxFile, '_-_build-metadata_-_') || !str_contains($inboxFile, '.json')) {
                 continue;
             }
             $this->queueFile($inboxFile);
         }
 
         // Iterate over our inbox files again, queueing them for parsing.
-        foreach (\Storage::files('inbox') as $inboxFile) {
+        foreach (Storage::files('inbox') as $inboxFile) {
             if (str_contains($inboxFile, '_-_build-metadata_-_') && str_contains($inboxFile, '.json')) {
                 continue;
             }
@@ -64,7 +62,7 @@ private function queueFile($inboxFile)
         $filename = str_replace('inbox/', '', $inboxFile);
         $pos = strpos($filename, '_-_');
         if ($pos === false) {
-            \Storage::move("inbox/{$filename}", "failed/{$filename}");
+            Storage::move("inbox/{$filename}", "failed/{$filename}");
             echo "Could not extract projectname from $filename\n";
             return;
         }
@@ -73,7 +71,7 @@ private function queueFile($inboxFile)
         $project = new Project();
         $project->FindByName($projectname);
         if (!$project->Id) {
-            \Storage::move("inbox/{$filename}", "failed/{$filename}");
+            Storage::move("inbox/{$filename}", "failed/{$filename}");
             echo "Could not find project $projectname\n";
             return;
         }
@@ -83,13 +81,13 @@ private function queueFile($inboxFile)
             $begin = $pos + 3;
             $end = strpos($filename, '_-_', $begin);
             if ($end === false) {
-                \Storage::move("inbox/{$filename}", "failed/{$filename}");
+                Storage::move("inbox/{$filename}", "failed/{$filename}");
                 echo "Could not extract authtoken from $filename\n";
                 return;
             }
             $len = $end - $begin;
             if (!AuthTokenUtil::checkToken(substr($filename, $begin, $len), $project->Id)) {
-                \Storage::move("inbox/{$filename}", "failed/{$filename}");
+                Storage::move("inbox/{$filename}", "failed/{$filename}");
                 echo "Invalid authentication token for $filename\n";
                 return;
             }

app/Console/Commands/RemoveUser.php

@@ -2,6 +2,7 @@
 
 namespace App\Console\Commands;
 
+use App\Models\User;
 use Illuminate\Console\Command;
 
 class RemoveUser extends Command
@@ -32,18 +33,16 @@ public function __construct()
 
     /**
      * Execute the console command.
-     *
-     * @return mixed
      */
     public function handle()
     {
         $email = $this->option('email');
         if (is_null($email)) {
-            $this->error("You must specify the --email option");
+            $this->error('You must specify the --email option');
             return;
         }
 
-        $user = \App\Models\User::where('email', $email)->first();
+        $user = User::where('email', $email)->first();
         if (!$user) {
             $this->error("User $email does not exist");
             return;

app/Console/Commands/SaveUser.php

@@ -33,19 +33,17 @@ public function __construct()
 
     /**
      * Create or update a user.
-     *
-     * @return mixed
      */
     public function handle()
     {
         $email = $this->option('email');
         if (is_null($email)) {
-            $this->error("You must specify the --email option");
+            $this->error('You must specify the --email option');
             return;
         }
 
         // Are we creating a new user or updating an existing one?
-        $user = \App\Models\User::where('email', $email)->first();
+        $user = User::where('email', $email)->first();
         if ($user === null) {
             $user = new User();
             $user->email = $email;

app/Console/Commands/UpdateDependencies.php

@@ -32,25 +32,25 @@ public function __construct()
 
     public function handle()
     {
-        $composerInstallArgs = "--no-dev --prefer-dist --no-interaction --no-progress --optimize-autoloader";
-        $composerUpdateArgs = "--no-dev --prefer-lowest --prefer-stable";
-        if ($this->option("dev")) {
-            $composerInstallArgs = "--no-interaction --no-progress --prefer-dist";
-            $composerUpdateArgs = "--prefer-lowest --prefer-stable";
+        $composerInstallArgs = '--no-dev --prefer-dist --no-interaction --no-progress --optimize-autoloader';
+        $composerUpdateArgs = '--no-dev --prefer-lowest --prefer-stable';
+        if ($this->option('dev')) {
+            $composerInstallArgs = '--no-interaction --no-progress --prefer-dist';
+            $composerUpdateArgs = '--prefer-lowest --prefer-stable';
         }
 
-        if ($this->option("upgrade")) {
-            exec("npm update");
+        if ($this->option('upgrade')) {
+            exec('npm update');
             exec("composer update $composerUpdateArgs");
         }
 
         //  PHP dependencies via composer
         exec("composer install $composerInstallArgs");
 
         // Update JavaScript dependencies via npm
-        exec("npm install");
+        exec('npm install');
 
         // Run laravel-mix to builds assets
-        exec("npm run production");
+        exec('npm run production');
     }
 }

app/Console/Commands/ValidateXml.php

@@ -3,8 +3,8 @@
 namespace App\Console\Commands;
 
 use App\Utils\SubmissionUtils;
-use Illuminate\Console\Command;
 use DOMDocument;
+use Illuminate\Console\Command;
 
 class ValidateXml extends Command
 {
@@ -26,7 +26,7 @@ public function handle(): int
     {
         // parse all input files from command line
         $xml_files_args = $this->argument('xml_file');
-        $schemas_dir = base_path()."/app/Validators/Schemas";
+        $schemas_dir = base_path() . '/app/Validators/Schemas';
 
         // process each of the input files
         $has_errors = false;
@@ -43,8 +43,8 @@ public function handle(): int
 
             // verify we identified a valid xml type
             if ($xml_type === '') {
-                $this->error("ERROR: Could not determine submission"
-                            ." file type for: '{$input_xml_file}'");
+                $this->error('ERROR: Could not determine submission'
+                            . " file type for: '{$input_xml_file}'");
                 $has_errors = true;
                 continue;
             }
@@ -53,7 +53,7 @@ public function handle(): int
             $schema_file = "{$schemas_dir}/{$xml_type}.xsd";
             if (!file_exists($schema_file)) {
                 $this->error("ERROR: Could not find schema file '{$schema_file}'"
-                            ." corresonding to input: '{$input_xml_file}'");
+                            . " corresonding to input: '{$input_xml_file}'");
                 $has_errors = true;
                 continue;
             }
@@ -72,7 +72,7 @@ public function handle(): int
                 foreach ($errors as $error) {
                     if ($error->level > 2) {
                         $this->error("ERROR: {$error->message} in {$error->file},"
-                            ." line: {$error->line}, column: {$error->column}");
+                            . " line: {$error->line}, column: {$error->column}");
                     }
                 }
                 libxml_clear_errors();
@@ -84,10 +84,10 @@ public function handle(): int
 
         // finally, report the results
         if ($has_errors) {
-            $this->error("FAILED: Some XML file checks did not pass!");
+            $this->error('FAILED: Some XML file checks did not pass!');
             return Command::FAILURE;
         } else {
-            $this->line("SUCCESS: All XML file checks passed.");
+            $this->line('SUCCESS: All XML file checks passed.');
             return Command::SUCCESS;
         }
     }

app/Console/Kernel.php

@@ -12,14 +12,13 @@ class Kernel extends ConsoleKernel
     /**
      * Define the application's command schedule.
      *
-     * @param  \Illuminate\Console\Scheduling\Schedule  $schedule
      * @return void
      */
     protected function schedule(Schedule $schedule)
     {
         $cdash_directory_name = env('CDASH_DIRECTORY', 'cdash');
         $cdash_app_dir = realpath(app_path($cdash_directory_name));
-        $output_filename = $cdash_app_dir . "/AuditReport.log";
+        $output_filename = $cdash_app_dir . '/AuditReport.log';
 
         $schedule->command('dependencies:audit')
             ->everySixHours()
@@ -46,7 +45,7 @@ protected function schedule(Schedule $schedule)
      */
     protected function commands()
     {
-        $this->load(__DIR__.'/Commands');
+        $this->load(__DIR__ . '/Commands');
 
         require base_path('routes/console.php');
     }

app/Exceptions/Handler.php

@@ -3,9 +3,12 @@
 namespace App\Exceptions;
 
 use Illuminate\Auth\AuthenticationException;
+use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
 use Illuminate\Http\JsonResponse;
+use Illuminate\Http\RedirectResponse;
+use Illuminate\Http\Request;
+use Illuminate\Http\Response;
 use Throwable;
-use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
 
 class Handler extends ExceptionHandler
 {
@@ -15,7 +18,6 @@ class Handler extends ExceptionHandler
      * @var array
      */
     protected $dontReport = [
-        //
     ];
 
     /**
@@ -31,7 +33,6 @@ class Handler extends ExceptionHandler
     /**
      * Report or log an exception.
      *
-     * @param  \Throwable  $exception
      * @return void
      */
     public function report(Throwable $exception)
@@ -42,9 +43,9 @@ public function report(Throwable $exception)
     /**
      * Render an exception into an HTTP response.
      *
-     * @param  \Illuminate\Http\Request  $request
-     * @param  \Throwable  $exception
-     * @return \Illuminate\Http\Response|\Illuminate\Http\JsonResponse|\Symfony\Component\HttpFoundation\Response
+     * @param Request $request
+     *
+     * @return Response|JsonResponse|\Symfony\Component\HttpFoundation\Response
      */
     public function render($request, Throwable $exception)
     {
@@ -70,9 +71,7 @@ public function render($request, Throwable $exception)
     }
 
     /**
-     * @param $request
-     * @param AuthenticationException $exception
-     * @return \Illuminate\Http\JsonResponse|\Illuminate\Http\RedirectResponse
+     * @return JsonResponse|RedirectResponse
      */
     public function unauthenticated($request, AuthenticationException $exception)
     {