From d0ec23f4326258317dc5681dd2e223cc19147619 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 29 May 2026 10:37:55 +0200 Subject: [PATCH] Rework --- composer.json | 2 +- .../AssertTypeSpecifyingExtension.php | 118 +++++++----------- .../ImpossibleCheckTypeMethodCallRuleTest.php | 10 -- 3 files changed, 48 insertions(+), 82 deletions(-) diff --git a/composer.json b/composer.json index 8d20484..14be9c3 100644 --- a/composer.json +++ b/composer.json @@ -8,7 +8,7 @@ "keywords": ["static analysis"], "require": { "php": "^7.4 || ^8.0", - "phpstan/phpstan": "^2.1.41" + "phpstan/phpstan": "^2.2.15" }, "require-dev": { "nikic/php-parser": "^5.1", diff --git a/src/Type/WebMozartAssert/AssertTypeSpecifyingExtension.php b/src/Type/WebMozartAssert/AssertTypeSpecifyingExtension.php index b03a105..7a83721 100644 --- a/src/Type/WebMozartAssert/AssertTypeSpecifyingExtension.php +++ b/src/Type/WebMozartAssert/AssertTypeSpecifyingExtension.php @@ -38,7 +38,6 @@ use PHPStan\ShouldNotHappenException; use PHPStan\Type\ArrayType; use PHPStan\Type\Constant\ConstantArrayTypeBuilder; -use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\IterableType; use PHPStan\Type\MixedType; use PHPStan\Type\NeverType; @@ -156,7 +155,7 @@ public function specifyTypes( ); } - [$expr, $rootExpr] = self::createExpression($scope, $staticMethodReflection->getName(), $node->getArgs()); + [$expr, $isEquality] = self::createExpression($scope, $staticMethodReflection->getName(), $node->getArgs()); if ($expr === null) { return new SpecifiedTypes([], []); } @@ -165,14 +164,17 @@ public function specifyTypes( $scope, $expr, TypeSpecifierContext::createTruthy(), - )->setRootExpr($rootExpr ?? $expr); + ); + if ($isEquality) { + $specifiedTypes = $specifiedTypes->setEquality(); + } - return $this->specifyRootExprIfSet($rootExpr, $scope, $specifiedTypes); + return $specifiedTypes; } /** * @param Arg[] $args - * @return array{?Expr, ?Expr} + * @return array{?Expr, bool} */ private function createExpression( Scope $scope, @@ -186,14 +188,14 @@ private function createExpression( $resolverResult = $resolver($scope, ...$args); if (is_array($resolverResult)) { - [$expr, $rootExpr] = $resolverResult; + [$expr, $isEquality] = $resolverResult; } else { $expr = $resolverResult; - $rootExpr = null; + $isEquality = false; } if ($expr === null) { - return [null, null]; + return [null, false]; } if (substr($name, 0, 6) === 'nullOr') { @@ -206,11 +208,11 @@ private function createExpression( ); } - return [$expr, $rootExpr]; + return [$expr, $isEquality]; } /** - * @return array + * @return array */ private function getExpressionResolvers(): array { @@ -634,22 +636,23 @@ private function getExpressionResolvers(): array ]; foreach (['contains', 'startsWith', 'endsWith'] as $name) { - $this->resolvers[$name] = static function (Scope $scope, Arg $value, Arg $subString) use ($name): array { - if ($scope->getType($subString->value)->isNonEmptyString()->yes()) { - return self::createIsNonEmptyStringAndSomethingExprPair($name, [$value, $subString]); - } - + $this->resolvers[$name] = static function (Scope $scope, Arg $value, Arg $subString): array { $expr = new FuncCall( new Name('is_string'), [$value], ); - $rootExpr = new BooleanAnd( - $expr, - new FuncCall(new Name('FAUX_FUNCTION_ ' . $name), [$value, $subString]), - ); + if ($scope->getType($subString->value)->isNonEmptyString()->yes()) { + $expr = new BooleanAnd( + $expr, + new NotIdentical( + $value->value, + new String_(''), + ), + ); + } - return [$expr, $rootExpr]; + return [$expr, true]; }; } @@ -669,7 +672,19 @@ private function getExpressionResolvers(): array 'notWhitespaceOnly', ]; foreach ($assertionsResultingAtLeastInNonEmptyString as $name) { - $this->resolvers[$name] = static fn (Scope $scope, Arg $value): array => self::createIsNonEmptyStringAndSomethingExprPair($name, [$value]); + $this->resolvers[$name] = static fn (Scope $scope, Arg $value): array => [ + new BooleanAnd( + new FuncCall( + new Name('is_string'), + [$value], + ), + new NotIdentical( + $value->value, + new String_(''), + ), + ), + true, + ]; } } @@ -687,7 +702,6 @@ private function handleAllNot( $scope, $node->getArgs()[0]->value, static fn (Type $type): Type => TypeCombinator::removeNull($type), - null, ); } @@ -703,7 +717,6 @@ private function handleAllNot( $scope, $node->getArgs()[0]->value, static fn (Type $type): Type => TypeCombinator::remove($type, $classNameType), - null, ); } @@ -713,7 +726,6 @@ private function handleAllNot( $scope, $node->getArgs()[0]->value, static fn (Type $type): Type => TypeCombinator::remove($type, $valueType), - null, ); } @@ -732,7 +744,7 @@ private function handleAll( { $args = $node->getArgs(); $args[0] = new Arg(new ArrayDimFetch($args[0]->value, new LNumber(0))); - [$expr, $rootExpr] = self::createExpression($scope, $methodName, $args); + [$expr, $isEquality] = self::createExpression($scope, $methodName, $args); if ($expr === null) { return new SpecifiedTypes(); } @@ -741,7 +753,7 @@ private function handleAll( $scope, $expr, TypeSpecifierContext::createTruthy(), - )->setRootExpr($rootExpr ?? $expr); + ); $sureNotTypes = $specifiedTypes->getSureNotTypes(); foreach ($specifiedTypes->getSureTypes() as $exprStr => [$exprNode, $type]) { @@ -754,12 +766,16 @@ private function handleAll( $type = $typeModifier($type); } - return $this->allArrayOrIterable( + $specifiedTypes = $this->allArrayOrIterable( $scope, $node->getArgs()[0]->value, static fn (): Type => $type, - $rootExpr, ); + break; + } + + if ($isEquality) { + $specifiedTypes = $specifiedTypes->setEquality(); } return $specifiedTypes; @@ -768,8 +784,7 @@ private function handleAll( private function allArrayOrIterable( Scope $scope, Expr $expr, - Closure $typeCallback, - ?Expr $rootExpr + Closure $typeCallback ): SpecifiedTypes { $currentType = TypeCombinator::intersect($scope->getType($expr), new IterableType(new MixedType(), new MixedType())); @@ -809,14 +824,12 @@ private function allArrayOrIterable( return new SpecifiedTypes([], []); } - $specifiedTypes = $this->typeSpecifier->create( + return $this->typeSpecifier->create( $expr, $specifiedType, TypeSpecifierContext::createTruthy(), $scope, - )->setRootExpr($rootExpr); - - return $this->specifyRootExprIfSet($rootExpr, $scope, $specifiedTypes); + ); } /** @@ -856,41 +869,4 @@ private static function buildAnyOfExpr(Scope $scope, Arg $value, Arg $items, cal return self::implodeExpr($resolvers, BooleanOr::class); } - /** - * @param Arg[] $args - * @return array{Expr, Expr} - */ - private static function createIsNonEmptyStringAndSomethingExprPair(string $name, array $args): array - { - $expr = new BooleanAnd( - new FuncCall( - new Name('is_string'), - [$args[0]], - ), - new NotIdentical( - $args[0]->value, - new String_(''), - ), - ); - - $rootExpr = new BooleanAnd( - $expr, - new FuncCall(new Name('FAUX_FUNCTION_ ' . $name), $args), - ); - - return [$expr, $rootExpr]; - } - - private function specifyRootExprIfSet(?Expr $rootExpr, Scope $scope, SpecifiedTypes $specifiedTypes): SpecifiedTypes - { - if ($rootExpr === null) { - return $specifiedTypes; - } - - // Makes consecutive calls with a rootExpr adding unknown info via FAUX_FUNCTION evaluate to true - return $specifiedTypes->unionWith( - $this->typeSpecifier->create($rootExpr, new ConstantBooleanType(true), TypeSpecifierContext::createTruthy(), $scope), - ); - } - } diff --git a/tests/Type/WebMozartAssert/ImpossibleCheckTypeMethodCallRuleTest.php b/tests/Type/WebMozartAssert/ImpossibleCheckTypeMethodCallRuleTest.php index 4be70b0..7a11915 100644 --- a/tests/Type/WebMozartAssert/ImpossibleCheckTypeMethodCallRuleTest.php +++ b/tests/Type/WebMozartAssert/ImpossibleCheckTypeMethodCallRuleTest.php @@ -19,8 +19,6 @@ protected function getRule(): Rule public function testExtension(): void { - $tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.'; - $this->analyse([__DIR__ . '/data/impossible-check.php'], [ [ 'Call to static method Webmozart\Assert\Assert::stringNotEmpty() with \'\' will always evaluate to false.', @@ -110,7 +108,6 @@ public function testExtension(): void [ 'Call to static method Webmozart\Assert\Assert::isInstanceOf() with Exception and class-string will always evaluate to true.', 119, - $tipText, ], [ 'Call to static method Webmozart\Assert\Assert::startsWith() with \'value\' and string will always evaluate to true.', @@ -119,7 +116,6 @@ public function testExtension(): void [ 'Call to static method Webmozart\Assert\Assert::implementsInterface() with \'WebmozartAssertImpossibleCheck\\\\Bar\' and \'WebmozartAssertImpossibleCheck\\\\Bar\' will always evaluate to false.', 134, - $tipText, ], ]); } @@ -196,34 +192,28 @@ public function testEqNotEq(): void public function testBug8(): void { - $tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.'; $this->analyse([__DIR__ . '/data/bug-8.php'], [ [ 'Call to static method Webmozart\Assert\Assert::numeric() with numeric-string will always evaluate to true.', 15, - $tipText, ], [ 'Call to static method Webmozart\Assert\Assert::numeric() with \'foo\' will always evaluate to false.', 16, - $tipText, ], [ 'Call to static method Webmozart\Assert\Assert::numeric() with \'17.19\' will always evaluate to true.', 17, - $tipText, ], ]); } public function testBug17(): void { - $tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.'; $this->analyse([__DIR__ . '/data/bug-17.php'], [ [ 'Call to static method Webmozart\Assert\Assert::implementsInterface() with \'DateTime\' and \'DateTimeInterface\' will always evaluate to true.', 9, - $tipText, ], ]); }