Skip to content

Commit

Permalink
Merge pull request #1667 from WordPress-Coding-Standards/feature/impr…
Browse files Browse the repository at this point in the history 
…ove-global-function-call-detection

Improve global function call detection
  • Loading branch information
@GaryJones
GaryJones authored Mar 29, 2019
2 parents 3ba3005 + 08f4a7f commit d3ac40a
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 53 deletions.
20 changes: 9 additions & 11 deletions WordPress/AbstractFunctionRestrictionsSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,15 @@ public function process_token( $stackPtr ) {
public function is_targetted_token( $stackPtr ) {

// Exclude function definitions, class methods, and namespaced calls.
if ( \T_STRING === $this->tokens[ $stackPtr ]['code'] && isset( $this->tokens[ ( $stackPtr - 1 ) ] ) ) {
if ( \T_STRING === $this->tokens[ $stackPtr ]['code'] ) {
if ( $this->is_class_object_call( $stackPtr ) === true ) {
return false;
}

if ( $this->is_token_namespaced( $stackPtr ) === true ) {
return false;
}

$prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true );

if ( false !== $prev ) {
Expand All @@ -222,21 +230,11 @@ public function is_targetted_token( $stackPtr ) {
\T_FUNCTION => \T_FUNCTION,
\T_CLASS => \T_CLASS,
\T_AS => \T_AS, // Use declaration alias.
\T_DOUBLE_COLON => \T_DOUBLE_COLON,
\T_OBJECT_OPERATOR => \T_OBJECT_OPERATOR,
);

if ( isset( $skipped[ $this->tokens[ $prev ]['code'] ] ) ) {
return false;
}

// Skip namespaced functions, ie: \foo\bar() not \bar().
if ( \T_NS_SEPARATOR === $this->tokens[ $prev ]['code'] ) {
$pprev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $prev - 1 ), null, true );
if ( false !== $pprev && \T_STRING === $this->tokens[ $pprev ]['code'] ) {
return false;
}
}
}

return true;
Expand Down
113 changes: 78 additions & 35 deletions WordPress/Sniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1461,23 +1461,12 @@ protected function is_in_isset_or_empty( $stackPtr ) {
}

if ( \T_STRING === $previous_code && 'array_key_exists' === $this->tokens[ $previous_non_empty ]['content'] ) {
$before_function = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $previous_non_empty - 1 ), null, true, null, true );

if ( false !== $before_function ) {
if ( \T_OBJECT_OPERATOR === $this->tokens[ $before_function ]['code']
|| \T_DOUBLE_COLON === $this->tokens[ $before_function ]['code']
) {
// Method call.
return false;
}
if ( $this->is_class_object_call( $previous_non_empty ) === true ) {
return false;
}

if ( \T_NS_SEPARATOR === $this->tokens[ $before_function ]['code'] ) {
$before_before = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $before_function - 1 ), null, true, null, true );
if ( false !== $before_before && \T_STRING === $this->tokens[ $before_before ]['code'] ) {
// Namespaced function call.
return false;
}
}
if ( $this->is_token_namespaced( $previous_non_empty ) === true ) {
return false;
}

$second_param = $this->get_function_call_parameter( $previous_non_empty, 2 );
Expand All @@ -1489,6 +1478,71 @@ protected function is_in_isset_or_empty( $stackPtr ) {
return false;
}

/**
* Check if a particular token is a (static or non-static) call to a class method or property.
*
* @internal Note: this may still mistake a namespaced function imported via a `use` statement for
* a global function!
*
* @since 2.1.0
*
* @param int $stackPtr The index of the token in the stack.
*
* @return bool
*/
protected function is_class_object_call( $stackPtr ) {
$before = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true, null, true );

if ( false === $before ) {
return false;
}

if ( \T_OBJECT_OPERATOR !== $this->tokens[ $before ]['code']
&& \T_DOUBLE_COLON !== $this->tokens[ $before ]['code']
) {
return false;
}

return true;
}

/**
* Check if a particular token is prefixed with a namespace.
*
* @internal This will give a false positive if the file is not namespaced and the token is prefixed
* with `namespace\`.
*
* @since 2.1.0
*
* @param int $stackPtr The index of the token in the stack.
*
* @return bool
*/
protected function is_token_namespaced( $stackPtr ) {
$prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true, null, true );

if ( false === $prev ) {
return false;
}

if ( \T_NS_SEPARATOR !== $this->tokens[ $prev ]['code'] ) {
return false;
}

$before_prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $prev - 1 ), null, true, null, true );
if ( false === $before_prev ) {
return false;
}

if ( \T_STRING !== $this->tokens[ $before_prev ]['code']
&& \T_NAMESPACE !== $this->tokens[ $before_prev ]['code']
) {
return false;
}

return true;
}

/**
* Check if something is only being sanitized.
*
Expand Down Expand Up @@ -1853,22 +1907,14 @@ protected function is_validated( $stackPtr, $array_key = null, $in_condition_onl
continue 2;
}

$previous_non_empty = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $i - 1 ), null, true, null, true );
if ( false !== $previous_non_empty ) {
if ( \T_OBJECT_OPERATOR === $this->tokens[ $previous_non_empty ]['code']
|| \T_DOUBLE_COLON === $this->tokens[ $previous_non_empty ]['code']
) {
// Method call.
continue 2;
}
if ( $this->is_class_object_call( $i ) === true ) {
// Method call.
continue 2;
}

if ( \T_NS_SEPARATOR === $this->tokens[ $previous_non_empty ]['code'] ) {
$pprev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $previous_non_empty - 1 ), null, true, null, true );
if ( false !== $pprev && \T_STRING === $this->tokens[ $pprev ]['code'] ) {
// Namespaced function call.
continue 2;
}
}
if ( $this->is_token_namespaced( $i ) === true ) {
// Namespaced function call.
continue 2;
}

$params = $this->get_function_call_parameters( $i );
Expand Down Expand Up @@ -2680,10 +2726,7 @@ public function is_use_of_global_constant( $stackPtr ) {
return false;
}

if ( false !== $prev
&& \T_NS_SEPARATOR === $this->tokens[ $prev ]['code']
&& \T_STRING === $this->tokens[ ( $prev - 1 ) ]['code']
) {
if ( $this->is_token_namespaced( $stackPtr ) === true ) {
// Namespaced constant of the same name.
return false;
}
Expand Down
5 changes: 1 addition & 4 deletions WordPress/Sniffs/WP/DiscouragedConstantsSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,7 @@ public function process_arbitrary_tstring( $stackPtr ) {
return;
}

if ( false !== $prev
&& \T_NS_SEPARATOR === $this->tokens[ $prev ]['code']
&& \T_STRING === $this->tokens[ ( $prev - 1 ) ]['code']
) {
if ( $this->is_token_namespaced( $stackPtr ) === true ) {
// Namespaced constant of the same name.
return;
}
Expand Down
3 changes: 1 addition & 2 deletions WordPress/Sniffs/WP/GlobalVariablesOverrideSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,7 @@ protected function process_global_statement( $stackPtr, $in_function_scope ) {
}

// Don't throw false positives for static class properties.
$previous = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $ptr - 1 ), null, true, null, true );
if ( false !== $previous && \T_DOUBLE_COLON === $this->tokens[ $previous ]['code'] ) {
if ( $this->is_class_object_call( $ptr ) === true ) {
continue;
}

Expand Down
5 changes: 5 additions & 0 deletions WordPress/Tests/PHP/DevelopmentFunctionsUnitTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,8 @@ phpinfo(); // Ok - within excluded group.
// phpcs:set WordPress.PHP.DevelopmentFunctions exclude[]
trigger_error(); // Error.
phpinfo(); // Error.

Wrapper_Class::var_dump(); // OK, not the native PHP function.
$wrapper ->var_dump(); // OK, not the native PHP function.
namespace\var_dump(); // OK as long as the file is namespaced.
MyNamespace\var_dump(); // OK, namespaced function.
2 changes: 1 addition & 1 deletion WordPress/Tests/WP/DiscouragedConstantsUnitTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ define( 'My\STYLESHEETPATH', 'something' );
if ( defined( 'STYLESHEETPATH' ) ) { // Ok.
// Do something unrelated.
}

echo namespace\STYLESHEETPATH; // "Magic" namespace operator.

/*
* These are all bad.
Expand Down

0 comments on commit d3ac40a

Please sign in to comment.