diff --git a/extension.neon b/extension.neon index 98a3ffa..ac7f226 100644 --- a/extension.neon +++ b/extension.neon @@ -100,7 +100,7 @@ services: - phpstan.broker.methodsClassReflectionExtension - - class: Pest\PHPStan\Type\Pest\ProtectedMethodCallIgnoreExtension + class: Pest\PHPStan\Type\Pest\TestCaseMemberVisibilityIgnoreExtension tags: - phpstan.ignoreErrorExtension diff --git a/src/Type/Pest/PestTestCaseWithTraitsType.php b/src/Type/Pest/PestTestCaseWithTraitsType.php index 22059fe..0ccee26 100644 --- a/src/Type/Pest/PestTestCaseWithTraitsType.php +++ b/src/Type/Pest/PestTestCaseWithTraitsType.php @@ -26,6 +26,12 @@ public function __construct( parent::__construct($className); } + /** @return list */ + public function getTraitNames(): array + { + return $this->traitNames; + } + #[Override] public function hasMethod(string $methodName): TrinaryLogic { diff --git a/src/Type/Pest/ProtectedMethodCallIgnoreExtension.php b/src/Type/Pest/ProtectedMethodCallIgnoreExtension.php deleted file mode 100644 index e108a43..0000000 --- a/src/Type/Pest/ProtectedMethodCallIgnoreExtension.php +++ /dev/null @@ -1,46 +0,0 @@ -getIdentifier() !== 'method.protected') { - return false; - } - - if (! $scope->isInAnonymousFunction()) { - return false; - } - - if (! $node instanceof MethodCall) { - return false; - } - - if (! $node->var instanceof Variable || $node->var->name !== 'this') { - return false; - } - - if (! $scope->hasVariableType('this')->yes()) { - return false; - } - - $thisType = $scope->getVariableType('this'); - - return new ObjectType(TestCase::class) - ->isSuperTypeOf($thisType) - ->yes(); - } -} diff --git a/src/Type/Pest/TestCaseMemberVisibilityIgnoreExtension.php b/src/Type/Pest/TestCaseMemberVisibilityIgnoreExtension.php new file mode 100644 index 0000000..f8606d4 --- /dev/null +++ b/src/Type/Pest/TestCaseMemberVisibilityIgnoreExtension.php @@ -0,0 +1,93 @@ +getIdentifier(); + + $receiver = match (true) { + $node instanceof MethodCall && in_array($identifier, self::METHOD_IDENTIFIERS, true) => $node->var, + $node instanceof StaticCall && in_array($identifier, self::STATIC_METHOD_IDENTIFIERS, true) => $node->class, + $node instanceof ClassConstFetch && in_array($identifier, self::CONSTANT_IDENTIFIERS, true) => $node->class, + default => null, + }; + + if (! $receiver instanceof Variable || $receiver->name !== 'this') { + return false; + } + + if (! $scope->isInAnonymousFunction()) { + return false; + } + + if (! $scope->hasVariableType('this')->yes()) { + return false; + } + + $thisType = $scope->getVariableType('this'); + + if (! new ObjectType(TestCase::class)->isSuperTypeOf($thisType)->yes()) { + return false; + } + + // @note: the closure is bound to a subclass of the test case, so its protected members are reachable. + if (str_ends_with($identifier, '.protected')) { + return true; + } + + // @note: a private member is reachable only when it is declared by a trait that uses() binds, because Pest flattens those traits onto the generated class. + $declaringClass = $this->declaringClassName($node, $thisType, $scope); + + return $declaringClass !== null + && $thisType instanceof PestTestCaseWithTraitsType + && in_array($declaringClass, $thisType->getTraitNames(), true); + } + + private function declaringClassName(Expr $node, Type $thisType, Scope $scope): ?string + { + if (! ($node instanceof MethodCall || $node instanceof StaticCall || $node instanceof ClassConstFetch)) { + return null; + } + + if (! $node->name instanceof Identifier) { + return null; + } + + $name = $node->name->toString(); + + if ($node instanceof ClassConstFetch) { + return $thisType->hasConstant($name)->yes() + ? $thisType->getConstant($name)->getDeclaringClass()->getName() + : null; + } + + return $thisType->hasMethod($name)->yes() + ? $thisType->getMethod($name, $scope)->getDeclaringClass()->getName() + : null; + } +} diff --git a/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-calls.php b/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-calls.php new file mode 100644 index 0000000..167c003 --- /dev/null +++ b/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-calls.php @@ -0,0 +1,28 @@ +protectedTraitMethod(); + $this->privateTraitMethod(); +}); + +it('can call protected and private static trait methods through $this', function (): void { + $this::protectedStaticTraitMethod(); + $this::privateStaticTraitMethod(); +}); + +it('can read protected and private trait constants through $this', function (): void { + expect($this::PROTECTED_TRAIT_CONSTANT)->toBe('protected'); + expect($this::PRIVATE_TRAIT_CONSTANT)->toBe('private'); +}); + +beforeEach(function (): void { + $this->privateTraitMethod(); + $this::privateStaticTraitMethod(); +}); diff --git a/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-errors.php b/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-errors.php new file mode 100644 index 0000000..9517a92 --- /dev/null +++ b/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-errors.php @@ -0,0 +1,21 @@ +runTest(); +}); + +it('still reports private trait methods called on an object other than $this', function (): void { + $other = new VisibilityTraitUser; + + $other->privateTraitMethod(); + $other::privateStaticTraitMethod(); + expect($other::PRIVATE_TRAIT_CONSTANT)->toBe('private'); +}); diff --git a/tests/Rules/TraitMemberVisibilityTest.php b/tests/Rules/TraitMemberVisibilityTest.php new file mode 100644 index 0000000..08b2ecc --- /dev/null +++ b/tests/Rules/TraitMemberVisibilityTest.php @@ -0,0 +1,71 @@ +analyse([ + __DIR__.'/../Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-calls.php', + ], []); +}); + +test('protected and private static trait methods are callable through $this in pest closures', function (): void { + RuleTestCase::$rule = RuleTestCase::resolveRule(CallStaticMethodsRule::class); + + $this->analyse([ + __DIR__.'/../Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-calls.php', + ], []); +}); + +test('private methods not flattened from a bound trait are still reported', function (): void { + RuleTestCase::$rule = RuleTestCase::resolveRule(CallMethodsRule::class); + + $this->analyse([ + __DIR__.'/../Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-errors.php', + ], [ + ['Call to private method runTest() of class PHPUnit\Framework\TestCase.', 12], + ['Call to private method privateTraitMethod() of class Tests\Type\Fixtures\VisibilityTraitUser.', 18], + ]); +}); + +test('private static trait methods called on another object are still reported', function (): void { + RuleTestCase::$rule = RuleTestCase::resolveRule(CallStaticMethodsRule::class); + + $this->analyse([ + __DIR__.'/../Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-errors.php', + ], [ + ['Call to private static method privateStaticTraitMethod() of class Tests\Type\Fixtures\VisibilityTraitUser.', 19], + ]); +}); + +test('protected and private trait constants are readable through $this in pest closures', function (): void { + RuleTestCase::$rule = RuleTestCase::resolveRule(ClassConstantRule::class); + + $this->analyse([ + __DIR__.'/../Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-calls.php', + ], []); +}); + +test('private trait constants read from another object are still reported', function (): void { + RuleTestCase::$rule = RuleTestCase::resolveRule(ClassConstantRule::class); + + $this->analyse([ + __DIR__.'/../Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-errors.php', + ], [ + ['Access to private constant PRIVATE_TRAIT_CONSTANT of class Tests\\Type\\Fixtures\\VisibilityTraitUser.', 20], + ]); +}); diff --git a/tests/Type/Fixtures/VisibilityTrait.php b/tests/Type/Fixtures/VisibilityTrait.php new file mode 100644 index 0000000..1d15791 --- /dev/null +++ b/tests/Type/Fixtures/VisibilityTrait.php @@ -0,0 +1,32 @@ +