From ac77fbe13d4ab196d5b57ed03f390951f4af2370 Mon Sep 17 00:00:00 2001 From: Brent Passmore Date: Fri, 2 Oct 2026 07:34:38 -0500 Subject: [PATCH 1/2] fix: allow private and static trait methods on $this in test closures Pest flattens a trait bound through uses() onto the generated test case, so a test closure can call the trait's private methods and its protected or private static methods. ProtectedMethodCallIgnoreExtension only covered method.protected on a $this->foo() call, so method.private, staticMethod.protected and staticMethod.private were still reported. The extension now also covers $this::foo() static calls. Private methods are allowed only when they are declared by a trait, so a private method inherited from a parent class or called on another object is still reported. Fixes pestphp/pest#1945 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ProtectedMethodCallIgnoreExtension.php | 46 +++++++++++++--- .../trait-visibility-calls.php | 23 ++++++++ .../trait-visibility-errors.php | 20 +++++++ tests/Rules/TraitVisibilityMethodCallTest.php | 52 +++++++++++++++++++ tests/Type/Fixtures/VisibilityTrait.php | 28 ++++++++++ tests/Type/Fixtures/VisibilityTraitUser.php | 10 ++++ 6 files changed, 171 insertions(+), 8 deletions(-) create mode 100644 tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-calls.php create mode 100644 tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-errors.php create mode 100644 tests/Rules/TraitVisibilityMethodCallTest.php create mode 100644 tests/Type/Fixtures/VisibilityTrait.php create mode 100644 tests/Type/Fixtures/VisibilityTraitUser.php diff --git a/src/Type/Pest/ProtectedMethodCallIgnoreExtension.php b/src/Type/Pest/ProtectedMethodCallIgnoreExtension.php index e108a43..79e25d2 100644 --- a/src/Type/Pest/ProtectedMethodCallIgnoreExtension.php +++ b/src/Type/Pest/ProtectedMethodCallIgnoreExtension.php @@ -6,18 +6,33 @@ use PhpParser\Node; use PhpParser\Node\Expr\MethodCall; +use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Expr\Variable; +use PhpParser\Node\Identifier; use PHPStan\Analyser\Error; use PHPStan\Analyser\IgnoreErrorExtension; use PHPStan\Analyser\Scope; use PHPStan\Type\ObjectType; +use PHPStan\Type\Type; use PHPUnit\Framework\TestCase; final class ProtectedMethodCallIgnoreExtension implements IgnoreErrorExtension { + private const array INSTANCE_IDENTIFIERS = ['method.protected', 'method.private']; + + private const array STATIC_IDENTIFIERS = ['staticMethod.protected', 'staticMethod.private']; + public function shouldIgnore(Error $error, Node $node, Scope $scope): bool { - if ($error->getIdentifier() !== 'method.protected') { + $identifier = $error->getIdentifier(); + + $receiver = match (true) { + $node instanceof MethodCall && in_array($identifier, self::INSTANCE_IDENTIFIERS, true) => $node->var, + $node instanceof StaticCall && in_array($identifier, self::STATIC_IDENTIFIERS, true) => $node->class, + default => null, + }; + + if (! $receiver instanceof Variable || $receiver->name !== 'this') { return false; } @@ -25,22 +40,37 @@ public function shouldIgnore(Error $error, Node $node, Scope $scope): bool return false; } - if (! $node instanceof MethodCall) { + if (! $scope->hasVariableType('this')->yes()) { return false; } - if (! $node->var instanceof Variable || $node->var->name !== 'this') { + $thisType = $scope->getVariableType('this'); + + if (! new ObjectType(TestCase::class)->isSuperTypeOf($thisType)->yes()) { return false; } - if (! $scope->hasVariableType('this')->yes()) { + // @note: the closure is bound to a subclass of the test case, so its protected members are reachable. + if ($identifier === 'method.protected' || $identifier === 'staticMethod.protected') { + return true; + } + + // @note: a private member is reachable only when it comes from a trait Pest flattens onto the test case. + return $this->isBoundTraitMethod($node, $thisType, $scope); + } + + private function isBoundTraitMethod(MethodCall|StaticCall $node, Type $thisType, Scope $scope): bool + { + if (! $node->name instanceof Identifier) { return false; } - $thisType = $scope->getVariableType('this'); + $methodName = $node->name->toString(); + + if (! $thisType->hasMethod($methodName)->yes()) { + return false; + } - return new ObjectType(TestCase::class) - ->isSuperTypeOf($thisType) - ->yes(); + return $thisType->getMethod($methodName, $scope)->getDeclaringClass()->isTrait(); } } 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..7346378 --- /dev/null +++ b/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-calls.php @@ -0,0 +1,23 @@ +protectedTraitMethod(); + $this->privateTraitMethod(); +}); + +it('can call protected and private static trait methods through $this', function (): void { + $this::protectedStaticTraitMethod(); + $this::privateStaticTraitMethod(); +}); + +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..8e2bf19 --- /dev/null +++ b/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-errors.php @@ -0,0 +1,20 @@ +runTest(); +}); + +it('still reports private trait methods called on an object other than $this', function (): void { + $other = new VisibilityTraitUser; + + $other->privateTraitMethod(); + $other::privateStaticTraitMethod(); +}); diff --git a/tests/Rules/TraitVisibilityMethodCallTest.php b/tests/Rules/TraitVisibilityMethodCallTest.php new file mode 100644 index 0000000..a5f8b75 --- /dev/null +++ b/tests/Rules/TraitVisibilityMethodCallTest.php @@ -0,0 +1,52 @@ +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], + ]); +}); diff --git a/tests/Type/Fixtures/VisibilityTrait.php b/tests/Type/Fixtures/VisibilityTrait.php new file mode 100644 index 0000000..1a39908 --- /dev/null +++ b/tests/Type/Fixtures/VisibilityTrait.php @@ -0,0 +1,28 @@ + Date: Fri, 2 Oct 2026 09:25:17 -0500 Subject: [PATCH 2/2] fix: limit private access to traits bound by uses() and cover constants Address review on #19: - Compare a private member's declaring class against the traits uses() binds (PestTestCaseWithTraitsType::getTraitNames()) instead of asking isTrait(), so only the traits Pest flattens onto the generated class are allowed. - Cover classConstant.protected and classConstant.private on $this::NAME, which the same flattening makes reachable at run time. - Rename ProtectedMethodCallIgnoreExtension to TestCaseMemberVisibilityIgnoreExtension, since it now covers private methods, static calls and constants. Co-Authored-By: Claude Opus 5.5 (1M context) --- extension.neon | 2 +- src/Type/Pest/PestTestCaseWithTraitsType.php | 6 ++ .../ProtectedMethodCallIgnoreExtension.php | 76 --------------- ...estCaseMemberVisibilityIgnoreExtension.php | 93 +++++++++++++++++++ .../trait-visibility-calls.php | 5 + .../trait-visibility-errors.php | 1 + ...Test.php => TraitMemberVisibilityTest.php} | 19 ++++ tests/Type/Fixtures/VisibilityTrait.php | 4 + 8 files changed, 129 insertions(+), 77 deletions(-) delete mode 100644 src/Type/Pest/ProtectedMethodCallIgnoreExtension.php create mode 100644 src/Type/Pest/TestCaseMemberVisibilityIgnoreExtension.php rename tests/Rules/{TraitVisibilityMethodCallTest.php => TraitMemberVisibilityTest.php} (70%) 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 79e25d2..0000000 --- a/src/Type/Pest/ProtectedMethodCallIgnoreExtension.php +++ /dev/null @@ -1,76 +0,0 @@ -getIdentifier(); - - $receiver = match (true) { - $node instanceof MethodCall && in_array($identifier, self::INSTANCE_IDENTIFIERS, true) => $node->var, - $node instanceof StaticCall && in_array($identifier, self::STATIC_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 ($identifier === 'method.protected' || $identifier === 'staticMethod.protected') { - return true; - } - - // @note: a private member is reachable only when it comes from a trait Pest flattens onto the test case. - return $this->isBoundTraitMethod($node, $thisType, $scope); - } - - private function isBoundTraitMethod(MethodCall|StaticCall $node, Type $thisType, Scope $scope): bool - { - if (! $node->name instanceof Identifier) { - return false; - } - - $methodName = $node->name->toString(); - - if (! $thisType->hasMethod($methodName)->yes()) { - return false; - } - - return $thisType->getMethod($methodName, $scope)->getDeclaringClass()->isTrait(); - } -} 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 index 7346378..167c003 100644 --- a/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-calls.php +++ b/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-calls.php @@ -17,6 +17,11 @@ $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 index 8e2bf19..9517a92 100644 --- a/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-errors.php +++ b/tests/Fixtures/CustomTestCaseInference/TraitVisibility/trait-visibility-errors.php @@ -17,4 +17,5 @@ $other->privateTraitMethod(); $other::privateStaticTraitMethod(); + expect($other::PRIVATE_TRAIT_CONSTANT)->toBe('private'); }); diff --git a/tests/Rules/TraitVisibilityMethodCallTest.php b/tests/Rules/TraitMemberVisibilityTest.php similarity index 70% rename from tests/Rules/TraitVisibilityMethodCallTest.php rename to tests/Rules/TraitMemberVisibilityTest.php index a5f8b75..08b2ecc 100644 --- a/tests/Rules/TraitVisibilityMethodCallTest.php +++ b/tests/Rules/TraitMemberVisibilityTest.php @@ -4,6 +4,7 @@ namespace Tests\Rules; +use PHPStan\Rules\Classes\ClassConstantRule; use PHPStan\Rules\Methods\CallMethodsRule; use PHPStan\Rules\Methods\CallStaticMethodsRule; use Tests\RuleTestCase; @@ -50,3 +51,21 @@ ['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 index 1a39908..1d15791 100644 --- a/tests/Type/Fixtures/VisibilityTrait.php +++ b/tests/Type/Fixtures/VisibilityTrait.php @@ -6,6 +6,10 @@ trait VisibilityTrait { + protected const string PROTECTED_TRAIT_CONSTANT = 'protected'; + + private const string PRIVATE_TRAIT_CONSTANT = 'private'; + protected static function protectedStaticTraitMethod(): string { return 'protected static';