Skip to content

Commit 9f2d7de

Browse files
phpstan-botphpstan-bot
committed
Resolve variable variables to their possible names in isset(), empty(), ??, ??= and unset()
- Add `PHPStan\Analyser\VariableNameResolver`, which maps a `Variable` node to the list of variable names it can refer to, each paired with the scope narrowed to that name. For `$$name`/`${$name}` the names come from the constant strings of the name expression, mirroring what `VariableHandler::resolveType()` already does. - `MutatingScope::issetCheck()` and `issetCheckUndefined()` previously only handled `Variable` nodes with a literal string name. Variable variables fell through to the generic tail, which reports every expression as "always set" - the source of the false positive `nullCoalesce.unnecessary` on `${$field} ?? null`. Both now go through the resolver. - Mirror the same handling in `PHPStan\Rules\IssetCheck` (extracted into `checkVariable()`), so variable variables produce the same `Variable $x ... is never defined.` / `... always exists and is not nullable.` messages as their plain counterparts, in `isset()`, `empty()`, `??` and `??=`. - `UnsetRule`: resolve variable variables in `canBeUnset()` and drop the `Node\Identifier` requirement on property fetches, so `unset($foo->{$name})` reports readonly/hooked property unsets like `unset($foo->ro)` does. - `PropertyReflectionFinder::findPropertyReflectionFromNode()`: resolve `Foo::${$name}` from a single constant string name, matching what the instance property fetch branch already did. - `PropertyDescriptor::describeProperty()`: use the property reflection's name instead of the fetch's name node, which printed `Foo::$name` (the variable holding the property name) for `$foo->{$name}`. - `DefinedVariableRule` now reuses `VariableNameResolver` instead of its own copy of the name-resolution logic. - Probed and found already correct: `$obj->{$name}` and `Foo::${$name}` returning a conservative "maybe" from `issetCheck()`, nullsafe property fetches, and array dim fetches on variable variables (which delegate to the variable itself).
1 parent 6f5a635 commit 9f2d7de

17 files changed

Lines changed: 575 additions & 72 deletions

‎src/Analyser/MutatingScope.php‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,19 +1085,29 @@ private function resolveType(string $exprString, Expr $node): Type
10851085
public function issetCheck(Expr $expr, callable $typeCallback, ?bool $result = null): ?bool
10861086
{
10871087
// mirrored in PHPStan\Rules\IssetCheck
1088-
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
1089-
$hasVariable = $this->hasVariableType($expr->name);
1088+
if ($expr instanceof Node\Expr\Variable) {
1089+
$variableScopes = VariableNameResolver::resolveNamesWithScopes($this, $expr);
1090+
if ($variableScopes === null) {
1091+
return null;
1092+
}
1093+
1094+
$hasVariable = TrinaryLogic::lazyExtremeIdentity(
1095+
$variableScopes,
1096+
static fn (array $variableScope): TrinaryLogic => $variableScope[1]->hasVariableType($variableScope[0]),
1097+
);
10901098
if ($hasVariable->maybe()) {
10911099
return null;
10921100
}
10931101

10941102
if ($result === null) {
10951103
if ($hasVariable->yes()) {
1096-
if ($expr->name === '_SESSION') {
1097-
return null;
1104+
foreach ($variableScopes as [$variableName]) {
1105+
if ($variableName === '_SESSION') {
1106+
return null;
1107+
}
10981108
}
10991109

1100-
return $typeCallback($this->getVariableType($expr->name));
1110+
return $typeCallback($this->getType($expr));
11011111
}
11021112

11031113
return false;
@@ -1209,8 +1219,16 @@ public function issetCheck(Expr $expr, callable $typeCallback, ?bool $result = n
12091219

12101220
private function issetCheckUndefined(Expr $expr): ?bool
12111221
{
1212-
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
1213-
$hasVariable = $this->hasVariableType($expr->name);
1222+
if ($expr instanceof Node\Expr\Variable) {
1223+
$variableScopes = VariableNameResolver::resolveNamesWithScopes($this, $expr);
1224+
if ($variableScopes === null) {
1225+
return null;
1226+
}
1227+
1228+
$hasVariable = TrinaryLogic::lazyExtremeIdentity(
1229+
$variableScopes,
1230+
static fn (array $variableScope): TrinaryLogic => $variableScope[1]->hasVariableType($variableScope[0]),
1231+
);
12141232
if (!$hasVariable->no()) {
12151233
return null;
12161234
}

‎src/Analyser/VariableNameResolver.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Analyser;
4+
5+
use PhpParser\Node\Expr\BinaryOp\Identical;
6+
use PhpParser\Node\Expr\Variable;
7+
use PhpParser\Node\Scalar\String_;
8+
use function is_string;
9+
10+
/**
11+
* Resolves which variable names a `Variable` node can refer to.
12+
*
13+
* For variable variables like `$$name` or `${$name}` the names come from the
14+
* constant strings the name expression can evaluate to. Each name is paired with
15+
* the scope narrowed by that name so callers see the variable types belonging to it.
16+
*/
17+
final class VariableNameResolver
18+
{
19+
20+
/**
21+
* Returns null when the names cannot be determined.
22+
*
23+
* @return non-empty-list<array{string, Scope}>|null
24+
*/
25+
public static function resolveNamesWithScopes(Scope $scope, Variable $variable): ?array
26+
{
27+
if (is_string($variable->name)) {
28+
return [[$variable->name, $scope]];
29+
}
30+
31+
$namesWithScopes = [];
32+
foreach ($scope->getType($variable->name)->getConstantStrings() as $constantString) {
33+
$name = $constantString->getValue();
34+
$namesWithScopes[] = [
35+
$name,
36+
$scope->filterByTruthyValue(new Identical($variable->name, new String_($name))),
37+
];
38+
}
39+
40+
if ($namesWithScopes === []) {
41+
return null;
42+
}
43+
44+
return $namesWithScopes;
45+
}
46+
47+
}

‎src/Rules/IssetCheck.php‎

Lines changed: 71 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PhpParser\Node;
66
use PhpParser\Node\Expr;
77
use PHPStan\Analyser\Scope;
8+
use PHPStan\Analyser\VariableNameResolver;
89
use PHPStan\DependencyInjection\AutowiredParameter;
910
use PHPStan\DependencyInjection\AutowiredService;
1011
use PHPStan\Node\Expr\PropertyInitializationExpr;
@@ -13,7 +14,6 @@
1314
use PHPStan\Type\NeverType;
1415
use PHPStan\Type\Type;
1516
use PHPStan\Type\VerbosityLevel;
16-
use function is_string;
1717
use function sprintf;
1818
use function str_starts_with;
1919

@@ -42,36 +42,23 @@ public function __construct(
4242
public function check(Expr $expr, Scope $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, ?IdentifierRuleError $error = null): ?IdentifierRuleError
4343
{
4444
// mirrored in PHPStan\Analyser\MutatingScope::issetCheck()
45-
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
46-
$hasVariable = $scope->hasVariableType($expr->name);
47-
if ($hasVariable->maybe()) {
45+
if ($expr instanceof Node\Expr\Variable) {
46+
$variableScopes = VariableNameResolver::resolveNamesWithScopes($scope, $expr);
47+
if ($variableScopes === null) {
4848
return null;
4949
}
5050

51-
if ($error === null) {
52-
if ($hasVariable->yes()) {
53-
if ($expr->name === '_SESSION') {
54-
return null;
55-
}
56-
57-
$type = $this->treatPhpDocTypesAsCertain ? $scope->getScopeType($expr) : $scope->getScopeNativeType($expr);
58-
if (!$type instanceof NeverType) {
59-
return $this->generateError(
60-
$type,
61-
sprintf('Variable $%s %s always exists and', $expr->name, $operatorDescription),
62-
$typeMessageCallback,
63-
$identifier,
64-
'variable',
65-
);
66-
}
51+
$variableErrors = [];
52+
foreach ($variableScopes as [$variableName, $variableScope]) {
53+
$variableError = $this->checkVariable($expr, $variableName, $variableScope, $operatorDescription, $identifier, $typeMessageCallback, $error);
54+
if ($variableError === null) {
55+
return null;
6756
}
6857

69-
return RuleErrorBuilder::message(sprintf('Variable $%s %s is never defined.', $expr->name, $operatorDescription))
70-
->identifier(sprintf('%s.variable', $identifier))
71-
->build();
58+
$variableErrors[] = $variableError;
7259
}
7360

74-
return $error;
61+
return $variableErrors[0];
7562
} elseif ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) {
7663
$type = $this->treatPhpDocTypesAsCertain
7764
? $scope->getScopeType($expr->var)
@@ -275,20 +262,74 @@ static function (Type $type) use ($typeMessageCallback): ?string {
275262

276263
/**
277264
* @param ErrorIdentifier $identifier
265+
* @param callable(Type): ?string $typeMessageCallback
278266
*/
279-
private function checkUndefined(Expr $expr, Scope $scope, string $operatorDescription, string $identifier): ?IdentifierRuleError
267+
private function checkVariable(
268+
Expr\Variable $expr,
269+
string $variableName,
270+
Scope $scope,
271+
string $operatorDescription,
272+
string $identifier,
273+
callable $typeMessageCallback,
274+
?IdentifierRuleError $error,
275+
): ?IdentifierRuleError
280276
{
281-
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
282-
$hasVariable = $scope->hasVariableType($expr->name);
283-
if (!$hasVariable->no()) {
284-
return null;
277+
$hasVariable = $scope->hasVariableType($variableName);
278+
if ($hasVariable->maybe()) {
279+
return null;
280+
}
281+
282+
if ($error === null) {
283+
if ($hasVariable->yes()) {
284+
if ($variableName === '_SESSION') {
285+
return null;
286+
}
287+
288+
$type = $this->treatPhpDocTypesAsCertain ? $scope->getScopeType($expr) : $scope->getScopeNativeType($expr);
289+
if (!$type instanceof NeverType) {
290+
return $this->generateError(
291+
$type,
292+
sprintf('Variable $%s %s always exists and', $variableName, $operatorDescription),
293+
$typeMessageCallback,
294+
$identifier,
295+
'variable',
296+
);
297+
}
285298
}
286299

287-
return RuleErrorBuilder::message(sprintf('Variable $%s %s is never defined.', $expr->name, $operatorDescription))
300+
return RuleErrorBuilder::message(sprintf('Variable $%s %s is never defined.', $variableName, $operatorDescription))
288301
->identifier(sprintf('%s.variable', $identifier))
289302
->build();
290303
}
291304

305+
return $error;
306+
}
307+
308+
/**
309+
* @param ErrorIdentifier $identifier
310+
*/
311+
private function checkUndefined(Expr $expr, Scope $scope, string $operatorDescription, string $identifier): ?IdentifierRuleError
312+
{
313+
if ($expr instanceof Node\Expr\Variable) {
314+
$variableScopes = VariableNameResolver::resolveNamesWithScopes($scope, $expr);
315+
if ($variableScopes === null) {
316+
return null;
317+
}
318+
319+
$variableErrors = [];
320+
foreach ($variableScopes as [$variableName, $variableScope]) {
321+
if (!$variableScope->hasVariableType($variableName)->no()) {
322+
return null;
323+
}
324+
325+
$variableErrors[] = RuleErrorBuilder::message(sprintf('Variable $%s %s is never defined.', $variableName, $operatorDescription))
326+
->identifier(sprintf('%s.variable', $identifier))
327+
->build();
328+
}
329+
330+
return $variableErrors[0];
331+
}
332+
292333
if ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) {
293334
$type = $this->treatPhpDocTypesAsCertain ? $scope->getScopeType($expr->var) : $scope->getScopeNativeType($expr->var);
294335
$dimType = $this->treatPhpDocTypesAsCertain ? $scope->getScopeType($expr->dim) : $scope->getScopeNativeType($expr->dim);

‎src/Rules/Properties/PropertyDescriptor.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
use PhpParser\Node;
66
use PHPStan\Analyser\Scope;
77
use PHPStan\DependencyInjection\AutowiredService;
8-
use PHPStan\Reflection\PropertyReflection;
8+
use PHPStan\Reflection\ExtendedPropertyReflection;
99
use PHPStan\Type\ObjectType;
1010
use PHPStan\Type\VerbosityLevel;
1111
use function sprintf;
@@ -17,7 +17,7 @@ final class PropertyDescriptor
1717
/**
1818
* @param Node\Expr\PropertyFetch|Node\Expr\StaticPropertyFetch $propertyFetch
1919
*/
20-
public function describeProperty(PropertyReflection $property, Scope $scope, $propertyFetch): string
20+
public function describeProperty(ExtendedPropertyReflection $property, Scope $scope, $propertyFetch): string
2121
{
2222
if ($propertyFetch instanceof Node\Expr\PropertyFetch) {
2323
$fetchedOnType = $scope->getType($propertyFetch->var);
@@ -31,13 +31,13 @@ public function describeProperty(PropertyReflection $property, Scope $scope, $pr
3131
$classDescription = $property->getDeclaringClass()->getDisplayName();
3232
}
3333

34-
/** @var Node\Identifier $name */
35-
$name = $propertyFetch->name;
34+
// the fetch name node is not usable for dynamic accesses like $foo->{$name}
35+
$name = $property->getName();
3636
if (!$property->isStatic()) {
37-
return sprintf('Property %s::$%s', $classDescription, $name->name);
37+
return sprintf('Property %s::$%s', $classDescription, $name);
3838
}
3939

40-
return sprintf('Static property %s::$%s', $classDescription, $name->name);
40+
return sprintf('Static property %s::$%s', $classDescription, $name);
4141
}
4242

4343
}

‎src/Rules/Properties/PropertyReflectionFinder.php‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,17 +103,22 @@ public function findPropertyReflectionFromNode($propertyFetch, Scope $scope): ?F
103103
return null;
104104
}
105105

106-
if (!$propertyFetch->name instanceof Node\Identifier) {
107-
return null;
108-
}
109-
110106
if ($propertyFetch->class instanceof Node\Name) {
111107
$propertyHolderType = $scope->resolveTypeByName($propertyFetch->class);
112108
} else {
113109
$propertyHolderType = $scope->getType($propertyFetch->class);
114110
}
115111

116-
return $this->findStaticPropertyReflection($propertyHolderType, $propertyFetch->name->name, $scope);
112+
if ($propertyFetch->name instanceof Node\Identifier) {
113+
return $this->findStaticPropertyReflection($propertyHolderType, $propertyFetch->name->name, $scope);
114+
}
115+
116+
$nameTypeConstantStrings = $scope->getType($propertyFetch->name)->getConstantStrings();
117+
if (count($nameTypeConstantStrings) === 1) {
118+
return $this->findStaticPropertyReflection($propertyHolderType, $nameTypeConstantStrings[0]->getValue(), $scope);
119+
}
120+
121+
return null;
117122
}
118123

119124
private function findInstancePropertyReflection(Type $propertyHolderType, string $propertyName, Scope $scope): ?FoundPropertyReflection

‎src/Rules/Variables/DefinedVariableRule.php‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,16 @@
33
namespace PHPStan\Rules\Variables;
44

55
use PhpParser\Node;
6-
use PhpParser\Node\Expr\BinaryOp\Identical;
76
use PhpParser\Node\Expr\Variable;
8-
use PhpParser\Node\Scalar\String_;
97
use PHPStan\Analyser\Scope;
8+
use PHPStan\Analyser\VariableNameResolver;
109
use PHPStan\DependencyInjection\AutowiredParameter;
1110
use PHPStan\DependencyInjection\RegisteredRule;
1211
use PHPStan\Rules\IdentifierRuleError;
1312
use PHPStan\Rules\Rule;
1413
use PHPStan\Rules\RuleErrorBuilder;
1514
use function array_merge;
1615
use function in_array;
17-
use function is_string;
1816
use function sprintf;
1917

2018
/**
@@ -40,23 +38,17 @@ public function getNodeType(): string
4038

4139
public function processNode(Node $node, Scope $scope): array
4240
{
43-
$errors = [];
44-
if (is_string($node->name)) {
45-
$variableNameScopes = [$node->name => $scope];
46-
} else {
47-
$nameType = $scope->getType($node->name);
48-
$variableNameScopes = [];
49-
foreach ($nameType->getConstantStrings() as $constantString) {
50-
$name = $constantString->getValue();
51-
$variableNameScopes[$name] = $scope->filterByTruthyValue(new Identical($node->name, new String_($name)));
52-
}
41+
$namesWithScopes = VariableNameResolver::resolveNamesWithScopes($scope, $node);
42+
if ($namesWithScopes === null) {
43+
return [];
5344
}
5445

55-
foreach ($variableNameScopes as $name => $variableScope) {
46+
$errors = [];
47+
foreach ($namesWithScopes as [$name, $variableScope]) {
5648
$errors = array_merge($errors, $this->processSingleVariable(
5749
$variableScope,
5850
$node,
59-
(string) $name, // @phpstan-ignore cast.useless
51+
$name,
6052
));
6153
}
6254

0 commit comments

Comments
 (0)