From c7dc8b24135486ad473cad652570803fd8aaacfd Mon Sep 17 00:00:00 2001 From: Emil Masiakowski Date: Thu, 20 Mar 2025 19:38:37 +0100 Subject: [PATCH] Fix isJsonString assertion --- src/Type/BeberleiAssert/AssertHelper.php | 78 ++++++++++++++++--- .../ImpossibleCheckTypeMethodCallRuleTest.php | 10 +++ ...sibleCheckTypeStaticMethodCallRuleTest.php | 1 + tests/Type/BeberleiAssert/data/data.php | 2 +- .../BeberleiAssert/data/impossible-check.php | 8 ++ 5 files changed, 87 insertions(+), 12 deletions(-) diff --git a/src/Type/BeberleiAssert/AssertHelper.php b/src/Type/BeberleiAssert/AssertHelper.php index 5d4e873..f3f73a3 100644 --- a/src/Type/BeberleiAssert/AssertHelper.php +++ b/src/Type/BeberleiAssert/AssertHelper.php @@ -38,6 +38,7 @@ use ReflectionObject; use function array_key_exists; use function count; +use function is_array; use function key; use function reset; @@ -62,7 +63,7 @@ public static function isSupported( } $resolver = $resolvers[$assertName]; - $resolverReflection = new ReflectionObject($resolver); + $resolverReflection = new ReflectionObject(Closure::fromCallable($resolver)); return count($args) >= count($resolverReflection->getMethod('__invoke')->getParameters()) - 1; } @@ -78,7 +79,7 @@ public static function specifyTypes( bool $nullOr ): SpecifiedTypes { - $expression = self::createExpression($scope, $assertName, $args); + [$expression, $rootExpr] = self::createExpression($scope, $assertName, $args); if ($expression === null) { return new SpecifiedTypes([], []); } @@ -93,11 +94,13 @@ public static function specifyTypes( ); } - return $typeSpecifier->specifyTypesInCondition( + $specifiedTypes = $typeSpecifier->specifyTypesInCondition( $scope, $expression, TypeSpecifierContext::createTruthy(), - ); + )->setRootExpr($rootExpr ?? $expression); + + return self::specifyRootExprIfSet($rootExpr, $scope, $specifiedTypes, $typeSpecifier); } public static function handleAll( @@ -239,21 +242,34 @@ private static function allArrayOrIterable( /** * @param Arg[] $args + * @return array{?Expr, ?Expr} */ private static function createExpression( Scope $scope, string $assertName, array $args - ): ?Expr + ): array { $resolvers = self::getExpressionResolvers(); $resolver = $resolvers[$assertName]; - return $resolver($scope, ...$args); + $resolverResult = $resolver($scope, ...$args); + if (is_array($resolverResult)) { + [$expr, $rootExpr] = $resolverResult; + } else { + $expr = $resolverResult; + $rootExpr = null; + } + + if ($expr === null) { + return [null, null]; + } + + return [$expr, $rootExpr]; } /** - * @return Closure[] + * @return array */ private static function getExpressionResolvers(): array { @@ -363,10 +379,6 @@ private static function getExpressionResolvers(): array $class, ], ), - 'isJsonString' => static fn (Scope $scope, Arg $value): Expr => new FuncCall( - new Name('is_string'), - [$value], - ), 'integerish' => static fn (Scope $scope, Arg $value): Expr => new FuncCall( new Name('is_numeric'), [$value], @@ -406,7 +418,51 @@ private static function getExpressionResolvers(): array ]; } + $assertionsResultingAtLeastInNonEmptyString = [ + 'isJsonString', + ]; + foreach ($assertionsResultingAtLeastInNonEmptyString as $name) { + self::$resolvers[$name] = static fn (Scope $scope, Arg $value): array => self::createIsNonEmptyStringAndSomethingExprPair($name, [$value]); + } + return self::$resolvers; } + /** + * @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 static function specifyRootExprIfSet(?Expr $rootExpr, Scope $scope, SpecifiedTypes $specifiedTypes, TypeSpecifier $typeSpecifier): SpecifiedTypes + { + if ($rootExpr === null) { + return $specifiedTypes; + } + + // Makes consecutive calls with a rootExpr adding unknown info via FAUX_FUNCTION evaluate to true + return $specifiedTypes->unionWith( + $typeSpecifier->create($rootExpr, new ConstantBooleanType(true), TypeSpecifierContext::createTruthy(), $scope), + ); + } + } diff --git a/tests/Type/BeberleiAssert/ImpossibleCheckTypeMethodCallRuleTest.php b/tests/Type/BeberleiAssert/ImpossibleCheckTypeMethodCallRuleTest.php index 9438816..735a93e 100644 --- a/tests/Type/BeberleiAssert/ImpossibleCheckTypeMethodCallRuleTest.php +++ b/tests/Type/BeberleiAssert/ImpossibleCheckTypeMethodCallRuleTest.php @@ -30,6 +30,16 @@ public function testExtension(): void 16, 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', ], + [ + 'Call to method Assert\AssertionChain::isJsonString() will always evaluate to true.', + 22, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + ], + [ + 'Call to method Assert\AssertionChain::isJsonString() will always evaluate to true.', + 25, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + ], ]); } diff --git a/tests/Type/BeberleiAssert/ImpossibleCheckTypeStaticMethodCallRuleTest.php b/tests/Type/BeberleiAssert/ImpossibleCheckTypeStaticMethodCallRuleTest.php index 0355c6c..7790b8a 100644 --- a/tests/Type/BeberleiAssert/ImpossibleCheckTypeStaticMethodCallRuleTest.php +++ b/tests/Type/BeberleiAssert/ImpossibleCheckTypeStaticMethodCallRuleTest.php @@ -23,6 +23,7 @@ public function testExtension(): void [ 'Call to static method Assert\Assertion::string() with string will always evaluate to true.', 12, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', ], [ 'Call to static method Assert\Assertion::allString() with array will always evaluate to true.', diff --git a/tests/Type/BeberleiAssert/data/data.php b/tests/Type/BeberleiAssert/data/data.php index 11d25be..57cf656 100644 --- a/tests/Type/BeberleiAssert/data/data.php +++ b/tests/Type/BeberleiAssert/data/data.php @@ -129,7 +129,7 @@ public function doFoo($a, $b, array $c, iterable $d, $e, $f, $g, $h, $i, $j, $k, \PHPStan\Testing\assertType('array|PHPStan\Type\BeberleiAssert\Foo>', $ab); Assertion::isJsonString($ac); - \PHPStan\Testing\assertType('string', $ac); + \PHPStan\Testing\assertType('non-empty-string', $ac); /** @var array{a?: string, b?: int} $ad */ $ad = doFoo(); diff --git a/tests/Type/BeberleiAssert/data/impossible-check.php b/tests/Type/BeberleiAssert/data/impossible-check.php index e9909a1..29cbab7 100644 --- a/tests/Type/BeberleiAssert/data/impossible-check.php +++ b/tests/Type/BeberleiAssert/data/impossible-check.php @@ -16,4 +16,12 @@ public function doBar(string $s) Assert::that($strings)->all()->string(); } + public function nonEmptyStringAndSomethingUnknownNarrow(string $a, array $b): void + { + Assert::that($a)->isJsonString(); + Assert::that($a)->isJsonString(); // only this should report + + Assert::thatAll($b)->isJsonString(); + Assert::thatAll($b)->isJsonString(); // only this should report + } }