diff --git a/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php b/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php index ae98644b..4f859fc4 100644 --- a/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php +++ b/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php @@ -11,7 +11,9 @@ namespace WordPressVIPMinimum\Sniffs\UserExperience; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\FilePath; use PHPCSUtils\Utils\GetTokensAsString; +use PHPCSUtils\Utils\PassedParameters; use PHPCSUtils\Utils\TextStrings; use WordPressCS\WordPress\AbstractFunctionParameterSniff; @@ -52,6 +54,7 @@ class AdminBarRemovalSniff extends AbstractFunctionParameterSniff { protected $target_functions = [ 'show_admin_bar' => true, 'add_filter' => true, + 'add_action' => true, // Alias of add_filter(). ]; /** @@ -156,8 +159,8 @@ public function register() { */ public function process_token( $stackPtr ) { - $file_name = $this->phpcsFile->getFilename(); - $file_extension = substr( strrchr( $file_name, '.' ), 1 ); + $file_name = FilePath::getName( $this->phpcsFile ); + $file_extension = pathinfo( $file_name, \PATHINFO_EXTENSION ); if ( $file_extension === 'css' ) { if ( $this->tokens[ $stackPtr ]['code'] === \T_STYLE ) { @@ -200,21 +203,42 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p $error = false; switch ( $matched_content ) { case 'show_admin_bar': + $show_param = PassedParameters::getParameterFromStack( $parameters, 1, 'show' ); + if ( $show_param === false ) { + break; + } + $error = true; - if ( $this->remove_only === true && $parameters[1]['raw'] === 'true' ) { + if ( $this->remove_only === true && $show_param['clean'] === 'true' ) { $error = false; } break; + case 'add_action': case 'add_filter': - $filter_name = TextStrings::stripQuotes( $parameters[1]['raw'] ); + $hook_name_param = PassedParameters::getParameterFromStack( $parameters, 1, 'hook_name' ); + if ( $hook_name_param === false ) { + break; + } + + $filter_name = TextStrings::stripQuotes( $hook_name_param['clean'] ); if ( $filter_name !== 'show_admin_bar' ) { break; } - $error = true; - if ( $this->remove_only === true && isset( $parameters[2]['raw'] ) && TextStrings::stripQuotes( $parameters[2]['raw'] ) === '__return_true' ) { - $error = false; + $callback_param = PassedParameters::getParameterFromStack( $parameters, 2, 'callback' ); + $error = true; + if ( $this->remove_only === true && $callback_param !== false ) { + $clean_param = strtolower( TextStrings::stripQuotes( $callback_param['clean'] ) ); + + $expected = Tokens::$emptyTokens + Tokens::$stringTokens; + $has_non_textstring = $this->phpcsFile->findNext( $expected, $callback_param['start'], ( $callback_param['end'] + 1 ), true ); + if ( ( $has_non_textstring === false && $clean_param === '__return_true' ) + || ( $has_non_textstring !== false + && preg_match( '`^\\\\?__return_true\s*\(\s*\.\.\.\s*\)$`', $clean_param ) === 1 ) + ) { + $error = false; + } } break; } diff --git a/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.inc b/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.inc index f452ae83..abc0a248 100644 --- a/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.inc +++ b/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.inc @@ -107,3 +107,77 @@ EOT; show_admin_bar( false ); // OK. +$this?->show_admin_bar( false ); // OK. +MyClass::add_filter( 'show_admin_bar', '__return_false' ); // OK. +echo ADD_FILTER; // OK. +namespace\add_filter( 'show_admin_bar', '__return_false' ); // OK. + +#[Show_Admin_Bar(false)] // OK. PHP 8.0+ class instantiation via an attribute. Can't contain a nested function call anyway. +function foo() {} + +array_walk($filters, \add_filter(...),); // OK. PHP 8.1 first class callable. + +// Incomplete function calls, should be ignored by the sniff. +$incorrect_but_ok = show_admin_bar(); // OK. +$incorrect_but_ok = add_filter(); // OK. + +// Safeguard that the sniff only flags the "show_admin_bar" filter. +add_filter( 'not_show_admin_bar', '__return_false', ); // OK. + +// Document that dynamic values will be flagged. +show_admin_bar( $unknown ); // Bad. +add_filter( 'show_admin_bar', $callable, ); // Bad. +add_filter('show_admin_bar', ...$params); // Bad. PHP 5.6 argument unpacking. + +// Document that fully qualified function calls and functions in unconventional case will correctly be recognized. +\add_filter( "show_admin_bar", '__return_true' ); // OK. +\Add_Filter( 'show_admin_bar', "__return_false", ); // Bad. + +\show_Admin_bar( true, ); // OK. +\show_admin_bar( false ); // Bad. +\SHOW_ADMIN_BAR( false, ); // Bad. + +// Comments in parameters should be ignored. +show_admin_bar( true /* turn it on */ ); // OK. +add_filter( + // Admin bar gives access to admin for users with the right permissions. + 'show_admin_bar', + '__return_false' +); // Bad. +add_filter( + 'show_admin_bar', + // Turn it on. + '__return_true' +); // OK. + +// Safeguard handling of function calls using PHP 8.0+ named parameters. +show_admin_bar( shown: false ); // OK, well not really, typo in param name, but that's not the concern of the sniff. +\show_admin_bar( show: true ); // OK. +show_admin_bar( show: $toggle ); // Bad. + +add_filter(callback: '__return_false', priority: 10); // OK, well, not really, missing required $hook_name param, but that's not the concern of this sniff. +\add_filter(callback: '__return_false', hook_name: 'not_our_target'); // OK. +add_filter(hookName: 'show_admin_bar', callback: '__return_false',); // OK, well, not really, typo in param name, but that's not the concern of the sniff. +add_filter( callback: '__return_true', hook_name: 'show_admin_bar', ); // Ok. +\add_filter( callback: '__return_false', hook_name: 'show_admin_bar', ); // Bad. + +// Bug: add_action() is an alias of add_filter. +add_action( 'show_admin_bar', $callable, ); // Bad. + +// Safeguard handling of filter callback being passed as PHP 8.1+ first class callable. +add_filter( 'show_admin_bar', __return_true(...) ); // OK. +add_filter( 'show_admin_bar', \__return_true( ... ) ); // OK. +add_action( 'show_admin_bar', __return_false ( ... ) , ); // Bad. +add_filter( 'show_admin_bar', \__return_false(...) ); // Bad. +add_filter( 'show_admin_bar', "__return_true(...)" ); // Bad. Invalid callback. + +// Bug fix: function names are case-insensitive. +\add_filter( "show_admin_bar", '__Return_TRUE' ); // OK. +add_filter( 'show_admin_bar', __Return_True(...) ); // OK. diff --git a/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.php b/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.php index 8973a169..88f7e823 100644 --- a/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.php +++ b/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.php @@ -51,6 +51,19 @@ public function getErrorList( $testFile = '' ) { 103 => 1, 104 => 1, 105 => 1, + 135 => 1, + 136 => 1, + 137 => 1, + 141 => 1, + 144 => 1, + 145 => 1, + 149 => 1, + 163 => 1, + 169 => 1, + 172 => 1, + 177 => 1, + 178 => 1, + 179 => 1, ]; case 'AdminBarRemovalUnitTest.css': @@ -85,15 +98,6 @@ public function getErrorList( $testFile = '' ) { * @return array Key is the line number, value is the number of expected warnings. */ public function getWarningList( $testFile = '' ) { - switch ( $testFile ) { - case 'AdminBarRemovalUnitTest.css': - return []; - - case 'AdminBarRemovalUnitTest.inc': - return []; - - default: - return []; - } + return []; } }