Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whitespace/ControlStructureSpacing: sync with upstream + minor tweaks #2333

Merged
merged 6 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 49 additions & 12 deletions WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@

namespace WordPressCS\WordPress\Sniffs\WhiteSpace;

use WordPressCS\WordPress\Sniff;
use PHPCSUtils\Tokens\Collections;
use PHP_CodeSniffer\Util\Tokens;
use WordPressCS\WordPress\Sniff;

/**
* Enforces spacing around logical operators and assignments, based upon Squiz code.
* Checks that control structures have the correct spacing around brackets, based upon Squiz code.
*
* @since 0.1.0
* @since 2013-06-11 This sniff no longer supports JS.
Expand All @@ -22,10 +23,10 @@
* @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.
* Last synced with base class 2021-11-20 at commit 7f11ffc8222b123c06345afd3261221561c3bb29.
* Note: This class has diverged quite far from the original. All the same, checking occasionally
* to see if there are upstream fixes made from which this sniff can benefit, is warranted.
* @link https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php
* @link https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Standards/Squiz/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php
*/
final class ControlStructureSpacingSniff extends Sniff {

Expand Down Expand Up @@ -53,16 +54,17 @@ final class ControlStructureSpacingSniff extends Sniff {
/**
* Tokens for which to ignore extra space on the inside of parenthesis.
*
* For do / else / try, there are no parenthesis, so skip it.
* For do / else / try / finally, there are no parenthesis, so skip it.
*
* @since 0.11.0
*
* @var array
*/
private $ignore_extra_space_after_open_paren = array(
\T_DO => true,
\T_ELSE => true,
\T_TRY => true,
\T_DO => true,
\T_ELSE => true,
\T_TRY => true,
\T_FINALLY => true,
);

/**
Expand All @@ -82,6 +84,8 @@ public function register() {
\T_ELSEIF,
\T_TRY,
\T_CATCH,
\T_FINALLY,
\T_MATCH,
);
}

Expand Down Expand Up @@ -284,17 +288,18 @@ public function process_token( $stackPtr ) {

// We ignore spacing for some structures that tend to have their own rules.
$ignore = array(
\T_FUNCTION => true,
\T_CLOSURE => true,
\T_DOC_COMMENT_OPEN_TAG => true,
\T_CLOSE_TAG => true,
\T_COMMENT => true,
);
$ignore += Tokens::$ooScopeTokens;
$ignore += Collections::closedScopes();

if ( ! isset( $ignore[ $this->tokens[ $firstContent ]['code'] ] )
&& $this->tokens[ $firstContent ]['line'] > ( $this->tokens[ $scopeOpener ]['line'] + 1 )
) {
$gap = ( $this->tokens[ $firstContent ]['line'] - $this->tokens[ $scopeOpener ]['line'] - 1 );
$this->phpcsFile->recordMetric( $stackPtr, 'Blank lines at start of control structure', $gap );

$error = 'Blank line found at start of control structure';
$fix = $this->phpcsFile->addFixableError( $error, $scopeOpener, 'BlankLineAfterStart' );

Expand All @@ -311,6 +316,8 @@ public function process_token( $stackPtr ) {
$this->phpcsFile->fixer->addNewline( $scopeOpener );
$this->phpcsFile->fixer->endChangeset();
}
} else {
$this->phpcsFile->recordMetric( $stackPtr, 'Blank lines at start of control structure', 0 );
}

if ( $firstContent !== $scopeCloser ) {
Expand All @@ -326,6 +333,9 @@ public function process_token( $stackPtr ) {
if ( ! isset( $ignore[ $this->tokens[ $checkToken ]['code'] ] )
&& $this->tokens[ $lastContent ]['line'] <= ( $this->tokens[ $scopeCloser ]['line'] - 2 )
) {
$gap = ( $this->tokens[ $scopeCloser ]['line'] - $this->tokens[ $lastContent ]['line'] - 1 );
$this->phpcsFile->recordMetric( $stackPtr, 'Blank lines at end of control structure', $gap );

for ( $i = ( $scopeCloser - 1 ); $i > $lastContent; $i-- ) {
if ( $this->tokens[ $i ]['line'] < $this->tokens[ $scopeCloser ]['line']
&& \T_OPEN_TAG !== $this->tokens[ $firstContent ]['code']
Expand Down Expand Up @@ -359,6 +369,8 @@ public function process_token( $stackPtr ) {
break;
}
}
} else {
$this->phpcsFile->recordMetric( $stackPtr, 'Blank lines at end of control structure', 0 );
}
}
unset( $ignore );
Expand All @@ -368,6 +380,16 @@ public function process_token( $stackPtr ) {
return;
}

if ( \T_MATCH === $this->tokens[ $stackPtr ]['code'] ) {
// Move the scope closer to the semicolon/comma.
$next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $scopeCloser + 1 ), null, true );
if ( false !== $next
&& ( \T_SEMICOLON === $this->tokens[ $next ]['code'] || \T_COMMA === $this->tokens[ $next ]['code'] )
) {
$scopeCloser = $next;
}
}

// {@internal This is just for the blank line check. Only whitespace should be considered,
// not "other" empty tokens.}}
$trailingContent = $this->phpcsFile->findNext( \T_WHITESPACE, ( $scopeCloser + 1 ), null, true );
Expand Down Expand Up @@ -405,6 +427,21 @@ public function process_token( $stackPtr ) {
return;
}

if ( \T_CATCH === $this->tokens[ $trailingContent ]['code'] && \T_TRY === $this->tokens[ $stackPtr ]['code'] ) {
// TRY with CATCH.
return;
}

if ( \T_FINALLY === $this->tokens[ $trailingContent ]['code'] && \T_CATCH === $this->tokens[ $stackPtr ]['code'] ) {
// CATCH with FINALLY.
return;
}

if ( \T_FINALLY === $this->tokens[ $trailingContent ]['code'] && \T_TRY === $this->tokens[ $stackPtr ]['code'] ) {
// TRY with FINALLY.
return;
}

if ( \T_CLOSE_TAG === $this->tokens[ $trailingContent ]['code'] ) {
// At the end of the script or embedded code.
return;
Expand All @@ -415,7 +452,7 @@ public function process_token( $stackPtr ) {
) {
// Another control structure's closing brace.
$owner = $this->tokens[ $trailingContent ]['scope_condition'];
if ( \in_array( $this->tokens[ $owner ]['code'], array( \T_FUNCTION, \T_CLOSURE, \T_CLASS, \T_ANON_CLASS, \T_INTERFACE, \T_TRAIT ), true ) ) {
if ( isset( Collections::closedScopes()[ $this->tokens[ $owner ]['code'] ] ) === true ) {
// The next content is the closing brace of a function, class, interface or trait
// so normal function/class rules apply and we can ignore it.
return;
Expand Down
83 changes: 83 additions & 0 deletions WordPress/Tests/WhiteSpace/ControlStructureSpacingUnitTest.1.inc
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,86 @@ if ( $foo ) {


}

// Handle (try/catch/) finally statements as well.
try {
// Something
} catch ( Exception $e ) {
// Something
} finally { // OK.
// Something
}

try {
// Something
} finally

{ // Bad.
// Something
}

try {
// Something
} finally{ // Bad.
// Something
}

if ( $condition ) {
try {
// Something
} finally {
// Something
}

} // Bad: blank line between.

// Handle PHP 8.0+ match expressions.
$expr = match ( $foo ) {
1 => 1,
2 => 2,
};

$expr = match($foo){
1 => 1,
2 => 2,
} ;

// phpcs:set WordPress.WhiteSpace.ControlStructureSpacing blank_line_check true
$expr = match ( $foo ) {

1 => 1,
2 => 2,

};

// phpcs:set WordPress.WhiteSpace.ControlStructureSpacing blank_line_check false

if ( $condition ) {
$expr = match ( $foo ) {
1 => 1,
2 => 2,
};


} // Bad: blank line between.

// Ignore spacing rules in combination with enums as they tend to have their own rules.
// phpcs:set WordPress.WhiteSpace.ControlStructureSpacing blank_line_check true
if ( $foo ) {


enum MyEnumA {
// Code here
}


}
// phpcs:set WordPress.WhiteSpace.ControlStructureSpacing blank_line_check false

enum MyEnumB {
if ( $foo ) {
// Code here
}


} // OK, defer to enum rules.
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,79 @@ if ( $foo ) {
// Something
} // End try/catch <- Bad: "blank line after".
}

// Handle (try/catch/) finally statements as well.
try {
// Something
} catch ( Exception $e ) {
// Something
} finally { // OK.
// Something
}

try {
// Something
} finally { // Bad.
// Something
}

try {
// Something
} finally { // Bad.
// Something
}

if ( $condition ) {
try {
// Something
} finally {
// Something
}
} // Bad: blank line between.

// Handle PHP 8.0+ match expressions.
$expr = match ( $foo ) {
1 => 1,
2 => 2,
};

$expr = match ( $foo ) {
1 => 1,
2 => 2,
} ;

// phpcs:set WordPress.WhiteSpace.ControlStructureSpacing blank_line_check true
$expr = match ( $foo ) {
1 => 1,
2 => 2,
};

// phpcs:set WordPress.WhiteSpace.ControlStructureSpacing blank_line_check false

if ( $condition ) {
$expr = match ( $foo ) {
1 => 1,
2 => 2,
};
} // Bad: blank line between.

// Ignore spacing rules in combination with enums as they tend to have their own rules.
// phpcs:set WordPress.WhiteSpace.ControlStructureSpacing blank_line_check true
if ( $foo ) {


enum MyEnumA {
// Code here
}


}
// phpcs:set WordPress.WhiteSpace.ControlStructureSpacing blank_line_check false

enum MyEnumB {
if ( $foo ) {
// Code here
}


} // OK, defer to enum rules.
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ public function getErrorList( $testFile = '' ) {
159 => 1,
169 => 1,
179 => 1,
195 => 1,
203 => 2,
212 => 1,
222 => 5,
228 => 1,
232 => 1,
241 => 1,
);

return $ret;
Expand Down