PHPCS: Enable cognitive complexity rule (#12931)

Co-authored-by: Pascal Birchler <pascalb@google.com> Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
GoogleForCreators · Jan 16, 2023 · c2b0521 · c2b0521
1 parent c15b362
commit c2b0521
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 76 deletions.
diff --git a/includes/AMP/Sanitization.php b/includes/AMP/Sanitization.php
@@ -138,7 +138,7 @@ public function validation_error_callback( array $error, array $data = [] ): boo
 	 * @param Document             $document Document instance.
 	 * @param array<string,string> $scripts  List of found scripts.
 	 */
-	protected function ensure_required_markup( Document $document, array $scripts ): void {
+	protected function ensure_required_markup( Document $document, array $scripts ): void { // phpcs:ignore SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh
 		/**
 		 * Link elements.
 		 *

diff --git a/includes/Admin/Cross_Origin_Isolation.php b/includes/Admin/Cross_Origin_Isolation.php
@@ -255,7 +255,7 @@ private function needs_isolation(): bool {
 	 * @param string $html HTML document as string.
 	 * @return string Processed HTML document.
 	 */
-	protected function replace_in_dom( string $html ): string {
+	protected function replace_in_dom( string $html ): string { // phpcs:ignore SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh
 		$site_url = site_url();
 
 		// See https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/crossorigin.

diff --git a/includes/KSES.php b/includes/KSES.php
@@ -206,7 +206,7 @@ public function filter_safe_style_css( $attr ) {
 	 * @param string $css A string of CSS rules.
 	 * @return string Filtered string of CSS rules.
 	 */
-	public function safecss_filter_attr( $css ): string {
+	public function safecss_filter_attr( $css ): string { // phpcs:ignore SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh
 		$css = wp_kses_no_null( $css );
 		$css = str_replace( [ "\n", "\r", "\t" ], '', $css );
 

diff --git a/includes/REST_API/Embed_Controller.php b/includes/REST_API/Embed_Controller.php
@@ -346,8 +346,6 @@ private function get_data_from_post( string $url ) {
 	 * Checks are supposedly from the hosted site blog.
 	 *
 	 * @SuppressWarnings(PHPMD.NPathComplexity)
-	 * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
-	 * @SuppressWarnings(PHPMD.CyclomaticComplexity)
 	 *
 	 * @since 1.2.0
 	 *
@@ -359,59 +357,8 @@ private function get_data_from_post( string $url ) {
 	 */
 	private function url_to_post( $url ): ?WP_Post {
 		$post          = null;
-		$switched_blog = false;
-
-		if ( is_multisite() ) {
-			/**
-			 * URL parts.
-			 *
-			 * @var array<string, string>|false $url_parts
-			 */
-			$url_parts = wp_parse_url( $url );
-			if ( ! $url_parts ) {
-				$url_parts = [];
-			}
-
-			$url_parts = wp_parse_args(
-				$url_parts,
-				[
-					'host' => '',
-					'path' => '/',
-				]
-			);
-
-			$qv = [
-				'domain'                 => $url_parts['host'],
-				'path'                   => '/',
-				'number'                 => 1,
-				'update_site_cache'      => false,
-				'update_site_meta_cache' => false,
-			];
-
-			// In case of subdirectory configs, set the path.
-			if ( ! is_subdomain_install() ) {
-				// Get "sub-site" part of "http://example.org/sub-site/web-stories/my-story/".
-				// But given just "http://example.org/web-stories/my-story/", don't treat "web-stories" as site path.
-				// This differs from the logic in get_oembed_response_data_for_url() which does not do this.
-				// TODO: Investigate possible core bug in get_oembed_response_data_for_url()?
-				$path    = explode( '/', ltrim( $url_parts['path'], '/' ) );
-				$path    = \count( $path ) > 2 ? reset( $path ) : false;
-				$network = get_network();
-				if ( $path && $network instanceof WP_Network ) {
-					$qv['path'] = $network->path . $path . '/';
-				}
-			}
-
-			$sites = (array) get_sites( $qv );
-			$site  = reset( $sites );
+		$switched_blog = $this->maybe_switch_site( $url );
 
-			if ( $site && get_current_blog_id() !== (int) $site->blog_id ) {
-				// phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.switch_to_blog_switch_to_blog
-				switch_to_blog( (int) $site->blog_id );
-
-				$switched_blog = true;
-			}
-		}
 
 		if ( function_exists( 'wpcom_vip_url_to_postid' ) ) {
 			$post_id = wpcom_vip_url_to_postid( $url );
@@ -480,6 +427,72 @@ private function url_to_post( $url ): ?WP_Post {
 		return $post;
 	}
 
+	/**
+	 * Maybe switch to site.
+	 *
+	 * @since 1.29.0
+	 *
+	 * @param string $url Permalink to check.
+	 */
+	private function maybe_switch_site( $url ): bool {
+		if ( ! is_multisite() ) {
+			return false;
+		}
+		$switched_blog = false;
+
+		/**
+		 * URL parts.
+		 *
+		 * @var array<string, string>|false $url_parts
+		 */
+		$url_parts = wp_parse_url( $url );
+		if ( ! $url_parts ) {
+			$url_parts = [];
+		}
+
+		$url_parts = wp_parse_args(
+			$url_parts,
+			[
+				'host' => '',
+				'path' => '/',
+			]
+		);
+
+		$qv = [
+			'domain'                 => $url_parts['host'],
+			'path'                   => '/',
+			'number'                 => 1,
+			'update_site_cache'      => false,
+			'update_site_meta_cache' => false,
+		];
+
+		// In case of subdirectory configs, set the path.
+		if ( ! is_subdomain_install() ) {
+			// Get "sub-site" part of "http://example.org/sub-site/web-stories/my-story/".
+			// But given just "http://example.org/web-stories/my-story/", don't treat "web-stories" as site path.
+			// This differs from the logic in get_oembed_response_data_for_url() which does not do this.
+			// TODO: Investigate possible core bug in get_oembed_response_data_for_url()?
+			$path    = explode( '/', ltrim( $url_parts['path'], '/' ) );
+			$path    = \count( $path ) > 2 ? reset( $path ) : false;
+			$network = get_network();
+			if ( $path && $network instanceof WP_Network ) {
+				$qv['path'] = $network->path . $path . '/';
+			}
+		}
+
+		$sites = (array) get_sites( $qv );
+		$site  = reset( $sites );
+
+		if ( $site && get_current_blog_id() !== (int) $site->blog_id ) {
+			// phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.switch_to_blog_switch_to_blog
+			switch_to_blog( (int) $site->blog_id );
+
+			$switched_blog = true;
+		}
+
+		return $switched_blog;
+	}
+
 	/**
 	 * Parses an HTML document to and returns the story's title and poster.
 	 *

diff --git a/includes/REST_API/Hotlinking_Controller.php b/includes/REST_API/Hotlinking_Controller.php
@@ -721,7 +721,7 @@ private function proxy_url_fallback( string $url, array $args ): void {
 	 * @param string $url Request URL.
 	 * @return string|false Original URL, resolved IP address, or false on failure.
 	 */
-	private function validate_url( string $url ) {
+	private function validate_url( string $url ) { // phpcs:ignore SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh
 		if ( '' === $url || is_numeric( $url ) ) {
 			return false;
 		}

diff --git a/includes/REST_API/Products_Controller.php b/includes/REST_API/Products_Controller.php
@@ -267,7 +267,7 @@ public function get_items( $request ) {
 	 * @param WP_REST_Request $request Request object.
 	 * @return WP_REST_Response Response object.
 	 */
-	public function prepare_item_for_response( $item, $request ): WP_REST_Response {
+	public function prepare_item_for_response( $item, $request ): WP_REST_Response { // phpcs:ignore SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh
 		$product = $item;
 		$fields  = $this->get_fields_for_response( $request );
 

diff --git a/includes/REST_API/Stories_Controller.php b/includes/REST_API/Stories_Controller.php
@@ -80,7 +80,7 @@ class Stories_Controller extends Stories_Base_Controller {
 	 * @param WP_REST_Request $request Request object.
 	 * @return WP_REST_Response Response object.
 	 */
-	public function prepare_item_for_response( $post, $request ): WP_REST_Response {
+	public function prepare_item_for_response( $post, $request ): WP_REST_Response { // phpcs:ignore SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh
 		/**
 		 * Request context.
 		 *

diff --git a/phpcs.xml.dist b/phpcs.xml.dist
@@ -197,14 +197,14 @@
   </rule>
 
   <!-- Requires arrow functions if possible. -->
-  <rule ref="SlevomatCodingStandard.Functions.RequireArrowFunction"/>
+  <rule ref="SlevomatCodingStandard.Functions.RequireArrowFunction" />
   <!-- Enforces correct space usage in array functions. -->
   <rule ref="SlevomatCodingStandard.Functions.ArrowFunctionDeclaration">
     <properties>
-      <property name="spacesCountAfterKeyword" value="0"/>
-      <property name="spacesCountBeforeArrow" value="1"/>
-      <property name="spacesCountAfterArrow" value="1"/>
-      <property name="allowMultiLine" value="true"/>
+      <property name="spacesCountAfterKeyword" value="0" />
+      <property name="spacesCountBeforeArrow" value="1" />
+      <property name="spacesCountAfterArrow" value="1" />
+      <property name="allowMultiLine" value="true" />
     </properties>
   </rule>
   <rule ref="SlevomatCodingStandard.Functions.StaticClosure" />
@@ -227,29 +227,42 @@
   <rule ref="SlevomatCodingStandard.Classes.ClassStructure">
     <properties>
       <property name="groups" type="array">
-        <element value="uses"/>
+        <element value="uses" />
 
-        <element value="enum cases"/>
+        <element value="enum cases" />
 
         <!-- Public constants are first but you don't care about the order of protected or private constants -->
-        <element value="public constants"/>
-        <element value="constants"/>
+        <element value="public constants" />
+        <element value="constants" />
 
         <!-- You don't care about the order among the properties. The same can be done with "properties" shortcut -->
-        <element value="public static properties, protected static properties, private static properties"/>
-        <element value="public properties, protected properties, private properties"/>
+        <element
+          value="public static properties, protected static properties, private static properties"
+        />
+        <element
+          value="public properties, protected properties, private properties"
+        />
 
         <!-- Constructor is first, then all public methods, then protected/private methods and magic methods are last -->
-        <element value="constructor"/>
-        <element value="all public methods"/>
-        <element value="methods"/>
-        <element value="magic methods"/>
+        <element value="constructor" />
+        <element value="all public methods" />
+        <element value="methods" />
+        <element value="magic methods" />
       </property>
     </properties>
   </rule>
 
   <rule ref="SlevomatCodingStandard.Variables">
-    <exclude name="SlevomatCodingStandard.Variables.DisallowSuperGlobalVariable" />
+    <exclude
+      name="SlevomatCodingStandard.Variables.DisallowSuperGlobalVariable"
+    />
+  </rule>
+
+  <!-- Enforces maximum cognitive complexity for functions.. -->
+  <rule ref="SlevomatCodingStandard.Complexity.Cognitive">
+    <properties>
+      <property name="maxComplexity" value="25" />
+    </properties>
   </rule>
 
   <!-- Requires use of null coalesce operator (??) when possible. -->