feat(ValidClassName): Check traits and enums for valid upperCamel names (#3497580)

Author: klausi

.github/workflows/testing.yml

@@ -84,4 +84,4 @@ jobs:
         # core is updated to that version.
         run: |
           cd drupal/core
-          ../../vendor/bin/phpcs -p -s --ignore=lib/Drupal/Core/Entity/EntityType.php,lib/Drupal/Core/Recipe/RecipeInputFormTrait.php,lib/Drupal/Core/Form/FormState.php,modules/migrate/src/Plugin/Migration.php,modules/views/src/ViewExecutable.php,modules/views/src/Plugin/views/style/StylePluginBase.php
+          ../../vendor/bin/phpcs -p -s --ignore=lib/Drupal/Core/Entity/EntityType.php,lib/Drupal/Core/Recipe/RecipeInputFormTrait.php,lib/Drupal/Core/Form/FormState.php,modules/migrate/src/Plugin/Migration.php,modules/views/src/ViewExecutable.php,modules/views/src/Plugin/views/style/StylePluginBase.php,core/lib/Drupal/Core/FileTransfer/FTP.php,core/lib/Drupal/Core/FileTransfer/SSH.php

coder_sniffer/Drupal/Sniffs/Commenting/ClassCommentSniff.php

@@ -53,16 +53,23 @@ public function register()
      */
     public function process(File $phpcsFile, $stackPtr)
     {
-        $tokens = $phpcsFile->getTokens();
-        $find   = Tokens::$methodPrefixes;
-        $find[T_WHITESPACE] = T_WHITESPACE;
-        $find[T_READONLY]   = T_READONLY;
+        $tokens         = $phpcsFile->getTokens();
+        $find           = ([
+            T_ABSTRACT   => T_ABSTRACT,
+            T_FINAL      => T_FINAL,
+            T_READONLY   => T_READONLY,
+            T_WHITESPACE => T_WHITESPACE,
+        ] + Tokens::$phpcsCommentTokens);
         $name           = $tokens[$stackPtr]['content'];
         $classCodeStart = $stackPtr;
 
         $previousContent = null;
         for ($commentEnd = ($stackPtr - 1); $commentEnd >= 0; $commentEnd--) {
             if (isset($find[$tokens[$commentEnd]['code']]) === true) {
+                if (isset(Tokens::$phpcsCommentTokens[$tokens[$commentEnd]['code']]) === true) {
+                    $classCodeStart = $commentEnd;
+                }
+
                 continue;
             }
 
@@ -78,7 +85,7 @@ public function process(File $phpcsFile, $stackPtr)
             }
 
             break;
-        }
+        }//end for
 
         if ($tokens[$commentEnd]['code'] !== T_DOC_COMMENT_CLOSE_TAG
             && $tokens[$commentEnd]['code'] !== T_COMMENT

coder_sniffer/Drupal/Sniffs/NamingConventions/ValidClassNameSniff.php

@@ -15,7 +15,7 @@
 /**
  * \Drupal\Sniffs\NamingConventions\ValidClassNameSniff.
  *
- * Ensures class and interface names start with a capital letter
+ * Ensures class, enum, interface and trait names start with a capital letter
  * and do not use _ separators.
  *
  * @category PHP
@@ -35,7 +35,9 @@ public function register()
     {
         return [
             T_CLASS,
+            T_ENUM,
             T_INTERFACE,
+            T_TRAIT,
         ];
 
     }//end register()
@@ -60,7 +62,7 @@ public function process(File $phpcsFile, $stackPtr)
 
         // Make sure the first letter is a capital.
         if (preg_match('|^[A-Z]|', $name) === 0) {
-            $error = '%s name must begin with a capital letter';
+            $error = '%s name must use UpperCamel naming and begin with a capital letter';
             $phpcsFile->addError($error, $stackPtr, 'StartWithCapital', $errorData);
         }
 
@@ -70,6 +72,15 @@ public function process(File $phpcsFile, $stackPtr)
             $phpcsFile->addError($error, $stackPtr, 'NoUnderscores', $errorData);
         }
 
+        // Ensure the name is not all uppercase.
+        // @todo We could make this more strict to check if there are more than
+        // 2 upper case characters in a row, but not decided yet.
+        // See https://www.drupal.org/project/coder/issues/3497433
+        if (strtoupper($name) === $name) {
+            $error = '%s name must use UpperCamel naming and not contain multiple upper case letters in a row';
+            $phpcsFile->addError($error, $stackPtr, 'NoUpperAcronyms', $errorData);
+        }
+
     }//end process()
 
 

tests/Drupal/NamingConventions/ValidClassNameUnitTest.inc

@@ -0,0 +1,25 @@
+<?php
+
+class CorrectClassName {}
+class CorrectClassWithAReallyLongName {}
+class INCORRECT_CLASS_NAME {}
+class INCORRECTCLASSNAME {}
+class incorrectLowercaseClassName {}
+
+interface CorrectInterfaceName {}
+interface CorrectInterfaceWithAReallyLongName {}
+interface INCORRECT_INTERFACE_NAME {}
+interface INCORRECTINTERFACENAME {}
+interface incorrectLowercaseInterfaceName {}
+
+trait CorrectTraitName {}
+trait CorrectTraitWithAReallyLongName {}
+trait INCORRECT_TRAIT_NAME {}
+trait INCORRECTTRAITNAME {}
+trait incorrectLowercaseTraitName {}
+
+enum CorrectEnumName {}
+enum CorrectEnumWithAReallyLongName {}
+enum INCORRECT_ENUM_NAME {}
+enum INCORRECTENUMNAME {}
+enum incorrectLowercaseEnumName {}

tests/Drupal/NamingConventions/ValidClassNameUnitTest.php

@@ -0,0 +1,58 @@
+<?php
+
+namespace Drupal\Test\NamingConventions;
+
+use Drupal\Test\CoderSniffUnitTest;
+
+class ValidClassNameUnitTest extends CoderSniffUnitTest
+{
+
+
+    /**
+     * Returns the lines where errors should occur.
+     *
+     * The key of the array should represent the line number and the value
+     * should represent the number of errors that should occur on that line.
+     *
+     * @param string $testFile The name of the file being tested.
+     *
+     * @return array<int, int>
+     */
+    protected function getErrorList(string $testFile): array
+    {
+        return [
+            5  => 2,
+            6  => 1,
+            7  => 1,
+            11 => 2,
+            12 => 1,
+            13 => 1,
+            17 => 2,
+            18 => 1,
+            19 => 1,
+            23 => 2,
+            24 => 1,
+            25 => 1,
+        ];
+
+    }//end getErrorList()
+
+
+    /**
+     * Returns the lines where warnings should occur.
+     *
+     * The key of the array should represent the line number and the value
+     * should represent the number of warnings that should occur on that line.
+     *
+     * @param string $testFile The name of the file being tested.
+     *
+     * @return array<int, int>
+     */
+    protected function getWarningList(string $testFile): array
+    {
+        return [];
+
+    }//end getWarningList()
+
+
+}//end class

tests/Drupal/NamingConventions/ValidEnumCaseUnitTest.inc

@@ -5,4 +5,9 @@ enum Test: int {
   case one = 1;
   // Must not contain underscores.
   case TWO_TEST = 2;
+  // Must not contain only upper case.
+  case THREE = 3;
+  // Upper case parts are allowed for now.
+  case FourJSONCase = 4;
+  case FiveAndAHorseCorrect = 5;
 }

tests/Drupal/NamingConventions/ValidEnumCaseUnitTest.php

@@ -22,7 +22,8 @@ protected function getErrorList(string $testFile): array
     {
         return [
             5 => 1,
-            7 => 1,
+            7 => 2,
+            9 => 1,
         ];
 
     }//end getErrorList()

tests/Drupal/bad/BadUnitTest.php

@@ -380,7 +380,7 @@ protected function getErrorList(string $testFile): array
                 827 => 1,
                 829 => 1,
                 836 => 1,
-                838 => 1,
+                838 => 3,
                 849 => 2,
                 860 => 2,
                 867 => 1,

tests/Drupal/good/good.php

@@ -1905,3 +1905,12 @@ public function __construct(
   ) {}
 
 }
+
+/**
+ * Doc block is here and an ignore directive is ok.
+ */
+// phpcs:ignore Drupal.NamingConventions.ValidClassName
+enum PUROSELY_WRONG_BUT_OK: int {
+  case One = 1;
+  case Two = 2;
+}