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

CS: remove yoda conditions from VIPCS codebase #571

Merged
merged 2 commits into from
Jul 30, 2020
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
6 changes: 5 additions & 1 deletion .phpcs.xml.dist
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
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 @@ -185,9 +185,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 @@ -198,7 +198,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
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
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
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
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
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
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