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

Allow for functions which set variables in the current scope #172

Open
jrfnl opened this issue May 12, 2020 · 3 comments
Open

Allow for functions which set variables in the current scope #172

jrfnl opened this issue May 12, 2020 · 3 comments
Labels

Comments

@jrfnl
Copy link
Collaborator

jrfnl commented May 12, 2020

In PHP there are a number of functions which can set variables in the current scope. Sometimes by reference to a passed parameter (first examples), sometimes just plainly in the current scope (second examples).

While the first set of examples is handled correctly by the sniff, the second set of examples is not.

Now, I'm not saying it will be easy to handle this or that I have a ready-made solution for this, but I figured opening an issue as a reminder to have a look at this at some point would be a good idea nevertheless.

1. Functions setting a variable by reference.

	parse_str($str, $output);
	echo $output['first']; 

	preg_match('`.*`', $first, $matches);
	var_dump($matches);

The sniff correctly doesn't throw an error for $matches or $output being used.

2. Functions setting variables in the current scope.

function foo() {
	$str = "first=value&second=value&arr[]=foo+bar&arr[]=baz";
	parse_str($str);
	echo $first;  // value
	echo $second;  // value
	echo $arr[0]; // foo bar
	echo $arr[1]; // baz
	
	$array = [
		'a' => 1,
		'b' => 'test',
	];
	extract($array);
	echo $a, $b;
}

The sniff as-is will throw errors for use of "undefined" $first, $second, $arr, $a, $b variables, while these are in actual fact all defined.

@jrfnl jrfnl added the bug label May 12, 2020
@sirbrillig
Copy link
Owner

sirbrillig commented May 12, 2020

I totally agree with number 2, and I'm not sure how to solve it easily. As for number 1, I'm not totally convinced it's incorrect. We have code in place already to detect pass-by-reference functions, so this code would pass:

$output = [];
parse_str($str, $output);

Without that first line, we're relying on PHP's "helpful" shortcut to use and define a variable at the same time. This ties in to the discussion here.

@jrfnl
Copy link
Collaborator Author

jrfnl commented May 12, 2020

Sorry for the confusion. This issue is about the second part. AFAIC (based on a simple test run), the examples from the first part are handled correctly already (which was already noted in the original issue description).

I only added the code for the first example because the functions involved overlap.

@sirbrillig
Copy link
Owner

Oops. Yes, sorry, I mis-read it! 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants