Skip to content

Commit c7dc8b2

Browse files
author
Emil Masiakowski
committed
Fix isJsonString assertion
1 parent ac3e916 commit c7dc8b2

File tree

5 files changed

+87
-12
lines changed

5 files changed

+87
-12
lines changed

Diff for: src/Type/BeberleiAssert/AssertHelper.php

+67-11
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
use ReflectionObject;
3939
use function array_key_exists;
4040
use function count;
41+
use function is_array;
4142
use function key;
4243
use function reset;
4344

@@ -62,7 +63,7 @@ public static function isSupported(
6263
}
6364

6465
$resolver = $resolvers[$assertName];
65-
$resolverReflection = new ReflectionObject($resolver);
66+
$resolverReflection = new ReflectionObject(Closure::fromCallable($resolver));
6667

6768
return count($args) >= count($resolverReflection->getMethod('__invoke')->getParameters()) - 1;
6869
}
@@ -78,7 +79,7 @@ public static function specifyTypes(
7879
bool $nullOr
7980
): SpecifiedTypes
8081
{
81-
$expression = self::createExpression($scope, $assertName, $args);
82+
[$expression, $rootExpr] = self::createExpression($scope, $assertName, $args);
8283
if ($expression === null) {
8384
return new SpecifiedTypes([], []);
8485
}
@@ -93,11 +94,13 @@ public static function specifyTypes(
9394
);
9495
}
9596

96-
return $typeSpecifier->specifyTypesInCondition(
97+
$specifiedTypes = $typeSpecifier->specifyTypesInCondition(
9798
$scope,
9899
$expression,
99100
TypeSpecifierContext::createTruthy(),
100-
);
101+
)->setRootExpr($rootExpr ?? $expression);
102+
103+
return self::specifyRootExprIfSet($rootExpr, $scope, $specifiedTypes, $typeSpecifier);
101104
}
102105

103106
public static function handleAll(
@@ -239,21 +242,34 @@ private static function allArrayOrIterable(
239242

240243
/**
241244
* @param Arg[] $args
245+
* @return array{?Expr, ?Expr}
242246
*/
243247
private static function createExpression(
244248
Scope $scope,
245249
string $assertName,
246250
array $args
247-
): ?Expr
251+
): array
248252
{
249253
$resolvers = self::getExpressionResolvers();
250254
$resolver = $resolvers[$assertName];
251255

252-
return $resolver($scope, ...$args);
256+
$resolverResult = $resolver($scope, ...$args);
257+
if (is_array($resolverResult)) {
258+
[$expr, $rootExpr] = $resolverResult;
259+
} else {
260+
$expr = $resolverResult;
261+
$rootExpr = null;
262+
}
263+
264+
if ($expr === null) {
265+
return [null, null];
266+
}
267+
268+
return [$expr, $rootExpr];
253269
}
254270

255271
/**
256-
* @return Closure[]
272+
* @return array<string, callable(Scope, Arg...): (Expr|array{?Expr, ?Expr}|null)>
257273
*/
258274
private static function getExpressionResolvers(): array
259275
{
@@ -363,10 +379,6 @@ private static function getExpressionResolvers(): array
363379
$class,
364380
],
365381
),
366-
'isJsonString' => static fn (Scope $scope, Arg $value): Expr => new FuncCall(
367-
new Name('is_string'),
368-
[$value],
369-
),
370382
'integerish' => static fn (Scope $scope, Arg $value): Expr => new FuncCall(
371383
new Name('is_numeric'),
372384
[$value],
@@ -406,7 +418,51 @@ private static function getExpressionResolvers(): array
406418
];
407419
}
408420

421+
$assertionsResultingAtLeastInNonEmptyString = [
422+
'isJsonString',
423+
];
424+
foreach ($assertionsResultingAtLeastInNonEmptyString as $name) {
425+
self::$resolvers[$name] = static fn (Scope $scope, Arg $value): array => self::createIsNonEmptyStringAndSomethingExprPair($name, [$value]);
426+
}
427+
409428
return self::$resolvers;
410429
}
411430

431+
/**
432+
* @param Arg[] $args
433+
* @return array{Expr, Expr}
434+
*/
435+
private static function createIsNonEmptyStringAndSomethingExprPair(string $name, array $args): array
436+
{
437+
$expr = new BooleanAnd(
438+
new FuncCall(
439+
new Name('is_string'),
440+
[$args[0]],
441+
),
442+
new NotIdentical(
443+
$args[0]->value,
444+
new String_(''),
445+
),
446+
);
447+
448+
$rootExpr = new BooleanAnd(
449+
$expr,
450+
new FuncCall(new Name('FAUX_FUNCTION_ ' . $name), $args),
451+
);
452+
453+
return [$expr, $rootExpr];
454+
}
455+
456+
private static function specifyRootExprIfSet(?Expr $rootExpr, Scope $scope, SpecifiedTypes $specifiedTypes, TypeSpecifier $typeSpecifier): SpecifiedTypes
457+
{
458+
if ($rootExpr === null) {
459+
return $specifiedTypes;
460+
}
461+
462+
// Makes consecutive calls with a rootExpr adding unknown info via FAUX_FUNCTION evaluate to true
463+
return $specifiedTypes->unionWith(
464+
$typeSpecifier->create($rootExpr, new ConstantBooleanType(true), TypeSpecifierContext::createTruthy(), $scope),
465+
);
466+
}
467+
412468
}

Diff for: tests/Type/BeberleiAssert/ImpossibleCheckTypeMethodCallRuleTest.php

+10
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,16 @@ public function testExtension(): void
3030
16,
3131
'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
3232
],
33+
[
34+
'Call to method Assert\AssertionChain::isJsonString() will always evaluate to true.',
35+
22,
36+
'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
37+
],
38+
[
39+
'Call to method Assert\AssertionChain::isJsonString() will always evaluate to true.',
40+
25,
41+
'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
42+
],
3343
]);
3444
}
3545

Diff for: tests/Type/BeberleiAssert/ImpossibleCheckTypeStaticMethodCallRuleTest.php

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ public function testExtension(): void
2323
[
2424
'Call to static method Assert\Assertion::string() with string will always evaluate to true.',
2525
12,
26+
'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
2627
],
2728
[
2829
'Call to static method Assert\Assertion::allString() with array<string> will always evaluate to true.',

Diff for: tests/Type/BeberleiAssert/data/data.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public function doFoo($a, $b, array $c, iterable $d, $e, $f, $g, $h, $i, $j, $k,
129129
\PHPStan\Testing\assertType('array<class-string<PHPStan\Type\BeberleiAssert\Foo>|PHPStan\Type\BeberleiAssert\Foo>', $ab);
130130

131131
Assertion::isJsonString($ac);
132-
\PHPStan\Testing\assertType('string', $ac);
132+
\PHPStan\Testing\assertType('non-empty-string', $ac);
133133

134134
/** @var array{a?: string, b?: int} $ad */
135135
$ad = doFoo();

Diff for: tests/Type/BeberleiAssert/data/impossible-check.php

+8
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,12 @@ public function doBar(string $s)
1616
Assert::that($strings)->all()->string();
1717
}
1818

19+
public function nonEmptyStringAndSomethingUnknownNarrow(string $a, array $b): void
20+
{
21+
Assert::that($a)->isJsonString();
22+
Assert::that($a)->isJsonString(); // only this should report
23+
24+
Assert::thatAll($b)->isJsonString();
25+
Assert::thatAll($b)->isJsonString(); // only this should report
26+
}
1927
}

0 commit comments

Comments
 (0)