Fix PR comments

WordPress · Nov 25, 2022 · ef579fc · ef579fc
1 parent 91aa89a
commit ef579fc
Show file tree

Hide file tree

Showing 6 changed files with 247 additions and 127 deletions.
diff --git a/WordPress/Docs/WP/CapabilitiesStandard.xml b/WordPress/Docs/WP/CapabilitiesStandard.xml
@@ -5,11 +5,28 @@
     >
     <standard>
     <![CDATA[
+Capabilities passed should be valid capabilities (custom capabilities can be added in the ruleset).
+    ]]>
+    </standard>
+    <code_comparison>
+        <code title="Valid: a WP native or registered custom user capability is used.">
+        <![CDATA[
+if ( author_can( $post, <em>'manage_sites'</em> ) ) { }
+        ]]>
+        </code>
+        <code title="Invalid: unknown/unsupported user capability is used.">
+        <![CDATA[
+map_meta_cap( <em>'manage_site'</em>, $user->ID );
+        ]]>
+        </code>
+    </code_comparison>
+    <standard>
+    <![CDATA[
 Always use user capabilities instead of roles.
     ]]>
     </standard>
     <code_comparison>
-        <code title="Valid: User capability is used.">
+        <code title="Valid: user capability is used.">
         <![CDATA[
 add_options_page(
     esc_html__( 'Options', 'textdomain' ),
@@ -20,7 +37,7 @@ add_options_page(
 );
         ]]>
         </code>
-        <code title="Invalid: User role is used instead of a capability.">
+        <code title="Invalid: user role is used instead of a capability.">
         <![CDATA[
 add_posts_page(
     __( 'Post visibility', 'textdomain' ),
@@ -34,18 +51,18 @@ add_posts_page(
     </code_comparison>
     <standard>
     <![CDATA[
-Capabilities passed should be valid capabilities (custom capabilities can be added in the ruleset).
+Don't use deprecated capabilities.
     ]]>
     </standard>
     <code_comparison>
         <code title="Valid: a WP native or registered custom user capability is used.">
         <![CDATA[
-if ( author_can( $post, <em>'manage_sites'</em> ) ) { }
+if ( author_can( $post, <em>'read'</em> ) ) { }
         ]]>
         </code>
-        <code title="Invalid: unknown/unsupported user capability is used.">
+        <code title="Invalid: deprecated user capability is used.">
         <![CDATA[
-map_meta_cap( <em>'manage_site'</em>, $user->ID );
+if ( author_can( $post, <em>'level_6'</em> ) ) { }
         ]]>
         </code>
     </code_comparison>

diff --git a/WordPress/Sniffs/WP/CapabilitiesSniff.php b/WordPress/Sniffs/WP/CapabilitiesSniff.php
@@ -19,7 +19,7 @@
 /**
  * Check that capabilities are used correctly.
  *
- * User capabilities should be used not roles nor deprecated capabilities.
+ * User capabilities should be used, not roles or deprecated capabilities.
  *
  * @package WPCS\WordPressCodingStandards
  *
@@ -318,8 +318,8 @@ class CapabilitiesSniff extends AbstractFunctionParameterSniff {
 	 * List of deprecated core capabilities.
 	 *
 	 * User Levels were deprecated in version 3.0.
-	 * To be updated after every major release.
-	 * Last updated for WordPress 6.1.0.
+	 *
+	 * {@internal To be updated after every major release. Last updated for WordPress 6.1.0.}
 	 *
 	 * @link https://github.com/WordPress/wordpress-develop/blob/master/tests/phpunit/tests/user/capabilities.php
 	 *
@@ -396,24 +396,26 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
 			return;
 		}
 
+		/*
+		 * As of this point we know that the `$capabilities` parameter only contains the one token
+		 * and that that token is a `T_CONSTANT_ENCAPSED_STRING`.
+		 */
 		$matched_parameter = TextStrings::stripQuotes( $this->tokens[ $first_non_empty ]['content'] );
 
+		if ( isset( $this->core_capabilities[ $matched_parameter ] ) ) {
+			return;
+		}
+
 		if ( empty( $matched_parameter ) ) {
 			$this->phpcsFile->addError(
-				'Found empty capability parameter in function call "%s()"',
+				'Found an empty capability parameter in the function call "%s()"',
 				$first_non_empty,
 				'Invalid',
-				array(
-					$matched_content,
-				)
+				array( $matched_content )
 			);
 			return;
 		}
 
-		if ( isset( $this->core_capabilities[ $matched_parameter ] ) ) {
-			return;
-		}
-
 		// Check if additional capabilities were registered via the ruleset and if the found capability matches any of those.
 		$custom_capabilities = $this->merge_custom_array( $this->custom_capabilities, array() );
 		if ( isset( $custom_capabilities[ $matched_parameter ] ) ) {
@@ -422,24 +424,28 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
 
 		if ( isset( $this->deprecated_capabilities[ $matched_parameter ] ) ) {
 			$this->get_wp_version_from_cli( $this->phpcsFile );
+			$is_error = version_compare( $this->deprecated_capabilities[ $matched_parameter ], $this->minimum_supported_version, '<' );
+
+			$data = array(
+				$matched_parameter,
+				$matched_content,
+				$this->deprecated_capabilities[ $matched_parameter ],
+			);
+
 			MessageHelper::addMessage(
 				$this->phpcsFile,
-				'The capability "%s" found in function call "%s()" has been deprecated since WordPress version %s.',
+				'The capability "%s", found in the function call "%s()", has been deprecated since WordPress version %s.',
 				$first_non_empty,
-				version_compare( $this->deprecated_capabilities[ $matched_parameter ], $this->minimum_supported_version, '<' ),
+				$is_error,
 				'Deprecated',
-				array(
-					$matched_parameter,
-					$matched_content,
-					$this->deprecated_capabilities[ $matched_parameter ],
-				)
+				$data
 			);
 			return;
 		}
 
 		if ( isset( $this->core_roles[ $matched_parameter ] ) ) {
 			$this->phpcsFile->addError(
-				'Capabilities should be used instead of roles. Found "%s" in function call "%s()"',
+				'Capabilities should be used instead of roles. Found "%s" in a function call "%s()"',
 				$first_non_empty,
 				'RoleFound',
 				array(
@@ -451,7 +457,7 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
 		}
 
 		$this->phpcsFile->addWarning(
-			'"%s" is an unknown role or capability. Check the "%s()" function call to ensure it is a capability and not a role.',
+			'"%s" is an unknown capability. Check the "%s()" function call, and that the capability is registered with WordPress. If this is a custom capability, make sure you have passed it to the custom_capabilities sniff property.',
 			$first_non_empty,
 			'Unknown',
 			array(

diff --git a/WordPress/Tests/WP/CapabilitiesUnitTest.inc → ...Press/Tests/WP/CapabilitiesUnitTest.1.inc b/WordPress/Tests/WP/CapabilitiesUnitTest.inc → ...Press/Tests/WP/CapabilitiesUnitTest.1.inc
@@ -1,5 +1,66 @@
 <?php
-add_posts_page( 'page_title', 'menu_title', 'admin' . 'istrator', 'menu_slug', 'function' ); // Warning.
+/*
+ * These shouldn't be flagged.
+ */
+if ( author_can( $post, 'read' ) ) { } // OK.
+map_meta_cap( 'edit_posts', $user->ID ); // OK.
+
+/*
+ * Low severity warnings, usually these need to be manually checked.
+ */
+add_posts_page( 'page_title', 'menu_title', 'admin' . 'istrator', 'menu_slug', 'function' ); // Low severity warning.
+if ( author_can( $post, $capability ) ) { } // Low severity warning.
+add_submenu_page(
+	'parent_slug',
+	'page_title',
+	'menu_title',
+	$variable, // Low severity warning.
+	'menu_slug',
+	'function'
+);
+add_menu_page( $pagetitle, $menu_title, $subscriber, 'handle', 'function', 'icon_url' ); // Low severity warning.
+add_plugins_page( 'page_title', 'menu_title', $cap, 'menu_slug', 'function' ); // Low severity warning.
+add_options_page( $pagetitle, $menu_title, CONSTANT, 'menu_slug', 'function' ); // Low severity warning.
+add_posts_page( 'page_title', 'menu_title', self /* comment */   ::   CAPABILITY, 'menu_slug', 'function' ); // Low severity warning.
+add_posts_page( 'page_title', 'menu_title', 'admin' /* comment */   . 'istrator', 'menu_slug', 'function' ); // Low severity warning.
+add_menu_page(
+	$p,
+	$t, // Comment.
+	$capability, // Low severity warning.
+);
+add_menu_page( $p, $t, 'admin' . 'istrator' ); // Low severity warning.
+add_menu_page($p, $t, $caps['level'] ); // Low severity warning.
+
+// Parse error, but just making sure we account for all possibilities.
+add_menu_page($p, $t, 'level_' 'level' ); // Low severity warning.
+
+/*
+ * Empty capability parameter.
+ */
+if ( author_can( $post, '' ) ) { } // Error.
+
+/*
+ * Deprecated capabilities
+ */
+// phpcs:set WordPress.WP.Capabilities minimum_supported_version 2.9.1
+if ( author_can( $post, 'level_3' ) ) { } // Warning.
+
+// phpcs:set WordPress.WP.Capabilities minimum_supported_version 5.9.0
+if ( author_can( $post, 'level_5' ) ) { } // Error.
+
+/*
+ * Unknown capabilities, could be that they need to be set in the property, but weren't.
+ */
+if ( author_can( $post, 'custom_cap' ) ) { } // Warning.
+if ( current_user_can( 'foo_bar' ) ) { } // Warning.
+if ( current_user_can_for_blog( '3', 'custom_cap' ) ) { } // Warning.
+add_users_page( 'page_title', 'menu_title', 'foo_bar', 'menu_slug', 'function' ); // Warning.
+add_management_page( 'page_title', 'menu_title', 'foo_bar', 'menu_slug', 'function' ); // Warning.
+add_menu_page( $pagetitle, 'menu_title', 'foo_bar', 'handle', 'function', 'icon_url' ); // Warning.
+
+/*
+ * Roles found instead of capabilities.
+ */
 add_posts_page( 'page_title', 'menu_title', 'administrator', 'menu_slug', 'function' ); // Error.
 add_media_page( 'page_title', 'menu_title', 'editor', 'menu_slug', 'function' ); // Error.
 add_pages_page( 'page_title', 'menu_title', 'author', 'menu_slug', 'function' ); // Error.
@@ -27,62 +88,30 @@ add_utility_page(
 	,'icon_url'
 );
 
+// PHP 8.0 named parameters support.
+add_menu_page( capability: 'foobar', page_title: $p, menu_title: $m ); // Warning.
 
-add_menu_page( $pagetitle, $menu_title, $subscriber, 'handle', 'function', 'icon_url' ); // Warning.
-add_plugins_page( 'page_title', 'menu_title', $cap, 'menu_slug', 'function' ); // Warning.
-add_users_page( 'page_title', 'menu_title', 'foo_bar', 'menu_slug', 'function' ); // Warning.
-add_management_page( 'page_title', 'menu_title', 'foo_bar', 'menu_slug', 'function' ); // Warning.
-add_options_page( $pagetitle, $menu_title, CONSTANT, 'menu_slug', 'function' ); // Warning.
-add_menu_page( $pagetitle, 'menu_title', 'foo_bar', 'handle', 'function', 'icon_url' ); // Warning.
-if ( author_can( $post, $capability ) ) { } // Warning.
-add_submenu_page(
-	'parent_slug',
-	'page_title',
-	'menu_title',
-	$variable, // Warning.
-	'menu_slug',
-	'function'
-);
-if ( author_can( $post, 'custom_cap' ) ) { } // Warning.
-
+/*
+ * Testing handling of the custom capabilities properties.
+ */
 // phpcs:set WordPress.WP.Capabilities custom_capabilities[] custom_cap,foo_bar
 if ( current_user_can( 'foo_bar' ) ) { } // OK.
 if ( author_can( $post, 'custom_cap' ) ) { } // OK.
+if ( author_can( $post, 'custom_capability' ) ) { } // Warning.
 
 // phpcs:set WordPress.WP.Capabilities custom_capabilities[]
 
-if ( current_user_can( 'foo_bar' ) ) { } // Warning.
-if ( current_user_can_for_blog( '3', 'custom_cap' ) ) { } // Warning.
-
-
-
-if ( author_can( $post, 'read' ) ) { } // OK.
-if ( author_can( $post, '' ) ) { } // Error.
+// Making sure that the warnings and errors are showing up in the case where we unset the custom capabilities.
 if ( author_can( $post, 'custom_cap' ) ) { } // Warning.
-
-map_meta_cap( 'edit_posts', $user->ID ); // OK.
 map_meta_cap( 'editor', $user->ID ); // Error.
 
-add_menu_page(
-	$p,
-	$t, // Comment.
-	$capability, // Low severity warning.
-);
-
-add_menu_page( $p, $t, 'admin' . 'istrator' ); // Low severity warning.
-
-// PHP 8.0 named parameters support.
-add_menu_page( capability: 'foobar', page_title: $p, menu_title: $m ); // Warning.
-
-add_menu_page($p, $t, $caps['level'] ); // Low severity warning.
-
-add_menu_page($p, $t, 'level_' . $level ); // Low severity warning.
-
-// Parse error, but just making sure we account for all possibilities.
-add_menu_page($p, $t, 'level_' 'level' ); // Low severity warning.
-
 // Another parse error, but the sniff should still handle this correctly (by bowing out).
 add_menu_page( $p, $t, /* deliberately empty */, $slug, );
 
-add_posts_page( 'page_title', 'menu_title', self /* comment */   ::   CAPABILITY, 'menu_slug', 'function' ); // Warning.
-add_posts_page( 'page_title', 'menu_title', 'admin' /* comment */   . 'istrator', 'menu_slug', 'function' ); // Warning.
+add_menu_page( [] ); // Should bow out because the parameter is not found.
+
+$obj->current_user_can( 'foo_bar' ); // Ok. We're not checking the method calls
+My\NamespaceS\add_posts_page( 'page_title', 'menu_title', 'administrator', 'menu_slug', 'function' ); // Ok. We're not checking namespaced functions.
+
+// Parse error, should be handled correctly by bowing out.
+add_posts_page( 'page_title',
diff --git a/WordPress/Tests/WP/CapabilitiesUnitTest.2.inc b/WordPress/Tests/WP/CapabilitiesUnitTest.2.inc
@@ -0,0 +1,14 @@
+<?php
+
+if ( author_can( $post, 'read' ) ) { } // OK.
+
+/*
+ * Deprecated capabilities - checking if setting from the CLI will work as well.
+ *
+ * WordPress version: 2.9.0
+ */
+if ( author_can( $post, 'level_3' ) ) { } // Warning.
+
+if ( author_can( $post, 'level_5' ) ) { } // Warning.
+
+add_options_page( 'page_title', 'menu_title', 'level_10', 'menu_slug', 'function' ); // Warning.
diff --git a/WordPress/Tests/WP/CapabilitiesUnitTest.3.inc b/WordPress/Tests/WP/CapabilitiesUnitTest.3.inc
@@ -0,0 +1,14 @@
+<?php
+
+if ( author_can( $post, 'read' ) ) { } // OK.
+
+/*
+ * Deprecated capabilities - checking if setting from the CLI will work as well.
+ *
+ * WordPress version: 6.1.0
+ */
+if ( author_can( $post, 'level_3' ) ) { } // Error.
+
+if ( author_can( $post, 'level_5' ) ) { } // Error.
+
+add_options_page( 'page_title', 'menu_title', 'level_10', 'menu_slug', 'function' ); // Error.