From a0586047b71a5440dca0fb3cd28cff17324cad7f Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Mon, 8 Aug 2022 10:39:04 +0100
Subject: [PATCH] Issue #3135933 by Spokje, jungle, ravi.shankar, quietone,
 jonathan1055, longwave, phenaproxima: Sort sniffs/rules in phpcs.xml.dist and
 write test to keep them sorted

(cherry picked from commit 6ef6ffcf468f07a9882265b11313ecfb8d8047cb)
---
 core/phpcs.xml.dist                        | 133 ++++++++++-----------
 core/tests/Drupal/Tests/PhpCs/SortTest.php |  99 +++++++++++++++
 2 files changed, 165 insertions(+), 67 deletions(-)
 create mode 100644 core/tests/Drupal/Tests/PhpCs/SortTest.php

diff --git a/core/phpcs.xml.dist b/core/phpcs.xml.dist
index b8d15f939f6..c8a0dbdf873 100644
--- a/core/phpcs.xml.dist
+++ b/core/phpcs.xml.dist
@@ -1,6 +1,21 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <ruleset name="drupal_core">
+  <arg name="extensions" value="inc,install,module,php,profile,test,theme,yml"/>
   <description>Default PHP CodeSniffer configuration for Drupal core.</description>
+
+  <!--Exclude folders used by common frontend tools. These folders match the file_scan_ignore_directories setting in default.settings.php-->
+  <exclude-pattern>*/bower_components/*</exclude-pattern>
+  <exclude-pattern>*/node_modules/*</exclude-pattern>
+  <!--Exclude third party code.-->
+  <exclude-pattern>./assets/vendor/*</exclude-pattern>
+  <!-- Exclude third-party code maintained within core that does not follow our standards. -->
+  <!-- @todo This rule may be removed when https://www.drupal.org/node/1848264 is resolved. -->
+  <exclude-pattern>./core/lib/Drupal/Component/Diff/</exclude-pattern>
+  <exclude-pattern>./core/tests/Drupal/Tests/Component/Annotation/Doctrine/</exclude-pattern>
+
+  <!--Exclude test files that are intentionally empty, or intentionally violate coding standards.-->
+  <exclude-pattern>./modules/system/tests/fixtures/HtaccessTest</exclude-pattern>
+
   <file>.</file>
   <file>../composer</file>
   <file>scripts/drupal.sh</file>
@@ -8,24 +23,9 @@
   <file>scripts/rebuild_token_calculator.sh</file>
   <file>scripts/run-tests.sh</file>
   <file>scripts/update-countries.sh</file>
-  <arg name="extensions" value="inc,install,module,php,profile,test,theme,yml"/>
-
-  <!--Exclude third party code.-->
-  <exclude-pattern>./assets/vendor/*</exclude-pattern>
-  <exclude-pattern>./core/tests/Drupal/Tests/Component/Annotation/Doctrine/</exclude-pattern>
-
-  <!--Exclude folders used by common frontend tools. These folders match the file_scan_ignore_directories setting in default.settings.php-->
-  <exclude-pattern>*/node_modules/*</exclude-pattern>
-  <exclude-pattern>*/bower_components/*</exclude-pattern>
-
-  <!--Exclude test files that are intentionally empty, or intentionally violate coding standards.-->
-  <exclude-pattern>./modules/system/tests/fixtures/HtaccessTest</exclude-pattern>
-
-  <!-- Exclude third-party code maintained within core that does not follow our standards. -->
-  <!-- @todo This rule may be removed when https://www.drupal.org/node/1848264 is resolved. -->
-  <exclude-pattern>./core/lib/Drupal/Component/Diff/</exclude-pattern>
 
   <!-- Only include specific sniffs that pass. This ensures that, if new sniffs are added, HEAD does not fail.-->
+
   <!-- Drupal sniffs -->
   <rule ref="Drupal.Arrays.Array">
     <!-- Sniff for these errors: CommaLastItem -->
@@ -33,6 +33,8 @@
     <exclude name="Drupal.Arrays.Array.ArrayIndentation"/>
     <exclude name="Drupal.Arrays.Array.LongLineDeclaration"/>
   </rule>
+  <rule ref="Drupal.CSS.ClassDefinitionNameSpacing"/>
+  <rule ref="Drupal.CSS.ColourDefinition"/>
   <rule ref="Drupal.Classes.ClassCreateInstance"/>
   <rule ref="Drupal.Classes.ClassDeclaration"/>
   <rule ref="Drupal.Classes.ClassFileName"/>
@@ -42,8 +44,6 @@
   <rule ref="Drupal.Classes.UnusedUseStatement"/>
   <rule ref="Drupal.Classes.UseGlobalClass"/>
   <rule ref="Drupal.Classes.UseLeadingBackslash"/>
-  <rule ref="Drupal.CSS.ClassDefinitionNameSpacing"/>
-  <rule ref="Drupal.CSS.ColourDefinition"/>
   <rule ref="Drupal.Commenting.ClassComment">
     <exclude name="Drupal.Commenting.ClassComment.Missing"/>
   </rule>
@@ -56,12 +56,12 @@
       TagsNotGrouped, ParamGroup -->
     <!-- ParamNotFirst still not decided for PHPUnit-based tests.
       @see https://www.drupal.org/node/2253915 -->
-    <exclude name="Drupal.Commenting.DocComment.ParamNotFirst"/>
-    <exclude name="Drupal.Commenting.DocComment.SpacingBeforeTags"/>
     <exclude name="Drupal.Commenting.DocComment.LongFullStop"/>
+    <exclude name="Drupal.Commenting.DocComment.MissingShort"/>
+    <exclude name="Drupal.Commenting.DocComment.ParamNotFirst"/>
     <exclude name="Drupal.Commenting.DocComment.ShortNotCapital"/>
     <exclude name="Drupal.Commenting.DocComment.ShortSingleLine"/>
-    <exclude name="Drupal.Commenting.DocComment.MissingShort"/>
+    <exclude name="Drupal.Commenting.DocComment.SpacingBeforeTags"/>
   </rule>
   <rule ref="Drupal.Commenting.DocCommentAlignment"/>
   <rule ref="Drupal.Commenting.DocCommentStar"/>
@@ -75,15 +75,8 @@
     <exclude name="Drupal.Commenting.FunctionComment.ParamCommentFullStop"/>
     <exclude name="Drupal.Commenting.FunctionComment.TypeHintMissing"/>
   </rule>
-  <rule ref="Drupal.Commenting.HookComment"/>
   <rule ref="Drupal.Commenting.GenderNeutralComment"/>
-  <rule ref="Drupal.Commenting.InlineVariableComment"/>
-  <rule ref="Drupal.Commenting.VariableComment">
-    <!-- Sniff for: DuplicateVar, EmptyVar, IncorrectVarType, InlineVariableName, WrongStyle -->
-    <exclude name="Drupal.Commenting.VariableComment.Missing"/>
-    <exclude name="Drupal.Commenting.VariableComment.MissingVar"/>
-    <exclude name="Drupal.Commenting.VariableComment.VarOrder"/>
-  </rule>
+  <rule ref="Drupal.Commenting.HookComment"/>
   <rule ref="Drupal.Commenting.InlineComment">
     <!-- Sniff for: NoSpaceBefore, SpacingBefore, WrongStyle -->
     <exclude name="Drupal.Commenting.InlineComment.DocBlock"/>
@@ -91,9 +84,17 @@
     <exclude name="Drupal.Commenting.InlineComment.NotCapital"/>
     <exclude name="Drupal.Commenting.InlineComment.SpacingAfter"/>
   </rule>
+  <rule ref="Drupal.Commenting.InlineVariableComment"/>
   <rule ref="Drupal.Commenting.PostStatementComment"/>
-  <rule ref="Drupal.ControlStructures.ElseIf"/>
+  <rule ref="Drupal.Commenting.VariableComment">
+    <!-- Sniff for: DuplicateVar, EmptyVar, InlineVariableName, WrongStyle -->
+    <exclude name="Drupal.Commenting.VariableComment.IncorrectVarType"/>
+    <exclude name="Drupal.Commenting.VariableComment.Missing"/>
+    <exclude name="Drupal.Commenting.VariableComment.MissingVar"/>
+    <exclude name="Drupal.Commenting.VariableComment.VarOrder"/>
+  </rule>
   <rule ref="Drupal.ControlStructures.ControlSignature"/>
+  <rule ref="Drupal.ControlStructures.ElseIf"/>
   <rule ref="Drupal.ControlStructures.InlineControlStructure"/>
   <rule ref="Drupal.Files.EndFileNewline"/>
   <rule ref="Drupal.Files.FileEncoding"/>
@@ -113,6 +114,8 @@
       Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps. -->
     <exclude name="Drupal.Methods.MethodDeclaration.Underscore"/>
   </rule>
+  <rule ref="Drupal.NamingConventions.ValidClassName"/>
+  <rule ref="Drupal.NamingConventions.ValidGlobal"/>
   <rule ref="Drupal.NamingConventions.ValidVariableName">
     <!-- Sniff for: LowerStart -->
     <exclude name="Drupal.NamingConventions.ValidVariableName.LowerCamelName"/>
@@ -143,9 +146,6 @@
   <rule ref="Drupal.WhiteSpace.ScopeIndent"/>
 
   <!-- Drupal Practice sniffs -->
-  <rule ref="DrupalPractice.Commenting.ExpectedException"/>
-  <rule ref="DrupalPractice.General.ExceptionT"/>
-  <rule ref="DrupalPractice.InfoFiles.NamespacedDependency"/>
   <rule ref="DrupalPractice.CodeAnalysis.VariableAnalysis">
     <!-- @todo exclude tests -->
     <exclude-pattern>*/tests/*</exclude-pattern>
@@ -156,14 +156,17 @@
       <property name="allowUnusedFunctionParameters" value="true"/>
     </properties>
   </rule>
+  <rule ref="DrupalPractice.CodeAnalysis.VariableAnalysis.UndefinedUnsetVariable">
+    <severity>0</severity>
+  </rule>
   <rule ref="DrupalPractice.CodeAnalysis.VariableAnalysis.UndefinedVariable">
     <!-- Setting severity to 0 to completely disable an error message in this sniff, without excluding the whole sniff -->
     <!-- See https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options#changing-the-default-severity-levels -->
     <severity>0</severity>
   </rule>
-  <rule ref="DrupalPractice.CodeAnalysis.VariableAnalysis.UndefinedUnsetVariable">
-    <severity>0</severity>
-  </rule>
+  <rule ref="DrupalPractice.Commenting.ExpectedException"/>
+  <rule ref="DrupalPractice.General.ExceptionT"/>
+  <rule ref="DrupalPractice.InfoFiles.NamespacedDependency"/>
 
   <!-- Generic sniffs -->
   <rule ref="Generic.Arrays.DisallowLongArraySyntax"/>
@@ -178,8 +181,6 @@
       <property name="checkClosures" value="true"/>
     </properties>
   </rule>
-  <rule ref="Drupal.NamingConventions.ValidClassName"/>
-  <rule ref="Drupal.NamingConventions.ValidGlobal"/>
   <rule ref="Generic.NamingConventions.ConstructorName"/>
   <rule ref="Generic.NamingConventions.UpperCaseConstantName"/>
   <rule ref="Generic.PHP.DeprecatedFunctions"/>
@@ -200,30 +201,29 @@
   <!-- PEAR sniffs -->
   <rule ref="PEAR.Files.IncludingFile"/>
   <!-- Disable some error messages that we do not want. -->
-  <rule ref="PEAR.Files.IncludingFile.UseIncludeOnce">
-    <severity>0</severity>
-  </rule>
   <rule ref="PEAR.Files.IncludingFile.UseInclude">
     <severity>0</severity>
   </rule>
-  <rule ref="PEAR.Files.IncludingFile.UseRequireOnce">
+  <rule ref="PEAR.Files.IncludingFile.UseIncludeOnce">
     <severity>0</severity>
   </rule>
   <rule ref="PEAR.Files.IncludingFile.UseRequire">
     <severity>0</severity>
   </rule>
-  <rule ref="PEAR.Functions.ValidDefaultValue"/>
-
-  <!-- PEAR sniffs -->
+  <rule ref="PEAR.Files.IncludingFile.UseRequireOnce">
+    <severity>0</severity>
+  </rule>
   <rule ref="PEAR.Functions.FunctionCallSignature"/>
   <!-- The sniffs inside PEAR.Functions.FunctionCallSignature silenced below are
     also silenced in Drupal CS' ruleset.xml. The code below is a 1-on-1 copy
     from that file. -->
-  <!-- Disable some error messages that we already cover. -->
-  <rule ref="PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket">
+  <rule ref="PEAR.Functions.FunctionCallSignature.CloseBracketLine">
     <severity>0</severity>
   </rule>
-  <rule ref="PEAR.Functions.FunctionCallSignature.SpaceBeforeCloseBracket">
+  <rule ref="PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket">
+    <severity>0</severity>
+  </rule>
+  <rule ref="PEAR.Functions.FunctionCallSignature.EmptyLine">
     <severity>0</severity>
   </rule>
   <!-- Disable some error messages that we do not want. -->
@@ -233,15 +233,14 @@
   <rule ref="PEAR.Functions.FunctionCallSignature.OpeningIndent">
     <severity>0</severity>
   </rule>
-  <rule ref="PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket">
+  <!-- Disable some error messages that we already cover. -->
+  <rule ref="PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket">
     <severity>0</severity>
   </rule>
-  <rule ref="PEAR.Functions.FunctionCallSignature.CloseBracketLine">
-    <severity>0</severity>
-  </rule>
-  <rule ref="PEAR.Functions.FunctionCallSignature.EmptyLine">
+  <rule ref="PEAR.Functions.FunctionCallSignature.SpaceBeforeCloseBracket">
     <severity>0</severity>
   </rule>
+  <rule ref="PEAR.Functions.ValidDefaultValue"/>
 
   <!-- PSR-2 sniffs -->
   <rule ref="PSR2.Classes.PropertyDeclaration">
@@ -255,8 +254,8 @@
   <!-- Squiz sniffs -->
   <rule ref="Squiz.Arrays.ArrayBracketSpacing"/>
   <rule ref="Squiz.Arrays.ArrayDeclaration">
-    <exclude name="Squiz.Arrays.ArrayDeclaration.NoKeySpecified"/>
     <exclude name="Squiz.Arrays.ArrayDeclaration.KeySpecified"/>
+    <exclude name="Squiz.Arrays.ArrayDeclaration.NoKeySpecified"/>
   </rule>
   <!-- Disable some error messages that we do not want. -->
   <rule ref="Squiz.Arrays.ArrayDeclaration.CloseBraceNotAligned">
@@ -286,10 +285,10 @@
   <rule ref="Squiz.Arrays.ArrayDeclaration.SingleLineNotAllowed">
     <severity>0</severity>
   </rule>
-  <rule ref="Squiz.Arrays.ArrayDeclaration.ValueNotAligned">
+  <rule ref="Squiz.Arrays.ArrayDeclaration.ValueNoNewline">
     <severity>0</severity>
   </rule>
-  <rule ref="Squiz.Arrays.ArrayDeclaration.ValueNoNewline">
+  <rule ref="Squiz.Arrays.ArrayDeclaration.ValueNotAligned">
     <severity>0</severity>
   </rule>
   <rule ref="Squiz.ControlStructures.ForEachLoopDeclaration"/>
@@ -346,10 +345,21 @@
   <rule ref="Squiz.ControlStructures.SwitchDeclaration.SpacingBeforeBreak">
     <severity>0</severity>
   </rule>
+  <rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing">
+    <properties>
+      <property name="equalsSpacing" value="1"/>
+    </properties>
+  </rule>
+  <rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing.NoSpaceBeforeArg">
+    <severity>0</severity>
+  </rule>
   <rule ref="Squiz.Functions.MultiLineFunctionDeclaration"/>
   <rule ref="Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine">
     <severity>0</severity>
   </rule>
+  <rule ref="Squiz.Functions.MultiLineFunctionDeclaration.CloseBracketLine">
+    <severity>0</severity>
+  </rule>
   <rule ref="Squiz.Functions.MultiLineFunctionDeclaration.ContentAfterBrace">
     <severity>0</severity>
   </rule>
@@ -360,17 +370,6 @@
   <rule ref="Squiz.Functions.MultiLineFunctionDeclaration.Indent">
     <severity>0</severity>
   </rule>
-  <rule ref="Squiz.Functions.MultiLineFunctionDeclaration.CloseBracketLine">
-    <severity>0</severity>
-  </rule>
-  <rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing">
-    <properties>
-      <property name="equalsSpacing" value="1"/>
-    </properties>
-  </rule>
-  <rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing.NoSpaceBeforeArg">
-    <severity>0</severity>
-  </rule>
   <rule ref="Squiz.PHP.LowercasePHPFunctions"/>
   <rule ref="Squiz.PHP.NonExecutableCode"/>
   <rule ref="Squiz.Strings.ConcatenationSpacing">
diff --git a/core/tests/Drupal/Tests/PhpCs/SortTest.php b/core/tests/Drupal/Tests/PhpCs/SortTest.php
new file mode 100644
index 00000000000..768c9340a5d
--- /dev/null
+++ b/core/tests/Drupal/Tests/PhpCs/SortTest.php
@@ -0,0 +1,99 @@
+<?php
+
+namespace Drupal\Tests\PhpCs;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Serializer\Encoder\XmlEncoder;
+
+/**
+ * Tests that phpcs.xml.dist is properly sorted.
+ *
+ * @group phpcs
+ */
+class SortTest extends TestCase {
+
+  /**
+   * The path of phpcs.xml.dist file.
+   *
+   * @var string
+   */
+  private $filePath;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    $this->filePath = __DIR__ . '/../../../../../core/phpcs.xml.dist';
+  }
+
+  /**
+   * Tests that the phpcs.xml.dist file exists.
+   */
+  public function testFileExists() {
+    $this->assertFileExists($this->filePath);
+  }
+
+  /**
+   * Tests that the phpcs.xml.dist file is properly sorted.
+   */
+  public function testSorted() {
+    $content = file_get_contents($this->filePath);
+    $xml_encoder = new XmlEncoder();
+    $xml_encoded = $xml_encoder->decode($content, 'xml');
+    $this->assertIsArray($xml_encoded);
+
+    $top_level_keys = array_keys($xml_encoded);
+    $this->assertSorted($top_level_keys);
+
+    $this->assertArrayHasKey('file', $xml_encoded);
+    $files = $xml_encoded['file'];
+    $this->assertSorted($files);
+
+    $this->assertArrayHasKey('exclude-pattern', $xml_encoded);
+    $excluded_patterns = $xml_encoded['exclude-pattern'];
+    $this->assertSorted($excluded_patterns);
+
+    $this->assertArrayHasKey('rule', $xml_encoded);
+    $rules = $xml_encoded['rule'];
+    $this->assertSorted($rules, '@ref');
+
+    foreach ($rules as $item) {
+      if (array_key_exists('exclude', $item)) {
+        $excluded = $item['exclude'];
+        $excluded = array_filter($excluded, static function ($item) {
+          return is_array($item) && array_key_exists('@name', $item);
+        });
+        $this->assertSorted($excluded, '@name');
+      }
+    }
+  }
+
+  /**
+   * A helper method to assert that an input array is sorted.
+   *
+   * Compared by values, if the $column is not null, the column of the value is
+   * used for comparing.
+   *
+   * @param array $input
+   *   The input array.
+   * @param null|string $column
+   *   The column of the value or NULL.
+   */
+  private function assertSorted(array $input, string $column = NULL) {
+    $input_sorted = $input;
+
+    if ($column === NULL) {
+      usort($input_sorted, static function ($a, $b) {
+        return strcmp($a, $b);
+      });
+    }
+    else {
+      usort($input_sorted, static function ($a, $b) use ($column) {
+        return strcmp($a[$column], $b[$column]);
+      });
+    }
+
+    $this->assertEquals($input, $input_sorted);
+  }
+
+}