Skip to content

Commit 2c92101

Browse files
ondrejmirtesondrejmirtes
committed
Allow passing return values of by-reference functions/methods as by-ref args
- Functions and methods that return by reference (declared with &) produce valid lvalues that can be passed to by-reference parameters - Added callReturnsByReference() helper to FunctionCallParametersCheck that resolves reflection for MethodCall, StaticCall, and FuncCall nodes and checks returnsByReference() - Injected ReflectionProvider into FunctionCallParametersCheck for resolving function reflections - New regression test in tests/PHPStan/Rules/Functions/data/bug-757.php
1 parent 7912480 commit 2c92101

23 files changed

+129
-7
lines changed

‎CLAUDE.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,10 @@ PHPStan tracks whether expressions/statements have side effects ("impure points"
306306

307307
Bugs occur when impure points are missed (e.g. inherited constructors of anonymous classes) or when `clearstatcache()` calls don't invalidate filesystem function return types.
308308

309+
### FunctionCallParametersCheck: by-reference argument validation
310+
311+
`FunctionCallParametersCheck` (`src/Rules/FunctionCallParametersCheck.php`) validates arguments passed to functions/methods. For by-reference parameters, it checks whether the argument is a valid lvalue (variable, array dim fetch, property fetch). It also allows function/method calls that return by reference (`&getString()`, `&staticGetString()`, `&refFunction()`), using `returnsByReference()` on the resolved reflection. The class is manually instantiated in ~20 test files, so adding a constructor parameter requires updating all of them. The `Scope` interface provides `getMethodReflection()` for method calls, while `ReflectionProvider` (injected into the class) is needed for resolving function reflections.
312+
309313
### Testing patterns
310314

311315
- **Rule tests**: Extend `RuleTestCase`, implement `getRule()`, call `$this->analyse([__DIR__ . '/data/my-test.php'], [...expected errors...])`. Expected errors are `[message, line]` pairs. Test data files live in `tests/PHPStan/Rules/*/data/`.

‎src/Rules/FunctionCallParametersCheck.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use PHPStan\Reflection\ExtendedParameterReflection;
1212
use PHPStan\Reflection\ParameterReflection;
1313
use PHPStan\Reflection\ParametersAcceptor;
14+
use PHPStan\Reflection\ReflectionProvider;
1415
use PHPStan\Reflection\ResolvedFunctionVariant;
1516
use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper;
1617
use PHPStan\Rules\Properties\PropertyReflectionFinder;
@@ -47,6 +48,7 @@ public function __construct(
4748
private NullsafeCheck $nullsafeCheck,
4849
private UnresolvableTypeHelper $unresolvableTypeHelper,
4950
private PropertyReflectionFinder $propertyReflectionFinder,
51+
private ReflectionProvider $reflectionProvider,
5052
#[AutowiredParameter(ref: '%checkFunctionArgumentTypes%')]
5153
private bool $checkArgumentTypes,
5254
#[AutowiredParameter]
@@ -462,6 +464,10 @@ public function check(
462464
continue;
463465
}
464466

467+
if ($this->callReturnsByReference($argumentValue, $scope)) {
468+
continue;
469+
}
470+
465471
$errors[] = RuleErrorBuilder::message(sprintf(
466472
$parameterPassedByReferenceMessage,
467473
$this->describeParameter($parameter, $argumentName === null ? $i + 1 : null),
@@ -690,4 +696,46 @@ private function describeParameter(ParameterReflection $parameter, int|string|nu
690696
return implode(' ', $parts);
691697
}
692698

699+
private function callReturnsByReference(Expr $expr, Scope $scope): bool
700+
{
701+
if ($expr instanceof Node\Expr\MethodCall) {
702+
if (!$expr->name instanceof Node\Identifier) {
703+
return false;
704+
}
705+
$calledOnType = $scope->getType($expr->var);
706+
$methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->name);
707+
if ($methodReflection === null) {
708+
return false;
709+
}
710+
return $methodReflection->returnsByReference()->yes();
711+
}
712+
713+
if ($expr instanceof Node\Expr\StaticCall) {
714+
if (!$expr->name instanceof Node\Identifier) {
715+
return false;
716+
}
717+
if ($expr->class instanceof Node\Name) {
718+
$calledOnType = $scope->resolveTypeByName($expr->class);
719+
} else {
720+
$calledOnType = $scope->getType($expr->class);
721+
}
722+
$methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->name);
723+
if ($methodReflection === null) {
724+
return false;
725+
}
726+
return $methodReflection->returnsByReference()->yes();
727+
}
728+
729+
if ($expr instanceof Node\Expr\FuncCall) {
730+
if ($expr->name instanceof Node\Name) {
731+
if ($this->reflectionProvider->hasFunction($expr->name, $scope)) {
732+
$functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope);
733+
return $functionReflection->returnsByReference()->yes();
734+
}
735+
}
736+
}
737+
738+
return false;
739+
}
740+
693741
}

‎tests/PHPStan/Analyser/Bug9307CallMethodsRuleTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ protected function getRule(): Rule
2424
$ruleLevelHelper = new RuleLevelHelper($reflectionProvider, true, false, true, true, false, false, true);
2525
return new CallMethodsRule(
2626
new MethodCallCheck($reflectionProvider, $ruleLevelHelper, true, true),
27-
new FunctionCallParametersCheck($ruleLevelHelper, new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), true, true, true, true),
27+
new FunctionCallParametersCheck($ruleLevelHelper, new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), $reflectionProvider, true, true, true, true),
2828
);
2929
}
3030

‎tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ protected function getRule(): Rule
3737
new NullsafeCheck(),
3838
new UnresolvableTypeHelper(),
3939
new PropertyReflectionFinder(),
40+
$reflectionProvider,
4041
true,
4142
true,
4243
true,

‎tests/PHPStan/Rules/Classes/ClassConstantAttributesRuleTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ protected function getRule(): Rule
3232
new NullsafeCheck(),
3333
new UnresolvableTypeHelper(),
3434
new PropertyReflectionFinder(),
35+
$reflectionProvider,
3536
true,
3637
true,
3738
true,

‎tests/PHPStan/Rules/Classes/ForbiddenNameCheckExtensionRuleTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ protected function getRule(): Rule
2727
return new InstantiationRule(
2828
$container,
2929
$reflectionProvider,
30-
new FunctionCallParametersCheck(new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), true, true, true, true),
30+
new FunctionCallParametersCheck(new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), $reflectionProvider, true, true, true, true),
3131
new ClassNameCheck(
3232
new ClassCaseSensitivityCheck($reflectionProvider, true),
3333
new ClassForbiddenNameCheck($container),

‎tests/PHPStan/Rules/Classes/InstantiationRuleTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ protected function getRule(): Rule
2727
return new InstantiationRule(
2828
$container,
2929
$reflectionProvider,
30-
new FunctionCallParametersCheck(new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), true, true, true, true),
30+
new FunctionCallParametersCheck(new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), $reflectionProvider, true, true, true, true),
3131
new ClassNameCheck(
3232
new ClassCaseSensitivityCheck($reflectionProvider, true),
3333
new ClassForbiddenNameCheck($container),

‎tests/PHPStan/Rules/Constants/ConstantAttributesRuleTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ protected function getRule(): Rule
3838
new NullsafeCheck(),
3939
new UnresolvableTypeHelper(),
4040
new PropertyReflectionFinder(),
41+
$reflectionProvider,
4142
true,
4243
true,
4344
true,

‎tests/PHPStan/Rules/EnumCases/EnumCaseAttributesRuleTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ protected function getRule(): Rule
3333
new NullsafeCheck(),
3434
new UnresolvableTypeHelper(),
3535
new PropertyReflectionFinder(),
36+
$reflectionProvider,
3637
true,
3738
true,
3839
true,

‎tests/PHPStan/Rules/Functions/ArrowFunctionAttributesRuleTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ protected function getRule(): Rule
3232
new NullsafeCheck(),
3333
new UnresolvableTypeHelper(),
3434
new PropertyReflectionFinder(),
35+
$reflectionProvider,
3536
true,
3637
true,
3738
true,

0 commit comments

Comments
 (0)