diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 214844aac1..a652ac9825 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3265,6 +3265,12 @@ public function filterByTruthyValue(Expr $expr): self } $specifiedTypes = $this->typeSpecifier->specifyTypesInCondition($this, $expr, TypeSpecifierContext::createTruthy()); + if ($specifiedTypes->shouldSpecifyOnly()) { + [$markerExpr, $markerValue] = $this->unwrapSpecifyOnlyMarker($expr, true); + $specifiedTypes = $specifiedTypes->unionWith( + $this->typeSpecifier->create($markerExpr, new ConstantBooleanType($markerValue), TypeSpecifierContext::createTrue(), $this), + ); + } $scope = $this->filterBySpecifiedTypes($specifiedTypes); $this->truthyScopes[$exprString] = $scope; @@ -3282,12 +3288,36 @@ public function filterByFalseyValue(Expr $expr): self } $specifiedTypes = $this->typeSpecifier->specifyTypesInCondition($this, $expr, TypeSpecifierContext::createFalsey()); + if ($specifiedTypes->shouldSpecifyOnly()) { + [$markerExpr, $markerValue] = $this->unwrapSpecifyOnlyMarker($expr, false); + $specifiedTypes = $specifiedTypes->unionWith( + $this->typeSpecifier->create($markerExpr, new ConstantBooleanType($markerValue), TypeSpecifierContext::createTrue(), $this), + ); + } $scope = $this->filterBySpecifiedTypes($specifiedTypes); $this->falseyScopes[$exprString] = $scope; return $scope; } + /** + * Strips BooleanNot wrappers from a specifyOnly condition so the boolean + * result marker is stored for the underlying call (e.g. `array_key_exists(...)`) + * rather than for the negated form (`!array_key_exists(...)`). The negated form + * is then derived from the inner value instead of being capped at bool. + * + * @return array{Expr, bool} + */ + private function unwrapSpecifyOnlyMarker(Expr $expr, bool $value): array + { + while ($expr instanceof Expr\BooleanNot) { + $expr = $expr->expr; + $value = !$value; + } + + return [$expr, $value]; + } + /** * @return static */ diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 41fdf89072..21087970b4 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,31 @@ 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()) { + // This is the expression-statement counterpart of the specifyOnly handling + // in MutatingScope::filterByTruthyValue(). A void assertion method used as a + // statement (e.g. `$this->assertValid($x);` with `@phpstan-assert =non-empty-string $x`) + // only narrows types as a side effect and never determines a check outcome, so + // ImpossibleCheckTypeHelper would otherwise have nothing to detect a duplicate + // call against. Store ConstantBooleanType(true) for the call expression so a + // second identical call is reported as always-true. Unlike filterByTruthyValue, + // we overwrite directly instead of going through TypeSpecifier::create(): void + // calls are not used for their return value, so the purity check there would + // drop the marker for the impure calls that need it most, and intersecting + // `true` with the void return type would produce *NEVER*. + $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..10db7aac42 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,45 @@ 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. + * + * Duplicate detection: filterByTruthyValue stores the call's boolean + * result via TypeSpecifier::create(), which respects purity checks. + * Pure/possibly-impure calls get duplicate detection; impure calls + * (hasSideEffects=yes) do not, to avoid overwriting the expression's + * real return type in scope. Void assertion methods used as statements + * get duplicate detection via NodeScopeResolver's expression statement + * path, which bypasses purity checks since void calls are not used + * for their return value. + * + * @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 +99,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 +113,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 +165,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 +205,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 +245,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 +279,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/Rules/Methods/MethodCallWithPossiblyRenamedNamedArgumentRule.php b/src/Rules/Methods/MethodCallWithPossiblyRenamedNamedArgumentRule.php index 55d7e9d8ad..b6d9790ef7 100644 --- a/src/Rules/Methods/MethodCallWithPossiblyRenamedNamedArgumentRule.php +++ b/src/Rules/Methods/MethodCallWithPossiblyRenamedNamedArgumentRule.php @@ -51,10 +51,6 @@ public function processNode(Node $node, NodeCallbackInvoker&Scope&CollectedDataE continue; } - if (!array_key_exists($prototypeParameterName, $prototypeMethodCalls)) { - continue; - } - $callsWithParameter = $prototypeMethodCalls[$prototypeParameterName]; foreach ($callsWithParameter as [$file, $line]) { $errors[] = RuleErrorBuilder::message(sprintf( 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..2c1f108050 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14705.php @@ -0,0 +1,155 @@ += 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 both the negated + * and the bare positive call. + * + * The condition is the BooleanNot `!array_key_exists(...)`. When the specifyOnly + * duplicate-detection marker is stored, the BooleanNot wrapper is stripped so the + * marker records the underlying `array_key_exists(...)` call as true. The negated + * form is then derived from that inner value (false), and the bare positive call + * reads the stored true directly. + * + * @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('true', array_key_exists($tag, $theInput)); + } + } + +} diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index a72250dfc0..2abd5c078f 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -507,6 +507,39 @@ 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'], [ + [ + 'Call to function array_key_exists() with \'name\'|\'owners\' and array{name: string, owners: array} will always evaluate to true.', + 150, + ], + [ + 'Call to function array_key_exists() with \'name\'|\'owners\' and array{name: string, owners: array} will always evaluate to true.', + 151, + ], + ]); + } + + #[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 + + } + } + } + +}