From 3d9d0b51c228ad7c89fdb6476d10925b37721ce8 Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Mon, 13 Jan 2025 10:55:38 +0300 Subject: [PATCH 01/16] test --- .../Drupal/Sniffs/Classes/TestSniff.php | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php diff --git a/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php b/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php new file mode 100644 index 00000000..a4d3213b --- /dev/null +++ b/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php @@ -0,0 +1,68 @@ + + */ + public function register() + { + return [T_ATTRIBUTE]; + + }//end register() + + + /** + * Processes this test, when one of its tokens is encountered. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The PHP_CodeSniffer file where the + * token was found. + * @param int $stackPtr The position in the PHP_CodeSniffer + * file's token stack where the token + * was found. + * + * @return void|int Optionally returns a stack pointer. The sniff will not be + * called again on the current file until the returned stack + * pointer is reached. Return $phpcsFile->numTokens + 1 to skip + * the rest of the file. + */ + public function process(File $phpcsFile, $stackPtr) + { + $tokens = $phpcsFile->getTokens(); + $end = $phpcsFile->findNext([T_ATTRIBUTE_END], ($stackPtr + 2)); + $shortContent = ''; + if () + for ($i = ($stackPtr + 1); $i < $end; $i++) { + $shortContent .= $tokens[$i]['content']; + } + if (preg_match('/^hook\(.hook_/i', $shortContent, $matches) === 1) { + var_dump($matches); + } + + }//end process() + + +}//end class From 49a47e31e457d9cdad99b86c7a722f9ceecee27d Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Mon, 13 Jan 2025 11:52:07 +0300 Subject: [PATCH 02/16] fix --- .../Drupal/Sniffs/Classes/TestSniff.php | 23 +++++++++++-------- tests/Drupal/bad/BadUnitTest.php | 1 + tests/Drupal/bad/bad.php | 8 +++++++ tests/Drupal/bad/bad.php.fixed | 8 +++++++ 4 files changed, 31 insertions(+), 9 deletions(-) diff --git a/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php b/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php index a4d3213b..f73ada77 100644 --- a/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php @@ -13,7 +13,7 @@ use PHP_CodeSniffer\Sniffs\Sniff; /** - * Checks that class references do not use FQN but use statements. + * Checks that Hook attribute argument name not starts with "hook_" prefix. * * @category PHP * @package PHP_CodeSniffer @@ -52,14 +52,19 @@ public function register() public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - $end = $phpcsFile->findNext([T_ATTRIBUTE_END], ($stackPtr + 2)); - $shortContent = ''; - if () - for ($i = ($stackPtr + 1); $i < $end; $i++) { - $shortContent .= $tokens[$i]['content']; - } - if (preg_match('/^hook\(.hook_/i', $shortContent, $matches) === 1) { - var_dump($matches); +// $shortContent = ''; +// $end = $phpcsFile->findNext([T_ATTRIBUTE_END], ($stackPtr + 1)); +// for ($i = ($stackPtr + 1); $i < $end; $i++) { +// $shortContent .= $tokens[$i]['content']; +// } +// $a = 1; + + if ($tokens[$stackPtr + 1]['type'] === 'T_STRING' + && $tokens[$stackPtr + 1]['content'] === 'Hook' + && $tokens[$stackPtr + 3]['type'] === 'T_CONSTANT_ENCAPSED_STRING' + && str_contains($tokens[$stackPtr + 3]['content'], 'hook_') + ) { + $phpcsFile->addWarning('Hook name should not start with "hook_" prefix. Hook name used:' . $tokens[$stackPtr + 3]['content'], $stackPtr + 3,'HookAttributePrefixName'); } }//end process() diff --git a/tests/Drupal/bad/BadUnitTest.php b/tests/Drupal/bad/BadUnitTest.php index e9a855ce..f455b4fb 100644 --- a/tests/Drupal/bad/BadUnitTest.php +++ b/tests/Drupal/bad/BadUnitTest.php @@ -435,6 +435,7 @@ protected function getWarningList(string $testFile): array 823 => 1, 824 => 1, 836 => 1, + 874 => 1, ]; }//end switch diff --git a/tests/Drupal/bad/bad.php b/tests/Drupal/bad/bad.php index cf249d9c..3e36f480 100644 --- a/tests/Drupal/bad/bad.php +++ b/tests/Drupal/bad/bad.php @@ -868,4 +868,12 @@ function test30(TestType $a = NULL) { echo "Hello"; } +/** + * Implements hook_node_view(). + */ +#[Hook('hook_node_view')] +function bad_node_view() { + +} + ?> \ No newline at end of file diff --git a/tests/Drupal/bad/bad.php.fixed b/tests/Drupal/bad/bad.php.fixed index 71ddbb72..c8d0c81e 100644 --- a/tests/Drupal/bad/bad.php.fixed +++ b/tests/Drupal/bad/bad.php.fixed @@ -921,3 +921,11 @@ function test29( function test30(?TestType $a = NULL) { echo "Hello"; } + +/** + * Implements hook_node_view(). + */ +#[Hook('node_view')] +function bad_node_view() { + +} From 3ec769a2ec8139f3f61f40132e2e21b35f21c23b Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Mon, 13 Jan 2025 12:25:02 +0300 Subject: [PATCH 03/16] fix --- coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php | 4 +++- tests/Drupal/bad/BadUnitTest.php | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php b/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php index f73ada77..7afb409e 100644 --- a/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php @@ -64,7 +64,9 @@ public function process(File $phpcsFile, $stackPtr) && $tokens[$stackPtr + 3]['type'] === 'T_CONSTANT_ENCAPSED_STRING' && str_contains($tokens[$stackPtr + 3]['content'], 'hook_') ) { - $phpcsFile->addWarning('Hook name should not start with "hook_" prefix. Hook name used:' . $tokens[$stackPtr + 3]['content'], $stackPtr + 3,'HookAttributePrefixName'); + $hookName = $tokens[$stackPtr + 3]['content']; + $phpcsFile->addWarning('Hook name should not start with "hook_" prefix. Hook name used:' . $hookName, $stackPtr + 3,'HookAttributePrefixName'); + $phpcsFile->fixer->replaceToken($stackPtr + 3, str_replace('hook_', '', $hookName)); } }//end process() diff --git a/tests/Drupal/bad/BadUnitTest.php b/tests/Drupal/bad/BadUnitTest.php index f455b4fb..0e0ab17e 100644 --- a/tests/Drupal/bad/BadUnitTest.php +++ b/tests/Drupal/bad/BadUnitTest.php @@ -384,7 +384,7 @@ protected function getErrorList(string $testFile): array 849 => 2, 860 => 2, 867 => 1, - 871 => 2, + 879 => 2, ]; }//end switch From 49ad673a564644ecf381a7836401673d36196ac9 Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Mon, 13 Jan 2025 12:28:30 +0300 Subject: [PATCH 04/16] use addFixableWarning --- coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php b/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php index 7afb409e..7a2ffc40 100644 --- a/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php @@ -65,7 +65,7 @@ public function process(File $phpcsFile, $stackPtr) && str_contains($tokens[$stackPtr + 3]['content'], 'hook_') ) { $hookName = $tokens[$stackPtr + 3]['content']; - $phpcsFile->addWarning('Hook name should not start with "hook_" prefix. Hook name used:' . $hookName, $stackPtr + 3,'HookAttributePrefixName'); + $phpcsFile->addFixableWarning('Hook name should not start with "hook_" prefix. Hook name used:' . $hookName, $stackPtr + 3,'HookAttributePrefixName'); $phpcsFile->fixer->replaceToken($stackPtr + 3, str_replace('hook_', '', $hookName)); } From a26ccc7d10a3d4162d33695632330e09ec327c53 Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Mon, 13 Jan 2025 12:38:15 +0300 Subject: [PATCH 05/16] move --- .../Sniffs/General/AttributeHookNoHookPrefix.php} | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) rename coder_sniffer/{Drupal/Sniffs/Classes/TestSniff.php => DrupalPractice/Sniffs/General/AttributeHookNoHookPrefix.php} (85%) diff --git a/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php b/coder_sniffer/DrupalPractice/Sniffs/General/AttributeHookNoHookPrefix.php similarity index 85% rename from coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php rename to coder_sniffer/DrupalPractice/Sniffs/General/AttributeHookNoHookPrefix.php index 7a2ffc40..848812f2 100644 --- a/coder_sniffer/Drupal/Sniffs/Classes/TestSniff.php +++ b/coder_sniffer/DrupalPractice/Sniffs/General/AttributeHookNoHookPrefix.php @@ -1,13 +1,13 @@ getTokens(); -// $shortContent = ''; -// $end = $phpcsFile->findNext([T_ATTRIBUTE_END], ($stackPtr + 1)); -// for ($i = ($stackPtr + 1); $i < $end; $i++) { -// $shortContent .= $tokens[$i]['content']; -// } -// $a = 1; if ($tokens[$stackPtr + 1]['type'] === 'T_STRING' && $tokens[$stackPtr + 1]['content'] === 'Hook' From 48b339fb0b87dc31ace140b9eca8dad22fdbbe88 Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Mon, 13 Jan 2025 13:08:28 +0300 Subject: [PATCH 06/16] move --- .../NamingConventions/ValidAttributeHookNameSniff.php} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename coder_sniffer/{DrupalPractice/Sniffs/General/AttributeHookNoHookPrefix.php => Drupal/Sniffs/NamingConventions/ValidAttributeHookNameSniff.php} (91%) diff --git a/coder_sniffer/DrupalPractice/Sniffs/General/AttributeHookNoHookPrefix.php b/coder_sniffer/Drupal/Sniffs/NamingConventions/ValidAttributeHookNameSniff.php similarity index 91% rename from coder_sniffer/DrupalPractice/Sniffs/General/AttributeHookNoHookPrefix.php rename to coder_sniffer/Drupal/Sniffs/NamingConventions/ValidAttributeHookNameSniff.php index 848812f2..daaec00a 100644 --- a/coder_sniffer/DrupalPractice/Sniffs/General/AttributeHookNoHookPrefix.php +++ b/coder_sniffer/Drupal/Sniffs/NamingConventions/ValidAttributeHookNameSniff.php @@ -1,13 +1,13 @@ addFixableWarning('Hook name should not start with "hook_" prefix. Hook name used:' . $hookName, $stackPtr + 3,'HookAttributePrefixName'); + $phpcsFile->addFixableWarning('Hook name should not start with "hook_" prefix. Hook name used:' . $hookName, $stackPtr + 3,'AttributePrefixHookName'); $phpcsFile->fixer->replaceToken($stackPtr + 3, str_replace('hook_', '', $hookName)); } From a8c8966e355e39844f6799e2cddbb427fb4aaaff Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Tue, 21 Jan 2025 00:01:20 +0300 Subject: [PATCH 07/16] move sniff --- .../ValidHookNameSniff.php} | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) rename coder_sniffer/Drupal/Sniffs/{NamingConventions/ValidAttributeHookNameSniff.php => Attributes/ValidHookNameSniff.php} (70%) diff --git a/coder_sniffer/Drupal/Sniffs/NamingConventions/ValidAttributeHookNameSniff.php b/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php similarity index 70% rename from coder_sniffer/Drupal/Sniffs/NamingConventions/ValidAttributeHookNameSniff.php rename to coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php index daaec00a..60a8e970 100644 --- a/coder_sniffer/Drupal/Sniffs/NamingConventions/ValidAttributeHookNameSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php @@ -1,13 +1,13 @@ getTokens(); - if ($tokens[$stackPtr + 1]['type'] === 'T_STRING' - && $tokens[$stackPtr + 1]['content'] === 'Hook' - && $tokens[$stackPtr + 3]['type'] === 'T_CONSTANT_ENCAPSED_STRING' - && str_contains($tokens[$stackPtr + 3]['content'], 'hook_') + if ($tokens[($stackPtr + 1)]['type'] === 'T_STRING' + && $tokens[($stackPtr + 1)]['content'] === 'Hook' + && $tokens[($stackPtr + 3)]['type'] === 'T_CONSTANT_ENCAPSED_STRING' + && str_contains($tokens[($stackPtr + 3)]['content'], 'hook_') ) { - $hookName = $tokens[$stackPtr + 3]['content']; - $phpcsFile->addFixableWarning('Hook name should not start with "hook_" prefix. Hook name used:' . $hookName, $stackPtr + 3,'AttributePrefixHookName'); - $phpcsFile->fixer->replaceToken($stackPtr + 3, str_replace('hook_', '', $hookName)); + $hookName = $tokens[($stackPtr + 3)]['content']; + $phpcsFile->addFixableWarning('Hook name should not start with "hook_" prefix. Hook name used:'.$hookName, ($stackPtr + 3), 'AttributePrefixHookName'); + $phpcsFile->fixer->replaceToken(($stackPtr + 3), str_replace('hook_', '', $hookName)); } }//end process() From 3329b895bec7c3d8881886a64c0b0c7f0becfb60 Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Tue, 21 Jan 2025 00:25:04 +0300 Subject: [PATCH 08/16] move tests --- .../Attributes/ValidHookNameUnitTest.inc | 22 +++++++++ .../ValidHookNameUnitTest.inc.fixed | 22 +++++++++ .../Attributes/ValidHookNameUnitTest.php | 45 +++++++++++++++++++ tests/Drupal/bad/BadUnitTest.php | 3 +- tests/Drupal/bad/bad.php | 8 ---- tests/Drupal/bad/bad.php.fixed | 8 ---- 6 files changed, 90 insertions(+), 18 deletions(-) create mode 100644 tests/Drupal/Attributes/ValidHookNameUnitTest.inc create mode 100644 tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed create mode 100644 tests/Drupal/Attributes/ValidHookNameUnitTest.php diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc new file mode 100644 index 00000000..dd11c1b6 --- /dev/null +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc @@ -0,0 +1,22 @@ + + */ + protected function getErrorList(string $testFile): array + { + return []; + + }//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 + */ + protected function getWarningList(string $testFile): array + { + return [11 => 1]; + + }//end getWarningList() + + +}//end class diff --git a/tests/Drupal/bad/BadUnitTest.php b/tests/Drupal/bad/BadUnitTest.php index 0e0ab17e..e9a855ce 100644 --- a/tests/Drupal/bad/BadUnitTest.php +++ b/tests/Drupal/bad/BadUnitTest.php @@ -384,7 +384,7 @@ protected function getErrorList(string $testFile): array 849 => 2, 860 => 2, 867 => 1, - 879 => 2, + 871 => 2, ]; }//end switch @@ -435,7 +435,6 @@ protected function getWarningList(string $testFile): array 823 => 1, 824 => 1, 836 => 1, - 874 => 1, ]; }//end switch diff --git a/tests/Drupal/bad/bad.php b/tests/Drupal/bad/bad.php index 3e36f480..cf249d9c 100644 --- a/tests/Drupal/bad/bad.php +++ b/tests/Drupal/bad/bad.php @@ -868,12 +868,4 @@ function test30(TestType $a = NULL) { echo "Hello"; } -/** - * Implements hook_node_view(). - */ -#[Hook('hook_node_view')] -function bad_node_view() { - -} - ?> \ No newline at end of file diff --git a/tests/Drupal/bad/bad.php.fixed b/tests/Drupal/bad/bad.php.fixed index c8d0c81e..71ddbb72 100644 --- a/tests/Drupal/bad/bad.php.fixed +++ b/tests/Drupal/bad/bad.php.fixed @@ -921,11 +921,3 @@ function test29( function test30(?TestType $a = NULL) { echo "Hello"; } - -/** - * Implements hook_node_view(). - */ -#[Hook('node_view')] -function bad_node_view() { - -} From 74a0534fc1c0d523df3fa9b1ab70fc6e04b5a32c Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Tue, 21 Jan 2025 00:53:07 +0300 Subject: [PATCH 09/16] add hook_info --- tests/Drupal/Attributes/ValidHookNameUnitTest.inc | 8 ++++++++ tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc index dd11c1b6..bcd997e3 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc @@ -20,3 +20,11 @@ function module_node_view() { function module_piratehook_view() { } + +/** + * Implements hook_hook_info(). + */ +#[Hook('hook_info')] +function module_hook_info() { + +} diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed index e24bf039..a05f8e83 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed @@ -20,3 +20,11 @@ function module_node_view() { function module_piratehook_view() { } + +/** + * Implements hook_hook_info(). + */ +#[Hook('hook_info')] +function module_hook_info() { + +} From 9c6f937b25c43ced7008615f5c4d4e224b049253 Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Tue, 21 Jan 2025 00:53:13 +0300 Subject: [PATCH 10/16] refactor --- .../Sniffs/Attributes/ValidHookNameSniff.php | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php b/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php index 60a8e970..808d0eb9 100644 --- a/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php @@ -22,6 +22,13 @@ class ValidHookNameSniff implements Sniff { + /** + * List of hooks that should not be fixed. + * + * @var string[] + */ + public array $hookExceptions = ['hook_info']; + /** * Returns an array of tokens this test wants to listen for. @@ -51,17 +58,31 @@ public function register() */ public function process(File $phpcsFile, $stackPtr) { - $tokens = $phpcsFile->getTokens(); - - if ($tokens[($stackPtr + 1)]['type'] === 'T_STRING' - && $tokens[($stackPtr + 1)]['content'] === 'Hook' - && $tokens[($stackPtr + 3)]['type'] === 'T_CONSTANT_ENCAPSED_STRING' - && str_contains($tokens[($stackPtr + 3)]['content'], 'hook_') + $tokens = $phpcsFile->getTokens(); + $attributeName = $phpcsFile->findNext(T_STRING, ($stackPtr + 1)); + if ($attributeName !== false + && $tokens[$attributeName]['content'] === 'Hook' ) { - $hookName = $tokens[($stackPtr + 3)]['content']; - $phpcsFile->addFixableWarning('Hook name should not start with "hook_" prefix. Hook name used:'.$hookName, ($stackPtr + 3), 'AttributePrefixHookName'); - $phpcsFile->fixer->replaceToken(($stackPtr + 3), str_replace('hook_', '', $hookName)); - } + $hookName = $phpcsFile->findNext(T_CONSTANT_ENCAPSED_STRING, ($attributeName + 2)); + if ($hookName !== false + ) { + // Remove outer quotes. + $hookNameValue = trim($tokens[$hookName]['content'], '"\''); + + if (in_array($hookNameValue, $this->hookExceptions) === false + && strpos($hookNameValue, 'hook_') === 0 + ) { + $fix = $phpcsFile->addFixableWarning("Hook name should not start with 'hook_'. Hook name used: $hookNameValue", $hookName, 'HookPrefix'); + if ($fix === true && strlen($hookNameValue) > 5) { + // Remove "hook_" prefix. + $hookNameValueFixed = substr($hookNameValue, 5); + // Return outer quotes. + $hookNameValueFixed = str_replace($hookNameValue, $hookNameValueFixed, $tokens[$hookName]['content']); + $phpcsFile->fixer->replaceToken($hookName, $hookNameValueFixed); + } + } + } + }//end if }//end process() From 0f46b154141a9b45eea26e3349832a7c2ec9df4b Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Tue, 21 Jan 2025 10:02:06 +0300 Subject: [PATCH 11/16] update tests --- .../Attributes/ValidHookNameUnitTest.inc | 83 +++++++++++++++++++ .../ValidHookNameUnitTest.inc.fixed | 82 ++++++++++++++++++ .../Attributes/ValidHookNameUnitTest.php | 9 +- 3 files changed, 173 insertions(+), 1 deletion(-) diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc index bcd997e3..b08943c4 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc @@ -7,24 +7,107 @@ /** * Implements hook_node_view(). + * + * Single quotes. */ #[Hook('hook_node_view')] function module_node_view() { } +/** + * Implements hook_node_load(). + * + * Double quotes. + */ +#[Hook("hook_node_load")] +function module_node_load() { + +} + + +/** + * Implements hook_node_delete(). + */ +#[Hook(hook: 'hook_node_delete')] +function module_node_delete() { + +} + +/** + * Implements hook_node_alter(). + */ +#[Hook(hook: 'hook_node_alter', module: 'custom_module')] +function module_node_alter() { + +} + /** * Implements hook_piratehook_view(). + * + * "hook" is a part of hook name. No warning. */ #[Hook('piratehook_view')] function module_piratehook_view() { } +/** + * Implements hook_hookpirate_view(). + * + * "hook" is a part of hook name. No warning. + */ +#[Hook('hookpirate_view')] +function module_hookpirate_view() { + +} + /** * Implements hook_hook_info(). + * + * "hook_info" is exception. No warning. */ #[Hook('hook_info')] function module_hook_info() { } + +/** + * Implements hook_hook_info(). + * + * "hook_info" is exception. No warning. + */ +#[Hook(hook: 'hook_info')] +function module_custom_hook_info() { + +} + +/** + * + */ +#[Hook('hook_user_cancel', 'userCancel', 'custom')] +class Hooks { + + /** + * + */ + public function userCancel() { + + } + +} + +/** + * + */ +#[Hook(hook: "hook_user_login", method: "userLogin", module: "views")] +class MyHooks { + + /** + * + */ + public function userLogin() { + + } + +} diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed index a05f8e83..b2f9a5fc 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed @@ -7,24 +7,106 @@ /** * Implements hook_node_view(). + * + * Single quotes. */ #[Hook('node_view')] function module_node_view() { } +/** + * Implements hook_node_load(). + * + * Double quotes. + */ +#[Hook("node_load")] +function module_node_load() { + +} + +/** + * Implements hook_node_delete(). + */ +#[Hook(hook: 'node_delete')] +function module_node_delete() { + +} + +/** + * Implements hook_node_alter(). + */ +#[Hook(hook: 'node_alter', module: 'custom_module')] +function module_node_alter() { + +} + /** * Implements hook_piratehook_view(). + * + * "hook" is a part of hook name. No warning. */ #[Hook('piratehook_view')] function module_piratehook_view() { } +/** + * Implements hook_hookpirate_view(). + * + * "hook" is a part of hook name. No warning. + */ +#[Hook('hookpirate_view')] +function module_hookpirate_view() { + +} + /** * Implements hook_hook_info(). + * + * "hook_info" is exception. No warning. */ #[Hook('hook_info')] function module_hook_info() { } + +/** + * Implements hook_hook_info(). + * + * "hook_info" is exception. No warning. + */ +#[Hook(hook: 'hook_info')] +function module_custom_hook_info() { + +} + +/** + * + */ +#[Hook('user_cancel', 'userCancel', 'custom')] +class Hooks { + + /** + * + */ + public function userCancel() { + + } + +} + +/** + * + */ +#[Hook(hook: "user_login", method: "userLogin", module: "views")] +class MyHooks { + + /** + * + */ + public function userLogin() { + + } + +} diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.php b/tests/Drupal/Attributes/ValidHookNameUnitTest.php index d3b17b21..02b731bc 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.php +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.php @@ -37,7 +37,14 @@ protected function getErrorList(string $testFile): array */ protected function getWarningList(string $testFile): array { - return [11 => 1]; + return [ + 13 => 1, + 23 => 1, + 32 => 1, + 40 => 1, + 88 => 1, + 103 => 1, + ]; }//end getWarningList() From 970f9a3272bc9be718df46278e9406c58b00e56f Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Tue, 21 Jan 2025 10:52:12 +0300 Subject: [PATCH 12/16] update tests --- .../Attributes/ValidHookNameUnitTest.inc | 49 ++++++++++++------- .../ValidHookNameUnitTest.inc.fixed | 45 +++++++++++------ .../Attributes/ValidHookNameUnitTest.php | 14 +++--- 3 files changed, 68 insertions(+), 40 deletions(-) diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc index b08943c4..169132a9 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc @@ -6,8 +6,14 @@ */ /** - * Implements hook_node_view(). - * + * Valid hook. + */ +#[Hook('valid')] +function module_valid() { + +} + +/** * Single quotes. */ #[Hook('hook_node_view')] @@ -16,8 +22,6 @@ function module_node_view() { } /** - * Implements hook_node_load(). - * * Double quotes. */ #[Hook("hook_node_load")] @@ -27,7 +31,7 @@ function module_node_load() { /** - * Implements hook_node_delete(). + * Attribute named argument. */ #[Hook(hook: 'hook_node_delete')] function module_node_delete() { @@ -35,7 +39,7 @@ function module_node_delete() { } /** - * Implements hook_node_alter(). + * Attribute named arguments. */ #[Hook(hook: 'hook_node_alter', module: 'custom_module')] function module_node_alter() { @@ -43,11 +47,9 @@ function module_node_alter() { } /** - * Implements hook_piratehook_view(). - * - * "hook" is a part of hook name. No warning. + * "hook" is a part of hook name. */ -#[Hook('piratehook_view')] +#[Hook('hook_piratehook_view')] function module_piratehook_view() { } @@ -57,14 +59,12 @@ function module_piratehook_view() { * * "hook" is a part of hook name. No warning. */ -#[Hook('hookpirate_view')] +#[Hook('hook_hookpirate_view')] function module_hookpirate_view() { } /** - * Implements hook_hook_info(). - * * "hook_info" is exception. No warning. */ #[Hook('hook_info')] @@ -73,12 +73,25 @@ function module_hook_info() { } /** - * Implements hook_hook_info(). - * - * "hook_info" is exception. No warning. + * Named argument. "hook_info" is exception. No warning. */ #[Hook(hook: 'hook_info')] -function module_custom_hook_info() { +function mymodule_hook_info() { + +} + +/** + * Valid hook. + */ +#[Hook('valid', 'validMethod', 'module')] +class ValidHooks { + + /** + * + */ + public function validMethod() { + + } } @@ -98,7 +111,7 @@ class Hooks { } /** - * + * Named arguments, double quotes. */ #[Hook(hook: "hook_user_login", method: "userLogin", module: "views")] class MyHooks { diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed index b2f9a5fc..ceeeb484 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed @@ -6,8 +6,14 @@ */ /** - * Implements hook_node_view(). - * + * Valid hook. + */ +#[Hook('valid')] +function module_valid() { + +} + +/** * Single quotes. */ #[Hook('node_view')] @@ -16,8 +22,6 @@ function module_node_view() { } /** - * Implements hook_node_load(). - * * Double quotes. */ #[Hook("node_load")] @@ -26,7 +30,7 @@ function module_node_load() { } /** - * Implements hook_node_delete(). + * Attribute named argument. */ #[Hook(hook: 'node_delete')] function module_node_delete() { @@ -34,7 +38,7 @@ function module_node_delete() { } /** - * Implements hook_node_alter(). + * Attribute named arguments. */ #[Hook(hook: 'node_alter', module: 'custom_module')] function module_node_alter() { @@ -42,9 +46,7 @@ function module_node_alter() { } /** - * Implements hook_piratehook_view(). - * - * "hook" is a part of hook name. No warning. + * "hook" is a part of hook name. */ #[Hook('piratehook_view')] function module_piratehook_view() { @@ -62,8 +64,6 @@ function module_hookpirate_view() { } /** - * Implements hook_hook_info(). - * * "hook_info" is exception. No warning. */ #[Hook('hook_info')] @@ -72,12 +72,25 @@ function module_hook_info() { } /** - * Implements hook_hook_info(). - * - * "hook_info" is exception. No warning. + * Named argument. "hook_info" is exception. No warning. */ #[Hook(hook: 'hook_info')] -function module_custom_hook_info() { +function mymodule_hook_info() { + +} + +/** + * Valid hook. + */ +#[Hook('valid', 'validMethod', 'module')] +class ValidHooks { + + /** + * + */ + public function validMethod() { + + } } @@ -97,7 +110,7 @@ class Hooks { } /** - * + * Named arguments, double quotes. */ #[Hook(hook: "user_login", method: "userLogin", module: "views")] class MyHooks { diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.php b/tests/Drupal/Attributes/ValidHookNameUnitTest.php index 02b731bc..ecc68281 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.php +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.php @@ -38,12 +38,14 @@ protected function getErrorList(string $testFile): array protected function getWarningList(string $testFile): array { return [ - 13 => 1, - 23 => 1, - 32 => 1, - 40 => 1, - 88 => 1, - 103 => 1, + 19 => 1, + 27 => 1, + 36 => 1, + 44 => 1, + 52 => 1, + 62 => 1, + 101 => 1, + 116 => 1, ]; }//end getWarningList() From 28a96226d0537242404db1fe0ccad4855c395e2b Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Tue, 21 Jan 2025 11:14:42 +0300 Subject: [PATCH 13/16] fix comment namespace --- coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php b/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php index 808d0eb9..0d521cfb 100644 --- a/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php @@ -1,6 +1,6 @@ Date: Tue, 21 Jan 2025 11:26:06 +0300 Subject: [PATCH 14/16] update comment --- tests/Drupal/Attributes/ValidHookNameUnitTest.inc | 2 +- tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc index 169132a9..8ea1f159 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc @@ -57,7 +57,7 @@ function module_piratehook_view() { /** * Implements hook_hookpirate_view(). * - * "hook" is a part of hook name. No warning. + * "hook" is a part of hook name. */ #[Hook('hook_hookpirate_view')] function module_hookpirate_view() { diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed index ceeeb484..8174eb98 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed @@ -56,7 +56,7 @@ function module_piratehook_view() { /** * Implements hook_hookpirate_view(). * - * "hook" is a part of hook name. No warning. + * "hook" is a part of hook name. */ #[Hook('hookpirate_view')] function module_hookpirate_view() { From d87116f9e9775aca3d83c1cf72469828837a34f5 Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Tue, 21 Jan 2025 18:28:38 +0300 Subject: [PATCH 15/16] remove hook_info special handling --- .../Sniffs/Attributes/ValidHookNameSniff.php | 11 +--------- .../Attributes/ValidHookNameUnitTest.inc | 20 ++----------------- .../ValidHookNameUnitTest.inc.fixed | 20 ++----------------- .../Attributes/ValidHookNameUnitTest.php | 4 ++-- 4 files changed, 7 insertions(+), 48 deletions(-) diff --git a/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php b/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php index 0d521cfb..cf434ffa 100644 --- a/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php @@ -22,13 +22,6 @@ class ValidHookNameSniff implements Sniff { - /** - * List of hooks that should not be fixed. - * - * @var string[] - */ - public array $hookExceptions = ['hook_info']; - /** * Returns an array of tokens this test wants to listen for. @@ -69,9 +62,7 @@ public function process(File $phpcsFile, $stackPtr) // Remove outer quotes. $hookNameValue = trim($tokens[$hookName]['content'], '"\''); - if (in_array($hookNameValue, $this->hookExceptions) === false - && strpos($hookNameValue, 'hook_') === 0 - ) { + if (strpos($hookNameValue, 'hook_') === 0) { $fix = $phpcsFile->addFixableWarning("Hook name should not start with 'hook_'. Hook name used: $hookNameValue", $hookName, 'HookPrefix'); if ($fix === true && strlen($hookNameValue) > 5) { // Remove "hook_" prefix. diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc index 8ea1f159..84f3edf5 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc @@ -16,8 +16,8 @@ function module_valid() { /** * Single quotes. */ -#[Hook('hook_node_view')] -function module_node_view() { +#[Hook('hook_info')] +function module_info() { } @@ -64,22 +64,6 @@ function module_hookpirate_view() { } -/** - * "hook_info" is exception. No warning. - */ -#[Hook('hook_info')] -function module_hook_info() { - -} - -/** - * Named argument. "hook_info" is exception. No warning. - */ -#[Hook(hook: 'hook_info')] -function mymodule_hook_info() { - -} - /** * Valid hook. */ diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed index 8174eb98..581029e8 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed @@ -16,8 +16,8 @@ function module_valid() { /** * Single quotes. */ -#[Hook('node_view')] -function module_node_view() { +#[Hook('info')] +function module_info() { } @@ -63,22 +63,6 @@ function module_hookpirate_view() { } -/** - * "hook_info" is exception. No warning. - */ -#[Hook('hook_info')] -function module_hook_info() { - -} - -/** - * Named argument. "hook_info" is exception. No warning. - */ -#[Hook(hook: 'hook_info')] -function mymodule_hook_info() { - -} - /** * Valid hook. */ diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.php b/tests/Drupal/Attributes/ValidHookNameUnitTest.php index ecc68281..6430c76f 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.php +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.php @@ -44,8 +44,8 @@ protected function getWarningList(string $testFile): array 44 => 1, 52 => 1, 62 => 1, - 101 => 1, - 116 => 1, + 85 => 1, + 100 => 1, ]; }//end getWarningList() From 826142d9a3db2657f340fe7d96873d0b6151e299 Mon Sep 17 00:00:00 2001 From: Nikolay Shapovalov Date: Sat, 1 Feb 2025 11:11:36 +0300 Subject: [PATCH 16/16] Fixes after review, add test case for "hook_" --- .../Sniffs/Attributes/ValidHookNameSniff.php | 15 ++++++++------- tests/Drupal/Attributes/ValidHookNameUnitTest.inc | 7 +++++++ .../Attributes/ValidHookNameUnitTest.inc.fixed | 8 ++++++++ tests/Drupal/Attributes/ValidHookNameUnitTest.php | 12 ++++++------ 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php b/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php index cf434ffa..1f8791f8 100644 --- a/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Attributes/ValidHookNameSniff.php @@ -57,16 +57,17 @@ public function process(File $phpcsFile, $stackPtr) && $tokens[$attributeName]['content'] === 'Hook' ) { $hookName = $phpcsFile->findNext(T_CONSTANT_ENCAPSED_STRING, ($attributeName + 2)); - if ($hookName !== false - ) { + if ($hookName !== false) { // Remove outer quotes. $hookNameValue = trim($tokens[$hookName]['content'], '"\''); - if (strpos($hookNameValue, 'hook_') === 0) { - $fix = $phpcsFile->addFixableWarning("Hook name should not start with 'hook_'. Hook name used: $hookNameValue", $hookName, 'HookPrefix'); - if ($fix === true && strlen($hookNameValue) > 5) { - // Remove "hook_" prefix. - $hookNameValueFixed = substr($hookNameValue, 5); + if (strpos($hookNameValue, 'hook_') === 0 && $hookNameValue !== 'hook_') { + // Remove "hook_" prefix. + $hookNameValueFixed = substr($hookNameValue, 5); + $message = sprintf("The hook name should not start with 'hook_', expected '%s' but found '%s'", $hookNameValueFixed, $hookNameValue); + + $fix = $phpcsFile->addFixableWarning($message, $hookName, 'HookPrefix'); + if ($fix === true) { // Return outer quotes. $hookNameValueFixed = str_replace($hookNameValue, $hookNameValueFixed, $tokens[$hookName]['content']); $phpcsFile->fixer->replaceToken($hookName, $hookNameValueFixed); diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc index 84f3edf5..5a65eabd 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc @@ -29,6 +29,13 @@ function module_node_load() { } +/** + * Not finished hook name. No warning + */ +#[Hook('hook_')] +function module_system() { + +} /** * Attribute named argument. diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed index 581029e8..e7097d85 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.inc.fixed @@ -29,6 +29,14 @@ function module_node_load() { } +/** + * Not finished hook name. No warning. + */ +#[Hook('hook_')] +function module_system() { + +} + /** * Attribute named argument. */ diff --git a/tests/Drupal/Attributes/ValidHookNameUnitTest.php b/tests/Drupal/Attributes/ValidHookNameUnitTest.php index 6430c76f..19b8275e 100644 --- a/tests/Drupal/Attributes/ValidHookNameUnitTest.php +++ b/tests/Drupal/Attributes/ValidHookNameUnitTest.php @@ -40,12 +40,12 @@ protected function getWarningList(string $testFile): array return [ 19 => 1, 27 => 1, - 36 => 1, - 44 => 1, - 52 => 1, - 62 => 1, - 85 => 1, - 100 => 1, + 43 => 1, + 51 => 1, + 59 => 1, + 69 => 1, + 92 => 1, + 107 => 1, ]; }//end getWarningList()