diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 214844aac1..3e9198be09 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3265,6 +3265,11 @@ public function filterByTruthyValue(Expr $expr): self } $specifiedTypes = $this->typeSpecifier->specifyTypesInCondition($this, $expr, TypeSpecifierContext::createTruthy()); + if ($specifiedTypes->shouldSpecifyOnly()) { + $specifiedTypes = $specifiedTypes->unionWith( + $this->typeSpecifier->create($expr, new ConstantBooleanType(true), TypeSpecifierContext::createTrue(), $this), + ); + } $scope = $this->filterBySpecifiedTypes($specifiedTypes); $this->truthyScopes[$exprString] = $scope; @@ -3282,6 +3287,11 @@ public function filterByFalseyValue(Expr $expr): self } $specifiedTypes = $this->typeSpecifier->specifyTypesInCondition($this, $expr, TypeSpecifierContext::createFalsey()); + if ($specifiedTypes->shouldSpecifyOnly()) { + $specifiedTypes = $specifiedTypes->unionWith( + $this->typeSpecifier->create($expr, new ConstantBooleanType(false), TypeSpecifierContext::createTrue(), $this), + ); + } $scope = $this->filterBySpecifiedTypes($specifiedTypes); $this->falseyScopes[$exprString] = $scope; diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 41fdf89072..d6ea8a7cd4 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -143,6 +143,7 @@ use PHPStan\ShouldNotHappenException; use PHPStan\TrinaryLogic; use PHPStan\Type\ClosureType; +use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\FileTypeMapper; @@ -1143,11 +1144,24 @@ public function processStmtNode( $this->callNodeCallback($nodeCallback, new NoopExpressionNode($stmt->expr, $hasAssign), $scope, $storage); } $scope = $result->getScope(); - $scope = $scope->filterBySpecifiedTypes($this->typeSpecifier->specifyTypesInCondition( + $specifiedTypes = $this->typeSpecifier->specifyTypesInCondition( $scope, $stmt->expr, TypeSpecifierContext::createNull(), - )); + ); + $scope = $scope->filterBySpecifiedTypes($specifiedTypes); + if ($specifiedTypes->shouldSpecifyOnly()) { + // Statement counterpart of the specifyOnly handling in filterByTruthyValue(): + // store the call's true result so a duplicate void assertion statement is + // reported as always-true. We overwrite directly (not via TypeSpecifier::create) + // because void calls have no return value to protect from the purity check. + $scope = $scope->filterBySpecifiedTypes( + (new SpecifiedTypes( + [$scope->getNodeKey($stmt->expr) => [$stmt->expr, new ConstantBooleanType(true)]], + [], + ))->setAlwaysOverwriteTypes(), + ); + } $hasYield = $result->hasYield(); $throwPoints = $result->getThrowPoints(); $impurePoints = $result->getImpurePoints(); diff --git a/src/Analyser/SpecifiedTypes.php b/src/Analyser/SpecifiedTypes.php index 5cfa65dc53..b070bcd041 100644 --- a/src/Analyser/SpecifiedTypes.php +++ b/src/Analyser/SpecifiedTypes.php @@ -13,6 +13,8 @@ final class SpecifiedTypes private bool $overwrite = false; + private bool $specifyOnly = false; + /** @var array */ private array $newConditionalExpressionHolders = []; @@ -51,12 +53,36 @@ public function setAlwaysOverwriteTypes(): self { $self = new self($this->sureTypes, $this->sureNotTypes); $self->overwrite = true; + $self->specifyOnly = $this->specifyOnly; $self->newConditionalExpressionHolders = $this->newConditionalExpressionHolders; $self->rootExpr = $this->rootExpr; return $self; } + /** + * Marks these SpecifiedTypes as only narrowing types, not determining + * the check outcome. ImpossibleCheckTypeHelper will not use sureTypes + * to report always-true/false for the check expression. + * + * @api + */ + public function setSpecifyOnly(): self + { + $self = new self($this->sureTypes, $this->sureNotTypes); + $self->overwrite = $this->overwrite; + $self->specifyOnly = true; + $self->newConditionalExpressionHolders = $this->newConditionalExpressionHolders; + $self->rootExpr = $this->rootExpr; + + return $self; + } + + public function shouldSpecifyOnly(): bool + { + return $this->specifyOnly; + } + /** * @api */ @@ -64,6 +90,7 @@ public function setRootExpr(?Expr $rootExpr): self { $self = new self($this->sureTypes, $this->sureNotTypes); $self->overwrite = $this->overwrite; + $self->specifyOnly = $this->specifyOnly; $self->newConditionalExpressionHolders = $this->newConditionalExpressionHolders; $self->rootExpr = $rootExpr; @@ -77,6 +104,7 @@ public function setNewConditionalExpressionHolders(array $newConditionalExpressi { $self = new self($this->sureTypes, $this->sureNotTypes); $self->overwrite = $this->overwrite; + $self->specifyOnly = $this->specifyOnly; $self->newConditionalExpressionHolders = $newConditionalExpressionHolders; $self->rootExpr = $this->rootExpr; @@ -128,6 +156,7 @@ public function removeExpr(string $exprString): self $self = new self($sureTypes, $sureNotTypes); $self->overwrite = $this->overwrite; + $self->specifyOnly = $this->specifyOnly; $self->newConditionalExpressionHolders = $this->newConditionalExpressionHolders; $self->rootExpr = $this->rootExpr; @@ -167,6 +196,9 @@ public function intersectWith(SpecifiedTypes $other): self if ($this->overwrite && $other->overwrite) { $result = $result->setAlwaysOverwriteTypes(); } + if ($this->specifyOnly || $other->specifyOnly) { + $result->specifyOnly = true; + } return $result->setRootExpr($rootExpr); } @@ -204,6 +236,9 @@ public function unionWith(SpecifiedTypes $other): self if ($this->overwrite || $other->overwrite) { $result = $result->setAlwaysOverwriteTypes(); } + if ($this->specifyOnly || $other->specifyOnly) { + $result->specifyOnly = true; + } $conditionalExpressionHolders = $this->newConditionalExpressionHolders; foreach ($other->newConditionalExpressionHolders as $exprString => $holders) { @@ -235,6 +270,7 @@ public function normalize(Scope $scope): self if ($this->overwrite) { $result = $result->setAlwaysOverwriteTypes(); } + $result->specifyOnly = $this->specifyOnly; return $result->setRootExpr($this->rootExpr); } diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 01ab4208a3..9d8fee96a9 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -1856,7 +1856,10 @@ static function (Type $type, callable $traverse) use ($templateTypeMap, &$contai $assertedType, $assert->isNegated() ? TypeSpecifierContext::createFalse() : TypeSpecifierContext::createTrue(), $scope, - )->setRootExpr($containsUnresolvedTemplate || $assert->isEquality() ? $call : null); + ); + if ($containsUnresolvedTemplate || $assert->isEquality()) { + $newTypes = $newTypes->setSpecifyOnly(); + } $types = $types !== null ? $types->unionWith($newTypes) : $newTypes; if (!$context->null() || !$assertedType instanceof ConstantBooleanType) { diff --git a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php index 0a1621c842..bd02624a40 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php @@ -273,6 +273,20 @@ public function findSpecifiedType( return null; } + if ($specifiedTypes->shouldSpecifyOnly()) { + if ($scope->hasExpressionType($node)->yes()) { + $nodeType = $this->treatPhpDocTypesAsCertain ? $scope->getType($node) : $scope->getNativeType($node); + if ($nodeType->isTrue()->yes()) { + return true; + } + if ($nodeType->isFalse()->yes()) { + return false; + } + } + + return null; + } + $sureTypes = $specifiedTypes->getSureTypes(); $sureNotTypes = $specifiedTypes->getSureNotTypes(); diff --git a/src/Type/Php/ArrayKeyExistsFunctionTypeSpecifyingExtension.php b/src/Type/Php/ArrayKeyExistsFunctionTypeSpecifyingExtension.php index c48fee653f..8dce9bb2ac 100644 --- a/src/Type/Php/ArrayKeyExistsFunctionTypeSpecifyingExtension.php +++ b/src/Type/Php/ArrayKeyExistsFunctionTypeSpecifyingExtension.php @@ -3,10 +3,7 @@ namespace PHPStan\Type\Php; use PhpParser\Node\Expr\ArrayDimFetch; -use PhpParser\Node\Expr\BinaryOp\Identical; -use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Expr\FuncCall; -use PhpParser\Node\Name; use PHPStan\Analyser\Scope; use PHPStan\Analyser\SpecifiedTypes; use PHPStan\Analyser\TypeSpecifier; @@ -115,7 +112,7 @@ public function specifyTypes( $arrayType->getIterableValueType(), $context, $scope, - ))->setRootExpr(new Identical($arrayDimFetch, new ConstFetch(new Name('__PHPSTAN_FAUX_CONSTANT')))); + ))->setSpecifyOnly(); } return new SpecifiedTypes(); diff --git a/src/Type/Php/PregMatchTypeSpecifyingExtension.php b/src/Type/Php/PregMatchTypeSpecifyingExtension.php index 399ee9126f..3460d6321a 100644 --- a/src/Type/Php/PregMatchTypeSpecifyingExtension.php +++ b/src/Type/Php/PregMatchTypeSpecifyingExtension.php @@ -83,7 +83,7 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n $matchedType, $context, $scope, - )->setRootExpr($node); + )->setSpecifyOnly(); if ($overwrite) { $types = $types->setAlwaysOverwriteTypes(); } diff --git a/src/Type/Php/StrContainingTypeSpecifyingExtension.php b/src/Type/Php/StrContainingTypeSpecifyingExtension.php index 84b50e00cf..6223f295ad 100644 --- a/src/Type/Php/StrContainingTypeSpecifyingExtension.php +++ b/src/Type/Php/StrContainingTypeSpecifyingExtension.php @@ -2,12 +2,7 @@ namespace PHPStan\Type\Php; -use PhpParser\Node\Arg; -use PhpParser\Node\Expr\BinaryOp\BooleanAnd; -use PhpParser\Node\Expr\BinaryOp\NotIdentical; use PhpParser\Node\Expr\FuncCall; -use PhpParser\Node\Name; -use PhpParser\Node\Scalar\String_; use PHPStan\Analyser\Scope; use PHPStan\Analyser\SpecifiedTypes; use PHPStan\Analyser\TypeSpecifier; @@ -89,15 +84,7 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n new IntersectionType($accessories), $context, $scope, - )->setRootExpr(new BooleanAnd( - new NotIdentical( - $args[$needleArg]->value, - new String_(''), - ), - new FuncCall(new Name('FAUX_FUNCTION'), [ - new Arg($args[$needleArg]->value), - ]), - )); + )->setSpecifyOnly(); } } diff --git a/tests/PHPStan/Analyser/nsrt/bug-14705.php b/tests/PHPStan/Analyser/nsrt/bug-14705.php new file mode 100644 index 0000000000..e5dc4525e5 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14705.php @@ -0,0 +1,149 @@ += 8.0 + +namespace Bug14705; + +use function PHPStan\Testing\assertType; + +class Foo +{ + + /** + * strpos with non-empty-string haystack should not report always-true. + * + * @param non-empty-string $haystack + * @param non-empty-string $needle + */ + public function strposNonEmpty(string $haystack, string $needle): void + { + if (strpos($haystack, $needle) !== false) { + assertType('non-empty-string', $haystack); + assertType('non-empty-string', $needle); + } + } + + /** + * str_contains with non-empty-string haystack should not report always-true. + * + * @param non-empty-string $haystack + */ + public function strContainsNonEmpty(string $haystack, string $needle): void + { + if (str_contains($haystack, $needle)) { + assertType('non-empty-string', $haystack); + assertType('string', $needle); + } + } + + /** + * str_starts_with with non-empty-string haystack should not report always-true. + * + * @param non-empty-string $haystack + */ + public function strStartsWithNonEmpty(string $haystack, string $needle): void + { + if (str_starts_with($haystack, $needle)) { + assertType('non-empty-string', $haystack); + assertType('string', $needle); + } + } + + /** + * str_ends_with with non-empty-string haystack should not report always-true. + * + * @param non-empty-string $haystack + */ + public function strEndsWithNonEmpty(string $haystack, string $needle): void + { + if (str_ends_with($haystack, $needle)) { + assertType('non-empty-string', $haystack); + assertType('string', $needle); + } + } + + /** + * array_key_exists with non-constant key on a non-empty-array should not report always-true. + * + * @param non-empty-array $array + */ + public function arrayKeyExistsNonEmpty(array $array, string $key): void + { + if (array_key_exists($key, $array)) { + assertType('non-empty-array', $array); + } + } + + /** + * @phpstan-assert-if-true =non-empty-string $foo + */ + public function isValid(string $foo): bool + { + return $foo !== ''; + } + + public function equalityAssertDuplicate(string $task): void + { + if ($this->isValid($task)) { + assertType('non-empty-string', $task); + if ($this->isValid($task)) { // reported as always-true + assertType('non-empty-string', $task); + } + } + } + + /** + * @phpstan-assert =non-empty-string $foo + */ + public function assertValid(string $foo): void + { + if ($foo === '') { + throw new \Exception(); + } + } + + public function voidAssertDuplicate(string $task): void + { + $this->assertValid($task); + assertType('non-empty-string', $task); + $this->assertValid($task); // reported as always-true + assertType('non-empty-string', $task); + } + + public function realpathElvis(string $fileName): void + { + $fileName = realpath($fileName) ?: $fileName; + assertType('string', $fileName); + } + + /** @param list $paths */ + public function realpathElvisWithLoop(string $fileName, array $paths): void + { + $fileName = realpath($fileName) ?: $fileName; + assertType('string', $fileName); + + foreach ($paths as $path) { + if (str_starts_with($fileName, $path)) { + assertType('string', $fileName); + } + } + } + + /** + * Duplicate array_key_exists after an early-continue narrows the negated + * call to false, while the non-negated call stays bool. + * + * @param array> $theInput + * @phpstan-param array{'name':string,'owners':array} $theInput + * @param array $theTags + */ + public function arrayKeyExistsDuplicateInLoop(array $theInput, array $theTags): void + { + foreach ($theTags as $tag) { + if (!array_key_exists($tag, $theInput)) { + continue; + } + assertType('false', !array_key_exists($tag, $theInput)); + assertType('bool', array_key_exists($tag, $theInput)); // could be true + } + } + +} diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index a72250dfc0..cdbc3a989a 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -507,6 +507,30 @@ public function testNonEmptySpecifiedString(): void $this->analyse([__DIR__ . '/data/non-empty-string-impossible-type.php'], []); } + public function testBug14705(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-14705.php'], []); + } + + #[RequiresPhp('>= 8.0')] + public function testBug14705Php8(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-14705-php8.php'], [ + [ + 'Call to function str_ends_with() with non-empty-string and non-empty-string will always evaluate to true.', + 50, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + ], + [ + 'Call to function str_contains() with non-empty-string and non-empty-string will always evaluate to true.', + 62, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + ], + ]); + } + public function testBug2755(): void { $this->treatPhpDocTypesAsCertain = true; diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php index 9dde818f42..ff1c9d403b 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php @@ -308,6 +308,22 @@ public function testBug10337(): void $this->analyse([__DIR__ . '/data/bug-10337.php'], []); } + public function testBug14705(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-14705.php'], [ + [ + 'Call to method Bug14705\Foo::isValid() with non-empty-string will always evaluate to true.', + 87, + 'If Bug14705\Foo::isValid() is impure, add @phpstan-impure PHPDoc tag above its declaration. Learn more: https://phpstan.org/blog/remembering-and-forgetting-returned-values', + ], + [ + 'Call to method Bug14705\Foo::assertValid() with non-empty-string will always evaluate to true.', + 107, + ], + ]); + } + public function testInTrait(): void { $this->treatPhpDocTypesAsCertain = true; diff --git a/tests/PHPStan/Rules/Comparison/data/bug-14705-php8.php b/tests/PHPStan/Rules/Comparison/data/bug-14705-php8.php new file mode 100644 index 0000000000..dcec6d3291 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-14705-php8.php @@ -0,0 +1,68 @@ += 8.0 + +namespace Bug14705Php8; + +class Foo +{ + + /** + * str_contains with non-empty-string haystack should not report always-true. + * + * @param non-empty-string $haystack + */ + public function strContainsNonEmpty(string $haystack, string $needle): void + { + if (str_contains($haystack, $needle)) { + + } + } + + /** + * str_starts_with with non-empty-string haystack should not report always-true. + * + * @param non-empty-string $haystack + */ + public function strStartsWithNonEmpty(string $haystack, string $needle): void + { + if (str_starts_with($haystack, $needle)) { + + } + } + + /** + * str_ends_with with non-empty-string haystack should not report always-true. + * + * @param non-empty-string $haystack + */ + public function strEndsWithNonEmpty(string $haystack, string $needle): void + { + if (str_ends_with($haystack, $needle)) { + + } + } + + /** + * @param non-empty-string $needle + */ + public function strEndsWithDuplicate(string $haystack, string $needle): void + { + if (str_ends_with($haystack, $needle)) { + if (str_ends_with($haystack, $needle)) { // reported as always-true + + } + } + } + + /** + * @param non-empty-string $needle + */ + public function strContainsDuplicate(string $haystack, string $needle): void + { + if (str_contains($haystack, $needle)) { + if (str_contains($haystack, $needle)) { // reported as always-true + + } + } + } + +}