Skip to content

Fix args are mistakenly handled as immediately-invoked #4145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Aug 1, 2025
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
10 changes: 8 additions & 2 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -5139,16 +5139,22 @@ private function processArgs(
$scopeToPass = $closureBindScope;
}

$parameterCallableType = null;
if ($parameterType !== null) {
$parameterCallableType = TypeUtils::findCallableType($parameterType);
}

if ($parameter instanceof ExtendedParameterReflection) {
$parameterCallImmediately = $parameter->isImmediatelyInvokedCallable();
if ($parameterCallImmediately->maybe()) {
$callCallbackImmediately = $calleeReflection instanceof FunctionReflection;
$callCallbackImmediately = $parameterCallableType !== null && $calleeReflection instanceof FunctionReflection;
} else {
$callCallbackImmediately = $parameterCallImmediately->yes();
}
} else {
$callCallbackImmediately = $calleeReflection instanceof FunctionReflection;
$callCallbackImmediately = $parameterCallableType !== null && $calleeReflection instanceof FunctionReflection;
}

if ($arg->value instanceof Expr\Closure) {
$restoreThisScope = null;
if (
Expand Down
18 changes: 18 additions & 0 deletions src/Type/TypeUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,24 @@ public static function findThisType(Type $type): ?ThisType
return null;
}

public static function findCallableType(Type $type): ?Type
{
if ($type->isCallable()->yes()) {
return $type;
}

if ($type instanceof UnionType) {
foreach ($type->getTypes() as $innerType) {
$callableType = self::findCallableType($innerType);
if ($callableType !== null) {
return $callableType;
}
}
}

return null;
}

/**
* @return HasPropertyType[]
*/
Expand Down
35 changes: 35 additions & 0 deletions stubs/core.stub
Original file line number Diff line number Diff line change
Expand Up @@ -327,3 +327,38 @@ function get_defined_constants(bool $categorize = false): array {}
*/
function getopt(string $short_options, array $long_options = [], &$rest_index = null) {}

/**
* @param callable|int $handler
* @param-later-invoked-callable $handler
*/
function pcntl_signal(int $signal, $handler, bool $restart_syscalls = true): bool {}

/**
* @param-later-invoked-callable $callback
*/
function set_error_handler(?callable $callback, int $error_levels = E_ALL): ?callable {}

/**
* @param-later-invoked-callable $callback
*/
function set_exception_handler(?callable $callback): ?callable {}

/**
* @param-later-invoked-callable $callback
*/
function spl_autoload_register(?callable $callback = null, bool $throw = true, bool $prepend = false): bool {}

/**
* @param-later-invoked-callable $callback
*/
function register_shutdown_function(callable $callback, mixed ...$args): void {}

/**
* @param-later-invoked-callable $callback
*/
function header_register_callback(callable $callback): bool {}

/**
* @param-later-invoked-callable $callback
*/
function register_tick_function(callable $callback, mixed ...$args): bool {}
21 changes: 20 additions & 1 deletion tests/PHPStan/Analyser/ExpressionResultTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
use PHPStan\ShouldNotHappenException;
use PHPStan\Testing\PHPStanTestCase;
use PHPStan\TrinaryLogic;
use PHPStan\Type\ArrayType;
use PHPStan\Type\IntegerType;
use PHPStan\Type\MixedType;
use PHPUnit\Framework\Attributes\DataProvider;
use function count;
use function get_class;
Expand Down Expand Up @@ -127,6 +129,22 @@ public static function dataIsAlwaysTerminating(): array
'var_dump(exit()."a");',
true,
],
[
'array_push($arr, fn() => "exit");',
false,
],
[
'array_push($arr, function() { exit(); });',
false,
],
[
'array_push($arr, "exit");',
false,
],
[
'array_unshift($arr, "exit");',
false,
],
];
}

Expand Down Expand Up @@ -155,7 +173,8 @@ public function testIsAlwaysTerminating(
/** @var ScopeFactory $scopeFactory */
$scopeFactory = self::getContainer()->getByType(ScopeFactory::class);
$scope = $scopeFactory->create(ScopeContext::create('test.php'))
->assignVariable('x', new IntegerType(), new IntegerType(), TrinaryLogic::createYes());
->assignVariable('x', new IntegerType(), new IntegerType(), TrinaryLogic::createYes())
->assignVariable('arr', new ArrayType(new MixedType(), new MixedType()), new ArrayType(new MixedType(), new MixedType()), TrinaryLogic::createYes());

$result = $nodeScopeResolver->processExprNode(
$stmt,
Expand Down
25 changes: 25 additions & 0 deletions tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -348,4 +348,29 @@ public function testBug13232d(): void
]);
}

#[RequiresPhp('>= 8.1')]
public function testBug13288(): void
{
$this->treatPhpDocTypesAsCertain = false;
$this->analyse([__DIR__ . '/data/bug-13288.php'], []);
}

public function testBug13311(): void
{
$this->treatPhpDocTypesAsCertain = false;
$this->analyse([__DIR__ . '/data/bug-13311.php'], []);
}

public function testBug13307(): void
{
$this->treatPhpDocTypesAsCertain = false;
$this->analyse([__DIR__ . '/data/bug-13307.php'], []);
}

public function testBug13331(): void
{
$this->treatPhpDocTypesAsCertain = false;
$this->analyse([__DIR__ . '/data/bug-13331.php'], []);
}

}
11 changes: 11 additions & 0 deletions tests/PHPStan/Rules/DeadCode/data/bug-13288.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php // lint >= 8.1

namespace Bug13288;

function error_to_exception(int $errno, string $errstr, string $errfile = 'unknown', int $errline = 0): never {
throw new \ErrorException($errstr, $errno, $errno, $errfile, $errline);
}

set_error_handler(error_to_exception(...));

echo 'ok';
24 changes: 24 additions & 0 deletions tests/PHPStan/Rules/DeadCode/data/bug-13307.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

namespace Bug13307;

function dd(): never {
exit(1);
}

class HelloWorld
{
public function testMethod(): string
{
var_dump("\Bug13307\dd");

return "test";
}

public function testMethod2(): string
{
var_dump("\Bug13307\DD");

return "test";
}
}
14 changes: 14 additions & 0 deletions tests/PHPStan/Rules/DeadCode/data/bug-13311.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

namespace Bug13311;

use Exception;

$error_handler = static function () { throw new Exception(); };
set_error_handler($error_handler, \E_NOTICE | \E_WARNING);

try {
$out = iconv($from, $to . $iconv_options, $str);
} catch (Throwable $e) {
$out = false;
}
12 changes: 12 additions & 0 deletions tests/PHPStan/Rules/DeadCode/data/bug-13331.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php declare(strict_types = 1);

namespace Bug13331;

$signalHandler = function () {
exit();
};

pcntl_async_signals(true);
pcntl_signal(SIGINT, $signalHandler);
pcntl_signal(SIGQUIT, $signalHandler);
pcntl_signal(SIGTERM, $signalHandler);
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,10 @@ function doBar3(): void
{
throw new \LogicException(); // error
}

function bug13288(array $a): void
{
array_push($a, function() {
throw new \LogicException(); // ok, as array_push() will not invoke the function
});
}
29 changes: 29 additions & 0 deletions tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,30 @@ public function testRule(): void
'Impure output between PHP opening and closing tags in pure function PureFunction\justContainsInlineHtml().',
160,
],
[
'Impure call to function array_push() in pure function PureFunction\bug13288().',
171,
],
[
'Impure call to function array_push() in pure function PureFunction\bug13288().',
175,
],
[
'Impure call to function array_push() in pure function PureFunction\bug13288().',
182,
],
[
'Impure exit in pure function PureFunction\bug13288b().',
200,
],
[
'Impure exit in pure function PureFunction\bug13288c().',
217,
],
[
'Impure exit in pure function PureFunction\bug13288d().',
230,
],
]);
}

Expand Down Expand Up @@ -180,4 +204,9 @@ public function testBug13201(): void
$this->analyse([__DIR__ . '/data/bug-13201.php'], []);
}

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

}
21 changes: 21 additions & 0 deletions tests/PHPStan/Rules/Pure/data/bug-12119.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

namespace Bug12119;

/** @phpstan-pure */
function testFunction(string $s): string { return ''; }

class HelloWorld
{
/** @phpstan-pure */
public function testMethod(string $s): string { return ''; }

/** @phpstan-pure */
public function sayHello(string $s): string
{
$a = $this->testMethod('random_int');
$b = testFunction('random_int');

return $a . $b;
}
}
85 changes: 85 additions & 0 deletions tests/PHPStan/Rules/Pure/data/pure-function.php
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,88 @@ function justContainsInlineHtml()
</table>
<?php
}

/** @phpstan-pure */
function bug13288(array $a)
{
array_push($a, function () { // error because by ref arg
exit(); // ok, as array_push() will not invoke the function
});

array_push($a, // error because by ref arg
fn() => exit() // ok, as array_push() will not invoke the function
);

$exitingClosure = function () {
exit();
};
array_push($a, // error because by ref arg
$exitingClosure // ok, as array_push() will not invoke the function
);

takesString("exit"); // ok, as the maybe callable type string is not typed with immediately-invoked-callable
}

/** @phpstan-pure */
function takesString(string $s) {
}

/** @phpstan-pure */
function bug13288b()
{
$exitingClosure = function () {
exit();
};

takesMixed($exitingClosure); // error because immediately invoked
}

/**
* @phpstan-pure
* @param-immediately-invoked-callable $m
*/
function takesMixed(mixed $m) {
}

/** @phpstan-pure */
function bug13288c()
{
$exitingClosure = function () {
exit();
};

takesMaybeCallable($exitingClosure);
}

/** @phpstan-pure */
function takesMaybeCallable(?callable $c) { // arguments passed to functions are considered "immediately called" by default
}

/** @phpstan-pure */
function bug13288d()
{
$exitingClosure = function () {
exit();
};
takesMaybeCallable2($exitingClosure);
}

/** @phpstan-pure */
function takesMaybeCallable2(?\Closure $c) { // Closures are considered "immediately called"
}

/** @phpstan-pure */
function bug13288e(MyClass $m)
{
$exitingClosure = function () {
exit();
};
$m->takesMaybeCallable($exitingClosure);
}

class MyClass {
/** @phpstan-pure */
function takesMaybeCallable(?callable $c) { // arguments passed to methods are considered "later called" by default
}
}

Loading