diff --git a/extension.neon b/extension.neon index ac7f226..3b1df66 100644 --- a/extension.neon +++ b/extension.neon @@ -123,6 +123,11 @@ services: tags: - phpstan.rules.rule + - + class: Pest\PHPStan\Rules\BoundTestCasePrivateMemberRule + tags: + - phpstan.rules.rule + rules: - Pest\PHPStan\Rules\DisallowedCallInDescribeRule - Pest\PHPStan\Rules\StaticTestClosureRule @@ -136,3 +141,4 @@ rules: - Pest\PHPStan\Rules\DescribeWithoutTestsRule - Pest\PHPStan\Rules\InvalidGroupNameRule - Pest\PHPStan\Rules\RedundantLocalUseRule + - Pest\PHPStan\Rules\BoundTestCasePrivateMemberRule diff --git a/rector.php b/rector.php index a6487ba..b455f74 100644 --- a/rector.php +++ b/rector.php @@ -17,6 +17,7 @@ ReadOnlyClassRector::class, __DIR__.'/tests/Type/data', __DIR__.'/tests/Rules/data', + __DIR__.'/tests/Type/Fixtures/PrivateMembers', __DIR__.'/tests/Fixtures/CustomTestCaseInference', __DIR__.'/tests/Fixtures/UsesHookClosureThis', UsesToExtendRector::class => [ diff --git a/src/Rules/BoundTestCasePrivateMemberRule.php b/src/Rules/BoundTestCasePrivateMemberRule.php new file mode 100644 index 0000000..bd3e9d2 --- /dev/null +++ b/src/Rules/BoundTestCasePrivateMemberRule.php @@ -0,0 +1,125 @@ + + */ +final class BoundTestCasePrivateMemberRule implements Rule +{ + #[Override] + public function getNodeType(): string + { + return Expr::class; + } + + /** + * @return list + */ + #[Override] + public function processNode(Node $node, Scope $scope): array + { + if (! $node instanceof MethodCall && ! $node instanceof StaticCall && ! $node instanceof ClassConstFetch) { + return []; + } + + // @note: a closure declared inside a class keeps that class as its scope, so private members stay reachable. + if (! $scope->isInAnonymousFunction() || $scope->isInClass()) { + return []; + } + + if (! $node->name instanceof Identifier) { + return []; + } + + $receiver = $node instanceof MethodCall ? $node->var : $node->class; + + if (! $receiver instanceof Variable || $receiver->name !== 'this') { + return []; + } + + if (! $scope->hasVariableType('this')->yes()) { + return []; + } + + $thisType = $scope->getVariableType('this'); + + if (! new ObjectType(TestCase::class)->isSuperTypeOf($thisType)->yes()) { + return []; + } + + $reflection = $this->memberReflection($node, $node->name->toString(), $thisType, $scope); + + if (! $reflection instanceof ExtendedMethodReflection && ! $reflection instanceof ClassConstantReflection) { + return []; + } + + if (! $reflection->isPrivate()) { + return []; + } + + $declaringClass = $reflection->getDeclaringClass(); + + // @note: PHPStan lets the member through only when its declaring class is a bind scope class, so anything outside that set is reported by PHPStan already. + if (! in_array($declaringClass->getName(), $thisType->getObjectClassNames(), true)) { + return []; + } + + $builder = match (true) { + $node instanceof ClassConstFetch => RuleErrorBuilder::message(sprintf( + 'Access to private constant %s of class %s.', + $reflection->getName(), + $declaringClass->getDisplayName(), + ))->identifier('classConstant.private'), + $node instanceof StaticCall => RuleErrorBuilder::message(sprintf( + 'Call to private static method %s() of class %s.', + $reflection->getName(), + $declaringClass->getDisplayName(), + ))->identifier('staticMethod.private'), + default => RuleErrorBuilder::message(sprintf( + 'Call to private method %s() of class %s.', + $reflection->getName(), + $declaringClass->getDisplayName(), + ))->identifier('method.private'), + }; + + return [$builder->line($node->getStartLine())->build()]; + } + + private function memberReflection( + MethodCall|StaticCall|ClassConstFetch $node, + string $member, + Type $thisType, + Scope $scope, + ): ExtendedMethodReflection|ClassConstantReflection|null { + if ($node instanceof ClassConstFetch) { + return $thisType->hasConstant($member)->yes() + ? $thisType->getConstant($member) + : null; + } + + return $thisType->hasMethod($member)->yes() + ? $thisType->getMethod($member, $scope) + : null; + } +} diff --git a/tests/Fixtures/CustomTestCaseInference/PrivateMembers/closure-inside-class.php b/tests/Fixtures/CustomTestCaseInference/PrivateMembers/closure-inside-class.php new file mode 100644 index 0000000..3e2e4aa --- /dev/null +++ b/tests/Fixtures/CustomTestCaseInference/PrivateMembers/closure-inside-class.php @@ -0,0 +1,28 @@ +ownPrivate().$this::OWN_PRIVATE_CONSTANT; + }; + } + + private function ownPrivate(): string + { + return 'own private'; + } +} diff --git a/tests/Fixtures/CustomTestCaseInference/PrivateMembers/private-members-bound-trait.php b/tests/Fixtures/CustomTestCaseInference/PrivateMembers/private-members-bound-trait.php new file mode 100644 index 0000000..b78e017 --- /dev/null +++ b/tests/Fixtures/CustomTestCaseInference/PrivateMembers/private-members-bound-trait.php @@ -0,0 +1,12 @@ +privateTraitHelper(); +}); diff --git a/tests/Fixtures/CustomTestCaseInference/PrivateMembers/private-members-default-testcase.php b/tests/Fixtures/CustomTestCaseInference/PrivateMembers/private-members-default-testcase.php new file mode 100644 index 0000000..38d46da --- /dev/null +++ b/tests/Fixtures/CustomTestCaseInference/PrivateMembers/private-members-default-testcase.php @@ -0,0 +1,12 @@ +runTest(); +}); + +it('leaves protected and public members of the default test case alone', function (): void { + $this->getActualOutputForAssertion(); + $this->getStatus(); +}); diff --git a/tests/Fixtures/CustomTestCaseInference/PrivateMembers/private-members-errors.php b/tests/Fixtures/CustomTestCaseInference/PrivateMembers/private-members-errors.php new file mode 100644 index 0000000..39dba56 --- /dev/null +++ b/tests/Fixtures/CustomTestCaseInference/PrivateMembers/private-members-errors.php @@ -0,0 +1,39 @@ +privateHelper(); + $this::privateStaticHelper(); + $x = $this::PRIVATE_CONSTANT; +}); + +it('reports private members reached through an arrow function', fn (): string => $this->privateHelper()); + +it('leaves private members of another object to phpstan', function (): void { + $other = new BoundTestCase; + + $other->privateHelper(); + $other::privateStaticHelper(); +}); + +it('leaves reachable members of the bound test case alone', function (): void { + $this->protectedHelper(); + $this->publicHelper(); +}); + +it('leaves private members inherited from a parent class to phpstan', function (): void { + $this->runTest(); +}); + +it('survives a dynamic member name on this', function (): void { + $method = 'privateHelper'; + $constant = 'PRIVATE_CONSTANT'; + + $this->$method(); + $x = $this::${$constant}; +}); diff --git a/tests/Rules/BoundTestCasePrivateMemberRuleTest.php b/tests/Rules/BoundTestCasePrivateMemberRuleTest.php new file mode 100644 index 0000000..9f1c360 --- /dev/null +++ b/tests/Rules/BoundTestCasePrivateMemberRuleTest.php @@ -0,0 +1,46 @@ +analyse([ + __DIR__.'/../Fixtures/CustomTestCaseInference/PrivateMembers/private-members-errors.php', + ], [ + ['Call to private method privateHelper() of class Tests\Type\Fixtures\PrivateMembers\BoundTestCase.', 10], + ['Call to private static method privateStaticHelper() of class Tests\Type\Fixtures\PrivateMembers\BoundTestCase.', 11], + ['Access to private constant PRIVATE_CONSTANT of class Tests\Type\Fixtures\PrivateMembers\BoundTestCase.', 12], + ['Call to private method privateHelper() of class Tests\Type\Fixtures\PrivateMembers\BoundTestCase.', 15], + ]); +}); + +test('private members of a trait bound through uses() are left alone', function (): void { + $this->analyse([ + __DIR__.'/../Fixtures/CustomTestCaseInference/PrivateMembers/private-members-bound-trait.php', + ], []); +}); + +test('private members of the default test case are reported', function (): void { + $this->analyse([ + __DIR__.'/../Fixtures/CustomTestCaseInference/PrivateMembers/private-members-default-testcase.php', + ], [ + ['Call to private method runTest() of class PHPUnit\Framework\TestCase.', 6], + ]); +}); + +test('a closure declared inside a class keeps its own scope', function (): void { + $this->analyse([ + __DIR__.'/../Fixtures/CustomTestCaseInference/PrivateMembers/closure-inside-class.php', + ], []); +}); diff --git a/tests/Type/Fixtures/PrivateMembers/BoundTestCase.php b/tests/Type/Fixtures/PrivateMembers/BoundTestCase.php new file mode 100644 index 0000000..1f5d0a7 --- /dev/null +++ b/tests/Type/Fixtures/PrivateMembers/BoundTestCase.php @@ -0,0 +1,33 @@ +