From 1b489176d1b86749b5befd8ca6409e80a62798a1 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 16 Nov 2024 13:45:12 +0100 Subject: [PATCH 1/6] Fix ConstantArrayType::isCallable --- src/Type/Constant/ConstantArrayType.php | 38 +++++++++++++++---- ...mpossibleCheckTypeFunctionCallRuleTest.php | 7 ++++ .../Rules/Comparison/data/bug-12063.php | 38 +++++++++++++++++++ 3 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-12063.php diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index 1b770d6889..ca902c4422 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -458,17 +458,41 @@ public function equals(Type $type): bool public function isCallable(): TrinaryLogic { - $typeAndMethods = $this->findTypeAndMethodNames(); - if ($typeAndMethods === []) { + $callableArray = $this->getClassOrObjectAndMethods(); + if ($callableArray === []) { return TrinaryLogic::createNo(); } - $results = array_map( - static fn (ConstantArrayTypeAndMethod $typeAndMethod): TrinaryLogic => $typeAndMethod->getCertainty(), - $typeAndMethods, - ); + [$classOrObject, $methods] = $callableArray; + if (count($methods->getConstantStrings()) === 0) { + return TrinaryLogic::createMaybe(); + } + + $type = $classOrObject->getObjectTypeOrClassStringObjectType(); + if (!$type->isObject()->yes()) { + return TrinaryLogic::createMaybe(); + } + + $hasMethodTrinary = []; + $phpVersion = PhpVersionStaticAccessor::getInstance(); + foreach ($methods->getConstantStrings() as $method) { + $has = $type->hasMethod($method->getValue()); + + if ($has->yes()) { + if (!$phpVersion->supportsCallableInstanceMethods()) { + $methodReflection = $type->getMethod($method->getValue(), new OutOfClassScope()); + if ($classOrObject->isString()->yes() && !$methodReflection->isStatic()) { + $has = $has->and(TrinaryLogic::createNo()); + } + } elseif ($this->isOptionalKey(0) || $this->isOptionalKey(1)) { + $has = $has->and(TrinaryLogic::createMaybe()); + } + } + + $hasMethodTrinary[] = $has; + } - return TrinaryLogic::createYes()->and(...$results); + return TrinaryLogic::extremeIdentity(...$hasMethodTrinary); } public function getCallableParametersAcceptors(ClassMemberAccessAnswerer $scope): array diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index 16529f3a74..a24507b9b2 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -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'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-12063.php b/tests/PHPStan/Rules/Comparison/data/bug-12063.php new file mode 100644 index 0000000000..0ab9335b84 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-12063.php @@ -0,0 +1,38 @@ += 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"); + } + } + } +} From 569753dc78b3b6b1cb309506e8a49781b6267a60 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sun, 17 Nov 2024 19:34:48 +0100 Subject: [PATCH 2/6] Rework --- src/Type/Constant/ConstantArrayType.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index ca902c4422..a376e33eaf 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -482,7 +482,7 @@ public function isCallable(): TrinaryLogic if (!$phpVersion->supportsCallableInstanceMethods()) { $methodReflection = $type->getMethod($method->getValue(), new OutOfClassScope()); if ($classOrObject->isString()->yes() && !$methodReflection->isStatic()) { - $has = $has->and(TrinaryLogic::createNo()); + $has = TrinaryLogic::createNo(); } } elseif ($this->isOptionalKey(0) || $this->isOptionalKey(1)) { $has = $has->and(TrinaryLogic::createMaybe()); From 8993085a165f2157c3ba1764d162469118f7db06 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Tue, 19 Nov 2024 11:28:28 +0100 Subject: [PATCH 3/6] Another solution --- src/Type/Constant/ConstantArrayType.php | 61 +++++-------------- .../Constant/ConstantArrayTypeAndMethod.php | 3 - 2 files changed, 15 insertions(+), 49 deletions(-) diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index a376e33eaf..f0a3645552 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -458,41 +458,17 @@ public function equals(Type $type): bool public function isCallable(): TrinaryLogic { - $callableArray = $this->getClassOrObjectAndMethods(); - if ($callableArray === []) { + $typeAndMethods = $this->findTypeAndMethodNames(); + if ($typeAndMethods === []) { return TrinaryLogic::createNo(); } - [$classOrObject, $methods] = $callableArray; - if (count($methods->getConstantStrings()) === 0) { - return TrinaryLogic::createMaybe(); - } - - $type = $classOrObject->getObjectTypeOrClassStringObjectType(); - if (!$type->isObject()->yes()) { - return TrinaryLogic::createMaybe(); - } - - $hasMethodTrinary = []; - $phpVersion = PhpVersionStaticAccessor::getInstance(); - foreach ($methods->getConstantStrings() as $method) { - $has = $type->hasMethod($method->getValue()); - - if ($has->yes()) { - if (!$phpVersion->supportsCallableInstanceMethods()) { - $methodReflection = $type->getMethod($method->getValue(), new OutOfClassScope()); - if ($classOrObject->isString()->yes() && !$methodReflection->isStatic()) { - $has = TrinaryLogic::createNo(); - } - } elseif ($this->isOptionalKey(0) || $this->isOptionalKey(1)) { - $has = $has->and(TrinaryLogic::createMaybe()); - } - } - - $hasMethodTrinary[] = $has; - } + $results = array_map( + static fn (ConstantArrayTypeAndMethod $typeAndMethod): TrinaryLogic => $typeAndMethod->getCertainty(), + $typeAndMethods, + ); - return TrinaryLogic::extremeIdentity(...$hasMethodTrinary); + return TrinaryLogic::extremeIdentity(...$results); } public function getCallableParametersAcceptors(ClassMemberAccessAnswerer $scope): array @@ -606,25 +582,18 @@ public function findTypeAndMethodNames(): array $phpVersion = PhpVersionStaticAccessor::getInstance(); foreach ($methods->getConstantStrings() as $method) { $has = $type->hasMethod($method->getValue()); - if ($has->no()) { - continue; - } - if ( - BleedingEdgeToggle::isBleedingEdge() - && $has->yes() - && !$phpVersion->supportsCallableInstanceMethods() - ) { - $methodReflection = $type->getMethod($method->getValue(), new OutOfClassScope()); - if ($classOrObject->isString()->yes() && !$methodReflection->isStatic()) { - continue; + if ($has->yes()) { + if (BleedingEdgeToggle::isBleedingEdge() && !$phpVersion->supportsCallableInstanceMethods()) { + $methodReflection = $type->getMethod($method->getValue(), new OutOfClassScope()); + if ($classOrObject->isString()->yes() && !$methodReflection->isStatic()) { + $has = TrinaryLogic::createNo(); + } + } elseif ($this->isOptionalKey(0) || $this->isOptionalKey(1)) { + $has = $has->and(TrinaryLogic::createMaybe()); } } - if ($this->isOptionalKey(0) || $this->isOptionalKey(1)) { - $has = $has->and(TrinaryLogic::createMaybe()); - } - $typeAndMethods[] = ConstantArrayTypeAndMethod::createConcrete($type, $method->getValue(), $has); } diff --git a/src/Type/Constant/ConstantArrayTypeAndMethod.php b/src/Type/Constant/ConstantArrayTypeAndMethod.php index 01e735d949..873ce75f7e 100644 --- a/src/Type/Constant/ConstantArrayTypeAndMethod.php +++ b/src/Type/Constant/ConstantArrayTypeAndMethod.php @@ -27,9 +27,6 @@ public static function createConcrete( TrinaryLogic $certainty, ): self { - if ($certainty->no()) { - throw new ShouldNotHappenException(); - } return new self($type, $method, $certainty); } From 4c27e0fb8530ccad0882d0dee342db3950855a2a Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Tue, 19 Nov 2024 11:59:26 +0100 Subject: [PATCH 4/6] Add No filter --- src/Type/Constant/ConstantArrayType.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index f0a3645552..323d1e2213 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -458,7 +458,7 @@ public function equals(Type $type): bool public function isCallable(): TrinaryLogic { - $typeAndMethods = $this->findTypeAndMethodNames(); + $typeAndMethods = $this->findTypeAndMethodNames(false); if ($typeAndMethods === []) { return TrinaryLogic::createNo(); } @@ -561,7 +561,7 @@ public function findTypeAndMethodName(): ?ConstantArrayTypeAndMethod } /** @return ConstantArrayTypeAndMethod[] */ - public function findTypeAndMethodNames(): array + public function findTypeAndMethodNames(bool $atLeastMaybe = true): array { $callableArray = $this->getClassOrObjectAndMethods(); if ($callableArray === []) { @@ -594,7 +594,9 @@ public function findTypeAndMethodNames(): array } } - $typeAndMethods[] = ConstantArrayTypeAndMethod::createConcrete($type, $method->getValue(), $has); + if (!$atLeastMaybe || !$has->no()) { + $typeAndMethods[] = ConstantArrayTypeAndMethod::createConcrete($type, $method->getValue(), $has); + } } return $typeAndMethods; From 980ccc1b91a577454ab121fa33145e2a44b6aa52 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Tue, 19 Nov 2024 18:30:57 +0100 Subject: [PATCH 5/6] Fix lint --- src/Type/Constant/ConstantArrayType.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index 323d1e2213..63e3d2dd8e 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -594,9 +594,10 @@ public function findTypeAndMethodNames(bool $atLeastMaybe = true): array } } - if (!$atLeastMaybe || !$has->no()) { - $typeAndMethods[] = ConstantArrayTypeAndMethod::createConcrete($type, $method->getValue(), $has); + if ($atLeastMaybe && $has->no()) { + continue; } + $typeAndMethods[] = ConstantArrayTypeAndMethod::createConcrete($type, $method->getValue(), $has); } return $typeAndMethods; From 58de58fc0cf9bed188d7fc5628b965b679b0c714 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Tue, 19 Nov 2024 19:43:03 +0100 Subject: [PATCH 6/6] Another solution --- src/Type/Constant/ConstantArrayType.php | 42 +++++++++++++------ .../Constant/ConstantArrayTypeAndMethod.php | 3 ++ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index 63e3d2dd8e..07f1e5d5ba 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -458,7 +458,7 @@ public function equals(Type $type): bool public function isCallable(): TrinaryLogic { - $typeAndMethods = $this->findTypeAndMethodNames(false); + $typeAndMethods = $this->findTypeAndMethodNames(); if ($typeAndMethods === []) { return TrinaryLogic::createNo(); } @@ -468,7 +468,19 @@ public function isCallable(): TrinaryLogic $typeAndMethods, ); - return TrinaryLogic::extremeIdentity(...$results); + $isCallable = TrinaryLogic::createYes()->and(...$results); + if ($isCallable->yes()) { + $callableArray = $this->getClassOrObjectAndMethods(); + if ($callableArray !== []) { + [$classOrObject, $methods] = $callableArray; + + if (count($methods->getConstantStrings()) !== count($typeAndMethods)) { + return TrinaryLogic::createMaybe(); + } + } + } + + return $isCallable; } public function getCallableParametersAcceptors(ClassMemberAccessAnswerer $scope): array @@ -561,7 +573,7 @@ public function findTypeAndMethodName(): ?ConstantArrayTypeAndMethod } /** @return ConstantArrayTypeAndMethod[] */ - public function findTypeAndMethodNames(bool $atLeastMaybe = true): array + public function findTypeAndMethodNames(): array { $callableArray = $this->getClassOrObjectAndMethods(); if ($callableArray === []) { @@ -582,21 +594,25 @@ public function findTypeAndMethodNames(bool $atLeastMaybe = true): array $phpVersion = PhpVersionStaticAccessor::getInstance(); foreach ($methods->getConstantStrings() as $method) { $has = $type->hasMethod($method->getValue()); + if ($has->no()) { + continue; + } - if ($has->yes()) { - if (BleedingEdgeToggle::isBleedingEdge() && !$phpVersion->supportsCallableInstanceMethods()) { - $methodReflection = $type->getMethod($method->getValue(), new OutOfClassScope()); - if ($classOrObject->isString()->yes() && !$methodReflection->isStatic()) { - $has = TrinaryLogic::createNo(); - } - } elseif ($this->isOptionalKey(0) || $this->isOptionalKey(1)) { - $has = $has->and(TrinaryLogic::createMaybe()); + if ( + BleedingEdgeToggle::isBleedingEdge() + && $has->yes() + && !$phpVersion->supportsCallableInstanceMethods() + ) { + $methodReflection = $type->getMethod($method->getValue(), new OutOfClassScope()); + if ($classOrObject->isString()->yes() && !$methodReflection->isStatic()) { + continue; } } - if ($atLeastMaybe && $has->no()) { - continue; + if ($this->isOptionalKey(0) || $this->isOptionalKey(1)) { + $has = $has->and(TrinaryLogic::createMaybe()); } + $typeAndMethods[] = ConstantArrayTypeAndMethod::createConcrete($type, $method->getValue(), $has); } diff --git a/src/Type/Constant/ConstantArrayTypeAndMethod.php b/src/Type/Constant/ConstantArrayTypeAndMethod.php index 873ce75f7e..01e735d949 100644 --- a/src/Type/Constant/ConstantArrayTypeAndMethod.php +++ b/src/Type/Constant/ConstantArrayTypeAndMethod.php @@ -27,6 +27,9 @@ public static function createConcrete( TrinaryLogic $certainty, ): self { + if ($certainty->no()) { + throw new ShouldNotHappenException(); + } return new self($type, $method, $certainty); }