Skip to content

fix: allow private and static trait methods on $this in test closures - #19

Merged
MrPunyapal merged 2 commits into
pestphp:5.xfrom
bpmore:fix/bound-trait-private-and-static-visibility
Oct 2, 2026
Merged

MrPunyapal merged 2 commits into
pestphp:5.xfrom
bpmore:fix/bound-trait-private-and-static-visibility

Conversation

@bpmore

@bpmore bpmore commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes pestphp/pest#1945.

The problem

Pest flattens a trait bound through uses() onto the generated test case, so a test closure can reach every member of that trait. #9 and #10 made PestTestCaseWithTraitsType resolve those members, which fixed the method.notFound half (pestphp/pest#1815). These forms still fail analysis but work at run time:

uses(CustomTestCase::class, VisibilityTrait::class);

it('uses the bound trait', function (): void {
    $this->protectedTraitMethod();        // passes: the old ProtectedMethodCallIgnoreExtension
    $this->privateTraitMethod();          // method.private
    $this::protectedStaticTraitMethod();  // staticMethod.protected
    $this::privateStaticTraitMethod();    // staticMethod.private
    $this::PROTECTED_TRAIT_CONSTANT;      // classConstant.protected
    $this::PRIVATE_TRAIT_CONSTANT;        // classConstant.private
});

PestTestCaseWithTraitsType returns the trait's own reflection, so each member is checked against the trait's declared visibility. The old ignore extension accepted only method.protected on a MethodCall.

The change

This extends the ignore extension rather than the composite type, the first of the two shapes in pestphp/pest#1945. It is the mechanism the plugin already uses for protected test-case methods. Reporting different visibility from PestTestCaseWithTraitsType would mean wrapping ExtendedMethodReflection and ClassConstantReflection, a much larger change.

ProtectedMethodCallIgnoreExtension is renamed TestCaseMemberVisibilityIgnoreExtension. It now handles:

  • method.protected and method.private on $this->foo();
  • staticMethod.protected and staticMethod.private on $this::foo();
  • classConstant.protected and classConstant.private on $this::NAME.

The existing guards are unchanged: inside an anonymous function, $this defined, and $this a PHPUnit\Framework\TestCase.

  • Protected forms are accepted as before, since the closure is bound to a subclass of the test case.
  • Private forms are accepted only when the member's declaring class is one of the traits uses() binds, read from the new PestTestCaseWithTraitsType::getTraitNames(). Only those traits are flattened into the generated class. A private member inherited from a parent class, such as PHPUnit\Framework\TestCase::runTest(), or one reached on any object other than $this, is still reported.

Tests

tests/Rules/TraitMemberVisibilityTest.php uses the existing custom-testcase-extension.neon setup and has 6 tests:

  • trait-visibility-calls.php binds a test case and a trait with uses() and uses every form above from it() and beforeEach() closures. CallMethodsRule, CallStaticMethodsRule and ClassConstantRule each expect no errors.
  • trait-visibility-errors.php covers what must still be reported: $this->runTest(), and private trait methods and a private trait constant reached through a different object (VisibilityTraitUser).

Each change below was made to the extension by hand and the test suite rerun:

change made to the extension result
none 6 of 6 pass
private always accepted the runTest() test fails
trait-list check replaced by "any declaring class" the runTest() test fails
constant identifiers removed the constants test fails
static identifiers removed the static methods test fails
$this receiver check removed all three "still reported" tests fail
protected branch removed still passes

The last row survives because every protected member that can raise an error here belongs to a bound trait, which the trait-list check accepts anyway. The branch keeps the extension's previous method.protected behavior, so nothing reported today starts being reported.

Checks

All green on this branch, on macOS:

  • pest: 520 passed, 636 assertions
  • phpstan analyse: 0 errors
  • rector --dry-run: 0 changed files
  • pint --test: passed

In a real suite

This was found in a suite whose Feature tests bind a support trait with 56 private and 23 protected methods, 18 of them static. That suite currently works around the problem by adding the trait to a static-analysis stub of the base TestCase. Removing that stub entry and using this branch's extension (on v5.2.0, the release that suite installs) takes the suite from 190 method.private, staticMethod.protected and staticMethod.private errors at level: max to 0. The only remaining error is trait.unused, which is expected because the trait is no longer used inside a class.

Generated with Claude Code.

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) <noreply@anthropic.com>

@MrPunyapal MrPunyapal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the write-up, this is close. One thing blocks it.

isTrait() is broader than the reason given for it

The reasoning is right: a private member is only flattened onto the generated class when a trait is bound through uses(), and a private method inherited from a parent class is not reachable at run time. The runTest() case is covered because PHPUnit\Framework\TestCase is not a trait.

getDeclaringClass()->isTrait() does not make that distinction. It is also true when the trait is used by the bound test case class, where the private method is declared on that parent rather than on the generated class:

class TraitUsingTestCase extends TestCase
{
    use VisibilityTrait;
}
uses(TraitUsingTestCase::class);

it('...', function (): void {
    $this->privateTraitMethod();
});

Pest binds the closure to the generated class (Testable.php:508, $this::class) and PHP private access is scope based, so the child scope cannot reach the parent's private member. I ran this against the branch:

Call to private method Tests\Type\Fixtures\TraitUsingTestCase::privateTraitMethod() from scope P\C\...\ProbeTest

CallMethodsRule reports nothing for the same file.

Comparing against the binding list fixes it

The flattened traits are already resolved, PestTestCaseType::resolve() puts them in PestTestCaseWithTraitsType::$traitNames. Checking the declaring class against that list keeps every case this PR is about and rejects the one above:

  • uses(CustomTestCase::class, VisibilityTrait::class) gives a PestTestCaseWithTraitsType with VisibilityTrait in the list, ignored
  • uses(TraitUsingTestCase::class) gives a plain ObjectType with no traits bound, reported
  • $this->runTest() has TestCase as the declaring class, reported

pest()->extend(...)->use(Trait::class)->in(...) lands in the same list, so that keeps working too.

A test next to the existing runTest() one for this case would close it.

Two smaller notes

ProtectedMethodCallIgnoreExtension now handles method.private, staticMethod.protected and staticMethod.private. Worth renaming the class and the file before it lands.

Private trait constants are the same flattening story and are still reported. With uses(CustomTestCase::class, StateTrait::class) and a private const NAME in the trait, $this::NAME reports classConstant.private and works at run time. Not a reason to hold this, but pest#1945 is not closed until it is.

Checks

518 passed, phpstan 0 errors, rector 0 changed files. Pint fails on my machine on 5.x too, same files, CRLF checkout, so I did not count it.

Comment thread src/Type/Pest/ProtectedMethodCallIgnoreExtension.php Outdated
Comment thread src/Type/Pest/ProtectedMethodCallIgnoreExtension.php Outdated
Comment thread src/Type/Pest/ProtectedMethodCallIgnoreExtension.php Outdated
Comment thread tests/Rules/TraitMemberVisibilityTest.php
Address review on pestphp#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) <noreply@anthropic.com>
@bpmore

bpmore commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review. All three points are addressed in 98335bf, and the PR description is updated to match.

Trait-list check. The private forms now compare the declaring class against PestTestCaseWithTraitsType::getTraitNames(), through a new getter, instead of asking isTrait(). The comment above the check now names the same condition.

Rename. The class is now TestCaseMemberVisibilityIgnoreExtension, and its file and extension.neon are updated. The test file is now TraitMemberVisibilityTest.php.

Private trait constants. classConstant.protected and classConstant.private on $this::NAME now go through the same checks, so Fixes pestphp/pest#1945 holds. There are tests for a bound trait's constants, which pass, and for a private trait constant read through another object, which is still reported.

On the uses(TraitUsingTestCase::class) case: no test was added, because the analyser does not reach it with or without this PR. The runtime error is real, as described. But CallMethodsRule reports nothing for that file on unchanged 5.x at e0b9fce, with the original ProtectedMethodCallIgnoreExtension in place. Here is the same probe file, with control lines in the same closure:

uses(TraitUsingTestCase::class);

it('probe', function (): void {
    $this->privateTraitMethod();        // not reported, on 5.x and on this branch
    $this::privateStaticTraitMethod();  // not reported, on 5.x and on this branch
    $this->doesNotExist();              // method.notFound, so the closure is analysed
    $this->runTest();                   // method.private, so visibility is checked in this closure
});

\PHPStan\dumpType($this) in that closure gives Tests\Type\Fixtures\TraitUsingTestCase. So the analyser treats the bound class's own private members, including those it gets from a trait it uses, as reachable from the closure. No error is raised that this extension could suppress or let through, so a test asserting it is reported fails on 5.x as well.

The trait-list check is still the right condition, and it is what the branch uses now. But closing that case needs the closure to be analysed as the generated subclass rather than the bound class. That would equally affect a private method declared directly on the bound test case, which is likewise not reported today. It seemed outside this PR. Happy to open a separate issue for it on pestphp/pest if that is useful.

Checks on this branch (macOS): pest 520 passed, phpstan 0 errors, rector --dry-run 0 changed files, pint --test passed. The PR description has the table of hand mutations. Of the six changes made to the extension, five fail a test. The one that survives, removing the protected branch, is explained there.

@MrPunyapal
MrPunyapal merged commit 19afd92 into pestphp:5.x Oct 2, 2026
10 checks passed
@MrPunyapal

Copy link
Copy Markdown
Member

thanks

@MrPunyapal

Copy link
Copy Markdown
Member

My bad, the blocking point in my review was wrong. My fixture was malformed, PHPStan could not reflect the test case class, so the binding was dropped and $this fell back to TestCase. Nothing was being suppressed. Your 5.x control was the right check, I confirmed the output is identical on e0b9fce, ac77fbe and 98335bf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: pest-plugin-phpstan reports private and static members of a uses()-bound trait as inaccessible in test closures

2 participants