Recognize [$obj, $method] as callable when method_exists($obj, $method) is known true in scope
#5547
+190
−2
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace Bug4510; | ||
|
|
||
| use function PHPStan\Testing\assertType; | ||
|
|
||
| class HelloWorld | ||
| { | ||
| public function existingMethod(): void {} | ||
|
|
||
| public function doSomething(string $method): void { | ||
| if (!method_exists($this, $method)) { | ||
| return; | ||
| } | ||
|
|
||
| [$this, $method](); // error - method_exists doesn't imply callable | ||
| } | ||
| } | ||
|
|
||
| function testMethodExists(string $method): void { | ||
| $instance = new HelloWorld(); | ||
| if (!method_exists($instance, $method)) { | ||
| return; | ||
| } | ||
|
|
||
| assertType('array{Bug4510\HelloWorld, string}', [$instance, $method]); | ||
| [$instance, $method](); // error - method_exists doesn't imply callable | ||
| } | ||
|
|
||
| function testIsCallableInlineArray(string $method): void { | ||
| $instance = new HelloWorld(); | ||
| if (!is_callable([$instance, $method])) { | ||
| return; | ||
| } | ||
|
|
||
| assertType('list{Bug4510\HelloWorld, string}&callable(): mixed', [$instance, $method]); | ||
| [$instance, $method](); // ok - is_callable verifies callability | ||
| } | ||
|
|
||
| function testMethodExistsWithClassString(string $method): void { | ||
| if (!method_exists(HelloWorld::class, $method)) { | ||
| return; | ||
| } | ||
|
|
||
| assertType("array{'Bug4510\\\\HelloWorld', string}", [HelloWorld::class, $method]); | ||
| [HelloWorld::class, $method](); // error - method_exists doesn't imply callable | ||
| } | ||
|
|
||
| function testIsCallableWithClassString(string $method): void { | ||
| if (!is_callable([HelloWorld::class, $method])) { | ||
| return; | ||
| } | ||
|
|
||
| assertType("list{'Bug4510\\\\HelloWorld', string}&callable(): mixed", [HelloWorld::class, $method]); | ||
| [HelloWorld::class, $method](); // ok - is_callable verifies callability | ||
| } | ||
|
|
||
| function testIsCallableExplicitKeys(string $method): void { | ||
| $instance = new HelloWorld(); | ||
| if (!is_callable([0 => $instance, 1 => $method])) { | ||
|
Contributor
There was a problem hiding this comment. seeing this makes me think we should also test with named arguments |
||
| return; | ||
| } | ||
|
|
||
| assertType('list{Bug4510\HelloWorld, string}&callable(): mixed', [0 => $instance, 1 => $method]); | ||
| [0 => $instance, 1 => $method](); // ok - is_callable verifies callability | ||
| } | ||
|
|
||
| function testIsCallableExplicitKeysWithClassString(string $method): void { | ||
| if (!is_callable([0 => HelloWorld::class, 1 => $method])) { | ||
| return; | ||
| } | ||
|
|
||
| assertType("list{'Bug4510\\\\HelloWorld', string}&callable(): mixed", [0 => HelloWorld::class, 1 => $method]); | ||
| [0 => HelloWorld::class, 1 => $method](); // ok - is_callable verifies callability | ||
| } | ||
|
|
||
| function testWithDynamicMethodExistsAndVariable(string $method): void { | ||
| $instance = new HelloWorld(); | ||
| $callable = [$instance, $method]; | ||
| if (!is_callable($callable)) { | ||
| return; | ||
| } | ||
|
|
||
| $callable(); // ok - is_callable on variable already worked | ||
| } | ||
|
|
||
| function testMethodExistsInElseBranch(string $method): void { | ||
| $instance = new HelloWorld(); | ||
| if (method_exists($instance, $method)) { | ||
| [$instance, $method](); // error - method_exists doesn't imply callable | ||
| } | ||
| } | ||
|
|
||
| function testIsCallableInElseBranch(string $method): void { | ||
| $instance = new HelloWorld(); | ||
| if (is_callable([$instance, $method])) { | ||
| [$instance, $method](); // ok - is_callable verifies callability | ||
| } | ||
| } | ||
|
|
||
| function testIsCallableNamedArg(string $method): void { | ||
| $instance = new HelloWorld(); | ||
| if (!is_callable(value: [$instance, $method])) { | ||
| return; | ||
| } | ||
|
|
||
| assertType('list{Bug4510\HelloWorld, string}&callable(): mixed', [$instance, $method]); | ||
| [$instance, $method](); // ok - is_callable verifies callability | ||
| } | ||
|
|
||
| function testMethodExistsNamedArgs(string $method): void { | ||
| $instance = new HelloWorld(); | ||
| if (!method_exists(object_or_class: $instance, method: $method)) { | ||
| return; | ||
| } | ||
|
|
||
| assertType('array{Bug4510\HelloWorld, string}', [$instance, $method]); | ||
| [$instance, $method](); // error - method_exists doesn't imply callable | ||
| } | ||
|
|
||
| function testNoMethodExists(string $method): void { | ||
| $instance = new HelloWorld(); | ||
| assertType('array{Bug4510\HelloWorld, string}', [$instance, $method]); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we limit this to 2 item arrays only?
that way we would not need to check any literal array we find, whether its callable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Added
count($expr->items) === 2as an early guard before theisCallable()->maybe()check, so we only evaluate callable-related logic on arrays that could actually be[$obj, $method]pairs. Tests and static analysis both pass.