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

Improve global function call detection #1667

Merged
merged 7 commits into from
Mar 29, 2019
20 changes: 9 additions & 11 deletions WordPress/AbstractFunctionRestrictionsSniff.php
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
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
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
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
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
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