Skip to content
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

Add new sniff to check whether a hook name is valid. #599

Merged
merged 5 commits into from
Jul 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions WordPress-Core/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -112,5 +112,6 @@
<rule ref="WordPress.PHP.YodaConditions"/>
<rule ref="WordPress.WP.I18n"/>
<rule ref="WordPress.Functions.DontExtract"/>
<rule ref="WordPress.NamingConventions.ValidHookName"/>

</ruleset>
38 changes: 38 additions & 0 deletions WordPress/Sniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,44 @@ abstract class WordPress_Sniff implements PHP_CodeSniffer_Sniff {
'wp_cache_delete' => true,
);

/**
* A list of functions that invoke WP hooks (filters/actions).
*
* @since 0.10.0
*
* @var array
*/
public static $hookInvokeFunctions = array(
'do_action' => true,
'do_action_ref_array' => true,
'do_action_deprecated' => true,
'apply_filters' => true,
'apply_filters_ref_array' => true,
'apply_filters_deprecated' => true,
);

/**
* A list of functions that are used to interact with the WP plugins API.
*
* @since 0.10.0
*
* @var array <string function name> => <int position of the hook name argument in function signature>
*/
public static $hookFunctions = array(
'has_filter' => 1,
'add_filter' => 1,
'remove_filter' => 1,
'remove_all_filters' => 1,
'doing_filter' => 1, // Hook name optional.
'has_action' => 1,
'add_action' => 1,
'doing_action' => 1, // Hook name optional.
'did_action' => 1,
'remove_action' => 1,
'remove_all_actions' => 1,
'current_filter' => 0, // No hook name argument.
);

/**
* The current file being sniffed.
*
Expand Down
284 changes: 284 additions & 0 deletions WordPress/Sniffs/NamingConventions/ValidHookNameSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,284 @@
<?php
/**
* WordPress Coding Standard.
*
* @category PHP
* @package PHP_CodeSniffer
* @link https://make.wordpress.org/core/handbook/best-practices/coding-standards/
*/

/**
* Use lowercase letters in action and filter names. Separate words via underscores.
*
* This sniff is only testing the hook invoke functions as when using 'add_action'/'add_filter'
* you can't influence the hook name.
*
* Hook names invoked with `do_action_deprecated()` and `apply_filters_deprecated()` are ignored.
*
* @link https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#naming-conventions
*
* @category PHP
* @package PHP_CodeSniffer
* @author Juliette Reinders Folmer <[email protected]>
*/
class WordPress_Sniffs_NamingConventions_ValidHookNameSniff implements PHP_CodeSniffer_Sniff {

/**
* Additional word separators.
*
* This public variable allows providing additional word separators which
* will be allowed in hook names via a property in the phpcs.xml config file.
*
* Example usage:
* <rule ref="WordPress.NamingConventions.ValidHookName">
* <properties>
* <property name="additionalWordDelimiters" value="-"/>
* </properties>
* </rule>
*
* Provide several extra delimiters as one string:
* <rule ref="WordPress.NamingConventions.ValidHookName">
* <properties>
* <property name="additionalWordDelimiters" value="-/."/>
* </properties>
* </rule>
*
* @var string
*/
public $additionalWordDelimiters = '';

/**
* Regular expression to test for correct punctuation of a hook name.
*
* The placeholder will be replaced by potentially provided additional
* word delimiters in the `prepare_regex()` method.
*
* @var string
*/
protected $punctuation_regex = '`[^\w%s]`';

/**
* Register this sniff.
*
* Prepares the punctuation regex and returns an array of tokens this test wants to listen for.
*
* @return array
*/
public function register() {
return array(
T_STRING,
);

} // end register()

/**
* Processes this test, when one of its tokens is encountered.
*
* @param PHP_CodeSniffer_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( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) {
$tokens = $phpcsFile->getTokens();
$token = $tokens[ $stackPtr ];
$regex = $this->prepare_regex();

// Check if one of the hook functions was found.
if ( ! isset( WordPress_Sniff::$hookInvokeFunctions[ $tokens[ $stackPtr ]['content'] ] ) ) {
return;
}

// Ignore deprecated hook names.
if ( strpos( $tokens[ $stackPtr ]['content'], '_deprecated' ) > 0 ) {
return;
}

$prev = $phpcsFile->findPrevious( T_WHITESPACE, ( $stackPtr - 1 ), null, true );

if ( false !== $prev ) {
// Skip sniffing if calling a same-named method, or on function definitions.
if ( in_array( $tokens[ $prev ]['code'], array( T_FUNCTION, T_DOUBLE_COLON, T_OBJECT_OPERATOR ), true ) ) {
return;
}

// Skip namespaced functions, ie: \foo\bar() not \bar().
$pprev = $phpcsFile->findPrevious( T_WHITESPACE, ( $prev - 1 ), null, true );
if ( false !== $pprev && T_NS_SEPARATOR === $tokens[ $prev ]['code'] && T_STRING === $tokens[ $pprev ]['code'] ) {
return;
}
}
unset( $prev, $pprev );

/*
Ok, so we have a proper hook call, let's find the position of the tokens
which together comprise the hook name.
*/
$start = $phpcsFile->findNext( array( T_WHITESPACE, T_OPEN_PARENTHESIS ), ( $stackPtr + 1 ), null, true, null, true );
$open = $phpcsFile->findNext( T_OPEN_PARENTHESIS, ( $stackPtr + 1 ), null, false, null, true );
$end = $phpcsFile->findNext( T_COMMA, ( $start + 1 ), null, false, null, true );
if ( false === $end || $end > $tokens[ $open ]['parenthesis_closer'] ) {
$end = $tokens[ $open ]['parenthesis_closer'];
}
if ( T_WHITESPACE === $tokens[ ( $end - 1 ) ]['code'] ) {
$end--;
}

$case_errors = 0;
$underscores = 0;
$content = array();
$expected = array();

for ( $i = $start; $i < $end; $i++ ) {
$content[ $i ] = $tokens[ $i ]['content'];
$expected[ $i ] = $tokens[ $i ]['content'];

if ( in_array( $tokens[ $i ]['code'], array( T_CONSTANT_ENCAPSED_STRING, T_DOUBLE_QUOTED_STRING ), true ) ) {
/*
Here be dragons - a double quoted string can contain extrapolated variables
which don't have to comply with these rules.
*/
if ( T_DOUBLE_QUOTED_STRING === $tokens[ $i ]['code'] ) {
$string = trim( $tokens[ $i ]['content'], '"' );
$transform = $this->transform_complex_string( $string, $regex );
$case_transform = $this->transform_complex_string( $string, $regex, 'case' );
$punct_transform = $this->transform_complex_string( $string, $regex, 'punctuation' );
} else {
$string = trim( $tokens[ $i ]['content'], '\'"' );
$transform = $this->transform( $string, $regex );
$case_transform = $this->transform( $string, $regex, 'case' );
$punct_transform = $this->transform( $string, $regex, 'punctuation' );
}

if ( $string === $transform ) {
continue;
}

if ( T_DOUBLE_QUOTED_STRING === $tokens[ $i ]['code'] ) {
$expected[ $i ] = '"' . $transform . '"';
} else {
$expected[ $i ] = '\'' . $transform . '\'';
}

if ( $string !== $case_transform ) {
$case_errors++;
}
if ( $string !== $punct_transform ) {
$underscores++;
}
}
}

$data = array(
implode( '', $expected ),
implode( '', $content ),
);

if ( $case_errors > 0 ) {
$error = 'Hook names should be lowercase. Expected: %s, but found: %s.';
$phpcsFile->addError( $error, $stackPtr, 'NotLowercase', $data );
}
if ( $underscores > 0 ) {
$error = 'Words in hook names should be separated using underscores. Expected: %s, but found: %s.';
$phpcsFile->addWarning( $error, $stackPtr, 'UseUnderscores', $data );
}

} // end process()

/**
* Prepare the punctuation regular expression.
*
* Merges the existing regular expression with potentially provided extra word delimiters to allow.
* This is done 'late' and for each found token as otherwise inline `@codingStandardsChangeSetting`
* directives would be ignored.
*
* @return string
*/
protected function prepare_regex() {
$extra = '';
if ( '' !== $this->additionalWordDelimiters && is_string( $this->additionalWordDelimiters ) ) {
$extra = preg_quote( $this->additionalWordDelimiters, '`' );
}

return sprintf( $this->punctuation_regex, $extra );

} // end prepare_regex()

/**
* Transform an arbitrary string to lowercase and replace punctuation and spaces with underscores.
*
* @param string $string The target string.
* @param string $regex The punctuation regular expression to use.
* @param string $transform_type Whether to a partial or complete transform.
* Valid values are: 'full', 'case', 'punctuation'.
* @return string
*/
protected function transform( $string, $regex, $transform_type = 'full' ) {

switch ( $transform_type ) {
case 'case':
return strtolower( $string );

case 'punctuation':
return preg_replace( $regex, '_', $string );

case 'full':
default:
return preg_replace( $regex, '_', strtolower( $string ) );
}
} // end transform()

/**
* Transform a complex string which may contain variable extrapolation.
*
* @param string $string The target string.
* @param string $regex The punctuation regular expression to use.
* @param string $transform_type Whether to a partial or complete transform.
* Valid values are: 'full', 'case', 'punctuation'.
* @return string
*/
protected function transform_complex_string( $string, $regex, $transform_type = 'full' ) {
$output = preg_split( '`([\{\}\$\[\] ])`', $string, -1, PREG_SPLIT_DELIM_CAPTURE );

$is_variable = false;
$has_braces = false;
$braces = 0;

foreach ( $output as $i => $part ) {
if ( in_array( $part, array( '$', '{' ), true ) ) {
$is_variable = true;
if ( '{' === $part ) {
$has_braces = true;
$braces++;
}
continue;
}

if ( true === $is_variable ) {
if ( '[' === $part ) {
$has_braces = true;
$braces++;
}
if ( in_array( $part, array( '}', ']' ), true ) ) {
$braces--;
}
if ( false === $has_braces && ' ' === $part ) {
$is_variable = false;
$output[ $i ] = $this->transform( $part, $regex, $transform_type );
}

if ( ( true === $has_braces && 0 === $braces ) && false === in_array( $output[ ( $i + 1 ) ], array( '{', '[' ), true ) ) {
$has_braces = false;
$is_variable = false;
}
continue;
}

$output[ $i ] = $this->transform( $part, $regex, $transform_type );
}

return implode( '', $output );
} // end transform_complex_string()

} // end class
17 changes: 17 additions & 0 deletions WordPress/Tests/NamingConventions/ValidHookNameUnitTest.1.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

// @codingStandardsChangeSetting WordPress.NamingConventions.ValidHookName additionalWordDelimiters -

// These should now be ok.
do_action( "admin_head-$hook_suffix" );
do_action( 'admin_head-media-upload_popup' );
apply_filters( "bulk_actions-{$this->screen->id}", $this->_actions );
apply_filters( "current_theme-supports-{$feature}", true, $args, $_wp_theme_features[$feature] );

// These should still give warnings.
do_action( "admin_head*$hook_suffix" ); // Warning - use underscore.
do_action( 'admin_head.media.upload_popup' ); // Warning - use underscore.
apply_filters( "bulk_actions {$this->screen->id}", $this->_actions ); // Warning - use underscore.
apply_filters( "current_theme/supports-{$feature}", true, $args, $_wp_theme_features[$feature] ); // Warning - use underscore.

// @codingStandardsChangeSetting WordPress.NamingConventions.ValidHookName additionalWordDelimiters _
17 changes: 17 additions & 0 deletions WordPress/Tests/NamingConventions/ValidHookNameUnitTest.2.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

// @codingStandardsChangeSetting WordPress.NamingConventions.ValidHookName additionalWordDelimiters -/.

// These should now be ok.
do_action( "admin_head-$hook_suffix" );
do_action( 'admin_head.media-upload_popup' );
apply_filters( "bulk_actions-{$this->screen->id}", $this->_actions );
apply_filters( "current_theme/supports-{$feature}", true, $args, $_wp_theme_features[$feature] );

// These should still give warnings.
do_action( "admin_head*$hook_suffix" ); // Warning - use underscore.
do_action( 'admin_head&media+upload_popup' ); // Warning - use underscore.
apply_filters( "bulk_actions {$this->screen->id}", $this->_actions ); // Warning - use underscore.
apply_filters( "current_theme#supports-{$feature}", true, $args, $_wp_theme_features[$feature] ); // Warning - use underscore.

// @codingStandardsChangeSetting WordPress.NamingConventions.ValidHookName additionalWordDelimiters _
Loading