shopware/production

Executing coding standards result on errors

amansilla opened this issue · 2 comments

PHP Version

7.4.20

Shopware Version

6.4.3.1

Expected behaviour

Runs without errors.

Actual behaviour

When executing the coding standards included in the project multiple style errors are detected.

composer require --dev symplify/easy-coding-standard
php -d memory_limit=-1 vendor/bin/ecs --config=easy-coding-standard.php check
---------------------------------------------------------------------------------------------
 src/Command/SystemSetupCommand.php:75
 ---------------------------------------------------------------------------------------------
 Variable assignment found within a condition. Did you mean to do a comparison ?

 Reported by: "Found"
 ---------------------------------------------------------------------------------------------

 ---------------------------------------------------------------------------------------------
 src/Command/SystemSetupCommand.php:79
 ---------------------------------------------------------------------------------------------
 Variable assignment found within a condition. Did you mean to do a comparison ?

 Reported by: "Found"
 ---------------------------------------------------------------------------------------------

 ---------------------------------------------------------------------------------------------
 src/Command/SystemSetupCommand.php:83
 ---------------------------------------------------------------------------------------------
 Variable assignment found within a condition. Did you mean to do a comparison ?

 Reported by: "Found"
 ---------------------------------------------------------------------------------------------


 [ERROR] Found 3 errors that need to be fixed manually.

After taking a look at the easy-coding-standard.php I see the responsable for this is the $containerConfigurator->import(SetList::CONTROL_STRUCTURES);. If I comment out that rule set and execute it again:

1) bin/console

    ---------- begin diff ----------
@@ -36,7 +36,7 @@

 $input = new ArgvInput();
 $env = $input->getParameterOption(['--env', '-e'], $_SERVER['APP_ENV'] ?? 'prod', true);
-$debug = ($_SERVER['APP_DEBUG'] ?? ($env !== 'prod')) && !$input->hasParameterOption('--no-debug', true);
+$debug = ($_SERVER['APP_DEBUG'] ?? ('prod' !== $env)) && !$input->hasParameterOption('--no-debug', true);

 if ($debug) {
     umask(0000);
@@ -50,11 +50,11 @@

 $shopwareVersion = InstalledVersions::getVersion('shopware/core') . '@' . InstalledVersions::getReference('shopware/core');

-if ($input->getFirstArgument() === 'system:install') {
+if ('system:install' === $input->getFirstArgument()) {
     $_SERVER['INSTALL'] = true;
 }

-if (trim($_SERVER['DATABASE_URL'] ?? '') === '') {
+if ('' === trim($_SERVER['DATABASE_URL'] ?? '')) {
     // fake DATABASE_URL
     $_SERVER['DATABASE_URL'] = Kernel::PLACEHOLDER_DATABASE_URL;
 } else {
    ----------- end diff -----------


Applied checkers:

 * PhpCsFixer\Fixer\Casing\LowercaseKeywordsFixer
 * PhpCsFixer\Fixer\ControlStructure\YodaStyleFixer



2) public/index.php

    ---------- begin diff ----------
@@ -20,7 +20,7 @@
     $baseURL = str_replace(basename(__FILE__), '', $_SERVER['SCRIPT_NAME']);
     $baseURL = rtrim($baseURL, '/');
     $installerURL = $baseURL . '/' . $basePath . '/index.php';
-    if (strpos($_SERVER['REQUEST_URI'], $basePath) === false) {
+    if (false === strpos($_SERVER['REQUEST_URI'], $basePath)) {
         header('Location: ' . $installerURL);
         exit;
     }
@@ -53,7 +53,7 @@
 }

 $appEnv = $_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? 'dev';
-$debug = (bool) ($_SERVER['APP_DEBUG'] ?? $_ENV['APP_DEBUG'] ?? ($appEnv !== 'prod'));
+$debug = (bool) ($_SERVER['APP_DEBUG'] ?? $_ENV['APP_DEBUG'] ?? ('prod' !== $appEnv));

 if ($debug) {
     umask(0000);
    ----------- end diff -----------


Applied checkers:

 * PhpCsFixer\Fixer\Casing\LowercaseKeywordsFixer
 * PhpCsFixer\Fixer\ControlStructure\YodaStyleFixer



3) src/Command/SystemGenerateJwtSecretCommand.php

    ---------- begin diff ----------
@@ -81,7 +81,7 @@
             'encrypt_key_cipher' => \OPENSSL_CIPHER_AES_256_CBC,
         ]);

-        if ($key === false) {
+        if (false === $key) {
             $io->error('Failed to generate key');

             return 1;
@@ -89,7 +89,7 @@

         // export private key
         $result = openssl_pkey_export_to_file($key, $privateKeyPath, $passphrase);
-        if ($result === false) {
+        if (false === $result) {
             $io->error('Could not export private key to file');

             return 1;
@@ -100,7 +100,7 @@
         if ($publicKeyPath) {
             // export public key
             $keyData = openssl_pkey_get_details($key);
-            if ($keyData === false) {
+            if (false === $keyData) {
                 $io->error('Failed to export public key');

                 return 1;
    ----------- end diff -----------


Applied checkers:

 * PhpCsFixer\Fixer\ClassNotation\VisibilityRequiredFixer
 * PhpCsFixer\Fixer\ControlStructure\YodaStyleFixer



4) src/Command/SystemInstallCommand.php

    ---------- begin diff ----------
@@ -47,7 +47,7 @@
         putenv('BLUE_GREEN_DEPLOYMENT=' . $_SERVER['BLUE_GREEN_DEPLOYMENT']);

         $dsn = trim((string) ($_ENV['DATABASE_URL'] ?? $_SERVER['DATABASE_URL'] ?? getenv('DATABASE_URL')));
-        if ($dsn === '') {
+        if ('' === $dsn) {
             $output->error("Environment variable 'DATABASE_URL' not defined.");

             return 1;
@@ -60,7 +60,7 @@
         }

         $params = parse_url($dsn);
-        if ($params === false) {
+        if (false === $params) {
             $output->error('dsn invalid');

             return 1;
@@ -199,7 +199,7 @@
     private function runCommands(array $commands, OutputInterface $output): int
     {
         $application = $this->getApplication();
-        if ($application === null) {
+        if (null === $application) {
             throw new \RuntimeException('No application initialised');
         }

@@ -212,7 +212,7 @@

             try {
                 $returnCode = $command->run(new ArrayInput($parameters, $command->getDefinition()), $output);
-                if ($returnCode !== 0 && !$allowedToFail) {
+                if (0 !== $returnCode && !$allowedToFail) {
                     return $returnCode;
                 }
             } catch (\Throwable $e) {
    ----------- end diff -----------


Applied checkers:

 * PhpCsFixer\Fixer\ClassNotation\VisibilityRequiredFixer
 * PhpCsFixer\Fixer\ControlStructure\YodaStyleFixer



5) src/Command/SystemSetupCommand.php

    ---------- begin diff ----------
@@ -121,7 +121,7 @@
         $env['APP_URL'] = $io->ask('URL to your /public folder', $input->getOption('app-url'), static function (string $value): string {
             $value = trim($value);

-            if ($value === '') {
+            if ('' === $value) {
                 throw new \RuntimeException('Shop URL is required.');
             }

@@ -169,7 +169,7 @@
         $env = [];

         $emptyValidation = static function (string $value): string {
-            if (trim($value) === '') {
+            if ('' === trim($value)) {
                 throw new \RuntimeException('This value is required.');
             }

@@ -274,7 +274,7 @@
         }

         $application = $this->getApplication();
-        if ($application === null) {
+        if (null === $application) {
             throw new \RuntimeException('No application initialised');
         }
         $command = $application->find('system:generate-jwt-secret');
    ----------- end diff -----------


Applied checkers:

 * PhpCsFixer\Fixer\ClassNotation\VisibilityRequiredFixer
 * PhpCsFixer\Fixer\ControlStructure\YodaStyleFixer



6) src/Command/SystemUpdateFinishCommand.php

    ---------- begin diff ----------
@@ -48,7 +48,7 @@
         $output = new ShopwareStyle($input, $output);

         $dsn = trim((string) ($_SERVER['DATABASE_URL'] ?? getenv('DATABASE_URL')));
-        if ($dsn === '' || $dsn === Kernel::PLACEHOLDER_DATABASE_URL) {
+        if ('' === $dsn || Kernel::PLACEHOLDER_DATABASE_URL === $dsn) {
             $output->note("Environment variable 'DATABASE_URL' not defined. Skipping " . $this->getName() . '...');

             return 0;
@@ -86,7 +86,7 @@
     private function runMigrations(OutputInterface $output): int
     {
         $application = $this->getApplication();
-        if ($application === null) {
+        if (null === $application) {
             throw new \RuntimeException('No application initialised');
         }
         $command = $application->find('database:migrate');
@@ -103,7 +103,7 @@
     private function installAssets(OutputInterface $output): int
     {
         $application = $this->getApplication();
-        if ($application === null) {
+        if (null === $application) {
             throw new \RuntimeException('No application initialised');
         }
         $command = $application->find('assets:install');
    ----------- end diff -----------


Applied checkers:

 * PhpCsFixer\Fixer\ClassNotation\VisibilityRequiredFixer
 * PhpCsFixer\Fixer\ControlStructure\YodaStyleFixer



7) src/Command/SystemUpdatePrepareCommand.php

    ---------- begin diff ----------
@@ -45,7 +45,7 @@
         $output = new ShopwareStyle($input, $output);

         $dsn = trim((string) ($_SERVER['DATABASE_URL'] ?? getenv('DATABASE_URL')));
-        if ($dsn === '' || $dsn === Kernel::PLACEHOLDER_DATABASE_URL) {
+        if ('' === $dsn || Kernel::PLACEHOLDER_DATABASE_URL === $dsn) {
             $output->note("Environment variable 'DATABASE_URL' not defined. Skipping " . $this->getName() . '...');

             return 0;
    ----------- end diff -----------


Applied checkers:

 * PhpCsFixer\Fixer\ClassNotation\VisibilityRequiredFixer
 * PhpCsFixer\Fixer\ControlStructure\YodaStyleFixer



8) src/Kernel.php

    ---------- begin diff ----------
@@ -30,11 +30,11 @@
             ?? $_SERVER['DATABASE_URL']
             ?? getenv('DATABASE_URL');

-        if (isset($_SERVER['INSTALL']) || $url === self::PLACEHOLDER_DATABASE_URL) {
+        if (isset($_SERVER['INSTALL']) || self::PLACEHOLDER_DATABASE_URL === $url) {
             return;
         }

-        if ($this->getEnvironment() === 'dev') {
+        if ('dev' === $this->getEnvironment()) {
             self::getConnection()->getConfiguration()->setSQLLogger(new DebugStack());
         }
    ----------- end diff -----------


Applied checkers:

 * PhpCsFixer\Fixer\ClassNotation\VisibilityRequiredFixer
 * PhpCsFixer\Fixer\ControlStructure\YodaStyleFixer



9) src/TestBootstrap.php

    ---------- begin diff ----------
@@ -38,7 +38,7 @@

 // export private key
 $result = openssl_pkey_export_to_file($key, $jwtDir . '/private.pem');
-if ($result === false) {
+if (false === $result) {
     throw new RuntimeException('Could not export private key to file');
 }
    ----------- end diff -----------


Applied checkers:

 * PhpCsFixer\Fixer\Casing\LowercaseKeywordsFixer
 * PhpCsFixer\Fixer\ControlStructure\YodaStyleFixer




 [WARNING] 9 errors are fixable! Just add "--fix" to console command and rerun to apply.

In this case responsible is the ruleset SetList::SYMFONY.

So it seems that the production repo is released with a code style ruleset that doesn't fit the project code. I'd be interested in implementing the fix if necessary but I need to know what is desired by the owners. Change rulesets or change code?

How to reproduce

composer require --dev symplify/easy-coding-standard
php -d memory_limit=-1 vendor/bin/ecs --config=easy-coding-standard.php check

please make sure that you have installed the right version of easy coding standard. refer to the composer.json of the platform repository.
unfortunately it happens from time to time, that newer versions of ECS break the build. so it needs to be the exact version which is locked

All right, installing the version included in the platform repo did the trick, now after executing coding standard check there is no errors. For installing the right version composer require --dev rewuirsymplify/easy-coding-standard:9.3.15. Right version to be used can be found here https://github.com/shopware/platform/blob/7b1f136d1419f128453ddbeb3e02240ef9ed1328/vendor-bin/cs-fixer/composer.json.

Thanks @mitelg 😉