UserExperience/AdminBarRemoval: various improvements

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;
 		}

WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.inc

@@ -107,3 +107,77 @@ EOT;
 </style>
 <?php
 /* phpcs:set WordPressVIPMinimum.UserExperience.AdminBarRemoval remove_only true */
+
+/*
+ * Safeguard against false positives for method calls and namespaced function calls
+ * and various other syntaxes.
+ */
+my\ns\show_admin_bar( false ); // OK.
+$this->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.

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<int, int> 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 [];
 	}
 }