-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enforce valid function names in the packages/block-library/src/*/*.php files #53438
Merged
Merged
Changes from all commits
Commits
Show all changes
49 commits
Select commit
Hold shift + click to select a range
42dd2b5
Commit WIP.
anton-vlasenko d7f7212
Commit WIP.
anton-vlasenko 881a594
1. Rename sniff.
anton-vlasenko 695e2bf
Remove redundant file.
anton-vlasenko 3b650ad
Fix sniff's name.
anton-vlasenko ce561b1
List allowed function names in the error message.
anton-vlasenko 10eae71
1. Fix doc blocks.
anton-vlasenko 332f003
Fix error message.
anton-vlasenko 8f74d2a
Improve error message.
anton-vlasenko 081259d
Add the new rule to the phpcs.xml.dist file.
anton-vlasenko 5bdb79e
Add doing it wrong as dynamic properties are not supported in PHP 8.2…
anton-vlasenko 975187f
Improve error message.
anton-vlasenko 1563116
Improve error message.
anton-vlasenko c3a6cdc
Improve error message.
anton-vlasenko addaea8
Ensure that the rule checks all PHP files, not just the index.php files.
anton-vlasenko 8bf5c72
Refactor variable names to use lowercase.
anton-vlasenko ecc45fd
Add exceptions to ValidBlockLibraryFunctionName rule.
anton-vlasenko a3ef82a
Add exceptions to ValidBlockLibraryFunctionName rule.
anton-vlasenko c6029de
Add exceptions to ValidBlockLibraryFunctionName rule.
anton-vlasenko 1e54af8
Add exceptions to ValidBlockLibraryFunctionName rule.
anton-vlasenko a51c0ba
Add exceptions to ValidBlockLibraryFunctionName rule.
anton-vlasenko 32c3a27
Add exceptions to ValidBlockLibraryFunctionName rule.
anton-vlasenko 0ee1871
Add exceptions to the ValidBlockLibraryFunctionName rule.
anton-vlasenko 50dfbcc
Add exceptions to the ValidBlockLibraryFunctionName rule.
anton-vlasenko 07a9a15
Add exceptions to the ValidBlockLibraryFunctionName rule.
anton-vlasenko e276723
Add exceptions to the ValidBlockLibraryFunctionName rule.
anton-vlasenko 4c12e14
Add exceptions to the ValidBlockLibraryFunctionName rule.
anton-vlasenko aed8f1b
Add exceptions to the ValidBlockLibraryFunctionName rule.
anton-vlasenko 2991293
Better sanitization of parameters.
anton-vlasenko 5266d88
Update docblock.
anton-vlasenko deccd15
Update changlog.
anton-vlasenko 6c3135e
Implement a check for edge cases where the function name terminates w…
anton-vlasenko 148c47a
Use blockquote instead of double quote in the error message.
anton-vlasenko aa7820f
Make the sanitize_directory_name method static.
anton-vlasenko 8c47356
Update the commment.
anton-vlasenko 93e42d7
Revert adding the phpcs:ignore statements.
anton-vlasenko c28feeb
Add ability to whilelist certain functions.
anton-vlasenko fdb738b
Revert adding the phpcs:ignore statements.
anton-vlasenko b6178b4
Revert adding the phpcs:ignore statements.
anton-vlasenko 9076c0d
Add the list of allowed functions.
anton-vlasenko b3c9010
Fix typo.
anton-vlasenko 888d82f
Add comment to php.xml.dist with the reasoning for the list.
anton-vlasenko 62e9c44
Revert the changes as we don't use phpcs:disable statements now.
anton-vlasenko a4bd446
Fix comment.
anton-vlasenko a869ff4
Remove redundant code.
anton-vlasenko 523e1ed
Improve text of the comment.
anton-vlasenko d1f38ac
Improve text of the comment.
anton-vlasenko 8ab646f
Replace "backported" with "consumes" (props @hellofromtonya).
anton-vlasenko 1c25491
Remove the last sentence.
anton-vlasenko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
171 changes: 171 additions & 0 deletions
171
...oding-standards/Gutenberg/Sniffs/NamingConventions/ValidBlockLibraryFunctionNameSniff.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,171 @@ | ||
<?php | ||
/** | ||
* Gutenberg Coding Standards. | ||
* | ||
* @package gutenberg/gutenberg-coding-standards | ||
* @link https://github.com/WordPress/gutenberg | ||
* @license https://opensource.org/licenses/MIT MIT | ||
*/ | ||
|
||
namespace GutenbergCS\Gutenberg\Sniffs\NamingConventions; | ||
|
||
use PHP_CodeSniffer\Files\File; | ||
use PHP_CodeSniffer\Sniffs\Sniff; | ||
|
||
/** | ||
* | ||
* This sniff checks function names to ensure they adhere to specified prefixes | ||
* determined by the parent directory name. It enforces that function names start | ||
* with one of the allowed prefixes defined in the sniffer configuration. | ||
* | ||
* @package gutenberg/gutenberg-coding-standards | ||
* | ||
* @since 1.0.0 | ||
*/ | ||
final class ValidBlockLibraryFunctionNameSniff implements Sniff { | ||
/** | ||
* Target prefixes. | ||
* | ||
* @var array | ||
*/ | ||
public $prefixes = array(); | ||
|
||
/** | ||
* These functions are considered permissible and will be ignored by the sniffer. | ||
* | ||
* @var array | ||
*/ | ||
public $allowed_functions = array(); | ||
|
||
/** | ||
* Registers the tokens that this sniff wants to listen for. | ||
* | ||
* @return array | ||
*/ | ||
public function register() { | ||
$this->onRegisterEvent(); | ||
|
||
return array( T_FUNCTION ); | ||
} | ||
|
||
/** | ||
* Processes function tokens. | ||
* | ||
* @param File $phpcsFile The file being scanned. | ||
* @param int $stackPtr The position of the current token | ||
* in the stack passed in $tokens. | ||
* | ||
* @return void | ||
*/ | ||
public function process( File $phpcsFile, $stackPtr ) { | ||
$tokens = $phpcsFile->getTokens(); | ||
$token = $tokens[ $stackPtr ]; | ||
|
||
if ( 'T_FUNCTION' !== $token['type'] ) { | ||
return; | ||
} | ||
|
||
$this->processFunctionToken( $phpcsFile, $stackPtr ); | ||
} | ||
|
||
/** | ||
* This method analyzes the function token and its name within the provided file. | ||
* It checks if the function name adheres to allowed prefixes based on the parent directory name. | ||
* If the function name is not valid, an error message is added to the code sniffer report. | ||
* | ||
* @param File $phpcsFile The file being scanned. | ||
* @param int $stackPointer The position of the current token | ||
* in the stack passed in $tokens. | ||
* | ||
* @return void | ||
*/ | ||
private function processFunctionToken( File $phpcsFile, $stackPointer ) { | ||
|
||
if ( empty( $this->prefixes ) ) { | ||
// Nothing to process. | ||
return; | ||
} | ||
|
||
$tokens = $phpcsFile->getTokens(); | ||
$function_token = $phpcsFile->findNext( T_STRING, $stackPointer ); | ||
|
||
$wrapping_tokens_to_check = array( | ||
T_CLASS, | ||
T_INTERFACE, | ||
T_TRAIT, | ||
); | ||
|
||
foreach ( $wrapping_tokens_to_check as $wrapping_token_to_check ) { | ||
if ( false !== $phpcsFile->getCondition( $function_token, $wrapping_token_to_check, false ) ) { | ||
// This sniff only processes functions, not class methods. | ||
return; | ||
} | ||
} | ||
|
||
$function_name = $tokens[ $function_token ]['content']; | ||
|
||
if ( in_array( $function_name, $this->allowed_functions, true ) ) { | ||
// The function name is included in the list of allowed functions; bypassing further checks. | ||
return; | ||
} | ||
|
||
$parent_directory_name = basename( dirname( $phpcsFile->getFilename() ) ); | ||
|
||
$allowed_function_prefixes = array(); | ||
$is_function_name_valid = false; | ||
foreach ( $this->prefixes as $prefix ) { | ||
$prefix = rtrim( $prefix, '_' ); | ||
$allowed_function_prefix = $prefix . '_' . self::sanitize_directory_name( $parent_directory_name ); | ||
$allowed_function_prefixes[] = $allowed_function_prefix; | ||
// Validate the name's correctness and ensure it does not end with an underscore. | ||
$regexp = sprintf( '/^%s(|_.+)$/', preg_quote( $allowed_function_prefix, '/' ) ); | ||
$is_function_name_valid |= ( 1 === preg_match( $regexp, $function_name ) ); | ||
} | ||
|
||
if ( $is_function_name_valid ) { | ||
return; | ||
} | ||
|
||
$error_message = "The function name `{$function_name}()` is invalid because PHP function names in this file should start with one of the following prefixes: `" | ||
. implode( '`, `', $allowed_function_prefixes ) . '`.'; | ||
$phpcsFile->addError( $error_message, $function_token, 'FunctionNameInvalid' ); | ||
} | ||
|
||
/** | ||
* The purpose of this method is to run callbacks | ||
* after the class properties have been set. | ||
*/ | ||
private function onRegisterEvent() { | ||
$this->prefixes = self::sanitize( $this->prefixes ); | ||
$this->allowed_functions = self::sanitize( $this->allowed_functions ); | ||
} | ||
|
||
/** | ||
* Sanitize a directory name by converting it to lowercase and replacing non-letter | ||
* and non-digit characters with underscores. | ||
* | ||
* @param string $directory_name | ||
* | ||
* @return string | ||
*/ | ||
private static function sanitize_directory_name( $directory_name ) { | ||
// Convert to lowercase. | ||
$directory_name = strtolower( $directory_name ); | ||
|
||
// Replace non-letter and non-digit characters with underscores. | ||
return preg_replace( '/[^a-z0-9]/', '_', $directory_name ); | ||
} | ||
|
||
/** | ||
* Sanitize an array of values by trimming each element and removing empty elements. | ||
* | ||
* @param array $values The values being sanitized. | ||
* | ||
* @return array | ||
*/ | ||
private static function sanitize( $values ) { | ||
$values = array_map( 'trim', $values ); | ||
|
||
return array_filter( $values ); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see functions that start
build_template_part_
, so maybe we could open alsobuild_block_core_template_part
in case folks want to use that. I don't have strong opinions on that, as they could also doblock_core_tempalte_part_build
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a preference either. However, please note that the linting rule not only checks the prefix name but also the directory name, as requested here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine as is for now, but we should keep monitoring and extending the list if that is useful.