DQA-0: Improve phpcs ruleset (#795)

.drone.yml

@@ -29,6 +29,7 @@ pipeline:
     volumes:
       - /cache/${DRONE_REPO_NAME}:/cache
     commands:
+      - git config --global --add safe.directory /test/toolkit
       - composer self-update --${COMPOSER_VERSION}
       - composer install --ansi --no-progress
       - composer outdated --no-dev --locked --direct --minor-only
@@ -44,6 +45,7 @@ pipeline:
     volumes:
       - /cache/${DRONE_REPO_NAME}:/cache
     commands:
+      - git config --global --add safe.directory /test/toolkit
       - composer self-update --${COMPOSER_VERSION}
       - composer update --prefer-lowest --prefer-stable --ansi --no-progress
     when:

composer.json

@@ -23,6 +23,7 @@
         "consolidation/robo": "^3.0 || ^4.0 || ^5.0",
         "cweagans/composer-patches": "^1.7 || ^2.0",
         "dantleech/gherkin-lint": "^0.2.3",
+        "drupal/coder": "^8.3.10",
         "drush/drush": "^11.0.4 || ^12.0 || ^13.0",
         "ec-europa/qa-automation": "^9.3",
         "ec-europa/toolkit-composer-plugin": "^0.0.1",

phpcs.xml

@@ -20,8 +20,12 @@
 
     <!-- Exclude patterns. -->
     <exclude-pattern>vendor/</exclude-pattern>
-    <exclude-pattern>tests/fixtures</exclude-pattern>
+    <exclude-pattern>tests/fixtures/</exclude-pattern>
+    <exclude-pattern>tests/sandbox/</exclude-pattern>
 
+    <rule ref="MySource.Debug.DebugCode"/>
+
+    <!-- Generic. -->
     <rule ref="Generic">
         <exclude name="Generic.Arrays.ArrayIndent.CloseBraceNotNewLine"/>
         <exclude name="Generic.Arrays.DisallowShortArraySyntax.Found"/>
@@ -44,7 +48,6 @@
         <exclude name="Generic.PHP.UpperCaseConstant.Found"/>
         <exclude name="Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed"/>
         <exclude name="Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsedHeredocCloser"/>
-        <!-- Disable metrics -->
         <exclude name="Generic.Metrics"/>
     </rule>
 
@@ -88,16 +91,14 @@
     <rule ref="Generic.PHP.UpperCaseConstant"/>
     <rule ref="Generic.WhiteSpace.DisallowTabIndent"/>
 
-    <!-- Use Unix newlines -->
     <rule ref="Generic.Files.LineEndings">
         <properties>
             <property name="eolChar" value="\n"/>
         </properties>
     </rule>
 
-    <rule ref="MySource.Debug.DebugCode"/>
+    <!-- PEAR. -->
     <rule ref="PEAR.Files.IncludingFile"/>
-    <!-- Disable some error messages that we do not want. -->
     <rule ref="PEAR.Files.IncludingFile.UseIncludeOnce">
         <severity>0</severity>
     </rule>
@@ -112,14 +113,12 @@
     </rule>
 
     <rule ref="PEAR.Functions.FunctionCallSignature"/>
-    <!-- Disable some error messages that we already cover. -->
     <rule ref="PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket">
         <severity>0</severity>
     </rule>
     <rule ref="PEAR.Functions.FunctionCallSignature.SpaceBeforeCloseBracket">
         <severity>0</severity>
     </rule>
-    <!-- Disable some error messages that we do not want. -->
     <rule ref="PEAR.Functions.FunctionCallSignature.Indent">
         <severity>0</severity>
     </rule>
@@ -135,17 +134,17 @@
     <rule ref="PEAR.Functions.FunctionCallSignature.OpeningIndent">
         <severity>0</severity>
     </rule>
-
     <rule ref="PEAR.Functions.ValidDefaultValue"/>
 
+    <!-- PSR2. -->
     <rule ref="PSR2.Namespaces.NamespaceDeclaration"/>
     <rule ref="PSR2.Namespaces.UseDeclaration"/>
     <rule ref="PSR2.Classes.ClassDeclaration.CloseBraceAfterBody">
         <severity>0</severity>
     </rule>
 
+    <!-- Squiz. -->
     <rule ref="Squiz.Arrays.ArrayDeclaration"/>
-    <!-- Disable some error messages that we do not want. -->
     <rule ref="Squiz.Arrays.ArrayDeclaration.CloseBraceNotAligned">
         <severity>0</severity>
     </rule>
@@ -179,14 +178,16 @@
     <rule ref="Squiz.Arrays.ArrayDeclaration.ValueNoNewline">
         <severity>0</severity>
     </rule>
-
     <rule ref="Squiz.Arrays.ArrayBracketSpacing"/>
 
     <rule ref="Squiz.Commenting.ClassComment.Missing" />
     <rule ref="Squiz.Commenting.FunctionComment.Missing" />
+    <rule ref="Squiz.Commenting.FunctionComment.ParamCommentNotCapital"/>
+    <rule ref="Squiz.Commenting.FunctionComment.ParamCommentFullStop"/>
+    <rule ref="Squiz.Commenting.FunctionComment.SpacingAfterParamName"/>
+    <rule ref="Squiz.Commenting.FunctionComment.ThrowsNoFullStop"/>
 
     <rule ref="Squiz.ControlStructures.ForEachLoopDeclaration"/>
-    <!-- Disable some error messages that we already cover. -->
     <rule ref="Squiz.ControlStructures.ForEachLoopDeclaration.AsNotLower">
         <severity>0</severity>
     </rule>
@@ -197,16 +198,13 @@
         <severity>0</severity>
     </rule>
     <rule ref="Squiz.ControlStructures.ForLoopDeclaration"/>
-    <!-- Disable some error messages that we already cover. -->
     <rule ref="Squiz.ControlStructures.ForLoopDeclaration.SpacingAfterOpen">
         <severity>0</severity>
     </rule>
     <rule ref="Squiz.ControlStructures.ForLoopDeclaration.SpacingBeforeClose">
         <severity>0</severity>
     </rule>
-
     <rule ref="Squiz.ControlStructures.SwitchDeclaration"/>
-    <!-- Disable some error messages that we do not want. -->
     <rule ref="Squiz.ControlStructures.SwitchDeclaration.BreakIndent">
         <severity>0</severity>
     </rule>
@@ -269,7 +267,6 @@
     <rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing.NoSpaceBeforeArg">
         <severity>0</severity>
     </rule>
-
     <rule ref="Squiz.Functions.MultiLineFunctionDeclaration"/>
     <rule ref="Squiz.Functions.MultiLineFunctionDeclaration.ContentAfterBrace">
         <severity>0</severity>
@@ -311,10 +308,48 @@
     <rule ref="Squiz.WhiteSpace.SemicolonSpacing"/>
     <rule ref="Squiz.WhiteSpace.SuperfluousWhitespace"/>
 
-    <rule ref="Drupal.Classes.UnusedUseStatement"/>
+    <!-- Drupal. -->
     <rule ref="Drupal.Classes.FullyQualifiedNamespace"/>
+    <rule ref="Drupal.Classes.UnusedUseStatement"/>
     <rule ref="Drupal.Classes.UseLeadingBackslash"/>
+    <rule ref="Drupal.Commenting.DataTypeNamespace"/>
+    <rule ref="Drupal.Commenting.DocComment"/>
+    <rule ref="Drupal.Commenting.DocCommentStar"/>
+    <rule ref="Drupal.Commenting.InlineComment"/>
+    <rule ref="Drupal.Commenting.FunctionComment.ParamCommentIndentation"/>
+    <rule ref="Drupal.Commenting.FunctionComment.ParamCommentFullStop"/>
+    <rule ref="Drupal.Commenting.FunctionComment.MissingReturnComment"/>
+    <rule ref="Drupal.Commenting.FunctionComment.MissingParamComment"/>
+    <rule ref="Drupal.Commenting.FunctionComment.ParamCommentNewLine"/>
     <rule ref="Drupal.Files.EndFileNewline"/>
-    <rule ref="Drupal.Commenting.DataTypeNamespace.DataTypeNamespace"/>
+    <rule ref="Drupal.Arrays.Array.CommaLastItem"/>
+    <rule ref="Drupal.Formatting"/>
+    <rule ref="Drupal.NamingConventions"/>
+    <rule ref="Drupal.Scope"/>
+    <rule ref="Drupal.Semantics"/>
+    <rule ref="Drupal.Strings"/>
+
+    <rule ref="DrupalPractice.CodeAnalysis"/>
+    <rule ref="DrupalPractice.Commenting"/>
+    <rule ref="DrupalPractice.Constants"/>
+    <rule ref="DrupalPractice.FunctionCalls"/>
+    <rule ref="DrupalPractice.FunctionDefinitions"/>
+    <rule ref="DrupalPractice.General"/>
+    <rule ref="DrupalPractice.InfoFiles"/>
+    <rule ref="DrupalPractice.Objects">
+        <exclude name="DrupalPractice.Objects.UnusedPrivateMethod"/>
+    </rule>
 
+    <rule ref="Generic.PHP.ForbiddenFunctions">
+        <properties>
+            <property name="forbiddenFunctions" type="array">
+                <element key="print" value="echo"/>
+                <element key="var_dump" value="null"/>
+                <element key="dd" value="null"/>
+                <element key="dump" value="null"/>
+                <element key="print_r" value="null"/>
+                <element key="var_export" value="null"/>
+            </property>
+        </properties>
+    </rule>
 </ruleset>

src/Task/Command/ConfigurationCommand.php

@@ -55,7 +55,6 @@ class ConfigurationCommand extends BaseTask implements BuilderAwareInterface
         'process' => ['required' => ['source'], 'defaults' => 'destination'],
         'append' => ['required' => ['file', 'text']],
         'run' => ['required' => 'command'],
-//        'process-php' => ['required' => ['source', 'destination'], 'defaults' => 'override'],
         'exec' => ['required' => 'command'],
         'drush' => ['required' => 'command'],
         'replace-block' => [

src/Task/File/Process.php

@@ -50,7 +50,9 @@ class Process extends BaseTask implements BuilderAwareInterface
      * Constructs a new Process task.
      *
      * @param string $source
+     *   The source file.
      * @param string $destination
+     *   The destination file, if empty assumes the $source as destination.
      */
     public function __construct(string $source, string $destination = '')
     {

src/Task/File/Tasks.php

@@ -19,8 +19,6 @@ trait Tasks
      *   The source file to process.
      * @param string $destination
      *   The destination file.
-     *
-     * @return \EcEuropa\Toolkit\Task\File\Process|\Robo\Collection\CollectionBuilder
      */
     protected function taskProcess(string $source, string $destination = '')
     {
@@ -32,8 +30,6 @@ protected function taskProcess(string $source, string $destination = '')
      *
      * @param string $filename
      *   The file to process.
-     *
-     * @return \EcEuropa\Toolkit\Task\File\ReplaceBlock|\Robo\Collection\CollectionBuilder
      */
     protected function taskReplaceBlock(string $filename = '')
     {

src/TaskRunner/AbstractCommands.php

@@ -24,8 +24,6 @@ abstract class AbstractCommands extends Tasks implements ConfigAwareInterface
      * Path to YAML configuration file containing command defaults.
      *
      * Command classes should implement this method.
-     *
-     * @return string
      */
     public function getConfigurationFile()
     {

src/TaskRunner/Commands/AxeCommands.php

@@ -22,7 +22,7 @@ class AxeCommands extends AbstractCommands
      * @var array|string[]
      */
     private array $dependencies = [
-        // cspell:disable
+        // cspell:disable .
         'libnss3-tools',
         'libatk1.0-0',
         'libatk-bridge2.0-0',
@@ -42,7 +42,7 @@ class AxeCommands extends AbstractCommands
         'libx11-xcb1',
         'libxcursor1',
         'xdg-utils',
-        // cspell:enable
+        // cspell:enable .
     ];
 
     /**
@@ -152,7 +152,7 @@ public function toolkitSetupAxeScan()
                 'node_modules/axe-scan/build/src/commands/summary.js',
             ];
             $from = 'const browser = await puppeteer.launch();';
-            // cspell:ignore setuid
+            // cspell:ignore setuid .
             $args = '["--no-sandbox", "--disable-setuid-sandbox", "--single-process", "--disable-impl-side-painting", "--disable-gpu-sandbox", "--disable-accelerated-2d-canvas", "--disable-accelerated-jpeg-decoding", "--disable-dev-shm-usage"]';
             $to = 'const browser = await puppeteer.launch({args: ' . $args . '});';
             foreach ($files as $file) {

src/TaskRunner/Commands/BlackfireCommands.php

@@ -51,7 +51,7 @@ public function toolkitRunBlackfire(ConsoleIO $io, array $options = [
         // Confirm that blackfire is properly installed.
         $test = $this->taskExec('which blackfire')->silent(true)
             ->run()->getMessage();
-        if (strpos($test, 'not found') !== false) {
+        if (str_contains($test, 'not found')) {
             $problems[] = 'The Blackfire is not installed, skipping.';
         }
 
@@ -152,7 +152,7 @@ public function toolkitRunBlackfire(ConsoleIO $io, array $options = [
                     'field_blackfire_ci_cd_url' => [['value' => $ciUrl]],
                     'field_blackfire_graph_url' => [['value' => $data['graph']]],
                     'field_blackfire_timeline_url' => [['value' => $data['timeline']]],
-                    // cspell:ignore recomendations
+                    // cspell:ignore recomendations .
                     'field_blackfire_recomendations' => [['value' => $data['recommendation']]],
                     'field_blackfire_memory' => [['value' => $data['memory']]],
                     'field_blackfire_wall_time' => [['value' => $data['wall_time']]],

src/TaskRunner/Commands/ComponentCheckCommands.php

@@ -17,7 +17,7 @@
 use Symfony\Component\Yaml\Yaml;
 
 /**
- * Command class for toolkit:component-check
+ * Command class for toolkit:component-check.
  *
  * @SuppressWarnings(PHPMD.ExcessiveClassComplexity)
  * @SuppressWarnings(PHPMD.ExcessiveClassLength)
@@ -662,7 +662,7 @@ public function componentComposer(ConsoleIO $io)
             }
         }
 
-        // Make sure not installed plugins are not present in composer.json
+        // Make sure not installed plugins are not present in composer.json.
         $installedPackages = $this->getJson('vendor/composer/installed.json', false);
         if (!empty($composerJson['config']['allow-plugins']) && !empty($installedPackages['packages'])) {
             $composerPlugins = array_filter(
@@ -913,7 +913,8 @@ protected function printComponentResults(ConsoleIO $io)
     /**
      * Helper function to validate the component.
      *
-     * @param array $package The package to validate.
+     * @param array $package
+     *   The package to validate.
      *
      * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      * @SuppressWarnings(PHPMD.NPathComplexity)