From 04a4563b82c419e43cc58393a78b21c44fcc29e2 Mon Sep 17 00:00:00 2001 From: Klaus Purer <klaus.purer@protonmail.ch> Date: Mon, 6 Jan 2025 10:46:24 +0100 Subject: [PATCH] feat(ValidClassName): Check traits and enums for valid upperCamel names (#3497580) --- .github/workflows/testing.yml | 2 +- .../Sniffs/Commenting/ClassCommentSniff.php | 17 ++++-- .../NamingConventions/ValidClassNameSniff.php | 15 ++++- .../ValidClassNameUnitTest.inc | 25 ++++++++ .../ValidClassNameUnitTest.php | 58 +++++++++++++++++++ .../ValidEnumCaseUnitTest.inc | 5 ++ .../ValidEnumCaseUnitTest.php | 3 +- tests/Drupal/bad/BadUnitTest.php | 2 +- tests/Drupal/good/good.php | 9 +++ 9 files changed, 126 insertions(+), 10 deletions(-) create mode 100644 tests/Drupal/NamingConventions/ValidClassNameUnitTest.inc create mode 100644 tests/Drupal/NamingConventions/ValidClassNameUnitTest.php diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 4b9df659..b7d917d0 100644 --- a/.github/workflows/testing.yml +++ b/.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 diff --git a/coder_sniffer/Drupal/Sniffs/Commenting/ClassCommentSniff.php b/coder_sniffer/Drupal/Sniffs/Commenting/ClassCommentSniff.php index a0c6bb6c..75de9c9b 100644 --- a/coder_sniffer/Drupal/Sniffs/Commenting/ClassCommentSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Commenting/ClassCommentSniff.php @@ -53,16 +53,23 @@ class ClassCommentSniff implements Sniff */ 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 @@ class ClassCommentSniff implements Sniff } break; - } + }//end for if ($tokens[$commentEnd]['code'] !== T_DOC_COMMENT_CLOSE_TAG && $tokens[$commentEnd]['code'] !== T_COMMENT diff --git a/coder_sniffer/Drupal/Sniffs/NamingConventions/ValidClassNameSniff.php b/coder_sniffer/Drupal/Sniffs/NamingConventions/ValidClassNameSniff.php index 11ae5f89..a7df880c 100644 --- a/coder_sniffer/Drupal/Sniffs/NamingConventions/ValidClassNameSniff.php +++ b/coder_sniffer/Drupal/Sniffs/NamingConventions/ValidClassNameSniff.php @@ -15,7 +15,7 @@ use PHP_CodeSniffer\Sniffs\Sniff; /** * \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 @@ class ValidClassNameSniff implements Sniff { return [ T_CLASS, + T_ENUM, T_INTERFACE, + T_TRAIT, ]; }//end register() @@ -60,7 +62,7 @@ class ValidClassNameSniff implements Sniff // 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 @@ class ValidClassNameSniff implements Sniff $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() diff --git a/tests/Drupal/NamingConventions/ValidClassNameUnitTest.inc b/tests/Drupal/NamingConventions/ValidClassNameUnitTest.inc new file mode 100644 index 00000000..dfcf852e --- /dev/null +++ b/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 {} diff --git a/tests/Drupal/NamingConventions/ValidClassNameUnitTest.php b/tests/Drupal/NamingConventions/ValidClassNameUnitTest.php new file mode 100644 index 00000000..37803750 --- /dev/null +++ b/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 diff --git a/tests/Drupal/NamingConventions/ValidEnumCaseUnitTest.inc b/tests/Drupal/NamingConventions/ValidEnumCaseUnitTest.inc index d1819d99..b6b42807 100644 --- a/tests/Drupal/NamingConventions/ValidEnumCaseUnitTest.inc +++ b/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; } diff --git a/tests/Drupal/NamingConventions/ValidEnumCaseUnitTest.php b/tests/Drupal/NamingConventions/ValidEnumCaseUnitTest.php index eea3216c..28a5135b 100644 --- a/tests/Drupal/NamingConventions/ValidEnumCaseUnitTest.php +++ b/tests/Drupal/NamingConventions/ValidEnumCaseUnitTest.php @@ -22,7 +22,8 @@ class ValidEnumCaseUnitTest extends CoderSniffUnitTest { return [ 5 => 1, - 7 => 1, + 7 => 2, + 9 => 1, ]; }//end getErrorList() diff --git a/tests/Drupal/bad/BadUnitTest.php b/tests/Drupal/bad/BadUnitTest.php index 15c4f045..e9a855ce 100644 --- a/tests/Drupal/bad/BadUnitTest.php +++ b/tests/Drupal/bad/BadUnitTest.php @@ -380,7 +380,7 @@ class BadUnitTest extends CoderSniffUnitTest 827 => 1, 829 => 1, 836 => 1, - 838 => 1, + 838 => 3, 849 => 2, 860 => 2, 867 => 1, diff --git a/tests/Drupal/good/good.php b/tests/Drupal/good/good.php index b3b828b7..bd8d911b 100644 --- a/tests/Drupal/good/good.php +++ b/tests/Drupal/good/good.php @@ -1905,3 +1905,12 @@ class CronHook { ) {} } + +/** + * 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; +} -- GitLab