Core: properly check formatting of function declaration statements

.github/workflows/basic-qa.yml

@@ -152,7 +152,7 @@ jobs:
         continue-on-error: true
         run: |
           set +e
-          $(pwd)/vendor/bin/phpcbf -pq ./WordPress/Tests/ --standard=WordPress --extensions=inc --exclude=Generic.PHP.Syntax --report=summary --ignore=/WordPress/Tests/NamingConventions/ValidVariableNameUnitTest.inc
+          $(pwd)/vendor/bin/phpcbf -pq ./WordPress/Tests/ --standard=WordPress --extensions=inc --exclude=Generic.PHP.Syntax --report=summary --ignore=/WordPress/Tests/NamingConventions/ValidVariableNameUnitTest.inc,/WordPress/Tests/WP/GlobalVariablesOverrideUnitTest.7.inc
           exitcode="$?"
           echo "EXITCODE=$exitcode" >> $GITHUB_OUTPUT
           exit "$exitcode"

WordPress-Core/ruleset.xml

@@ -199,19 +199,37 @@
 	<!-- Covers rule: Define a function like so: function my_function( $param1 = 'foo', $param2 = 'bar' ) { -->
 	<rule ref="Generic.Functions.OpeningFunctionBraceKernighanRitchie">
 		<properties>
-			<property name="checkClosures" value="true"/>
+			<property name="checkClosures" value="false"/>
 		</properties>
 	</rule>
+	<!-- Prevent duplicate message. This is now checked by the Squiz.Functions.MultiLineFunctionDeclaration sniff. -->
+	<rule ref="Generic.Functions.OpeningFunctionBraceKernighanRitchie.ContentAfterBrace">
+		<severity>0</severity>
+	</rule>
+
+	<rule ref="Squiz.Functions.MultiLineFunctionDeclaration"/>
+	<!-- WP demands brace on same line, not on the next line. Silence errors related to "brace on same line". -->
+	<rule ref="Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine">
+		<severity>0</severity>
+	</rule>
+	<rule ref="Squiz.Functions.MultiLineFunctionDeclaration.BraceSpacing">
+		<severity>0</severity>
+	</rule>
+	<rule ref="Squiz.Functions.MultiLineFunctionDeclaration.BraceIndent">
+		<severity>0</severity>
+	</rule>
+	<!-- Prevent duplicate message. This is already checked by the Generic.WhiteSpace.LanguageConstructSpacing sniff. -->
+	<rule ref="Squiz.Functions.MultiLineFunctionDeclaration.SpaceAfterUse">
+		<severity>0</severity>
+	</rule>
+
 	<rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing">
 		<properties>
 			<property name="equalsSpacing" value="1"/>
 			<property name="requiredSpacesAfterOpen" value="1"/>
 			<property name="requiredSpacesBeforeClose" value="1"/>
 		</properties>
 	</rule>
-	<rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingBeforeClose">
-		<severity>0</severity>
-	</rule>
 	<rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingAfterVariadic">
 		<severity>0</severity>
 	</rule>

WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php

@@ -71,7 +71,7 @@ public function process_token( $stackPtr ) {
 		) {
 			// Retrieve all embeds, but use only the initial variable name part.
 			$interpolated_variables = array_map(
-				function( $embed ) {
+				function ( $embed ) {
 					return '$' . preg_replace( '`^(\{?\$\{?\(?)([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)(.*)$`', '$2', $embed );
 				},
 				TextStrings::getEmbeds( $this->tokens[ $stackPtr ]['content'] )

WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php

@@ -11,7 +11,6 @@
 
 use WordPressCS\WordPress\Sniff;
 use PHP_CodeSniffer\Util\Tokens;
-use PHPCSUtils\Utils\UseStatements;
 
 /**
  * Enforces spacing around logical operators and assignments, based upon Squiz code.
@@ -23,6 +22,7 @@
  * @since   0.3.0      This sniff now has the ability to fix most errors it flags.
  * @since   0.7.0      This class now extends the WordPressCS native `Sniff` class.
  * @since   0.13.0     Class name changed: this class is now namespaced.
+ * @since   3.0.0      Checks related to function declarations have been removed from this sniff.
  *
  * Last synced with base class 2017-01-15 at commit b024ad84656c37ef5733c6998ebc1e60957b2277.
  * Note: This class has diverged quite far from the original. All the same, checking occasionally
@@ -52,33 +52,19 @@ final class ControlStructureSpacingSniff extends Sniff {
 	 */
 	public $space_before_colon = 'required';
 
-	/**
-	 * How many spaces should be between a T_CLOSURE and T_OPEN_PARENTHESIS.
-	 *
-	 * `function[*]() {...}`
-	 *
-	 * @since 0.7.0
-	 *
-	 * @var int
-	 */
-	public $spaces_before_closure_open_paren = -1;
-
 	/**
 	 * Tokens for which to ignore extra space on the inside of parenthesis.
 	 *
-	 * For functions, this is already checked by the Squiz.Functions.FunctionDeclarationArgumentSpacing sniff.
 	 * For do / else / try, there are no parenthesis, so skip it.
 	 *
 	 * @since 0.11.0
 	 *
 	 * @var array
 	 */
 	private $ignore_extra_space_after_open_paren = array(
-		\T_FUNCTION => true,
-		\T_CLOSURE  => true,
-		\T_DO       => true,
-		\T_ELSE     => true,
-		\T_TRY      => true,
+		\T_DO   => true,
+		\T_ELSE => true,
+		\T_TRY  => true,
 	);
 
 	/**
@@ -96,9 +82,6 @@ public function register() {
 			\T_DO,
 			\T_ELSE,
 			\T_ELSEIF,
-			\T_FUNCTION,
-			\T_CLOSURE,
-			\T_USE,
 			\T_TRY,
 			\T_CATCH,
 		);
@@ -112,18 +95,8 @@ public function register() {
 	 * @return void
 	 */
 	public function process_token( $stackPtr ) {
-		$this->spaces_before_closure_open_paren = (int) $this->spaces_before_closure_open_paren;
-
-		if ( \T_USE === $this->tokens[ $stackPtr ]['code']
-			&& UseStatements::isClosureUse( $this->phpcsFile, $stackPtr ) === false
-		) {
-			return;
-		}
-
 		if ( isset( $this->tokens[ ( $stackPtr + 1 ) ] ) && \T_WHITESPACE !== $this->tokens[ ( $stackPtr + 1 ) ]['code']
 			&& ! ( \T_ELSE === $this->tokens[ $stackPtr ]['code'] && \T_COLON === $this->tokens[ ( $stackPtr + 1 ) ]['code'] )
-			&& ! ( \T_CLOSURE === $this->tokens[ $stackPtr ]['code']
-				&& 0 >= $this->spaces_before_closure_open_paren )
 		) {
 			$error = 'Space after opening control structure is required';
 			$fix   = $this->phpcsFile->addFixableError( $error, $stackPtr, 'NoSpaceAfterStructureOpen' );
@@ -133,12 +106,8 @@ public function process_token( $stackPtr ) {
 			}
 		}
 
-		if ( ! isset( $this->tokens[ $stackPtr ]['scope_closer'] ) ) {
-
-			if ( \T_USE === $this->tokens[ $stackPtr ]['code'] ) {
-				$scopeOpener = $this->phpcsFile->findNext( \T_OPEN_CURLY_BRACKET, ( $stackPtr + 1 ) );
-				$scopeCloser = $this->tokens[ $scopeOpener ]['scope_closer'];
-			} elseif ( \T_WHILE !== $this->tokens[ $stackPtr ]['code'] ) {
+		if ( ! isset( $this->tokens[ $stackPtr ]['scope_opener'], $this->tokens[ $stackPtr ]['scope_closer'] ) ) {
+			if ( \T_WHILE !== $this->tokens[ $stackPtr ]['code'] ) {
 				return;
 			}
 		} else {
@@ -174,102 +143,15 @@ public function process_token( $stackPtr ) {
 
 		$parenthesisOpener = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
 
-		// If this is a function declaration.
-		if ( \T_FUNCTION === $this->tokens[ $stackPtr ]['code'] ) {
-
-			if ( \T_STRING === $this->tokens[ $parenthesisOpener ]['code'] ) {
-
-				$function_name_ptr = $parenthesisOpener;
-
-			} elseif ( \T_BITWISE_AND === $this->tokens[ $parenthesisOpener ]['code'] ) {
-
-				// This function returns by reference, i.e. `function &function_name() {}`.
-				$parenthesisOpener = $this->phpcsFile->findNext(
-					Tokens::$emptyTokens,
-					( $parenthesisOpener + 1 ),
-					null,
-					true
-				);
-				$function_name_ptr = $parenthesisOpener;
-			}
-
-			if ( isset( $function_name_ptr ) ) {
-				$parenthesisOpener = $this->phpcsFile->findNext(
-					Tokens::$emptyTokens,
-					( $parenthesisOpener + 1 ),
-					null,
-					true
-				);
-
-				// Checking space between name and open parentheses, i.e. `function my_function[*](...) {}`.
-				if ( ( $function_name_ptr + 1 ) !== $parenthesisOpener ) {
-
-					$error = 'Space between function name and opening parenthesis is prohibited.';
-					$fix   = $this->phpcsFile->addFixableError(
-						$error,
-						$stackPtr,
-						'SpaceBeforeFunctionOpenParenthesis',
-						array( $this->tokens[ ( $function_name_ptr + 1 ) ]['content'] )
-					);
-
-					if ( true === $fix ) {
-						$this->phpcsFile->fixer->replaceToken( ( $function_name_ptr + 1 ), '' );
-					}
-				}
-			}
-		} elseif ( \T_CLOSURE === $this->tokens[ $stackPtr ]['code'] ) {
-
-			// Check if there is a use () statement.
-			if ( isset( $this->tokens[ $parenthesisOpener ]['parenthesis_closer'] ) ) {
-
-				$usePtr = $this->phpcsFile->findNext(
-					Tokens::$emptyTokens,
-					( $this->tokens[ $parenthesisOpener ]['parenthesis_closer'] + 1 ),
-					null,
-					true,
-					null,
-					true
-				);
-
-				// If it is, we set that as the "scope opener".
-				if ( \T_USE === $this->tokens[ $usePtr ]['code'] ) {
-					$scopeOpener = $usePtr;
-				}
-			}
-		}
-
 		if ( \T_COLON !== $this->tokens[ $parenthesisOpener ]['code']
-			&& \T_FUNCTION !== $this->tokens[ $stackPtr ]['code']
+			&& ( $stackPtr + 1 ) === $parenthesisOpener
 		) {
+			// Checking space between keyword and open parenthesis, i.e. `if[*](...) {}`.
+			$error = 'No space before opening parenthesis is prohibited';
+			$fix   = $this->phpcsFile->addFixableError( $error, $stackPtr, 'NoSpaceBeforeOpenParenthesis' );
 
-			if ( \T_CLOSURE === $this->tokens[ $stackPtr ]['code']
-				&& 0 === $this->spaces_before_closure_open_paren
-			) {
-
-				if ( ( $stackPtr + 1 ) !== $parenthesisOpener ) {
-					// Checking space between keyword and open parenthesis, i.e. `function[*](...) {}`.
-					$error = 'Space before closure opening parenthesis is prohibited';
-					$fix   = $this->phpcsFile->addFixableError( $error, $stackPtr, 'SpaceBeforeClosureOpenParenthesis' );
-
-					if ( true === $fix ) {
-						$this->phpcsFile->fixer->replaceToken( ( $stackPtr + 1 ), '' );
-					}
-				}
-			} elseif (
-				(
-					\T_CLOSURE !== $this->tokens[ $stackPtr ]['code']
-					|| 1 === $this->spaces_before_closure_open_paren
-				)
-				&& ( $stackPtr + 1 ) === $parenthesisOpener
-			) {
-
-				// Checking space between keyword and open parenthesis, i.e. `if[*](...) {}`.
-				$error = 'No space before opening parenthesis is prohibited';
-				$fix   = $this->phpcsFile->addFixableError( $error, $stackPtr, 'NoSpaceBeforeOpenParenthesis' );
-
-				if ( true === $fix ) {
-					$this->phpcsFile->fixer->addContent( $stackPtr, ' ' );
-				}
+			if ( true === $fix ) {
+				$this->phpcsFile->fixer->addContent( $stackPtr, ' ' );
 			}
 		}
 
@@ -292,7 +174,7 @@ public function process_token( $stackPtr ) {
 
 		if ( \T_CLOSE_PARENTHESIS !== $this->tokens[ ( $parenthesisOpener + 1 ) ]['code'] ) {
 			if ( \T_WHITESPACE !== $this->tokens[ ( $parenthesisOpener + 1 ) ]['code'] ) {
-				// Checking space directly after the open parenthesis, i.e. `$value = my_function([*]...)`.
+				// Checking space directly after the open parenthesis, i.e. `if ([*]...) {}`.
 				$error = 'No space after opening parenthesis is prohibited';
 				$fix   = $this->phpcsFile->addFixableError( $error, $stackPtr, 'NoSpaceAfterOpenParenthesis' );
 
@@ -351,11 +233,6 @@ public function process_token( $stackPtr ) {
 				}
 
 				if ( \T_WHITESPACE !== $this->tokens[ ( $parenthesisCloser + 1 ) ]['code']
-					&& ! ( // Do NOT flag : immediately following ) for return types declarations.
-						\T_COLON === $this->tokens[ ( $parenthesisCloser + 1 ) ]['code']
-						&& ( isset( $this->tokens[ $parenthesisCloser ]['parenthesis_owner'] ) === false
-							|| in_array( $this->tokens[ $this->tokens[ $parenthesisCloser ]['parenthesis_owner'] ]['code'], array( \T_FUNCTION, \T_CLOSURE ), true ) )
-					)
 					&& ( isset( $scopeOpener ) && \T_COLON !== $this->tokens[ $scopeOpener ]['code'] )
 				) {
 					$error = 'Space between opening control structure and closing parenthesis is required';
@@ -367,10 +244,7 @@ public function process_token( $stackPtr ) {
 				}
 			}
 
-			// Ignore this for function declarations. Handled by the OpeningFunctionBraceKernighanRitchie sniff.
-			if ( \T_FUNCTION !== $this->tokens[ $stackPtr ]['code']
-				&& \T_CLOSURE !== $this->tokens[ $stackPtr ]['code']
-				&& isset( $this->tokens[ $parenthesisOpener ]['parenthesis_owner'] )
+			if ( isset( $this->tokens[ $parenthesisOpener ]['parenthesis_owner'] )
 				&& ( isset( $scopeOpener )
 				&& $this->tokens[ $parenthesisCloser ]['line'] !== $this->tokens[ $scopeOpener ]['line'] )
 			) {
@@ -392,7 +266,6 @@ public function process_token( $stackPtr ) {
 			} elseif ( \T_WHITESPACE === $this->tokens[ ( $parenthesisCloser + 1 ) ]['code']
 				&& ' ' !== $this->tokens[ ( $parenthesisCloser + 1 ) ]['content']
 			) {
-
 				// Checking space between the close parenthesis and the open brace, i.e. `if (...) [*]{}`.
 				$error = 'Expected exactly one space between closing parenthesis and opening control structure; "%s" found.';
 				$fix   = $this->phpcsFile->addFixableError(

WordPress/Tests/WP/GlobalVariablesOverrideUnitTest.1.inc

@@ -183,20 +183,20 @@ function global_vars() {
 	return $closure( $pagenow ); // OK, not an assignment.
 }
 
-// Verify skipping over rest of the function when live coding/parse error in nested scope structure.
-function global_vars() {
-	global $pagenow;
 
-	$closure = function ( $pagenow ) {
-		global $feeds;
 
-		$nested_closure_with_parse_error = function ( $feeds )
 
-		$feeds = 'something'; // Bad, but ignored because of the parse error in the closure.
-	};
 
-	$pagenow = 'something'; // Bad, should be picked up. Tests that skipping on parse error doesn't skip too far.
-}
+
+
+
+
+
+
+
+
+
+
 
 $GLOBALS[] = 'something';
 $GLOBALS[103] = 'something';
@@ -314,4 +314,3 @@ list(
 // Live coding/parse error.
 // This has to be the last test in the file!
 list( $tab, $tabs
-

WordPress/Tests/WP/GlobalVariablesOverrideUnitTest.7.inc

@@ -0,0 +1,20 @@
+<?php
+
+/*
+ * Separate test file to isolate the parse error test.
+ */
+
+// Verify skipping over rest of the function when live coding/parse error in nested scope structure.
+function global_vars() {
+	global $pagenow;
+
+	$closure = function ( $pagenow ) {
+		global $feeds;
+
+		$nested_closure_with_parse_error = function ( $feeds )
+
+		$feeds = 'something'; // Bad, but ignored because of the parse error in the closure.
+	};
+
+	$pagenow = 'something'; // Bad, should be picked up. Tests that skipping on parse error doesn't skip too far.
+}

WordPress/Tests/WP/GlobalVariablesOverrideUnitTest.php

@@ -59,7 +59,6 @@ public function getErrorList( $testFile = '' ) {
 					143 => 1,
 					146 => 1,
 					181 => 1,
-					198 => 1,
 					212 => 4,
 					230 => 2,
 					231 => 2,
@@ -91,6 +90,11 @@ public function getErrorList( $testFile = '' ) {
 					29 => 1,
 				);
 
+			case 'GlobalVariablesOverrideUnitTest.7.inc':
+				return array(
+					19 => 1,
+				);
+
 			default:
 				return array();
 		}