Skip to content

Commit 306542c

Browse files
GaryJonesGaryJones
authored
Merge pull request #2328 from WordPress/feature/core-check-function-declarations
2 parents 9cf9f24 + 602d9ef commit 306542c

10 files changed

+84
-436
lines changed

‎.github/workflows/basic-qa.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ jobs:
152152
continue-on-error: true
153153
run: |
154154
set +e
155-
$(pwd)/vendor/bin/phpcbf -pq ./WordPress/Tests/ --standard=WordPress --extensions=inc --exclude=Generic.PHP.Syntax --report=summary --ignore=/WordPress/Tests/NamingConventions/ValidVariableNameUnitTest.inc
155+
$(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
156156
exitcode="$?"
157157
echo "EXITCODE=$exitcode" >> $GITHUB_OUTPUT
158158
exit "$exitcode"

‎WordPress-Core/ruleset.xml

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,19 +199,37 @@
199199
<!-- Covers rule: Define a function like so: function my_function( $param1 = 'foo', $param2 = 'bar' ) { -->
200200
<rule ref="Generic.Functions.OpeningFunctionBraceKernighanRitchie">
201201
<properties>
202-
<property name="checkClosures" value="true"/>
202+
<property name="checkClosures" value="false"/>
203203
</properties>
204204
</rule>
205+
<!-- Prevent duplicate message. This is now checked by the Squiz.Functions.MultiLineFunctionDeclaration sniff. -->
206+
<rule ref="Generic.Functions.OpeningFunctionBraceKernighanRitchie.ContentAfterBrace">
207+
<severity>0</severity>
208+
</rule>
209+
210+
<rule ref="Squiz.Functions.MultiLineFunctionDeclaration"/>
211+
<!-- WP demands brace on same line, not on the next line. Silence errors related to "brace on same line". -->
212+
<rule ref="Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine">
213+
<severity>0</severity>
214+
</rule>
215+
<rule ref="Squiz.Functions.MultiLineFunctionDeclaration.BraceSpacing">
216+
<severity>0</severity>
217+
</rule>
218+
<rule ref="Squiz.Functions.MultiLineFunctionDeclaration.BraceIndent">
219+
<severity>0</severity>
220+
</rule>
221+
<!-- Prevent duplicate message. This is already checked by the Generic.WhiteSpace.LanguageConstructSpacing sniff. -->
222+
<rule ref="Squiz.Functions.MultiLineFunctionDeclaration.SpaceAfterUse">
223+
<severity>0</severity>
224+
</rule>
225+
205226
<rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing">
206227
<properties>
207228
<property name="equalsSpacing" value="1"/>
208229
<property name="requiredSpacesAfterOpen" value="1"/>
209230
<property name="requiredSpacesBeforeClose" value="1"/>
210231
</properties>
211232
</rule>
212-
<rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingBeforeClose">
213-
<severity>0</severity>
214-
</rule>
215233
<rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingAfterVariadic">
216234
<severity>0</severity>
217235
</rule>

‎WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public function process_token( $stackPtr ) {
7171
) {
7272
// Retrieve all embeds, but use only the initial variable name part.
7373
$interpolated_variables = array_map(
74-
function( $embed ) {
74+
function ( $embed ) {
7575
return '$' . preg_replace( '`^(\{?\$\{?\(?)([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)(.*)$`', '$2', $embed );
7676
},
7777
TextStrings::getEmbeds( $this->tokens[ $stackPtr ]['content'] )

‎WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php

Lines changed: 14 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
use WordPressCS\WordPress\Sniff;
1313
use PHP_CodeSniffer\Util\Tokens;
14-
use PHPCSUtils\Utils\UseStatements;
1514

1615
/**
1716
* Enforces spacing around logical operators and assignments, based upon Squiz code.
@@ -23,6 +22,7 @@
2322
* @since 0.3.0 This sniff now has the ability to fix most errors it flags.
2423
* @since 0.7.0 This class now extends the WordPressCS native `Sniff` class.
2524
* @since 0.13.0 Class name changed: this class is now namespaced.
25+
* @since 3.0.0 Checks related to function declarations have been removed from this sniff.
2626
*
2727
* Last synced with base class 2017-01-15 at commit b024ad84656c37ef5733c6998ebc1e60957b2277.
2828
* Note: This class has diverged quite far from the original. All the same, checking occasionally
@@ -52,33 +52,19 @@ final class ControlStructureSpacingSniff extends Sniff {
5252
*/
5353
public $space_before_colon = 'required';
5454

55-
/**
56-
* How many spaces should be between a T_CLOSURE and T_OPEN_PARENTHESIS.
57-
*
58-
* `function[*]() {...}`
59-
*
60-
* @since 0.7.0
61-
*
62-
* @var int
63-
*/
64-
public $spaces_before_closure_open_paren = -1;
65-
6655
/**
6756
* Tokens for which to ignore extra space on the inside of parenthesis.
6857
*
69-
* For functions, this is already checked by the Squiz.Functions.FunctionDeclarationArgumentSpacing sniff.
7058
* For do / else / try, there are no parenthesis, so skip it.
7159
*
7260
* @since 0.11.0
7361
*
7462
* @var array
7563
*/
7664
private $ignore_extra_space_after_open_paren = array(
77-
\T_FUNCTION => true,
78-
\T_CLOSURE => true,
79-
\T_DO => true,
80-
\T_ELSE => true,
81-
\T_TRY => true,
65+
\T_DO => true,
66+
\T_ELSE => true,
67+
\T_TRY => true,
8268
);
8369

8470
/**
@@ -96,9 +82,6 @@ public function register() {
9682
\T_DO,
9783
\T_ELSE,
9884
\T_ELSEIF,
99-
\T_FUNCTION,
100-
\T_CLOSURE,
101-
\T_USE,
10285
\T_TRY,
10386
\T_CATCH,
10487
);
@@ -112,18 +95,8 @@ public function register() {
11295
* @return void
11396
*/
11497
public function process_token( $stackPtr ) {
115-
$this->spaces_before_closure_open_paren = (int) $this->spaces_before_closure_open_paren;
116-
117-
if ( \T_USE === $this->tokens[ $stackPtr ]['code']
118-
&& UseStatements::isClosureUse( $this->phpcsFile, $stackPtr ) === false
119-
) {
120-
return;
121-
}
122-
12398
if ( isset( $this->tokens[ ( $stackPtr + 1 ) ] ) && \T_WHITESPACE !== $this->tokens[ ( $stackPtr + 1 ) ]['code']
12499
&& ! ( \T_ELSE === $this->tokens[ $stackPtr ]['code'] && \T_COLON === $this->tokens[ ( $stackPtr + 1 ) ]['code'] )
125-
&& ! ( \T_CLOSURE === $this->tokens[ $stackPtr ]['code']
126-
&& 0 >= $this->spaces_before_closure_open_paren )
127100
) {
128101
$error = 'Space after opening control structure is required';
129102
$fix = $this->phpcsFile->addFixableError( $error, $stackPtr, 'NoSpaceAfterStructureOpen' );
@@ -133,12 +106,8 @@ public function process_token( $stackPtr ) {
133106
}
134107
}
135108

136-
if ( ! isset( $this->tokens[ $stackPtr ]['scope_closer'] ) ) {
137-
138-
if ( \T_USE === $this->tokens[ $stackPtr ]['code'] ) {
139-
$scopeOpener = $this->phpcsFile->findNext( \T_OPEN_CURLY_BRACKET, ( $stackPtr + 1 ) );
140-
$scopeCloser = $this->tokens[ $scopeOpener ]['scope_closer'];
141-
} elseif ( \T_WHILE !== $this->tokens[ $stackPtr ]['code'] ) {
109+
if ( ! isset( $this->tokens[ $stackPtr ]['scope_opener'], $this->tokens[ $stackPtr ]['scope_closer'] ) ) {
110+
if ( \T_WHILE !== $this->tokens[ $stackPtr ]['code'] ) {
142111
return;
143112
}
144113
} else {
@@ -174,102 +143,15 @@ public function process_token( $stackPtr ) {
174143

175144
$parenthesisOpener = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
176145

177-
// If this is a function declaration.
178-
if ( \T_FUNCTION === $this->tokens[ $stackPtr ]['code'] ) {
179-
180-
if ( \T_STRING === $this->tokens[ $parenthesisOpener ]['code'] ) {
181-
182-
$function_name_ptr = $parenthesisOpener;
183-
184-
} elseif ( \T_BITWISE_AND === $this->tokens[ $parenthesisOpener ]['code'] ) {
185-
186-
// This function returns by reference, i.e. `function &function_name() {}`.
187-
$parenthesisOpener = $this->phpcsFile->findNext(
188-
Tokens::$emptyTokens,
189-
( $parenthesisOpener + 1 ),
190-
null,
191-
true
192-
);
193-
$function_name_ptr = $parenthesisOpener;
194-
}
195-
196-
if ( isset( $function_name_ptr ) ) {
197-
$parenthesisOpener = $this->phpcsFile->findNext(
198-
Tokens::$emptyTokens,
199-
( $parenthesisOpener + 1 ),
200-
null,
201-
true
202-
);
203-
204-
// Checking space between name and open parentheses, i.e. `function my_function[*](...) {}`.
205-
if ( ( $function_name_ptr + 1 ) !== $parenthesisOpener ) {
206-
207-
$error = 'Space between function name and opening parenthesis is prohibited.';
208-
$fix = $this->phpcsFile->addFixableError(
209-
$error,
210-
$stackPtr,
211-
'SpaceBeforeFunctionOpenParenthesis',
212-
array( $this->tokens[ ( $function_name_ptr + 1 ) ]['content'] )
213-
);
214-
215-
if ( true === $fix ) {
216-
$this->phpcsFile->fixer->replaceToken( ( $function_name_ptr + 1 ), '' );
217-
}
218-
}
219-
}
220-
} elseif ( \T_CLOSURE === $this->tokens[ $stackPtr ]['code'] ) {
221-
222-
// Check if there is a use () statement.
223-
if ( isset( $this->tokens[ $parenthesisOpener ]['parenthesis_closer'] ) ) {
224-
225-
$usePtr = $this->phpcsFile->findNext(
226-
Tokens::$emptyTokens,
227-
( $this->tokens[ $parenthesisOpener ]['parenthesis_closer'] + 1 ),
228-
null,
229-
true,
230-
null,
231-
true
232-
);
233-
234-
// If it is, we set that as the "scope opener".
235-
if ( \T_USE === $this->tokens[ $usePtr ]['code'] ) {
236-
$scopeOpener = $usePtr;
237-
}
238-
}
239-
}
240-
241146
if ( \T_COLON !== $this->tokens[ $parenthesisOpener ]['code']
242-
&& \T_FUNCTION !== $this->tokens[ $stackPtr ]['code']
147+
&& ( $stackPtr + 1 ) === $parenthesisOpener
243148
) {
149+
// Checking space between keyword and open parenthesis, i.e. `if[*](...) {}`.
150+
$error = 'No space before opening parenthesis is prohibited';
151+
$fix = $this->phpcsFile->addFixableError( $error, $stackPtr, 'NoSpaceBeforeOpenParenthesis' );
244152

245-
if ( \T_CLOSURE === $this->tokens[ $stackPtr ]['code']
246-
&& 0 === $this->spaces_before_closure_open_paren
247-
) {
248-
249-
if ( ( $stackPtr + 1 ) !== $parenthesisOpener ) {
250-
// Checking space between keyword and open parenthesis, i.e. `function[*](...) {}`.
251-
$error = 'Space before closure opening parenthesis is prohibited';
252-
$fix = $this->phpcsFile->addFixableError( $error, $stackPtr, 'SpaceBeforeClosureOpenParenthesis' );
253-
254-
if ( true === $fix ) {
255-
$this->phpcsFile->fixer->replaceToken( ( $stackPtr + 1 ), '' );
256-
}
257-
}
258-
} elseif (
259-
(
260-
\T_CLOSURE !== $this->tokens[ $stackPtr ]['code']
261-
|| 1 === $this->spaces_before_closure_open_paren
262-
)
263-
&& ( $stackPtr + 1 ) === $parenthesisOpener
264-
) {
265-
266-
// Checking space between keyword and open parenthesis, i.e. `if[*](...) {}`.
267-
$error = 'No space before opening parenthesis is prohibited';
268-
$fix = $this->phpcsFile->addFixableError( $error, $stackPtr, 'NoSpaceBeforeOpenParenthesis' );
269-
270-
if ( true === $fix ) {
271-
$this->phpcsFile->fixer->addContent( $stackPtr, ' ' );
272-
}
153+
if ( true === $fix ) {
154+
$this->phpcsFile->fixer->addContent( $stackPtr, ' ' );
273155
}
274156
}
275157

@@ -292,7 +174,7 @@ public function process_token( $stackPtr ) {
292174

293175
if ( \T_CLOSE_PARENTHESIS !== $this->tokens[ ( $parenthesisOpener + 1 ) ]['code'] ) {
294176
if ( \T_WHITESPACE !== $this->tokens[ ( $parenthesisOpener + 1 ) ]['code'] ) {
295-
// Checking space directly after the open parenthesis, i.e. `$value = my_function([*]...)`.
177+
// Checking space directly after the open parenthesis, i.e. `if ([*]...) {}`.
296178
$error = 'No space after opening parenthesis is prohibited';
297179
$fix = $this->phpcsFile->addFixableError( $error, $stackPtr, 'NoSpaceAfterOpenParenthesis' );
298180

@@ -351,11 +233,6 @@ public function process_token( $stackPtr ) {
351233
}
352234

353235
if ( \T_WHITESPACE !== $this->tokens[ ( $parenthesisCloser + 1 ) ]['code']
354-
&& ! ( // Do NOT flag : immediately following ) for return types declarations.
355-
\T_COLON === $this->tokens[ ( $parenthesisCloser + 1 ) ]['code']
356-
&& ( isset( $this->tokens[ $parenthesisCloser ]['parenthesis_owner'] ) === false
357-
|| in_array( $this->tokens[ $this->tokens[ $parenthesisCloser ]['parenthesis_owner'] ]['code'], array( \T_FUNCTION, \T_CLOSURE ), true ) )
358-
)
359236
&& ( isset( $scopeOpener ) && \T_COLON !== $this->tokens[ $scopeOpener ]['code'] )
360237
) {
361238
$error = 'Space between opening control structure and closing parenthesis is required';
@@ -367,10 +244,7 @@ public function process_token( $stackPtr ) {
367244
}
368245
}
369246

370-
// Ignore this for function declarations. Handled by the OpeningFunctionBraceKernighanRitchie sniff.
371-
if ( \T_FUNCTION !== $this->tokens[ $stackPtr ]['code']
372-
&& \T_CLOSURE !== $this->tokens[ $stackPtr ]['code']
373-
&& isset( $this->tokens[ $parenthesisOpener ]['parenthesis_owner'] )
247+
if ( isset( $this->tokens[ $parenthesisOpener ]['parenthesis_owner'] )
374248
&& ( isset( $scopeOpener )
375249
&& $this->tokens[ $parenthesisCloser ]['line'] !== $this->tokens[ $scopeOpener ]['line'] )
376250
) {
@@ -392,7 +266,6 @@ public function process_token( $stackPtr ) {
392266
} elseif ( \T_WHITESPACE === $this->tokens[ ( $parenthesisCloser + 1 ) ]['code']
393267
&& ' ' !== $this->tokens[ ( $parenthesisCloser + 1 ) ]['content']
394268
) {
395-
396269
// Checking space between the close parenthesis and the open brace, i.e. `if (...) [*]{}`.
397270
$error = 'Expected exactly one space between closing parenthesis and opening control structure; "%s" found.';
398271
$fix = $this->phpcsFile->addFixableError(

‎WordPress/Tests/WP/GlobalVariablesOverrideUnitTest.1.inc

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -183,20 +183,20 @@ function global_vars() {
183183
return $closure( $pagenow ); // OK, not an assignment.
184184
}
185185

186-
// Verify skipping over rest of the function when live coding/parse error in nested scope structure.
187-
function global_vars() {
188-
global $pagenow;
189186

190-
$closure = function ( $pagenow ) {
191-
global $feeds;
192187

193-
$nested_closure_with_parse_error = function ( $feeds )
194188

195-
$feeds = 'something'; // Bad, but ignored because of the parse error in the closure.
196-
};
197189

198-
$pagenow = 'something'; // Bad, should be picked up. Tests that skipping on parse error doesn't skip too far.
199-
}
190+
191+
192+
193+
194+
195+
196+
197+
198+
199+
200200

201201
$GLOBALS[] = 'something';
202202
$GLOBALS[103] = 'something';
@@ -314,4 +314,3 @@ list(
314314
// Live coding/parse error.
315315
// This has to be the last test in the file!
316316
list( $tab, $tabs
317-

‎WordPress/Tests/WP/GlobalVariablesOverrideUnitTest.7.inc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
/*
4+
* Separate test file to isolate the parse error test.
5+
*/
6+
7+
// Verify skipping over rest of the function when live coding/parse error in nested scope structure.
8+
function global_vars() {
9+
global $pagenow;
10+
11+
$closure = function ( $pagenow ) {
12+
global $feeds;
13+
14+
$nested_closure_with_parse_error = function ( $feeds )
15+
16+
$feeds = 'something'; // Bad, but ignored because of the parse error in the closure.
17+
};
18+
19+
$pagenow = 'something'; // Bad, should be picked up. Tests that skipping on parse error doesn't skip too far.
20+
}

‎WordPress/Tests/WP/GlobalVariablesOverrideUnitTest.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ public function getErrorList( $testFile = '' ) {
5959
143 => 1,
6060
146 => 1,
6161
181 => 1,
62-
198 => 1,
6362
212 => 4,
6463
230 => 2,
6564
231 => 2,
@@ -91,6 +90,11 @@ public function getErrorList( $testFile = '' ) {
9190
29 => 1,
9291
);
9392

93+
case 'GlobalVariablesOverrideUnitTest.7.inc':
94+
return array(
95+
19 => 1,
96+
);
97+
9498
default:
9599
return array();
96100
}

0 commit comments

Comments
 (0)