From e860d7322f639aa6f65722fcbbcca9bce392eb28 Mon Sep 17 00:00:00 2001
From: Dennis Snell <dennis.snell@automattic.com>
Date: Wed, 20 Mar 2024 11:33:46 +0100
Subject: [PATCH] Generate hook names using parsed Node instead of
 pretty-printed name.

Previously, the hook-name printer was reyling on the parser's pretty
printer to generate names, and then post-processing the name to
extract string literal values and concatenations.

Unfortunately this left some cases unhandled, specifically the case
where concatenations are joining function calls with string literals.
Any number of other unexpected situations might arise, and there can
be defects in the redundant code attempting to parse PHP syntax.

In this patch the pretty-printed version of the expression is used
only as a fallback in unrecognized situations. Primarily, a direct
encoding from the parsed syntax tree to string is used to rely on
the parser's own handling of syntax, and making it clearer how to
add additional support for other syntaxes.
---
 lib/class-hook-reflector.php | 85 +++++++++++++++++++++++++-----------
 1 file changed, 59 insertions(+), 26 deletions(-)

diff --git a/lib/class-hook-reflector.php b/lib/class-hook-reflector.php
index 50075e0..9a58c21 100644
--- a/lib/class-hook-reflector.php
+++ b/lib/class-hook-reflector.php
@@ -11,40 +11,73 @@
 class Hook_Reflector extends BaseReflector {
 
 	/**
-	 * @return string
-	 */
-	public function getName() {
-		$printer = new PHPParser_PrettyPrinter_Default;
-		return $this->cleanupName( $printer->prettyPrintExpr( $this->node->args[0]->value ) );
-	}
-
-	/**
-	 * @param string $name
+	 * Hook names are the first argument to actions and filters.
+	 *
+	 * These are expected to be string values or concatenations of strings and variables.
+	 *
+	 * Example:
 	 *
+	 *     from: apply_filters( 'option_' . $option_name, $option_value );
+	 *     name: option_{$option_name}
+	 *
+	 *     from: do_action( 'wp_insert_post', $post_ID, $post, true );
+	 *     name: wp_insert_post
+	 *
+	 *     from: do_action( "{$old_status}_to_{$new_status}", $post );
+	 *     name: {$old_status}_to_{$new_status}
+	 *
+	 *     from: do_action( $filter_name, $args );
+	 *     name: {$filter_name}
+	 *
+	 * @param ?\PhpParser\Node $node Use this instead of the current node.
 	 * @return string
 	 */
-	private function cleanupName( $name ) {
-		$matches = array();
-
-		// quotes on both ends of a string
-		if ( preg_match( '/^[\'"]([^\'"]*)[\'"]$/', $name, $matches ) ) {
-			return $matches[1];
-		}
+	public function getName( $node = null ) {
+		/**
+		 * The current Node being examined.
+		 *
+		 * @var \PhpParser\Node
+		 */
+		$node    = $node ?? $this->node->args[0]->value;
+		$printer = new PHPParser_PrettyPrinter_Default;
+		$name    = $printer->prettyPrintExpr( $node );
 
-		// two concatenated things, last one of them a variable
-		if ( preg_match(
-			'/(?:[\'"]([^\'"]*)[\'"]\s*\.\s*)?' . // First filter name string (optional)
-			'(\$[^\s]*)' .                        // Dynamic variable
-			'(?:\s*\.\s*[\'"]([^\'"]*)[\'"])?/',  // Second filter name string (optional)
-			$name, $matches ) ) {
+		if ( $node instanceof \PhpParser\Node\Scalar\String_ ) {
+			// "'action'" -> "action"
+			return $node->value;
+		} elseif ( $node instanceof \PhpParser\Node\Scalar\Encapsed ) {
+			// '"action_{$var}"' -> 'action_{$var}'
+			$name = '';
 
-			if ( isset( $matches[3] ) ) {
-				return $matches[1] . '{' . $matches[2] . '}' . $matches[3];
-			} else {
-				return $matches[1] . '{' . $matches[2] . '}';
+			foreach ( $node->parts as $part ) {
+				if ( is_string( $part ) ) {
+					$name .= $part;
+				} else {
+					$name .= $this->getName( $part );
+				}
 			}
+
+			return $name;
+		} elseif ( $node instanceof \PhpParser\Node\Expr\BinaryOp\Concat ) {
+			// '"action_" . $var' -> 'action_{$var}'
+			return $this->getName( $node->left ) . $this->getName( $node->right );
+		} elseif ( $node instanceof \PhpParser\Node\Expr\PropertyFetch ) {
+			// '$this->action' -> '{$this->action}'
+			return "{{$name}}";
+		} elseif ( $node instanceof \PhpParser\Node\Expr\Variable ) {
+			// '$action' -> '{$action}'
+			return "{\${$node->name}}";
 		}
 
+		/*
+		 * If none of these known constructions match, then
+		 * fallback to the pretty-printed version of the node.
+		 *
+		 * For improving the quality of the hook-name generation,
+		 * replace this return statement by throwing an exception
+		 * to determine which cases aren't handled, and then add
+		 * them above.
+		 */
 		return $name;
 	}