Greetings,
You've really got my OCD tied in knots! :)
On Mon, Dec 19, 2016 at 7:29 AM, Richard Quadling <rquadling@xxxxxxxxx>
wrote:
> A very common pattern and the one that is documented is ...
>
> <?php
> $collection = [];
> foreach($collection as &$item) {
> // Payload.
> }
> unset($item);
>
> From PHP's perspective, this is all fine.
>
> From a static analysis perspective though, $item is a problem in the
> unset().
>
I think the key here is that all tools must be aware of how PHP treats
variables like $item. If you reference them before assigning a value to
them, in some contexts you're fine while in others you'll get a warning.
It's important to know how and why and in which situations it matters.
echo $x;
This will emit a warning because you are reading the value of an unset
variable, and tools would be correct to complain.
$x[] = 5;
This will happily create an empty array and assign it to $x without
warning. This is specified in PHP's specification and is safe, but the
context is more likely to be
function double(array $in) {
foreach ($in as $num) {
$x[] = 2 * $num;
}
return $x; // warning and null if $in is empty
}
For this reason, I always initialize arrays even if I can prove an item is
always assigned to it before using it.
Finally, we have unset(). This is a language construct instead of a
function, and if the comments on its documentation page [1] and my
experience are to believed, it accepts variables that have not been given a
value yet. PHP is unlike most other C-like languages in that variables are
created the first time the interpreter sees their name. They may not have a
value (i.e. "be set"), but you can reference them and pass them to unset()
and isset() without worry.
In the above code, $item will not exist as the collection is empty.
I would argue that $item *does* exist because the interpreter will create
it as a local variable as soon as it sees its name. It just won't have a
value. At least two of the comments on that page specifically mention that
it's okay to pass variables that have no value to unset().
"This is probably trivial but there is no error for unsetting a
non-existing variable."
and
"You don't need to check that a variable is set before you unset it."
Given that, I am fine with keeping the call to unset outside of the loop.
Static analysis tools should understand that isset() and unset() happily
accept variables without a value and not flag them as an error, or at the
very least make them an optional warning. Moving the call inside the loop
seems like you're appeasing the tool for the tool's sake. You shouldn't
assign null or some other dummy value to $matches before calling
preg_match(), and you shouldn't have to make sure a variable holds a
reference before passing it to unset().
I'm curious, though. Are you getting warnings from PHPStan or other tools?
Cheers!
David
[1]: http://php.net/manual/en/function.unset.php
