Skip to content

Commit 8e63627

Browse files
committed
Functions/DynamicCalls: bug fix - don't blindly use the next text string
A variable value may be build up of multiple tokens. As it was, the sniff would look for the first text string token after the equal sign within the variable assignment statement, but this disregards that: 1. The text string token found may not be the only token in the statement. 2. A statement can end on a PHP close tag (possibly a bug in PHPCS itself, but that's another matter), which would lead the sniff to look at the next statement for text strings. Fixed now. Includes unit tests, the first four of which resulted in false positives previously.
1 parent df8c596 commit 8e63627

File tree

2 files changed

+40
-13
lines changed

2 files changed

+40
-13
lines changed

WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -112,26 +112,36 @@ private function collect_variables() {
112112
}
113113

114114
/*
115-
* Find encapsulated string ( "" ).
115+
* Find assignments which only assign a plain text string.
116116
*/
117-
$t_item_key = $this->phpcsFile->findNext(
118-
[ T_CONSTANT_ENCAPSED_STRING ],
119-
$t_item_key + 1,
120-
null,
121-
false,
122-
null,
123-
true
124-
);
117+
$end_of_statement = $this->phpcsFile->findNext( [ T_SEMICOLON, T_CLOSE_TAG ], ( $t_item_key + 1 ) );
118+
$value_ptr = null;
119+
120+
for ( $i = $t_item_key + 1; $i < $end_of_statement; $i++ ) {
121+
if ( isset( Tokens::$emptyTokens[ $this->tokens[ $i ]['code'] ] ) === true ) {
122+
continue;
123+
}
124+
125+
if ( $this->tokens[ $i ]['code'] !== T_CONSTANT_ENCAPSED_STRING ) {
126+
// Not a plain text string value. Value cannot be determined reliably.
127+
return;
128+
}
129+
130+
$value_ptr = $i;
131+
}
125132

126-
if ( $t_item_key === false ) {
133+
if ( isset( $value_ptr ) === false ) {
134+
// Parse error. Bow out.
127135
return;
128136
}
129137

130138
/*
131-
* We have found variable-assignment, register its name and value in the
132-
* internal array for later usage.
139+
* If we reached the end of the loop and the $value_ptr was set, we know for sure
140+
* this was a plain text string variable assignment.
141+
*
142+
* Register its name and value in the internal array for later usage.
133143
*/
134-
$current_var_value = $this->tokens[ $t_item_key ]['content'];
144+
$current_var_value = $this->tokens[ $value_ptr ]['content'];
135145

136146
$this->variables_arr[ $current_var_name ] = str_replace( "'", '', $current_var_value );
137147
}

WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.inc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,20 @@ $my_okay_func(); // OK.
1313

1414
$test_with_comment /*comment*/ = 'func_get_args';
1515
$test_with_comment(); // Bad.
16+
17+
$test_getting_the_actual_value_1 = function_call( 'extract' );
18+
$test_getting_the_actual_value_1(); // OK. Unclear what the actual variable value will be.
19+
20+
$test_getting_the_actual_value_2 = $array['compact'];
21+
$test_getting_the_actual_value_2(); // OK. Unclear what the actual variable value will be.
22+
23+
$test_getting_the_actual_value_3 = 10 ?>
24+
<div>html</div>
25+
<?php
26+
echo 'extract';
27+
$test_getting_the_actual_value_3(); // OK. Broken function call, but not calling extract().
28+
29+
$test_getting_the_actual_value_4 = 'get_defined_vars' . $source;
30+
$test_getting_the_actual_value_4(); // OK. Unclear what the actual variable value will be.
31+
32+
$ensure_no_notices_are_thrown_on_parse_error = /*comment*/ ;

0 commit comments

Comments
 (0)