Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,10 @@ PHPStan tracks whether expressions/statements have side effects ("impure points"

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.

### FunctionCallParametersCheck: by-reference argument validation

`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.

### Testing patterns

- **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/`.
Expand Down
48 changes: 48 additions & 0 deletions src/Rules/FunctionCallParametersCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use PHPStan\Reflection\ExtendedParameterReflection;
use PHPStan\Reflection\ParameterReflection;
use PHPStan\Reflection\ParametersAcceptor;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Reflection\ResolvedFunctionVariant;
use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper;
use PHPStan\Rules\Properties\PropertyReflectionFinder;
Expand Down Expand Up @@ -47,6 +48,7 @@ public function __construct(
private NullsafeCheck $nullsafeCheck,
private UnresolvableTypeHelper $unresolvableTypeHelper,
private PropertyReflectionFinder $propertyReflectionFinder,
private ReflectionProvider $reflectionProvider,
#[AutowiredParameter(ref: '%checkFunctionArgumentTypes%')]
private bool $checkArgumentTypes,
#[AutowiredParameter]
Expand Down Expand Up @@ -462,6 +464,10 @@ public function check(
continue;
}

if ($this->callReturnsByReference($argumentValue, $scope)) {
continue;
}

$errors[] = RuleErrorBuilder::message(sprintf(
$parameterPassedByReferenceMessage,
$this->describeParameter($parameter, $argumentName === null ? $i + 1 : null),
Expand Down Expand Up @@ -690,4 +696,46 @@ private function describeParameter(ParameterReflection $parameter, int|string|nu
return implode(' ', $parts);
}

private function callReturnsByReference(Expr $expr, Scope $scope): bool
{
if ($expr instanceof Node\Expr\MethodCall) {
if (!$expr->name instanceof Node\Identifier) {
return false;
}
$calledOnType = $scope->getType($expr->var);
$methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->name);
if ($methodReflection === null) {
return false;
}
return $methodReflection->returnsByReference()->yes();
}

if ($expr instanceof Node\Expr\StaticCall) {
if (!$expr->name instanceof Node\Identifier) {
return false;
}
if ($expr->class instanceof Node\Name) {
$calledOnType = $scope->resolveTypeByName($expr->class);
} else {
$calledOnType = $scope->getType($expr->class);
}
$methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->name);
if ($methodReflection === null) {
return false;
}
return $methodReflection->returnsByReference()->yes();
}

if ($expr instanceof Node\Expr\FuncCall) {
if ($expr->name instanceof Node\Name) {
if ($this->reflectionProvider->hasFunction($expr->name, $scope)) {
$functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope);
return $functionReflection->returnsByReference()->yes();
}
}
}

return false;
}

}
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/Bug9307CallMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ protected function getRule(): Rule
$ruleLevelHelper = new RuleLevelHelper($reflectionProvider, true, false, true, true, false, false, true);
return new CallMethodsRule(
new MethodCallCheck($reflectionProvider, $ruleLevelHelper, true, true),
new FunctionCallParametersCheck($ruleLevelHelper, new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), true, true, true, true),
new FunctionCallParametersCheck($ruleLevelHelper, new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), $reflectionProvider, true, true, true, true),
);
}

Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ protected function getRule(): Rule
return new InstantiationRule(
$container,
$reflectionProvider,
new FunctionCallParametersCheck(new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), true, true, true, true),
new FunctionCallParametersCheck(new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), $reflectionProvider, true, true, true, true),
new ClassNameCheck(
new ClassCaseSensitivityCheck($reflectionProvider, true),
new ClassForbiddenNameCheck($container),
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Rules/Classes/InstantiationRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ protected function getRule(): Rule
return new InstantiationRule(
$container,
$reflectionProvider,
new FunctionCallParametersCheck(new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), true, true, true, true),
new FunctionCallParametersCheck(new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), $reflectionProvider, true, true, true, true),
new ClassNameCheck(
new ClassCaseSensitivityCheck($reflectionProvider, true),
new ClassForbiddenNameCheck($container),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
4 changes: 3 additions & 1 deletion tests/PHPStan/Rules/Functions/CallCallablesRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ class CallCallablesRuleTest extends RuleTestCase

protected function getRule(): Rule
{
$ruleLevelHelper = new RuleLevelHelper(self::createReflectionProvider(), true, false, true, $this->checkExplicitMixed, false, false, true);
$reflectionProvider = self::createReflectionProvider();
$ruleLevelHelper = new RuleLevelHelper($reflectionProvider, true, false, true, $this->checkExplicitMixed, false, false, true);
return new CallCallablesRule(
new FunctionCallParametersCheck(
$ruleLevelHelper,
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ protected function getRule(): Rule
$broker = self::createReflectionProvider();
return new CallToFunctionParametersRule(
$broker,
new FunctionCallParametersCheck(new RuleLevelHelper($broker, true, false, true, $this->checkExplicitMixed, $this->checkImplicitMixed, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), true, true, true, true),
new FunctionCallParametersCheck(new RuleLevelHelper($broker, true, false, true, $this->checkExplicitMixed, $this->checkImplicitMixed, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), $broker, true, true, true, true),
);
}

Expand Down Expand Up @@ -308,6 +308,25 @@ public function testPassingNonVariableToParameterPassedByReference(): void
]);
}

public function testBug757(): void
{
$this->analyse([__DIR__ . '/data/bug-757.php'], [
[
'Parameter #1 $s of function Bug757\useString is passed by reference, so it expects variables only.',
37,
],
[
'Parameter #1 $s of function Bug757\useString is passed by reference, so it expects variables only.',
40,
],
]);
}

public function testBug13711(): void
{
$this->analyse([__DIR__ . '/data/bug-13711.php'], []);
}

public function testImplodeOnPhp74(): void
{
$errors = [
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Rules/Functions/CallUserFuncRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class CallUserFuncRuleTest extends RuleTestCase
protected function getRule(): Rule
{
$reflectionProvider = self::createReflectionProvider();
return new CallUserFuncRule($reflectionProvider, new FunctionCallParametersCheck(new RuleLevelHelper($reflectionProvider, true, false, true, true, false, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), true, true, true, true));
return new CallUserFuncRule($reflectionProvider, new FunctionCallParametersCheck(new RuleLevelHelper($reflectionProvider, true, false, true, true, false, false, true), new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), $reflectionProvider, true, true, true, true));
}

#[RequiresPhp('>= 8.0')]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Rules/Functions/ParamAttributesRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
20 changes: 20 additions & 0 deletions tests/PHPStan/Rules/Functions/data/bug-13711.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php declare(strict_types = 1);

namespace Bug13711;

class Foo
{
/** @var array<string> */
private array $array = [];

/** @return array<string> */
public function &getList(): array
{
return $this->array;
}

public function rewind(): void
{
reset($this->getList());
}
}
41 changes: 41 additions & 0 deletions tests/PHPStan/Rules/Functions/data/bug-757.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Bug757;

class A {
/** @var string */
public $foo = "bar";

public function &getString() : string {
return $this->foo;
}

public function getStringNoRef() : string {
return $this->foo;
}

public static function &staticGetString() : string {
static $s = "bar";
return $s;
}
}

function &refFunction() : string {
static $s = "bar";
return $s;
}

function noRefFunction() : string {
return "bar";
}

function useString(string &$s) : void {}

function () {
$a = new A();
useString($a->getString()); // ok - returns by reference
useString($a->getStringNoRef()); // error - does not return by reference
useString(A::staticGetString()); // ok - returns by reference
useString(refFunction()); // ok - returns by reference
useString(noRefFunction()); // error - does not return by reference
};
2 changes: 1 addition & 1 deletion tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ protected function getRule(): Rule
$ruleLevelHelper = new RuleLevelHelper($reflectionProvider, $this->checkNullables, $this->checkThisOnly, $this->checkUnionTypes, $this->checkExplicitMixed, $this->checkImplicitMixed, false, true);
return new CallMethodsRule(
new MethodCallCheck($reflectionProvider, $ruleLevelHelper, true, true),
new FunctionCallParametersCheck($ruleLevelHelper, new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), true, true, true, true),
new FunctionCallParametersCheck($ruleLevelHelper, new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), $reflectionProvider, true, true, true, true),
);
}

Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Rules/Methods/MethodAttributesRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Rules/Traits/TraitAttributesRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ protected function getRule(): Rule
new NullsafeCheck(),
new UnresolvableTypeHelper(),
new PropertyReflectionFinder(),
$reflectionProvider,
true,
true,
true,
Expand Down
Loading