AbstractFunctionParameterSniff: fix first class callables and function imports (#2518)

* AbstractFunctionParameterSniff: fix first class callables and function imports

The `AbstractFunctionParameterSniff` class was incorrectly flagging first class callables and function imports as a function call without parameters. This commit fixes this behavior by introducing the `AbstractFunctionParameterSniff::is_targetted_token()` method. This method calls the parent method and then performs two additional checks to make sniffs extending this class ignore first class callables and function imports.

Since there are no tests for the abstract classes and the plan is to replace those classes with similar PHPCSUtils classes, I opted to add a few tests to the `I18nTextDomainFixer` tests that generate false positives before the changes implemented in this commit.

* AbstractFunctionParameterSniff: fix handling of unfinished function calls

Before this commit, the `AbstractFunctionParameterSniff` class would incorrectly treat an unfinished function call (missing closing parenthesis) as a function call without parameters. This behavior is now changed and the class will bail early when the closing parenthesis is missing and assume it is a syntax error or live coding.

Author: rodrigoprimo

WordPress/AbstractFunctionParameterSniff.php

@@ -9,6 +9,7 @@
 
 namespace WordPressCS\WordPress;
 
+use PHP_CodeSniffer\Util\Tokens;
 use PHPCSUtils\Utils\PassedParameters;
 use WordPressCS\WordPress\AbstractFunctionRestrictionsSniff;
 
@@ -77,6 +78,44 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
 		}
 	}
 
+	/**
+	 * Verify if the current token is a function call. Behaves like the parent method, except that
+	 * it returns false if there is no opening parenthesis after the function name (likely a
+	 * function import) or if it is a first class callable.
+	 *
+	 * @param int $stackPtr The position of the current token in the stack.
+	 *
+	 * @return bool
+	 */
+	public function is_targetted_token( $stackPtr ) {
+		if ( ! parent::is_targetted_token( $stackPtr ) ) {
+			return false;
+		}
+
+		$next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+
+		if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next ]['code'] ) {
+			// Not a function call (likely a function import).
+			return false;
+		}
+
+		if ( isset( $this->tokens[ $next ]['parenthesis_closer'] ) === false ) {
+			// Syntax error or live coding: missing closing parenthesis.
+			return false;
+		}
+
+		// First class callable.
+		$firstNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $next + 1 ), null, true );
+		if ( \T_ELLIPSIS === $this->tokens[ $firstNonEmpty ]['code'] ) {
+			$secondNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $firstNonEmpty + 1 ), null, true );
+			if ( \T_CLOSE_PARENTHESIS === $this->tokens[ $secondNonEmpty ]['code'] ) {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
 	/**
 	 * Process the parameters of a matched function.
 	 *

WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc

@@ -246,5 +246,40 @@ esc_html_x(
 	context: $context,
 );
 
+/*
+ * Test that `AbstractFunctionParameterSniff::is_targetted_token()` does not treat first class
+ * callables and function imports as a function call without parameters. This test is added here as
+ * there are no dedicated tests for the WPCS abstract classes. The WPCS abstract classes will be
+ * replaced with PHPCSUtils similar classes in the future, so it is not worth creating dedicated
+ * tests at this point.
+ */
+use function __;
+use function __ as my_function;
+use function
+    __ /* comment */
+    as /* comment */
+    my_function;
+use function
+    _n, // comment
+    _e, /* comment */
+    __ as my_function;
+add_action('my_action', __(...));
+add_action(
+    'my_action',
+    __ /* comment */
+    (
+        /* comment */ ... /* comment */
+    )
+);
+// The tests below ensure that the AbstractFunctionParameterSniff does not incorrectly ignore
+// function calls with variable unpacking. But they are also false positives in the context of the
+// I18nTextDomainFixer sniff and will be addressed in a future update.
+__(...$args);
+__ (
+    ...
+    /* comment */
+    $args
+);
+
 // phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[]
 // phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain false

WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc.fixed

@@ -251,5 +251,41 @@ esc_html_x(
 	context: $context,
 );
 
+/*
+ * Test that `AbstractFunctionParameterSniff::is_targetted_token()` does not treat first class
+ * callables and function imports as a function call without parameters. This test is added here as
+ * there are no dedicated tests for the WPCS abstract classes. The WPCS abstract classes will be
+ * replaced with PHPCSUtils similar classes in the future, so it is not worth creating dedicated
+ * tests at this point.
+ */
+use function __;
+use function __ as my_function;
+use function
+    __ /* comment */
+    as /* comment */
+    my_function;
+use function
+    _n, // comment
+    _e, /* comment */
+    __ as my_function;
+add_action('my_action', __(...));
+add_action(
+    'my_action',
+    __ /* comment */
+    (
+        /* comment */ ... /* comment */
+    )
+);
+// The tests below ensure that the AbstractFunctionParameterSniff does not incorrectly ignore
+// function calls with variable unpacking. But they are also false positives in the context of the
+// I18nTextDomainFixer sniff and will be addressed in a future update.
+__(...$args, 'something-else');
+__ (
+    ...
+    /* comment */
+    $args,
+    'something-else'
+);
+
 // phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[]
 // phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain false

WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.7.inc

@@ -0,0 +1,18 @@
+<?php
+// phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[] old-domain
+// phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain something-else
+
+/*
+ * Intentional parse error (nothing after opening parenthesis).
+ * This should be the only test in this file.
+ *
+ * Test to document that `AbstractFunctionParameterSniff::is_targetted_token()` ignores unfinished
+ * function calls. This test is added here as there are no dedicated tests for the WPCS abstract
+ * classes. The WPCS abstract classes will be replaced with PHPCSUtils similar classes in the
+ * future, so it is not worth creating dedicated tests at this point.
+ */
+
+__(
+
+// phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[]
+// phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain false

WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.php

@@ -17,6 +17,7 @@
  *
  * @since 1.2.0
  *
+ * @covers \WordPressCS\WordPress\AbstractFunctionParameterSniff::is_targetted_token
  * @covers \WordPressCS\WordPress\Sniffs\Utils\I18nTextDomainFixerSniff
  */
 final class I18nTextDomainFixerUnitTest extends AbstractSniffUnitTest {
@@ -148,6 +149,8 @@ public function getErrorList( $testFile = '' ) {
 					241 => 1,
 					242 => 1,
 					245 => 1,
+					277 => 1,
+					278 => 1,
 				);
 
 			default: