Skip to content
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.

Sign up for GitHub

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

Fix ConstantArrayType::isCallable #3717

Open
wants to merge 6 commits into
base: 2.1.x
Choose a base branch
from
+58 −1
Open

Fix ConstantArrayType::isCallable #3717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1b48917
Fix ConstantArrayType::isCallable
VincentLanglet Nov 16, 2024
569753d
Rework
VincentLanglet Nov 17, 2024
8993085
Another solution
VincentLanglet Nov 19, 2024
4c27e0f
Add No filter
VincentLanglet Nov 19, 2024
980ccc1
Fix lint
VincentLanglet Nov 19, 2024
58de58f
Another solution
VincentLanglet Nov 19, 2024
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
14 changes: 13 additions & 1 deletion src/Type/Constant/ConstantArrayType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,19 @@ public function isCallable(): TrinaryLogic
$typeAndMethods,
);

return TrinaryLogic::createYes()->and(...$results);
$isCallable = TrinaryLogic::createYes()->and(...$results);
if ($isCallable->yes()) {
$callableArray = $this->getClassOrObjectAndMethods();
Copy link
Member

Choose a reason for hiding this comment

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

This is ineffective. This method was already called once inside findTypeAndMethodNames above. It's a bit of a mess right now.

I just made sure this kind of mistake isn't possible: 30b9cd8

So the fix should probably be done inside findTypeAndMethodNames but I'm not sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the fix should probably be done inside findTypeAndMethodNames but I'm not sure.

Not sure too. Depends of the purpose of the method.

Currently the findTypeAndMethodNames returns all the possible callable with at least a Maybe certainty. Cf #3637 (comment)

Currently it's not possible to create

ConstantArrayTypeAndMethod::createConcrete($type, $method->getValue(), TrinaryLogic::createNo());

So if you have a string 'foo'|'bar' and only 'foo' is a callable name, if it will returns 'foo'.
But it doesn't mean that 'foo'|'bar' is always a callable.

That's why I added the logic in isCallable to check the count of possible callable didn't drop (which mean some value has No as certainty).

if ($callableArray !== []) {
[$classOrObject, $methods] = $callableArray;

if (count($methods->getConstantStrings()) !== count($typeAndMethods)) {
return TrinaryLogic::createMaybe();
}
}
}

return $isCallable;
}

public function getCallableParametersAcceptors(ClassMemberAccessAnswerer $scope): array
Expand Down
7 changes: 7 additions & 0 deletions tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1102,4 +1102,11 @@ public function testAlwaysTruePregMatch(): void
$this->analyse([__DIR__ . '/data/always-true-preg-match.php'], []);
}

public function testBug12063(): void
{
$this->checkAlwaysTrueCheckTypeFunctionCall = true;
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-12063.php'], []);
}

}
38 changes: 38 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-12063.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php declare(strict_types=1); // lint >= 7.4

namespace Bug12063;

use BadFunctionCallException;

final class View
{
public function existingMethod(): void
{
}
}

final class TwigExtension
{
private View $viewFunctions;

public function __construct(View $viewFunctions)
{
$this->viewFunctions = $viewFunctions;
}

public function iterateFunctions(): void
{
$functionMappings = [
'i_exist' => 'existingMethod',
'i_dont_exist' => 'nonExistingMethod'
];

$functions = [];
foreach ($functionMappings as $nameFrom => $nameTo) {
$callable = [$this->viewFunctions, $nameTo];
if (!is_callable($callable)) {
throw new BadFunctionCallException("Function $nameTo does not exist in view functions");
}
}
}
}
Loading