diff --git a/extension.neon b/extension.neon index f0f672d..d8f4e41 100644 --- a/extension.neon +++ b/extension.neon @@ -26,6 +26,30 @@ services: - class: Pest\PHPStan\Analysis\Expectation\ExpectationSemanticAnalyzer + - + class: Pest\PHPStan\Analysis\Expectation\ExpectationNarrowingResolver + + - + class: Pest\PHPStan\Type\Pest\ExpectationTypeSpecifyingExtension + arguments: + className: Pest\Expectation + tags: + - phpstan.typeSpecifier.methodTypeSpecifyingExtension + + - + class: Pest\PHPStan\Type\Pest\ExpectationTypeSpecifyingExtension + arguments: + className: Pest\Mixins\Expectation + tags: + - phpstan.typeSpecifier.methodTypeSpecifyingExtension + + - + class: Pest\PHPStan\Type\Pest\ExpectationTypeSpecifyingExtension + arguments: + className: Pest\Expectations\OppositeExpectation + tags: + - phpstan.typeSpecifier.methodTypeSpecifyingExtension + - class: Pest\PHPStan\Type\Pest\PestFunctionReturnTypeExtension tags: diff --git a/src/Analysis/Expectation/ExpectationMatcherRegistry.php b/src/Analysis/Expectation/ExpectationMatcherRegistry.php index b2bd271..3c87d3f 100644 --- a/src/Analysis/Expectation/ExpectationMatcherRegistry.php +++ b/src/Analysis/Expectation/ExpectationMatcherRegistry.php @@ -28,6 +28,11 @@ public function assertedTypeFor(string $methodName, MethodCall $methodCall, Scop return $this->assertionRegistry->assertedTypeFor($methodName, $methodCall, $scope); } + public function assertsExactTypeFor(string $methodName, MethodCall $methodCall, Scope $scope): bool + { + return $this->assertionRegistry->assertsExactTypeFor($methodName, $methodCall, $scope); + } + public function metadataFor(string $methodName): ?MatcherSemanticMetadata { if (array_key_exists($methodName, $this->metadataCache)) { diff --git a/src/Analysis/Expectation/ExpectationNarrowing.php b/src/Analysis/Expectation/ExpectationNarrowing.php new file mode 100644 index 0000000..5308a8a --- /dev/null +++ b/src/Analysis/Expectation/ExpectationNarrowing.php @@ -0,0 +1,29 @@ + Matcher name => uses loose (==) comparison */ + private const array COMPARISON_METHODS = ['toBe' => false, 'toEqual' => true]; + + private const string COMPARISON_PARAMETER = 'expected'; + + public function __construct( + private readonly ExpectationMatcherRegistry $matcherRegistry, + ) {} + + /** + * @return list + */ + public function resolve(MethodCall $methodCall, Scope $scope): array + { + $links = []; + $current = $methodCall; + + while (($current instanceof MethodCall || $current instanceof PropertyFetch) && $current->name instanceof Identifier) { + $links[] = $current; + $current = $current->var; + } + + $subject = $this->resolveSubject($current, $scope); + if (! $subject instanceof Expr) { + return []; + } + + $narrowings = []; + $negated = false; + + foreach (array_reverse($links) as $link) { + if ($link instanceof PropertyFetch) { + /** @var Identifier $name */ + $name = $link->name; + if ($name->name === self::NEGATE_METHOD) { + $negated = ! $negated; + + continue; + } + + return $narrowings; + } + + if ($link->isFirstClassCallable()) { + return $narrowings; + } + + /** @var Identifier $name */ + $name = $link->name; + $methodName = $name->name; + + if ($methodName === self::NEGATE_METHOD && $link->getArgs() === []) { + $negated = ! $negated; + + continue; + } + + if ($methodName === self::REBIND_METHOD) { + $rebound = MatcherArgument::first($link, self::REBIND_PARAMETER); + if (! $rebound instanceof Expr) { + return $narrowings; + } + + if ($this->mayBeExpectation($rebound, $scope)) { + return $narrowings; + } + + $subject = $rebound; + $negated = false; + + continue; + } + + if (in_array($methodName, self::PASSTHROUGH_METHODS, true)) { + continue; + } + + if (! method_exists(MixinsExpectation::class, $methodName)) { + return $narrowings; + } + + if (isset(self::COMPARISON_METHODS[$methodName])) { + $compared = MatcherArgument::first($link, self::COMPARISON_PARAMETER); + if ($compared instanceof Expr) { + $narrowings[] = ExpectationNarrowing::comparison($subject, $compared, self::COMPARISON_METHODS[$methodName], $negated); + } + + $negated = false; + + continue; + } + + if ($negated && ! $this->matcherRegistry->assertsExactTypeFor($methodName, $link, $scope)) { + $negated = false; + + continue; + } + + $assertedType = $this->matcherRegistry->assertedTypeFor($methodName, $link, $scope); + + if ($assertedType instanceof Type) { + $narrowings[] = ExpectationNarrowing::type($subject, $assertedType, $negated); + } + + $negated = false; + } + + return $narrowings; + } + + /** @return bool True when and() may unwrap the argument to an inner value we cannot track */ + private function mayBeExpectation(Expr $expr, Scope $scope): bool + { + return ! new ObjectType(Expectation::class)->isSuperTypeOf($scope->getType($expr))->no(); + } + + private function resolveSubject(Expr $root, Scope $scope): ?Expr + { + if (! $root instanceof FuncCall || ! $root->name instanceof Name) { + return null; + } + + if ($root->name->toLowerString() !== 'expect') { + return null; + } + + if (! new ObjectType(Expectation::class)->isSuperTypeOf($scope->getType($root))->yes()) { + return null; + } + + return $root->getArgs()[0]->value ?? null; + } +} diff --git a/src/Analysis/Expectation/MatcherArgument.php b/src/Analysis/Expectation/MatcherArgument.php new file mode 100644 index 0000000..f9321d8 --- /dev/null +++ b/src/Analysis/Expectation/MatcherArgument.php @@ -0,0 +1,43 @@ +isFirstClassCallable()) { + return null; + } + + foreach ($methodCall->getArgs() as $position => $argument) { + if ($argument->unpack) { + return null; + } + + if ($argument->name instanceof Identifier) { + if ($argument->name->name === $parameterName) { + return $argument->value; + } + + continue; + } + + if ($position === 0) { + return $argument->value; + } + } + + return null; + } +} diff --git a/src/Analysis/Expectation/MatcherAssertionRegistry.php b/src/Analysis/Expectation/MatcherAssertionRegistry.php index 7800450..ab9d3cf 100644 --- a/src/Analysis/Expectation/MatcherAssertionRegistry.php +++ b/src/Analysis/Expectation/MatcherAssertionRegistry.php @@ -4,6 +4,7 @@ namespace Pest\PHPStan\Analysis\Expectation; +use PhpParser\Node\Expr; use PhpParser\Node\Expr\MethodCall; use PHPStan\Analyser\Scope; use PHPStan\Type\Accessory\AccessoryArrayListType; @@ -12,6 +13,7 @@ use PHPStan\Type\BooleanType; use PHPStan\Type\CallableType; use PHPStan\Type\Constant\ConstantBooleanType; +use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\FloatType; use PHPStan\Type\IntegerType; use PHPStan\Type\IntersectionType; @@ -80,6 +82,8 @@ final class MatcherAssertionRegistry 'toBeResource' => self::RESOURCE, ]; + private const string INSTANCE_OF_PARAMETER = 'class'; + /** @var array */ private array $staticAssertedTypeCache = []; @@ -146,26 +150,43 @@ public function assertedTypeFor(string $methodName, MethodCall $methodCall, Scop return $assertedType; } + /** @return bool True when the asserted type mirrors the matcher exactly, so it may also be removed */ + public function assertsExactTypeFor(string $methodName, MethodCall $methodCall, Scope $scope): bool + { + if ($this->assertionFor($methodName) !== self::INSTANCE_OF) { + return true; + } + + return count($this->constantClassNames($methodCall, $scope)) === 1; + } + private function resolveToBeInstanceOf(MethodCall $methodCall, Scope $scope): Type { - $args = $methodCall->getArgs(); + $classNames = $this->constantClassNames($methodCall, $scope); - if ($args === []) { + if ($classNames === []) { return new ObjectWithoutClassType; } - $classType = $scope->getType($args[0]->value); - $classNames = $classType->getConstantStrings(); + $objectTypes = array_map( + static fn (ConstantStringType $name): ObjectType => new ObjectType($name->getValue()), + $classNames + ); + + return TypeCombinator::union(...$objectTypes); + } - if ($classNames !== []) { - $objectTypes = array_map( - static fn ($name): ObjectType => new ObjectType($name->getValue()), - $classNames - ); + /** + * @return list + */ + private function constantClassNames(MethodCall $methodCall, Scope $scope): array + { + $class = MatcherArgument::first($methodCall, self::INSTANCE_OF_PARAMETER); - return TypeCombinator::union(...$objectTypes); + if (! $class instanceof Expr) { + return []; } - return new ObjectWithoutClassType; + return $scope->getType($class)->getConstantStrings(); } } diff --git a/src/Type/Pest/ExpectationTypeSpecifyingExtension.php b/src/Type/Pest/ExpectationTypeSpecifyingExtension.php new file mode 100644 index 0000000..3757827 --- /dev/null +++ b/src/Type/Pest/ExpectationTypeSpecifyingExtension.php @@ -0,0 +1,83 @@ +className; + } + + public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void + { + $this->typeSpecifier = $typeSpecifier; + } + + public function isMethodSupported(MethodReflection $methodReflection, MethodCall $node, TypeSpecifierContext $context): bool + { + return $context->null(); + } + + public function specifyTypes(MethodReflection $methodReflection, MethodCall $node, Scope $scope, TypeSpecifierContext $context): SpecifiedTypes + { + $specifiedTypes = new SpecifiedTypes; + + foreach ($this->narrowingResolver->resolve($node, $scope) as $narrowing) { + $context = $narrowing->negated + ? TypeSpecifierContext::createTruthy()->negate() + : TypeSpecifierContext::createTruthy(); + + if ($narrowing->comparedExpr instanceof Expr) { + $comparison = $narrowing->loose + ? new Equal($narrowing->subject, $narrowing->comparedExpr) + : new Identical($narrowing->subject, $narrowing->comparedExpr); + + $specifiedTypes = $specifiedTypes->unionWith( + $this->typeSpecifier->specifyTypesInCondition($scope, $comparison, $context), + ); + + continue; + } + + if (! $narrowing->assertedType instanceof Type) { + continue; + } + + $specifiedTypes = $specifiedTypes->unionWith($this->typeSpecifier->create( + $narrowing->subject, + $narrowing->assertedType, + $context, + $scope, + )); + } + + return $specifiedTypes; + } +} diff --git a/tests/Rules/RedundantExpectationRuleTest.php b/tests/Rules/RedundantExpectationRuleTest.php index d6c0be4..a9c6046 100644 --- a/tests/Rules/RedundantExpectationRuleTest.php +++ b/tests/Rules/RedundantExpectationRuleTest.php @@ -127,6 +127,18 @@ ]); }); +test('redundancy is reported across separate expect() statements', function (): void { + $this->analyse([ + __DIR__.'/data/redundant-expectation-cross-statement.php', + ], [ + [ + 'Calling toBeInt() on Expectation; assertion is redundant.', + 8, + 'The expectation value is already guaranteed to satisfy toBeInt().', + ], + ]); +}); + test('every redundant matcher combination is reported without false positives', function (): void { $this->analyse([ __DIR__.'/data/redundant-expectation-exhaustive.php', diff --git a/tests/Rules/data/impossible-expectation-exhaustive.php b/tests/Rules/data/impossible-expectation-exhaustive.php index 270acb0..130c1ab 100644 --- a/tests/Rules/data/impossible-expectation-exhaustive.php +++ b/tests/Rules/data/impossible-expectation-exhaustive.php @@ -144,19 +144,39 @@ expect(new Post)->toBeInstanceOf(RuntimeException::class); }); -it('mixed can be anything', function (): void { +it('mixed can be a string', function (): void { /** @var mixed $value */ $value = null; expect($value)->toBeString(); +}); + +it('mixed can be an int', function (): void { + /** @var mixed $value */ + $value = null; expect($value)->toBeInt(); +}); + +it('mixed can be an array', function (): void { + /** @var mixed $value */ + $value = null; expect($value)->toBeArray(); +}); + +it('mixed can be an instance', function (): void { + /** @var mixed $value */ + $value = null; expect($value)->toBeInstanceOf(stdClass::class); }); -it('unions may match either branch', function (): void { +it('union may match the string branch', function (): void { /** @var int|string $value */ $value = 1; expect($value)->toBeString(); +}); + +it('union may match the int branch', function (): void { + /** @var int|string $value */ + $value = 1; expect($value)->toBeInt(); }); @@ -164,6 +184,11 @@ /** @var string|null $value */ $value = null; expect($value)->toBeNull(); +}); + +it('nullable may be a string', function (): void { + /** @var string|null $value */ + $value = null; expect($value)->toBeString(); }); diff --git a/tests/Rules/data/redundant-expectation-cross-statement.php b/tests/Rules/data/redundant-expectation-cross-statement.php new file mode 100644 index 0000000..75a9925 --- /dev/null +++ b/tests/Rules/data/redundant-expectation-cross-statement.php @@ -0,0 +1,9 @@ +toBeInt(); + expect($value)->toBeInt(); +}); diff --git a/tests/Type/ExpectTypeTest.php b/tests/Type/ExpectTypeTest.php index 583c1a7..914a594 100644 --- a/tests/Type/ExpectTypeTest.php +++ b/tests/Type/ExpectTypeTest.php @@ -97,3 +97,9 @@ })->with(function (): Iterator { yield from TestCase::gatherAssertTypes(__DIR__.'/data/test-hook-properties-exhaustive.php'); }); + +test('expectation narrowing types', function (string $assertType, string $file, mixed ...$args): void { + $this->assertFileAsserts($assertType, $file, ...$args); +})->with(function (): Iterator { + yield from TestCase::gatherAssertTypes(__DIR__.'/data/expectation-narrowing.php'); +}); diff --git a/tests/Type/data/expectation-narrowing.php b/tests/Type/data/expectation-narrowing.php new file mode 100644 index 0000000..ac6c9ee --- /dev/null +++ b/tests/Type/data/expectation-narrowing.php @@ -0,0 +1,299 @@ +toBeInt(); + assertType('int', $value); +} + +function testToBeStringNarrows(): void +{ + /** @var int|string $value */ + $value = random_int(0, 1) === 1 ? 1 : 'a'; + expect($value)->toBeString(); + assertType('string', $value); +} + +function testToBeNullNarrows(): void +{ + /** @var string|null $value */ + $value = random_int(0, 1) === 1 ? 'a' : null; + expect($value)->toBeNull(); + assertType('null', $value); +} + +function testToBeInstanceOfNarrows(): void +{ + $value = random_int(0, 1) === 1 ? new RuntimeException('x') : 'a'; + expect($value)->toBeInstanceOf(RuntimeException::class); + assertType('RuntimeException', $value); +} + +function testToBeTrueNarrows(): void +{ + $value = random_int(0, 1) === 1; + expect($value)->toBeTrue(); + assertType('true', $value); +} + +function testChainedMatchersAllNarrow(): void +{ + /** @var string|null $value */ + $value = random_int(0, 1) === 1 ? 'a' : null; + expect($value)->toBeString()->toStartWith('a'); + assertType('string', $value); +} + +function testAndRebindsTheSubject(): void +{ + /** @var int|string $first */ + $first = random_int(0, 1) === 1 ? 1 : 'a'; + /** @var int|string $second */ + $second = random_int(0, 1) === 1 ? 1 : 'a'; + expect($first)->toBeInt()->and($second)->toBeString(); + assertType('int', $first); + assertType('string', $second); +} + +function testWhenIsPassthrough(): void +{ + /** @var int|string $value */ + $value = random_int(0, 1) === 1 ? 1 : 'a'; + expect($value)->when(true, fn ($e) => $e)->toBeInt(); + assertType('int', $value); +} + +function testJsonBreaksTheSubjectLink(): void +{ + /** @var int|string $value */ + $value = random_int(0, 1) === 1 ? 1 : 'a'; + expect($value)->toBeString()->json()->toBeArray(); + assertType('string', $value); +} + +function testEachDoesNotNarrowTheSubject(): void +{ + /** @var array $values */ + $values = []; + expect($values)->each->toBeInt(); + assertType('array', $values); +} + +function testHigherOrderPropertyDoesNotNarrow(): void +{ + /** @var int|string $value Stays wide: the extension never fires on higher order chains */ + $value = random_int(0, 1) === 1 ? 1 : 'a'; + expect($value)->toBeInt()->foo->toBeString(); + assertType('int|string', $value); +} + +function testExpectWithoutArgumentsIsIgnored(): void +{ + expect()->toBeNull(); +} + +function testAssignedExpectationAlsoNarrowsTheSubject(): void +{ + /** @var int|string $value Narrowing also applies when the chain is an assigned expression */ + $value = random_int(0, 1) === 1 ? 1 : 'a'; + $expectation = expect($value)->toBeInt(); + assertType('Pest\Expectation', $expectation); + assertType('int', $value); +} + +function testNotToBeNullRemovesNull(): void +{ + /** @var string|null $value */ + $value = random_int(0, 1) === 1 ? 'a' : null; + expect($value)->not->toBeNull(); + assertType('string', $value); +} + +function testNotMethodRemovesNull(): void +{ + /** @var string|null $value */ + $value = random_int(0, 1) === 1 ? 'a' : null; + expect($value)->not()->toBeNull(); + assertType('string', $value); +} + +function testNotAppliesToOneMatcherOnly(): void +{ + /** @var string|null $value */ + $value = random_int(0, 1) === 1 ? 'a' : null; + expect($value)->not->toBeNull()->toBeString(); + assertType('string', $value); +} + +function testNotToBeInstanceOfRemovesTheClass(): void +{ + $value = random_int(0, 1) === 1 ? new RuntimeException('x') : 'a'; + expect($value)->not->toBeInstanceOf(RuntimeException::class); + assertType("'a'", $value); +} + +function testNotToBeStringOnUnion(): void +{ + /** @var int|string $value */ + $value = random_int(0, 1) === 1 ? 1 : 'a'; + expect($value)->not->toBeString(); + assertType('int', $value); +} + +function testToBeNarrowsToTheComparedConstant(): void +{ + /** @var int|string $value */ + $value = random_int(0, 1) === 1 ? 1 : 'a'; + expect($value)->toBe(1); + assertType('1', $value); +} + +function testToBeNarrowsToTheComparedExpressionType(): void +{ + $value = random_int(0, 1) === 1 ? new RuntimeException('x') : 'a'; + $expected = new RuntimeException('x'); + expect($value)->toBe($expected); + assertType('RuntimeException', $value); +} + +function testNotToBeRemovesTheConstant(): void +{ + /** @var 'a'|'b' $value */ + $value = 'a'; + expect($value)->not->toBe('a'); + assertType("'b'", $value); +} + +function testToEqualNarrowsLoosely(): void +{ + /** @var string|null $value Loose == against null also matches the empty string */ + $value = random_int(0, 1) === 1 ? 'a' : null; + expect($value)->toEqual(null); + assertType("''|null", $value); +} + +function testNotFollowedByAndRebindDoesNotLeakNegation(): void +{ + /** @var string|null $a */ + $a = random_int(0, 1) === 1 ? 'a' : null; + /** @var int|string $b */ + $b = random_int(0, 1) === 1 ? 1 : 'x'; + expect($a)->not->toBeNull()->and($b)->toBeInt(); + assertType('string', $a); + assertType('int', $b); +} + +function testDoubleNegationCancelsOut(): void +{ + /** @var string|null $value Pest throws on not->not, so the narrowed code never runs; this pins the resolver's bookkeeping */ + $value = random_int(0, 1) === 1 ? 'a' : null; + expect($value)->not->not->toBeNull(); + assertType('null', $value); +} + +function testToBeWithNoArgumentsDoesNotNarrow(): void +{ + /** @var int|string $value */ + $value = random_int(0, 1) === 1 ? 1 : 'a'; + expect($value)->toBe(); + assertType('int|string', $value); +} + +function testNotToBeInstanceOfWithClassStringVariableDoesNotNarrow(): void +{ + /** @var RuntimeException|string $value */ + $value = random_int(0, 1) === 1 ? new RuntimeException('x') : 'a'; + /** @var class-string $class Unknown class: any object may still pass the negated matcher */ + $class = RuntimeException::class; + expect($value)->not->toBeInstanceOf($class); + assertType('RuntimeException|string', $value); +} + +function testNotToBeInstanceOfWithMultipleClassStringsDoesNotNarrow(): void +{ + /** @var LogicException|RuntimeException|string $value Only one of the two classes is checked at runtime */ + $value = random_int(0, 1) === 1 ? new RuntimeException('x') : 'a'; + $class = random_int(0, 1) === 1 ? RuntimeException::class : LogicException::class; + expect($value)->not->toBeInstanceOf($class); + assertType('LogicException|RuntimeException|string', $value); +} + +function testToBeInstanceOfWithClassStringVariableNarrowsToObject(): void +{ + /** @var RuntimeException|string $value */ + $value = random_int(0, 1) === 1 ? new RuntimeException('x') : 'a'; + /** @var class-string $class Positive narrowing may over-approximate to any object and stay sound */ + $class = RuntimeException::class; + expect($value)->toBeInstanceOf($class); + assertType('RuntimeException', $value); +} + +function testAndWithExpectationArgumentStopsNarrowing(): void +{ + /** @var int|string $first */ + $first = random_int(0, 1) === 1 ? 1 : 'a'; + /** @var int|string $second and() rebinds to the inner value of the passed expectation */ + $second = random_int(0, 1) === 1 ? 1 : 'a'; + $expectation = expect($second); + expect($first)->toBeInt()->and($expectation)->toBeString(); + assertType('int', $first); + assertType('int|string', $second); + assertType('Pest\Expectation', $expectation); +} + +function testAndWithInlineExpectationStopsNarrowing(): void +{ + /** @var int|string $first */ + $first = random_int(0, 1) === 1 ? 1 : 'a'; + /** @var int|string $second and() rebinds to the inner value, so the printed argument is not the subject */ + $second = random_int(0, 1) === 1 ? 1 : 'a'; + expect($first)->toBeInt()->and(expect($second))->toBeString(); + assertType('int', $first); + assertType('int|string', $second); +} + +function testNamedComparisonArgumentIsReadByName(): void +{ + /** @var int|string $value */ + $value = random_int(0, 1) === 1 ? 1 : 'a'; + expect($value)->toBe(message: 'not one', expected: 1); + assertType('1', $value); +} + +function testComparisonWithoutTheComparedValueDoesNotNarrow(): void +{ + /** @var int|string $value */ + $value = random_int(0, 1) === 1 ? 1 : 'a'; + expect($value)->toBe(message: 'the compared value is missing'); + assertType('int|string', $value); +} + +function testNamedInstanceOfArgumentIsReadByName(): void +{ + /** @var RuntimeException|string $value */ + $value = random_int(0, 1) === 1 ? new RuntimeException('x') : 'a'; + expect($value)->not->toBeInstanceOf(message: 'still an exception', class: RuntimeException::class); + assertType('string', $value); +} + +function testFirstClassCallableChainDoesNotNarrow(): void +{ + /** @var string|null $value PHPStan never offers first-class callable nodes to type-specifying extensions */ + $value = random_int(0, 1) === 1 ? 'a' : null; + /** @var int|string $other */ + $other = random_int(0, 1) === 1 ? 1 : 'a'; + expect($value)->not->toBeNull()->and($other)->toBeInt(...); + assertType('string|null', $value); + assertType('int|string', $other); +}