Skip to content

Commit

Permalink
Merge pull request #571 from Automattic/fix/no-yoda-in-own-codebase
Browse files Browse the repository at this point in the history
  • Loading branch information
@GaryJones
GaryJones authored Jul 30, 2020
2 parents a3334db + fe6ccec commit 00d4f35
Show file tree
Hide file tree
Showing 45 changed files with 290 additions and 286 deletions.
6 changes: 5 additions & 1 deletion .phpcs.xml.dist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<exclude name="WordPress.Files.FileName"/>
<exclude name="WordPress.NamingConventions.ValidVariableName"/>
<exclude name="Generic.Arrays.DisallowShortArraySyntax"/>
<exclude name="WordPress.PHP.YodaConditions"/>
</rule>

<rule ref="WordPress-Docs"/>
Expand All @@ -32,9 +33,12 @@

<rule ref="PSR2.Methods.FunctionClosingBrace"/>

<!-- Disallow long array syntax -->
<!-- Disallow long array syntax. -->
<rule ref="Generic.Arrays.DisallowLongArraySyntax"/>

<!-- Disallow Yoda conditions. -->
<rule ref="Generic.ControlStructures.DisallowYodaConditions"/>

<!-- Check code for cross-version PHP compatibility. -->
<config name="testVersion" value="5.4-"/>
<rule ref="PHPCompatibility">
Expand Down
10 changes: 5 additions & 5 deletions WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ abstract class AbstractVariableRestrictionsSniff extends Sniff {
*/
public function register() {
// Retrieve the groups only once and don't set up a listener if there are no groups.
if ( false === $this->setup_groups() ) {
if ( $this->setup_groups() === false ) {
return [];
}

Expand Down Expand Up @@ -138,7 +138,7 @@ public function process_token( $stackPtr ) {
if ( \in_array( $token['code'], [ \T_OBJECT_OPERATOR, \T_DOUBLE_COLON ], true ) ) { // This only works for object vars and array members.
$method = $this->phpcsFile->findNext( \T_WHITESPACE, $stackPtr + 1, null, true );
$possible_parenthesis = $this->phpcsFile->findNext( \T_WHITESPACE, $method + 1, null, true );
if ( \T_OPEN_PARENTHESIS === $this->tokens[ $possible_parenthesis ]['code'] ) {
if ( $this->tokens[ $possible_parenthesis ]['code'] === \T_OPEN_PARENTHESIS ) {
return; // So .. it is a function after all !
}
}
Expand Down Expand Up @@ -190,9 +190,9 @@ public function process_token( $stackPtr ) {

$patterns = array_map( [ $this, 'test_patterns' ], $patterns );
$pattern = implode( '|', $patterns );
$delim = ( \T_OPEN_SQUARE_BRACKET !== $token['code'] && \T_HEREDOC !== $token['code'] ) ? '\b' : '';
$delim = ( $token['code'] !== \T_OPEN_SQUARE_BRACKET && $token['code'] !== \T_HEREDOC ) ? '\b' : '';

if ( \T_DOUBLE_QUOTED_STRING === $token['code'] || \T_HEREDOC === $token['code'] ) {
if ( $token['code'] === \T_DOUBLE_QUOTED_STRING || $token['code'] === \T_HEREDOC ) {
$var = $token['content'];
}

Expand All @@ -203,7 +203,7 @@ public function process_token( $stackPtr ) {
$this->addMessage(
$group['message'],
$stackPtr,
'error' === $group['type'],
$group['type'] === 'error',
$this->string_to_errorcode( $groupName . '_' . $match[1] ),
[ $var ]
);
Expand Down
36 changes: 18 additions & 18 deletions WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,15 +218,15 @@ protected function processTokenWithinScope( File $phpcsFile, $stackPtr, $currSco
$methodName = $phpcsFile->getDeclarationName( $stackPtr );

$parentClassName = $phpcsFile->findExtendedClassName( $currScope );
if ( false === $parentClassName ) {
if ( $parentClassName === false ) {
// This class does not extend any other class.
return;
}

// Meed to define the originalParentClassName since we might override the parentClassName due to signature notations grouping.
$originalParentClassName = $parentClassName;

if ( false === array_key_exists( $parentClassName, $this->checkClasses ) ) {
if ( array_key_exists( $parentClassName, $this->checkClasses ) === false ) {
// This class does not extend a class we are interested in.
foreach ( $this->checkClassesGroups as $parent => $children ) {
// But it might be one of the grouped classes.
Expand All @@ -237,14 +237,14 @@ protected function processTokenWithinScope( File $phpcsFile, $stackPtr, $currSco
}
}
}
if ( false === array_key_exists( $parentClassName, $this->checkClasses ) ) {
if ( array_key_exists( $parentClassName, $this->checkClasses ) === false ) {
// This class really does not extend a class we are interested in.
return;
}
}

if ( false === array_key_exists( $methodName, $this->checkClasses[ $parentClassName ] ) &&
false === in_array( $methodName, $this->checkClasses[ $parentClassName ], true )
if ( array_key_exists( $methodName, $this->checkClasses[ $parentClassName ] ) === false &&
in_array( $methodName, $this->checkClasses[ $parentClassName ], true ) === false
) {
// This method is not a one we are interested in.
return;
Expand All @@ -258,11 +258,11 @@ protected function processTokenWithinScope( File $phpcsFile, $stackPtr, $currSco
$extra_params = array_slice( $signatureParams, count( $parentSignature ) - count( $signatureParams ) );
$all_extra_params_have_default = true;
foreach ( $extra_params as $extra_param ) {
if ( false === array_key_exists( 'default', $extra_param ) || 'true' !== $extra_param['default'] ) {
if ( array_key_exists( 'default', $extra_param ) === false || $extra_param['default'] !== 'true' ) {
$all_extra_params_have_default = false;
}
}
if ( true === $all_extra_params_have_default ) {
if ( $all_extra_params_have_default === true ) {
return; // We're good.
}
}
Expand All @@ -274,16 +274,16 @@ protected function processTokenWithinScope( File $phpcsFile, $stackPtr, $currSco

$i = 0;
foreach ( $parentSignature as $key => $param ) {
if ( true === is_array( $param ) ) {
if ( is_array( $param ) === true ) {
if (
(
true === array_key_exists( 'default', $param ) &&
false === array_key_exists( 'default', $signatureParams[ $i ] )
array_key_exists( 'default', $param ) === true &&
array_key_exists( 'default', $signatureParams[ $i ] ) === false
) || (
true === array_key_exists( 'pass_by_reference', $param ) &&
array_key_exists( 'pass_by_reference', $param ) === true &&
$param['pass_by_reference'] !== $signatureParams[ $i ]['pass_by_reference']
) || (
true === array_key_exists( 'variable_length', $param ) &&
array_key_exists( 'variable_length', $param ) === true &&
$param['variable_length'] !== $signatureParams[ $i ]['variable_length']
)
) {
Expand Down Expand Up @@ -329,26 +329,26 @@ private function generateParamList( $methodSignature ) {
$paramList = [];
foreach ( $methodSignature as $param => $options ) {
$paramName = '$';
if ( false === is_array( $options ) ) {
if ( is_array( $options ) === false ) {
$paramList[] = '$' . $options;
continue;
}

if ( true === array_key_exists( 'name', $options ) ) {
if ( array_key_exists( 'name', $options ) === true ) {
$paramName = $options['name'];
} else {
$paramName .= $param;
}

if ( true === array_key_exists( 'variable_length', $options ) && true === $options['variable_length'] ) {
if ( array_key_exists( 'variable_length', $options ) === true && $options['variable_length'] === true ) {
$paramName = '...' . $paramName;
}

if ( true === array_key_exists( 'pass_by_reference', $options ) && true === $options['pass_by_reference'] ) {
if ( array_key_exists( 'pass_by_reference', $options ) === true && $options['pass_by_reference'] === true ) {
$paramName = '&' . $paramName;
}

if ( true === array_key_exists( 'default', $options ) && false === empty( $options['default'] ) ) {
if ( array_key_exists( 'default', $options ) === true && empty( $options['default'] ) === false ) {
$paramName .= ' = ' . trim( $options['default'] );
}

Expand All @@ -371,7 +371,7 @@ protected function loadFunctionNamesInScope( File $phpcsFile, $currScope ) {
$tokens = $phpcsFile->getTokens();

for ( $i = ( $tokens[ $currScope ]['scope_opener'] + 1 ); $i < $tokens[ $currScope ]['scope_closer']; $i++ ) {
if ( T_FUNCTION !== $tokens[ $i ]['code'] ) {
if ( $tokens[ $i ]['code'] !== T_FUNCTION ) {
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion WordPressVIPMinimum/Sniffs/Classes/RestrictedExtendClassesSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public function getGroups() {
public function process_matched_token( $stackPtr, $group_name, $matched_content ) {
$tokens = $this->phpcsFile->getTokens();

if ( T_EXTENDS !== $tokens[ $stackPtr ]['code'] ) {
if ( $tokens[ $stackPtr ]['code'] !== T_EXTENDS ) {
// If not extending, bail.
return;
}
Expand Down
12 changes: 6 additions & 6 deletions WordPressVIPMinimum/Sniffs/Compatibility/ZoninatorSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,26 @@ public function register() {
*/
public function process_token( $stackPtr ) {

if ( 'wpcom_vip_load_plugin' !== $this->tokens[ $stackPtr ]['content'] ) {
if ( $this->tokens[ $stackPtr ]['content'] !== 'wpcom_vip_load_plugin' ) {
return;
}

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

if ( T_OPEN_PARENTHESIS !== $this->tokens[ $openBracket ]['code'] ) {
if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) {
// Not a function call.
return;
}

$plugin_name = $this->phpcsFile->findNext( Tokens::$emptyTokens, $openBracket + 1, null, true );

if ( 'zoninator' !== $this->remove_wrapping_quotation_marks( $this->tokens[ $plugin_name ]['content'] ) ) {
if ( $this->remove_wrapping_quotation_marks( $this->tokens[ $plugin_name ]['content'] ) !== 'zoninator' ) {
return;
}

$comma = $this->phpcsFile->findNext( Tokens::$emptyTokens, $plugin_name + 1, null, true );

if ( ! $comma || 'PHPCS_T_COMMA' !== $this->tokens[ $comma ]['code'] ) {
if ( ! $comma || $this->tokens[ $comma ]['code'] !== 'PHPCS_T_COMMA' ) {
// We are loading the default version.
return;
}
Expand All @@ -63,15 +63,15 @@ public function process_token( $stackPtr ) {

$comma = $this->phpcsFile->findNext( Tokens::$emptyTokens, $folder + 1, null, true );

if ( ! $comma || 'PHPCS_T_COMMA' !== $this->tokens[ $comma ]['code'] ) {
if ( ! $comma || $this->tokens[ $comma ]['code'] !== 'PHPCS_T_COMMA' ) {
// We are loading the default version.
return;
}

$version = $this->phpcsFile->findNext( Tokens::$emptyTokens, $comma + 1, null, true );
$version = $this->remove_wrapping_quotation_marks( $this->tokens[ $version ]['content'] );

if ( true === version_compare( $version, '0.8', '>=' ) ) {
if ( version_compare( $version, '0.8', '>=' ) === true ) {
$message = 'Zoninator of version >= v0.8 requires WordPress core REST API. Please, make sure the `wpcom_vip_load_wp_rest_api()` is being called on all sites loading this file.';
$this->phpcsFile->addWarning( $message, $stackPtr, 'RequiresRESTAPI' );
}
Expand Down
8 changes: 4 additions & 4 deletions WordPressVIPMinimum/Sniffs/Constants/ConstantStringSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,26 @@ public function register() {
*/
public function process_token( $stackPtr ) {

if ( false === in_array( $this->tokens[ $stackPtr ]['content'], [ 'define', 'defined' ], true ) ) {
if ( in_array( $this->tokens[ $stackPtr ]['content'], [ 'define', 'defined' ], true ) === false ) {
return;
}

// Find the next non-empty token.
$nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true, null, true );

if ( T_OPEN_PARENTHESIS !== $this->tokens[ $nextToken ]['code'] ) {
if ( $this->tokens[ $nextToken ]['code'] !== T_OPEN_PARENTHESIS ) {
// Not a function call.
return;
}

if ( false === isset( $this->tokens[ $nextToken ]['parenthesis_closer'] ) ) {
if ( isset( $this->tokens[ $nextToken ]['parenthesis_closer'] ) === false ) {
// Not a function call.
return;
}

$nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextToken + 1, null, true, null, true );

if ( T_CONSTANT_ENCAPSED_STRING !== $this->tokens[ $nextToken ]['code'] ) {
if ( $this->tokens[ $nextToken ]['code'] !== T_CONSTANT_ENCAPSED_STRING ) {
$message = 'Constant name, as a string, should be used along with `%s()`.';
$data = [ $this->tokens[ $stackPtr ]['content'] ];
$this->phpcsFile->addError( $message, $nextToken, 'NotCheckingConstantName', $data );
Expand Down
18 changes: 9 additions & 9 deletions WordPressVIPMinimum/Sniffs/Constants/RestrictedConstantsSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,18 @@ public function register() {
*/
public function process_token( $stackPtr ) {

if ( T_STRING === $this->tokens[ $stackPtr ]['code'] ) {
if ( $this->tokens[ $stackPtr ]['code'] === T_STRING ) {
$constantName = $this->tokens[ $stackPtr ]['content'];
} else {
$constantName = trim( $this->tokens[ $stackPtr ]['content'], "\"'" );
}

if ( false === in_array( $constantName, $this->restrictedConstantNames, true ) && false === in_array( $constantName, $this->restrictedConstantDeclaration, true ) ) {
if ( in_array( $constantName, $this->restrictedConstantNames, true ) === false && in_array( $constantName, $this->restrictedConstantDeclaration, true ) === false ) {
// Not the constant we are looking for.
return;
}

if ( T_STRING === $this->tokens[ $stackPtr ]['code'] && true === in_array( $constantName, $this->restrictedConstantNames, true ) ) {
if ( $this->tokens[ $stackPtr ]['code'] === T_STRING && in_array( $constantName, $this->restrictedConstantNames, true ) === true ) {
$message = 'Code is touching the `%s` constant. Make sure it\'s used appropriately.';
$data = [ $constantName ];
$this->phpcsFile->addWarning( $message, $stackPtr, 'UsingRestrictedConstant', $data );
Expand All @@ -79,12 +79,12 @@ public function process_token( $stackPtr ) {
// Find the previous non-empty token.
$openBracket = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $stackPtr - 1, null, true, null, true );

if ( T_OPEN_PARENTHESIS !== $this->tokens[ $openBracket ]['code'] ) {
if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) {
// Not a function call.
return;
}

if ( false === isset( $this->tokens[ $openBracket ]['parenthesis_closer'] ) ) {
if ( isset( $this->tokens[ $openBracket ]['parenthesis_closer'] ) === false ) {
// Not a function call.
return;
}
Expand All @@ -93,17 +93,17 @@ public function process_token( $stackPtr ) {
$search = Tokens::$emptyTokens;
$search[] = T_BITWISE_AND;
$previous = $this->phpcsFile->findPrevious( $search, $openBracket - 1, null, true );
if ( T_FUNCTION === $this->tokens[ $previous ]['code'] ) {
if ( $this->tokens[ $previous ]['code'] === T_FUNCTION ) {
// It's a function definition, not a function call.
return;
}

if ( true === in_array( $this->tokens[ $previous ]['code'], Tokens::$functionNameTokens, true ) ) {
if ( in_array( $this->tokens[ $previous ]['code'], Tokens::$functionNameTokens, true ) === true ) {
$data = [ $constantName ];
if ( 'define' === $this->tokens[ $previous ]['content'] ) {
if ( $this->tokens[ $previous ]['content'] === 'define' ) {
$message = 'The definition of `%s` constant is prohibited. Please use a different name.';
$this->phpcsFile->addError( $message, $previous, 'DefiningRestrictedConstant', $data );
} elseif ( true === in_array( $constantName, $this->restrictedConstantNames, true ) ) {
} elseif ( in_array( $constantName, $this->restrictedConstantNames, true ) === true ) {
$message = 'Code is touching the `%s` constant. Make sure it\'s used appropriately.';
$this->phpcsFile->addWarning( $message, $previous, 'UsingRestrictedConstant', $data );
}
Expand Down
22 changes: 11 additions & 11 deletions WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,35 +94,35 @@ public function register() {
public function process_token( $stackPtr ) {
$nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true, null, true );

if ( T_OPEN_PARENTHESIS === $this->tokens[ $nextToken ]['code'] ) {
if ( $this->tokens[ $nextToken ]['code'] === T_OPEN_PARENTHESIS ) {
// The construct is using parenthesis, grab the next non empty token.
$nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextToken + 1, null, true, null, true );
}

if ( T_DIR === $this->tokens[ $nextToken ]['code'] || '__DIR__' === $this->tokens[ $nextToken ]['content'] ) {
if ( $this->tokens[ $nextToken ]['code'] === T_DIR || $this->tokens[ $nextToken ]['content'] === '__DIR__' ) {
// The construct is using __DIR__ which is fine.
return;
}

if ( T_VARIABLE === $this->tokens[ $nextToken ]['code'] ) {
if ( $this->tokens[ $nextToken ]['code'] === T_VARIABLE ) {
$message = 'File inclusion using variable (`%s`). Probably needs manual inspection.';
$data = [ $this->tokens[ $nextToken ]['content'] ];
$this->phpcsFile->addWarning( $message, $nextToken, 'UsingVariable', $data );
return;
}

if ( T_STRING === $this->tokens[ $nextToken ]['code'] ) {
if ( true === in_array( $this->tokens[ $nextToken ]['content'], $this->getPathFunctions, true ) ) {
if ( $this->tokens[ $nextToken ]['code'] === T_STRING ) {
if ( in_array( $this->tokens[ $nextToken ]['content'], $this->getPathFunctions, true ) === true ) {
// The construct is using one of the function for getting correct path which is fine.
return;
}

if ( true === in_array( $this->tokens[ $nextToken ]['content'], $this->allowedConstants, true ) ) {
if ( in_array( $this->tokens[ $nextToken ]['content'], $this->allowedConstants, true ) === true ) {
// The construct is using one of the allowed constants which is fine.
return;
}

if ( true === array_key_exists( $this->tokens[ $nextToken ]['content'], $this->restrictedConstants ) ) {
if ( array_key_exists( $this->tokens[ $nextToken ]['content'], $this->restrictedConstants ) === true ) {
// The construct is using one of the restricted constants.
$message = '`%s` constant might not be defined or available. Use `%s()` instead.';
$data = [ $this->tokens[ $nextToken ]['content'], $this->restrictedConstants[ $this->tokens[ $nextToken ]['content'] ] ];
Expand All @@ -131,22 +131,22 @@ public function process_token( $stackPtr ) {
}

$nextNextToken = $this->phpcsFile->findNext( array_merge( Tokens::$emptyTokens, [ T_COMMENT ] ), $nextToken + 1, null, true, null, true );
if ( 1 === preg_match( '/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/', $this->tokens[ $nextToken ]['content'] ) && T_OPEN_PARENTHESIS !== $this->tokens[ $nextNextToken ]['code'] ) {
if ( preg_match( '/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/', $this->tokens[ $nextToken ]['content'] ) === 1 && $this->tokens[ $nextNextToken ]['code'] !== T_OPEN_PARENTHESIS ) {
// The construct is using custom constant, which needs manual inspection.
$message = 'File inclusion using custom constant (`%s`). Probably needs manual inspection.';
$data = [ $this->tokens[ $nextToken ]['content'] ];
$this->phpcsFile->addWarning( $message, $nextToken, 'UsingCustomConstant', $data );
return;
}

if ( 0 === strpos( $this->tokens[ $nextToken ]['content'], '$' ) ) {
if ( strpos( $this->tokens[ $nextToken ]['content'], '$' ) === 0 ) {
$message = 'File inclusion using variable (`%s`). Probably needs manual inspection.';
$data = [ $this->tokens[ $nextToken ]['content'] ];
$this->phpcsFile->addWarning( $message, $nextToken, 'UsingVariable', $data );
return;
}

if ( true === in_array( $this->tokens[ $nextToken ]['content'], $this->slashingFunctions, true ) ) {
if ( in_array( $this->tokens[ $nextToken ]['content'], $this->slashingFunctions, true ) === true ) {
// The construct is using one of the slashing functions, it's probably correct.
return;
}
Expand All @@ -163,7 +163,7 @@ public function process_token( $stackPtr ) {
return;
}

if ( T_CONSTANT_ENCAPSED_STRING === $this->tokens[ $nextToken ]['code'] && filter_var( str_replace( [ '"', "'" ], '', $this->tokens[ $nextToken ]['content'] ), FILTER_VALIDATE_URL ) ) {
if ( $this->tokens[ $nextToken ]['code'] === T_CONSTANT_ENCAPSED_STRING && filter_var( str_replace( [ '"', "'" ], '', $this->tokens[ $nextToken ]['content'] ), FILTER_VALIDATE_URL ) ) {
$message = 'Include path must be local file source, external URLs are prohibited on WordPress VIP.';
$this->phpcsFile->addError( $message, $nextToken, 'ExternalURL' );
return;
Expand Down
Loading

0 comments on commit 00d4f35

Please sign in to comment.