Add ValidTaxonomySlugSniff #2485
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,276 @@ | ||
| <?php | ||
| /** | ||
| * WordPress Coding Standard. | ||
| * | ||
| * @package WPCS\WordPressCodingStandards | ||
| * @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards | ||
| * @license https://opensource.org/licenses/MIT MITnamespace WordPressCS\WordPress; | ||
IanDelMar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| */ | ||
|
|
||
| namespace WordPressCS\WordPress; | ||
|
|
||
| use PHP_CodeSniffer\Util\Tokens; | ||
| use PHPCSUtils\Tokens\Collections; | ||
| use PHPCSUtils\Utils\PassedParameters; | ||
| use PHPCSUtils\Utils\TextStrings; | ||
| use WordPressCS\WordPress\AbstractFunctionParameterSniff; | ||
|
|
||
| /** | ||
| * Validates names passed to a function call. | ||
| * | ||
| * Checks slugs for the presence of invalid characters, excessive length, | ||
| * and reserved keywords. | ||
| */ | ||
IanDelMar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| abstract class AbstractValidSlugSniff extends AbstractFunctionParameterSniff { | ||
|
|
||
| /** | ||
| * Slug type. E.g. 'post type' for a post type slug. | ||
| * | ||
| * @var string | ||
| */ | ||
| protected $slug_type; | ||
|
|
||
| /** | ||
| * Plural of the slug type. E.g. 'post types' for a post type slug. | ||
| * | ||
| * @var string | ||
| */ | ||
| protected $slug_type_plural; | ||
|
|
||
| /** | ||
| * Max length of a slug is limited by the SQL field. | ||
| * | ||
| * @var int | ||
| */ | ||
| protected $max_length; | ||
|
|
||
| /** | ||
| * Regex to validate the characters that can be used as the slug. | ||
| * | ||
| * @var string | ||
| */ | ||
| protected $valid_characters; | ||
IanDelMar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Array of reserved names which can not be used by themes and plugins. | ||
IanDelMar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * | ||
| * @var array<string, true> Key is reserved name, value irrelevant. | ||
| */ | ||
| protected $reserved_names; | ||
|
|
||
| /** | ||
| * All valid tokens for the slug parameter. | ||
| * | ||
| * Set in `register()`. | ||
| * | ||
| * @var array<int|string, int|string> | ||
| */ | ||
| private $valid_tokens = array(); | ||
|
|
||
| /** | ||
| * Constructor. | ||
| * | ||
| * @since 3.0.0 | ||
| */ | ||
| public function __construct() { | ||
|
There was a problem hiding this comment. I'm not keen on this "abstract methods to properties" setting of sniff specific information. I'm not even sure this abstract is a good idea to begin with. Abstract sniffs should generally not throw errors. That should be done in the actual sniffs themselves. Doing so in the abstract greatly reduces the re-usability of any such abstract (as the error may be needed for slug type A, but not for slug type B or the error may need to be worded slightly differently as the effect of the issue is different for a certain slug type etc etc). And if you take out the errors and just keep the logic, there is barely anything left of the abstract. The only acceptable way to throw errors from an abstract is via a separate method which can be overloaded in the concrete sniff. Along the same lines, the abstract methods currently defined in this sniff may not apply for all slug types, so again, this reduces the re-usability. Considering the original sniff already used class constants (for constant information) and removing those is no-go (breaking change), it could be considered to use class constants (and maybe the odd non-abstract method) as "feature toggles" + for the actual information. I'm thinking along the lines of logic like this (pseudo code): if (defined('static::MAX_LENGTH')) {
// Execute the max length check.
}Having said that, I'm not convinced of the value of the abstract sniff for something this specific as checking slug name rules. |
||
| $this->target_functions = $this->get_target_functions(); | ||
| $this->slug_type = $this->get_slug_type(); | ||
| $this->slug_type_plural = $this->get_slug_type_plural(); | ||
| $this->valid_characters = $this->get_valid_characters(); | ||
| $this->max_length = $this->get_max_length(); | ||
| $this->reserved_names = $this->get_reserved_names(); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieve function and parameter(s) pairs this sniff is looking for. | ||
| * | ||
| * The parameter or an array of parameters keyed by target function. | ||
| * An array of names is supported to allow for functions for which the | ||
| * parameter names have undergone name changes over time. | ||
| * | ||
| * @return array<string, string|array<string>> Function parameter(s) pairs. | ||
| */ | ||
| abstract protected function get_target_functions(); | ||
|
|
||
| /** | ||
| * Retrieve the slug type. | ||
| * | ||
| * @return string The slug type. | ||
| */ | ||
| abstract protected function get_slug_type(); | ||
|
|
||
| /** | ||
| * Retrieve the plural slug type. | ||
| * | ||
| * @return string The plural slug type. | ||
| */ | ||
| abstract protected function get_slug_type_plural(); | ||
|
|
||
| /** | ||
| * Retrieve the regex to validate the characters that can be used as | ||
| * the slug. | ||
| * | ||
| * @return string Regular expression. | ||
| */ | ||
| abstract protected function get_valid_characters(); | ||
|
|
||
| /** | ||
| * Retrieve the max length of a slug. | ||
| * | ||
| * The length is limited by the SQL field. | ||
IanDelMar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * | ||
| * @return int The maximum length of a slug. | ||
| */ | ||
| abstract protected function get_max_length(); | ||
|
|
||
| /** | ||
| * Retrieve the reserved names which can not be used by themes and plugins. | ||
| * | ||
| * @return array<string, true> Key is reserved name, value irrelevant. | ||
| */ | ||
| abstract protected function get_reserved_names(); | ||
|
|
||
| /** | ||
| * Returns an array of tokens this test wants to listen for. | ||
| * | ||
| * @return array | ||
| */ | ||
| public function register() { | ||
| $this->valid_tokens = Tokens::$textStringTokens + Tokens::$heredocTokens + Tokens::$emptyTokens; | ||
IanDelMar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return parent::register(); | ||
| } | ||
|
|
||
| /** | ||
| * Process the parameter of a matched function. | ||
| * | ||
| * Errors on invalid names when reserved keywords are used, | ||
IanDelMar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * the name is too long, or contains invalid characters. | ||
| * | ||
| * @param int $stackPtr The position of the current token in the stack. | ||
| * @param string $group_name The name of the group which was matched. | ||
| * @param string $matched_content The token content (function name) which was matched | ||
| * in lowercase. | ||
| * @param array $parameters Array with information about the parameters. | ||
| * | ||
| * @return void | ||
| */ | ||
| public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { | ||
| $slug_param = PassedParameters::getParameterFromStack( | ||
| $parameters, | ||
| 1, | ||
|
There was a problem hiding this comment. Please don't hard-code the parameter position in an abstract sniff. This reduces re-usability. This information should come from the concrete sniff. There was a problem hiding this comment. The parameter name must be provided by the concrete sniff. If that's the case, the offset is superseded unless the parameter name is incorrect. Am I understanding this correctly? There was a problem hiding this comment.
You are totally misunderstanding this. In PHP, parameters can be passed positionally, i.e. To get the correct parameter from a function call and handle both ways of passing parameters, the And as this is an abstract sniff, the sniff cannot rely on the "slug"-like parameter always being in the first position. There was a problem hiding this comment. I see! I thought this was checking against the name in the function declaration, rather than checking for named parameters. My mistake. If I had been more attentive, I would have noticed the line, 'First check for a named parameter.' As you may have noticed, I'm not very familiar with the WPCS or the PHPCS codebase. 😅 😇 There was a problem hiding this comment. PHPCS has no access to the function declaration (unless the function is declared in the same file). For this type of code analysis, you only look at the function call. |
||
| $this->target_functions[ $matched_content ] | ||
| ); | ||
| if ( false === $slug_param || '' === $slug_param['clean'] ) { | ||
| // Error for using empty slug. | ||
| $this->phpcsFile->addError( | ||
| '%s() called without a %s slug. The slug must be a non-empty string.', | ||
| false === $slug_param ? $stackPtr : $slug_param['start'], | ||
| 'Empty', | ||
| array( | ||
| $matched_content, | ||
| $this->slug_type, | ||
| ) | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| $string_start = $this->phpcsFile->findNext( Collections::textStringStartTokens(), $slug_param['start'], ( $slug_param['end'] + 1 ) ); | ||
| $string_pos = $this->phpcsFile->findNext( Tokens::$textStringTokens, $slug_param['start'], ( $slug_param['end'] + 1 ) ); | ||
|
|
||
| $has_invalid_tokens = $this->phpcsFile->findNext( $this->valid_tokens, $slug_param['start'], ( $slug_param['end'] + 1 ), true ); | ||
| if ( false !== $has_invalid_tokens || false === $string_pos ) { | ||
| // Check for non string based slug parameter (we cannot determine if this is valid). | ||
| $this->phpcsFile->addWarning( | ||
| 'The %s slug is not a string literal. It is not possible to automatically determine the validity of this slug. Found: %s.', | ||
| $stackPtr, | ||
| 'NotStringLiteral', | ||
| array( | ||
| $this->slug_type, | ||
| $slug_param['clean'], | ||
| ), | ||
| 3 | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| $slug = TextStrings::getCompleteTextString( $this->phpcsFile, $string_start ); | ||
| if ( isset( Tokens::$heredocTokens[ $this->tokens[ $string_start ]['code'] ] ) ) { | ||
| // Trim off potential indentation from PHP 7.3 flexible heredoc/nowdoc content. | ||
| $slug = ltrim( $slug ); | ||
| } | ||
|
|
||
| // Warn for dynamic parts in the slug parameter. | ||
| if ( 'T_DOUBLE_QUOTED_STRING' === $this->tokens[ $string_pos ]['type'] | ||
| || ( 'T_HEREDOC' === $this->tokens[ $string_pos ]['type'] | ||
| && strpos( $this->tokens[ $string_pos ]['content'], '$' ) !== false ) | ||
| ) { | ||
| $this->phpcsFile->addWarning( | ||
| 'The %s slug may, or may not, get too long with dynamic contents and could contain invalid characters. Found: "%s".', | ||
| $string_pos, | ||
| 'PartiallyDynamic', | ||
| array( | ||
| $this->slug_type, | ||
| $slug, | ||
| ) | ||
| ); | ||
| $slug = TextStrings::stripEmbeds( $slug ); | ||
| } | ||
|
|
||
| if ( preg_match( $this->valid_characters, $slug ) === 0 ) { | ||
| // Error for invalid characters. | ||
| $this->phpcsFile->addError( | ||
| '%s() called with invalid %s "%s". %s contains invalid characters. Only lowercase alphanumeric characters, dashes, and underscores are allowed.', | ||
| $string_pos, | ||
| 'InvalidCharacters', | ||
| array( | ||
| $matched_content, | ||
| $this->slug_type, | ||
| ucfirst( $this->slug_type ), | ||
| $slug, | ||
IanDelMar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ) | ||
| ); | ||
| } | ||
|
|
||
| if ( isset( $this->reserved_names[ $slug ] ) ) { | ||
| // Error for using reserved slug names. | ||
| $this->phpcsFile->addError( | ||
| '%s() called with reserved %s "%s". Reserved %s should not be used as they interfere with the functioning of WordPress itself.', | ||
| $string_pos, | ||
| 'Reserved', | ||
| array( | ||
| $matched_content, | ||
| $this->slug_type, | ||
| $slug, | ||
| $this->slug_type_plural, | ||
| ) | ||
| ); | ||
| } elseif ( stripos( $slug, 'wp_' ) === 0 ) { | ||
| // Error for using reserved slug prefix. | ||
| $this->phpcsFile->addError( | ||
| 'The %s passed to %s() uses a prefix reserved for WordPress itself. Found: "%s".', | ||
| $string_pos, | ||
| 'ReservedPrefix', | ||
| array( | ||
| $this->slug_type, | ||
| $matched_content, | ||
| $slug, | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| // Error for slugs that are too long. | ||
| if ( strlen( $slug ) > $this->max_length ) { | ||
| $this->phpcsFile->addError( | ||
| 'A %s slug must not exceed %d characters. Found: "%s" (%d characters).', | ||
| $string_pos, | ||
| 'TooLong', | ||
| array( | ||
| $this->slug_type, | ||
| $this->max_length, | ||
| $slug, | ||
| strlen( $slug ), | ||
| ) | ||
| ); | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.