Skip to content

Commit b093d62

Browse files
committed
Functions/DynamicCalls: bug fix - fix memory + performance issue
The sniff maintains a cache of all the variables it has seen and their assigned value. When a variable is encountered, it would: * Check if it was an (plain text) assignment and if so, register the variable name + value to the cache. * Next, call the `find_dynamic_calls()` method, which first checks if any variables have been registered to the cache before doing anything. * And then checks for dynamic function calls and if one is found, checks if the variable used is one registered in the cache with a value we are looking for. This is highly inefficient as text string variable assignments are common and, as it was, _every single one_ would be added to the cache. With a large code base, that means that the cache could grow pretty large. It also means that the logic to determine if something is a dynamic function call would be executed even when there would be no text strings registered in the cache which could match any of the ones we're looking for. By changing the order of the logic, the memory leak and performance inefficiency is removed. With the updated logic, the sniff will: * Check if it was an (plain text) assignment **and if the text string matches one we're looking for** and if so, register the variable name + value to the cache. * Next, call the `find_dynamic_calls()` method, which first checks if any variables have been registered to the cache before doing anything. * And then checks for dynamic function calls. This means that if none of the previous assignments encountered matches any of the target text strings (~ 99% of the time), this sniff will bow out at step 2 before executing the logic to check if a variable assignment is a dynamic function call.
1 parent e607fb0 commit b093d62

File tree

1 file changed

+17
-24
lines changed

1 file changed

+17
-24
lines changed

WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,15 @@ class DynamicCallsSniff extends Sniff {
3131
* @var array
3232
*/
3333
private $blacklisted_functions = [
34-
'assert',
35-
'compact',
36-
'extract',
37-
'func_get_args',
38-
'func_get_arg',
39-
'func_num_args',
40-
'get_defined_vars',
41-
'mb_parse_str',
42-
'parse_str',
34+
'assert' => true,
35+
'compact' => true,
36+
'extract' => true,
37+
'func_get_args' => true,
38+
'func_get_arg' => true,
39+
'func_num_args' => true,
40+
'get_defined_vars' => true,
41+
'mb_parse_str' => true,
42+
'parse_str' => true,
4343
];
4444

4545
/**
@@ -138,11 +138,17 @@ private function collect_variables() {
138138
/*
139139
* If we reached the end of the loop and the $value_ptr was set, we know for sure
140140
* this was a plain text string variable assignment.
141-
*
142-
* Register its name and value in the internal array for later usage.
143141
*/
144142
$current_var_value = $this->strip_quotes( $this->tokens[ $value_ptr ]['content'] );
145143

144+
if ( isset( $this->blacklisted_functions[ $current_var_value ] ) === false ) {
145+
// Text string is not one of the ones we're looking for.
146+
return;
147+
}
148+
149+
/*
150+
* Register the variable name and value in the internal array for later usage.
151+
*/
146152
$this->variables_arr[ $current_var_name ] = $current_var_value;
147153
}
148154

@@ -183,19 +189,6 @@ private function find_dynamic_calls() {
183189

184190
$t_item_key = $this->stackPtr + $i;
185191

186-
/*
187-
* We have a variable match, but make sure it contains name of a function which is on our blacklist.
188-
*/
189-
190-
if ( ! in_array(
191-
$this->variables_arr[ $this->tokens[ $this->stackPtr ]['content'] ],
192-
$this->blacklisted_functions,
193-
true
194-
) ) {
195-
return;
196-
}
197-
198-
// We do, so report.
199192
$message = 'Dynamic calling is not recommended in the case of %s.';
200193
$data = [ $this->variables_arr[ $this->tokens[ $this->stackPtr ]['content'] ] ];
201194
$this->phpcsFile->addError( $message, $t_item_key, 'DynamicCalls', $data );

0 commit comments

Comments
 (0)