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
38 changes: 20 additions & 18 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@
use function is_array;
use function is_int;
use function is_string;
use function max;
use function sprintf;
use function strtolower;
use function trim;
Expand Down Expand Up @@ -3301,33 +3302,34 @@ private function doCreateCallableParameters(Scope $scope, Expr $closureExpr, ?ar

$acceptors = $passedToType->getCallableParametersAcceptors($scope);
foreach ($acceptors as $acceptor) {
$acceptorParameters = array_map(static fn (ParameterReflection $callableParameter) => new NativeParameterReflection(
$callableParameter->getName(),
$callableParameter->isOptional(),
$callableParameter->getType(),
$callableParameter->passedByReference(),
$callableParameter->isVariadic(),
$callableParameter->getDefaultValue(),
), $acceptor->getParameters());

if ($callableParameters === null) {
$callableParameters = array_map(static fn (ParameterReflection $callableParameter) => new NativeParameterReflection(
$callableParameter->getName(),
$callableParameter->isOptional(),
$callableParameter->getType(),
$callableParameter->passedByReference(),
$callableParameter->isVariadic(),
$callableParameter->getDefaultValue(),
), $acceptor->getParameters());
$callableParameters = $acceptorParameters;
continue;
}

$newParameters = [];
foreach ($acceptor->getParameters() as $i => $callableParameter) {
$parameterCount = max(count($callableParameters), count($acceptorParameters));
for ($i = 0; $i < $parameterCount; $i++) {
if (!array_key_exists($i, $acceptorParameters)) {
$newParameters[] = $callableParameters[$i]->toOptional();
continue;
}

if (!array_key_exists($i, $callableParameters)) {
$newParameters[] = $callableParameter;
$newParameters[] = $acceptorParameters[$i]->toOptional();
continue;
}

$newParameters[] = $callableParameters[$i]->union(new NativeParameterReflection(
$callableParameter->getName(),
$callableParameter->isOptional(),
$callableParameter->getType(),
$callableParameter->passedByReference(),
$callableParameter->isVariadic(),
$callableParameter->getDefaultValue(),
));
$newParameters[] = $callableParameters[$i]->union($acceptorParameters[$i]);
}

$callableParameters = $newParameters;
Expand Down
17 changes: 17 additions & 0 deletions src/Reflection/Native/NativeParameterReflection.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,23 @@ public function getDefaultValue(): ?Type
return $this->defaultValue;
}

/** Used when merging signatures where only some of them declare this parameter. */
public function toOptional(): self
{
if ($this->optional) {
return $this;
}

return new self(
$this->name,
true,
$this->type,
$this->passedByReference,
$this->variadic,
$this->defaultValue,
);
}

public function union(self $other): self
{
return new self(
Expand Down
8 changes: 8 additions & 0 deletions tests/PHPStan/Analyser/AnalyserIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1608,6 +1608,14 @@ public function testBug14707(): void
$this->assertNoErrors($errors);
}

#[RequiresPhp('>= 8.0.0')]
public function testBug15003(): void
{
// crash
$errors = $this->runAnalyse(__DIR__ . '/data/bug-15003.php');
$this->assertNoErrors($errors);
}

/**
* @param string[]|null $allAnalysedFiles
* @return list<Error>
Expand Down
82 changes: 82 additions & 0 deletions tests/PHPStan/Analyser/data/bug-15003.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php // lint >= 8.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crash tests should be moved into a AnalyserIntegrationTest

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test should 1:1 reflect the snippet the user reported in https://phpstan.org/r/fa4c94b1-ab67-442d-8c89-36ea65b86346

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed the review — the crash reproducer now lives in AnalyserIntegrationTest.

What changed (commit 0915dc14c):

  • tests/PHPStan/Analyser/nsrt/bug-15003.phptests/PHPStan/Analyser/data/bug-15003.php, with the assertType() calls dropped (a crash test doesn't need them) and two adjustments so the file analyses cleanly at the integration test's level:
    • InvokableRule::__invoke() got a : void return type (otherwise "has no return type specified");
    • the two Field calls are wrapped in functions that return the result, instead of being standalone statements ("call on a separate line has no effect").
  • AnalyserIntegrationTest::testBug15003() added, #[RequiresPhp('>= 8.0.0')], asserting no errors.

Verified the moved test still catches the bug: with src/Analyser/NodeScopeResolver.php and src/Reflection/Native/NativeParameterReflection.php checked out from the pre-fix commit, testBug15003 errors with Call to undefined method PHPStan\Reflection\Php\ExtendedDummyParameter::union(); with the fix it passes.

nsrt/closure-passed-to-union-of-callables.php stays where it is — it asserts inferred closure parameter types (order independence, variadics, the Invokable<string> acceptor), so it's a type inference test rather than a crash test, even though its three-acceptor case exercises the same code path.

make tests (17708 tests, 97 skipped) and make phpstan are both green.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for make tests to finish; I'll report when it lands.


declare(strict_types = 1);

namespace Bug15003;

use Closure;

/**
* @phpstan-type Foo InvokableClass|callable(string, mixed): int
*/
class TypeImportShortcut {}

interface InvokableClass
{
/**
* @param Closure(string, ?string=): string $fail
*/
public function __invoke(string $foo, Closure $fail): int;
}

/** @phpstan-import-type Foo from TypeImportShortcut */
class A
{

/** @param callable(string):Foo|Foo $param */
public function foo($param): void {}

}

(new A)->foo(function(string $foo) {
return 5;
});

interface InvokableRule
{

/**
* @param Closure(string): string $fail
*/
public function __invoke(string $attribute, mixed $value, Closure $fail): void;

}

/**
* @phpstan-type FieldValidationRule InvokableRule|(callable(string, mixed, Closure): void)
* @phpstan-type ValidationRules array<int, FieldValidationRule>|FieldValidationRule
*/
final class Field
{

/**
* @param (callable(string): ValidationRules)|ValidationRules $rules
*/
public function rules($rules): self
{
return $this;
}

/**
* @param (callable(string): ValidationRules)|ValidationRules ...$rules
*/
public function creationRules($rules): self
{
return $this;
}

}

function rules(): Field
{
return (new Field())->rules(function ($attribute, $value, $fail) {
});
}

function creationRules(): Field
{
return (new Field())->creationRules([
function ($attribute, $value, $fail) {
},
]);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php declare(strict_types = 1);

namespace ClosurePassedToUnionOfCallables;

use function PHPStan\Testing\assertType;

/** @template T */
final class Invokable
{

/** @param T $a */
public function __invoke($a, string $b): void
{
}

}

final class Foo
{

/**
* @param callable(int, string): void|callable(int): void $cb
*/
public function longestFirst($cb): void
{
}

/**
* @param callable(int): void|callable(int, string): void $cb
*/
public function shortestFirst($cb): void
{
}

/**
* @param callable(int): void|Invokable<string>|callable(bool, float): void $cb
*/
public function withInvokable($cb): void
{
}

/**
* @param callable(int...): void|callable(string, string): void $cb
*/
public function variadicFirst($cb): void
{
}

/**
* @param callable(string, string): void|callable(int...): void $cb
*/
public function variadicLast($cb): void
{
}

public function run(): void
{
$this->longestFirst(function ($a, $b): void {
assertType('int', $a);
assertType('string', $b);
});

$this->shortestFirst(function ($a, $b): void {
assertType('int', $a);
assertType('string', $b);
});

$this->longestFirst(fn ($a, $b) => assertType('int', $a));
$this->longestFirst(fn ($a, $b) => assertType('string', $b));
$this->shortestFirst(fn ($a, $b) => assertType('int', $a));
$this->shortestFirst(fn ($a, $b) => assertType('string', $b));

$this->withInvokable(function ($a, $b): void {
assertType('bool|int|string', $a);
assertType('float|string', $b);
});

$this->variadicFirst(function ($a, $b): void {
assertType('int|string', $a);
assertType('string', $b);
});

$this->variadicLast(function ($a, $b): void {
assertType('int|string', $a);
assertType('string', $b);
});
}

}
Loading