-
-
Notifications
You must be signed in to change notification settings - Fork 934
Closed
Labels
Description
Feature request
The documentation for PHP's foreach contains a warning about the dangers of byref foreach.
Example code with error: https://phpstan.org/r/940c636d-f30d-4105-9bc4-b7a0f3ceb63f
Because of this, I wrote a custom rule to handle this case:
Details
<?php
declare(strict_types=1);
namespace App\Tools\Phpstan\Rule;
use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\ShouldNotHappenException;
/**
* @implements Rule<Node\Stmt\Foreach_>
*/
class MissingUnsetAfterByRefForeachRule implements Rule
{
public function getNodeType(): string
{
return Node\Stmt\Foreach_::class;
}
public function processNode(Node $node, Scope $scope): array
{
if (!$node->byRef) {
return [];
}
if (
!$node->valueVar instanceof Node\Expr\Variable
|| !\is_string($node->valueVar->name)
) {
throw new ShouldNotHappenException();
}
$byRefVarName = $node->valueVar->name;
/** @var \WeakReference<Node>|null $weakNextNode */
$weakNextNode = $node->getAttribute('weak_next');
$nextNode = $weakNextNode?->get();
if ($nextNode instanceof Node\Stmt\Unset_) {
foreach ($nextNode->vars as $var) {
if (
$var instanceof Node\Expr\Variable
&& $var->name === $byRefVarName
) {
return [];
}
}
}
return [
RuleErrorBuilder::message(\sprintf('Missing unset($%s) after by ref foreach.', $byRefVarName))
->identifier('hc.missingUnsetAfterByRefForeach')
->build(),
];
}
}I think it would be nice to include a rule into the main phpstan project as this weird PHP behavior can catch us devs off guard.
For this particular implementation, I had to add the phpparser's built in new \PhpParser\NodeVisitor\NodeConnectingVisitor(weakReferences: true) as a phpstan.parser.richParserNodeVisitor. At least for my project, this didn't cause any big memory usage or slow anything down.
Did PHPStan help you today? Did it make you happy in any way?
<3