Revisit PHPStan fixes to WP_HTML_Processor with reverts, ignores, and fixes

phpstan.neon.dist

@@ -33,7 +33,6 @@ includes:
 	- tests/phpstan/baselines/booleanNot.alwaysTrue.neon
 	- tests/phpstan/baselines/booleanOr.alwaysFalse.neon
 	- tests/phpstan/baselines/booleanOr.alwaysTrue.neon
-	- tests/phpstan/baselines/catch.neverThrown.neon
 	- tests/phpstan/baselines/class.notFound.neon
 	- tests/phpstan/baselines/deadCode.unreachable.neon
 	- tests/phpstan/baselines/empty.offset.neon
@@ -163,3 +162,31 @@ parameters:
 			identifier: greaterOrEqual.alwaysFalse
 			count: 1
 			path: src/wp-includes/html-api/class-wp-html-processor.php
+		-
+			message: '#^Left side of && is always true\.$#'
+			identifier: booleanAnd.leftAlwaysTrue
+			count: 1
+			path: src/wp-includes/html-api/class-wp-html-processor.php
+		-
+			message: '#^Unreachable statement \- code above always terminates\.$#'
+			identifier: deadCode.unreachable
+			count: 15
+			path: src/wp-includes/html-api/class-wp-html-processor.php
+		-
+			message: '#^If condition is always true\.$#'
+			identifier: if.alwaysTrue
+			count: 2
+			path: src/wp-includes/html-api/class-wp-html-processor.php
+		-
+			message: '#^Dead catch \- Exception is never thrown in the try block\.$#'
+			identifier: catch.neverThrown
+			count: 1
+			path: src/wp-includes/html-api/class-wp-html-processor.php
+		-
+			# The early return at the start of step() narrows $last_error to null, and PHPStan does not see that
+			# bookmark_token() sets it to ERROR_EXCEEDED_MAX_BOOKMARKS before throwing. So it reports that the
+			# check in the catch block can never match, when in fact it always does.
+			message: '#^Strict comparison using \=\=\= between ''exceeded\-max…'' and null will always evaluate to false\.$#'
+			identifier: identical.alwaysFalse
+			count: 1
+			path: src/wp-includes/html-api/class-wp-html-processor.php

src/wp-includes/html-api/class-wp-html-processor.php

@@ -2773,27 +2773,28 @@ private function step_in_body(): bool {
 
 					$this->state->stack_of_open_elements->remove_node( $node );
 					return true;
-				}
+				} else {
+					/*
+					 * > If the stack of open elements does not have a form element in scope,
+					 * > then this is a parse error; return and ignore the token.
+					 *
+					 * Note that unlike in the clause above, this is checking for any FORM in scope.
+					 */
+					if ( ! $this->state->stack_of_open_elements->has_element_in_scope( 'FORM' ) ) {
+						// Parse error: ignore the token.
+						return $this->step();
+					}
 
-				/*
-				 * > If the stack of open elements does not have a form element in scope,
-				 * > then this is a parse error; return and ignore the token.
-				 *
-				 * Note that unlike in the clause above, this is checking for any FORM in scope.
-				 */
-				if ( ! $this->state->stack_of_open_elements->has_element_in_scope( 'FORM' ) ) {
-					// Parse error: ignore the token.
-					return $this->step();
-				}
+					$this->generate_implied_end_tags();
 
-				$this->generate_implied_end_tags();
+					if ( ! $this->state->stack_of_open_elements->current_node_is( 'FORM' ) ) {
+						// @todo Indicate a parse error once it's possible. This error does not impact the logic here.
+					}
 
-				if ( ! $this->state->stack_of_open_elements->current_node_is( 'FORM' ) ) {
-					// @todo Indicate a parse error once it's possible. This error does not impact the logic here.
+					$this->state->stack_of_open_elements->pop_until( 'FORM' );
+					return true;
 				}
-
-				$this->state->stack_of_open_elements->pop_until( 'FORM' );
-				return true;
+				break;
 
 			/*
 			 * > An end tag whose tag name is "p"
@@ -3355,6 +3356,8 @@ private function step_in_body(): bool {
 			 */
 			return $this->in_body_any_other_end_tag();
 		}
+
+		$this->bail( 'Should not have been able to reach end of IN BODY processing. Check HTML API code.' );
 	}
 
 	/**
@@ -3371,6 +3374,7 @@ private function step_in_body(): bool {
 	 * @return bool Whether an element was found.
 	 */
 	private function in_body_any_other_end_tag(): bool {
+		$node       = null;
 		$token_name = $this->get_token_name();
 
 		/*
@@ -4963,18 +4967,20 @@ private function step_in_foreign_content(): bool {
 				$this->state->stack_of_open_elements->pop();
 			}
 			return true;
-		} else {
-			/*
-			 * > An end tag whose name is "script", if the current node is an SVG script element.
-			 */
-			if ( 'SCRIPT' === $this->state->current_token->node_name && 'svg' === $this->state->current_token->namespace ) {
-				$this->state->stack_of_open_elements->pop();
-				return true;
-			}
+		}
 
-			/*
-			 * > Any other end tag
-			 */
+		/*
+		 * > An end tag whose name is "script", if the current node is an SVG script element.
+		 */
+		if ( $this->is_tag_closer() && 'SCRIPT' === $this->state->current_token->node_name && 'svg' === $this->state->current_token->namespace ) {
+			$this->state->stack_of_open_elements->pop();
+			return true;
+		}
+
+		/*
+		 * > Any other end tag
+		 */
+		if ( $this->is_tag_closer() ) {
 			$node = $this->state->stack_of_open_elements->current_node();
 			if ( $tag_name !== $node->node_name ) {
 				// @todo Indicate a parse error once it's possible.
@@ -5074,6 +5080,8 @@ private function step_in_foreign_content(): bool {
 					$this->bail( "Unaware of the requested parsing mode: '{$this->state->insertion_mode}'." );
 			}
 		}
+
+		$this->bail( 'Should not have been able to reach end of IN FOREIGN CONTENT processing. Check HTML API code.' );
 	}
 
 	/*
@@ -5092,7 +5100,7 @@ private function step_in_foreign_content(): bool {
 	 * @return string Name of created bookmark.
 	 */
 	private function bookmark_token() {
-		if ( ! parent::set_bookmark( ++$this->bookmark_counter ) ) {
+		if ( ! parent::set_bookmark( (string) ++$this->bookmark_counter ) ) {
 			$this->last_error = self::ERROR_EXCEEDED_MAX_BOOKMARKS;
 			throw new Exception( 'could not allocate bookmark' );
 		}

tests/phpstan/baselines/argument.type.neon

@@ -988,11 +988,6 @@ parameters:
 			identifier: argument.type
 			count: 1
 			path: ../../../src/wp-includes/general-template.php
-		-
-			message: '#^Parameter \#1 \$name of method WP_HTML_Tag_Processor\:\:set_bookmark\(\) expects string, int given\.$#'
-			identifier: argument.type
-			count: 1
-			path: ../../../src/wp-includes/html-api/class-wp-html-processor.php
 		-
 			message: '#^Parameter \#1 \$user_id of function get_userdata expects int, string given\.$#'
 			identifier: argument.type

tests/phpstan/baselines/deadCode.unreachable.neon

@@ -53,11 +53,6 @@ parameters:
 			identifier: deadCode.unreachable
 			count: 1
 			path: ../../../src/wp-includes/class-wp-block.php
-		-
-			message: '#^Unreachable statement \- code above always terminates\.$#'
-			identifier: deadCode.unreachable
-			count: 12
-			path: ../../../src/wp-includes/html-api/class-wp-html-processor.php
 		-
 			message: '#^Unreachable statement \- code above always terminates\.$#'
 			identifier: deadCode.unreachable

tests/phpstan/baselines/identical.alwaysFalse.neon

@@ -23,11 +23,6 @@ parameters:
 			identifier: identical.alwaysFalse
 			count: 1
 			path: ../../../src/wp-admin/plugins.php
-		-
-			message: '#^Strict comparison using \=\=\= between ''exceeded\-max…'' and null will always evaluate to false\.$#'
-			identifier: identical.alwaysFalse
-			count: 1
-			path: ../../../src/wp-includes/html-api/class-wp-html-processor.php
 		-
 			message: '#^Strict comparison using \=\=\= between 3000000000 and 2147483647 will always evaluate to false\.$#'
 			identifier: identical.alwaysFalse

tests/phpstan/baselines/if.alwaysTrue.neon

@@ -43,8 +43,3 @@ parameters:
 			identifier: if.alwaysTrue
 			count: 1
 			path: ../../../src/wp-content/themes/twentytwenty/template-parts/modal-menu.php
-		-
-			message: '#^If condition is always true\.$#'
-			identifier: if.alwaysTrue
-			count: 1
-			path: ../../../src/wp-includes/html-api/class-wp-html-processor.php

tests/phpstan/baselines/variable.undefined.neon

@@ -798,11 +798,6 @@ parameters:
 			identifier: variable.undefined
 			count: 1
 			path: ../../../src/wp-includes/general-template.php
-		-
-			message: '#^Variable \$node might not be defined\.$#'
-			identifier: variable.undefined
-			count: 2
-			path: ../../../src/wp-includes/html-api/class-wp-html-processor.php
 		-
 			message: '#^Variable \$current_blog might not be defined\.$#'
 			identifier: variable.undefined