Skip to content

Commit 1f150cc

Browse files
committed
Fix "will always evaluate to true" function call with named arguments
1 parent a38e400 commit 1f150cc

File tree

3 files changed

+29
-21
lines changed

3 files changed

+29
-21
lines changed

src/Analyser/TypeSpecifier.php

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -482,24 +482,21 @@ public function specifyTypesInCondition(
482482
$parametersAcceptor = null;
483483

484484
$functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope);
485+
$normalizedExpr = $expr;
485486
if (count($expr->getArgs()) > 0) {
486487
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs($scope, $expr->getArgs(), $functionReflection->getVariants(), $functionReflection->getNamedArgumentsVariants());
487-
$expr = ArgumentsNormalizer::reorderFuncArguments($parametersAcceptor, $expr) ?? $expr;
488+
$normalizedExpr = ArgumentsNormalizer::reorderFuncArguments($parametersAcceptor, $expr) ?? $expr;
488489
}
489490

490491
foreach ($this->getFunctionTypeSpecifyingExtensions() as $extension) {
491-
if (!$extension->isFunctionSupported($functionReflection, $expr, $context)) {
492+
if (!$extension->isFunctionSupported($functionReflection, $normalizedExpr, $context)) {
492493
continue;
493494
}
494495

495-
return $extension->specifyTypes($functionReflection, $expr, $scope, $context);
496+
return $extension->specifyTypes($functionReflection, $normalizedExpr, $scope, $context);
496497
}
497498

498499
if (count($expr->getArgs()) > 0) {
499-
if ($parametersAcceptor === null) {
500-
throw new ShouldNotHappenException();
501-
}
502-
503500
$specifiedTypes = $this->specifyTypesFromConditionalReturnType($context, $expr, $parametersAcceptor, $scope);
504501
if ($specifiedTypes !== null) {
505502
return $specifiedTypes;
@@ -531,9 +528,10 @@ public function specifyTypesInCondition(
531528
// lazy create parametersAcceptor, as creation can be expensive
532529
$parametersAcceptor = null;
533530

531+
$normalizedExpr = $expr;
534532
if (count($expr->getArgs()) > 0) {
535533
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs($scope, $expr->getArgs(), $methodReflection->getVariants(), $methodReflection->getNamedArgumentsVariants());
536-
$expr = ArgumentsNormalizer::reorderMethodArguments($parametersAcceptor, $expr) ?? $expr;
534+
$normalizedExpr = ArgumentsNormalizer::reorderMethodArguments($parametersAcceptor, $expr) ?? $expr;
537535
}
538536

539537
$referencedClasses = $methodCalledOnType->getObjectClassNames();
@@ -543,19 +541,15 @@ public function specifyTypesInCondition(
543541
) {
544542
$methodClassReflection = $this->reflectionProvider->getClass($referencedClasses[0]);
545543
foreach ($this->getMethodTypeSpecifyingExtensionsForClass($methodClassReflection->getName()) as $extension) {
546-
if (!$extension->isMethodSupported($methodReflection, $expr, $context)) {
544+
if (!$extension->isMethodSupported($methodReflection, $normalizedExpr, $context)) {
547545
continue;
548546
}
549547

550-
return $extension->specifyTypes($methodReflection, $expr, $scope, $context);
548+
return $extension->specifyTypes($methodReflection, $normalizedExpr, $scope, $context);
551549
}
552550
}
553551

554552
if (count($expr->getArgs()) > 0) {
555-
if ($parametersAcceptor === null) {
556-
throw new ShouldNotHappenException();
557-
}
558-
559553
$specifiedTypes = $this->specifyTypesFromConditionalReturnType($context, $expr, $parametersAcceptor, $scope);
560554
if ($specifiedTypes !== null) {
561555
return $specifiedTypes;
@@ -592,9 +586,10 @@ public function specifyTypesInCondition(
592586
// lazy create parametersAcceptor, as creation can be expensive
593587
$parametersAcceptor = null;
594588

589+
$normalizedExpr = $expr;
595590
if (count($expr->getArgs()) > 0) {
596591
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs($scope, $expr->getArgs(), $staticMethodReflection->getVariants(), $staticMethodReflection->getNamedArgumentsVariants());
597-
$expr = ArgumentsNormalizer::reorderStaticCallArguments($parametersAcceptor, $expr) ?? $expr;
592+
$normalizedExpr = ArgumentsNormalizer::reorderStaticCallArguments($parametersAcceptor, $expr) ?? $expr;
598593
}
599594

600595
$referencedClasses = $calleeType->getObjectClassNames();
@@ -604,19 +599,15 @@ public function specifyTypesInCondition(
604599
) {
605600
$staticMethodClassReflection = $this->reflectionProvider->getClass($referencedClasses[0]);
606601
foreach ($this->getStaticMethodTypeSpecifyingExtensionsForClass($staticMethodClassReflection->getName()) as $extension) {
607-
if (!$extension->isStaticMethodSupported($staticMethodReflection, $expr, $context)) {
602+
if (!$extension->isStaticMethodSupported($staticMethodReflection, $normalizedExpr, $context)) {
608603
continue;
609604
}
610605

611-
return $extension->specifyTypes($staticMethodReflection, $expr, $scope, $context);
606+
return $extension->specifyTypes($staticMethodReflection, $normalizedExpr, $scope, $context);
612607
}
613608
}
614609

615610
if (count($expr->getArgs()) > 0) {
616-
if ($parametersAcceptor === null) {
617-
throw new ShouldNotHappenException();
618-
}
619-
620611
$specifiedTypes = $this->specifyTypesFromConditionalReturnType($context, $expr, $parametersAcceptor, $scope);
621612
if ($specifiedTypes !== null) {
622613
return $specifiedTypes;

tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,4 +1025,11 @@ public function testBug12412(): void
10251025
$this->analyse([__DIR__ . '/data/bug-12412.php'], []);
10261026
}
10271027

1028+
#[RequiresPhp('>= 8.2')]
1029+
public function testBug13291(): void
1030+
{
1031+
$this->treatPhpDocTypesAsCertain = true;
1032+
$this->analyse([__DIR__ . '/data/bug-13291.php'], []);
1033+
}
1034+
10281035
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php // lint >= 8.2
2+
3+
namespace Bug13291;
4+
5+
function test(bool $someBool): true {
6+
var_dump($someBool);
7+
return true;
8+
}
9+
10+
test(someBool: true);

0 commit comments

Comments
 (0)