Skip to content

feat: type a bare test() call as the bound test case - #22

Open
ragulka wants to merge 3 commits into
pestphp:5.xfrom
ragulka:feat/type-bare-test-call
Open

ragulka wants to merge 3 commits into
pestphp:5.xfrom
ragulka:feat/type-bare-test-call

Conversation

@ragulka

@ragulka ragulka commented Oct 5, 2026

Copy link
Copy Markdown

The problem

$this in a test closure is typed as the bound test case. A bare test() call is not. Many suites call test()-> instead of $this->, so those calls get no checking:

uses(Tests\TestCase::class);

it('mocks a service', function (): void {
    $this->mokc(Service::class);   // error: Call to an undefined method Tests\TestCase::mokc()
    test()->mokc(Service::class);  // no error: type is TestCall|HigherOrderTapProxy
});

Projects hide the noise with a blanket ignore of Call to an undefined method Pest\PendingCalls\TestCall|Pest\Support\HigherOrderTapProxy::…. That ignore also hides real typos. Properties set through test()->name = … in a beforeEach stay mixed.

The change

Two commits.

  1. TestFunctionReturnTypeExtension types a bare test() call as the bound test case. It is the same type $this gets in test closures. It uses a new PestTestCaseType::resolveIfBound(). resolve() is unchanged.
    • A file with no uses() or pest() binding keeps the declared return type of test(). Helper files outside a binding report nothing new.
    • test('description') is unchanged.
  2. PestHookPropertyReader accepts a bare test() as the assignment receiver, as well as $this. test()->name = … in a beforeEach now types the property. Reads work through the existing TestCaseDynamicPropertyTypeExtension.
beforeEach(function (): void {
    test()->name = 'Ada';
});

it('reads it', function (): void {
    expect(test()->name)->toBeString(); // before: mixed
});

Tests

  • tests/Fixtures/CustomTestCaseInference/Feature/test-function-custom-testcase.php: test() in a bound file, including hook properties.
  • tests/Type/data/test-function.php: test() in an unbound file keeps its declared type.
  • tests/Type/data/test-hook-properties.php: hook properties assigned through test().

Each new bound test fails when the matching change is disabled.

Checks

  • pest: 529 passed (520 before)
  • phpstan analyse at level max: 0 errors
  • rector --dry-run: 0 changes
  • pint --test: pass

The suite needed php -d memory_limit=2G vendor/bin/pest, run serially. At the default 128M it runs out of memory on a clean checkout too.

🤖 Generated with Claude Code

ragulka and others added 2 commits October 5, 2026 15:39
test() without arguments returns a proxy to the running test case, but
PHPStan only saw the declared HigherOrderTapProxy|TestCall, so method
calls through it were unchecked. Resolve it the same way $this is
resolved in test closures. Files no binding covers keep the declared
type, so helper files outside a Pest.php binding report nothing new.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The hook property reader only collected `$this->name = …` assignments,
so a hook written as `test()->name = …` left the property mixed. A bare
test() proxies the running test case, so treat it as the same
receiver. Reads through test() resolve via the existing property
extension now that test() is typed as the test case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
}

// @note: a bare test() proxies the running test case, so it gets the same type as $this in a test closure; an unbound file keeps the declared type.
return $this->pestTestCaseType->resolveIfBound($scope->getFile());

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.

Should this be limited to test and hook closures?

Right now any bare test() call in a bound file gets the test case type, including calls inside normal helper functions. The new testBareTestCallInHelperFunction test makes that behavior explicit.

A file binding tells us which test case Pest will use, but it does not mean test() is always running in a test context. This seems broader than $this inference, which only exists inside the bound closures.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah, good catch! Indeed, $this behaves a bit different, and I agree test() should follow it.

At runtime, a bare test() returns the test case only while a test is running. Inside a helper function, the type depends on the caller, so we can't know it.

Changed in 9ac88da.

Side note: $this (and also a bare test()) is typed as the test case inside a describe() body too, although that body runs before any test does (so it can't be the test case). It was like that before this PR and likely doesn't matter in practice, so I left it alone, but I'm happy to fix it if you want.

A bare test() returns the running test case only while a test runs.
In a helper function the type depends on the caller, so it now keeps
the declared type. Inside test, hook and describe closures, and in
closures nested in them, test() gets the same type as $this.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ragulka
ragulka requested a review from MrPunyapal October 7, 2026 07:12

@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.

test() doesn't branch on $this, it branches on TestSuite::getInstance()->test instanceof TestCase. So the hasVariableType('this') guard is a proxy for that and the two come apart. In a static closure there's no $this, but the test is still running, so test() is the proxy and we bail. Nothing errors, the declared union still has HigherOrderTapProxy, so the suite stays green, but test()->createHelper() on a custom test case stays unchecked.

Not sure there's a fix. A static closure has no $this to read, so there's nothing to key off. Fine to leave it open, but testBareTestCallInStaticClosureKeepsDeclaredType asserting it as correct should probably go or be inverted.

isFunctionSupported matches any function named test. Pest declares it behind if (! function_exists('test')), so a project that defines its own global test() first never gets Pest's, and this retypes it. FunctionReflection::getFileName() against pestphp/pest/src/Functions.php would be safer.

resolveIfBound() is only used as a null check, the type gets thrown away since we return $scope->getVariableType('this'). Fine today, but isBound() reads better than instanceof Type.

Two things in the description: pest is 531 passed on 9ac88da, not 529. CI hasn't run, all four workflow runs are action_required.

@odolbeau

Copy link
Copy Markdown

Thanks for this! One case it leaves out, which is where we hit the error in practice: a bare test() inside a file-level helper.

function sendWebhook(array $content): TestResponse
{
    return test()->postRawContent('/webhook', ...); // still TestCall|HigherOrderTapProxy
}

There's no $this there, so the guard bails. But in a bound test file, a helper like this only runs while a test is running. In practice the proxy is the only outcome. Could the bound test case type apply there too?

Also, once test() is typed as the test case, calls to protected methods will probably report method.protected, for example test()->assertDatabaseHas() or test()->mock(). TestCaseMemberVisibilityIgnoreExtension only accepts $this as the receiver, while at runtime the proxy reaches them through HigherOrderMessage. It may be worth accepting a bare test() receiver there as well.

@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.

Reviewed at 9ac88da.

The change does what it says inside test closures and I would like to see it land, but not in this state. Four things.

1. CI has not run

All four workflow runs on this branch are action_required, so nothing here has been through CI yet. Approve them and get it green before a merge.

2. A bare test() in a file-level helper keeps the old type

That is the case odolbeau raised above. A helper in a bound test file still gets TestCall|HigherOrderTapProxy, so test()->postRawContent(...) is still unresolved. The $this check is what bails, and a helper that is called during a test does get the proxy at run time, so typing it as the bound test case is the better default. A helper called at collection time would then be typed wrong, but that is the rarer case.

If you drop the $this check, TestCaseMemberVisibilityIgnoreExtension has to change in the same commit. It returns early unless the receiver is a $this variable (line 40) and unless the scope is inside an anonymous function (line 44). Neither holds in a helper, so test()->mock() and test()->createMock() would start reporting method.protected. The private member branch also reads $this from the scope (line 52), which a helper does not have. This only needs widening for helpers: inside a test closure it does not come up, because the closure scope already counts as being inside the test case and protected access is granted whatever the receiver is.

3. test()->todo() becomes a false positive

I ran the pending call methods on a bare test() inside a running test. test()->with(), test()->skip() and test()->group() all throw Call to undefined method, so reporting those is correct. test()->todo('...') returns a TestCall and works, and the new type reports it as undefined. Small, but it works today and nothing covers it.

4. Minor

resolveIfBound($scope->getFile()) instanceof Type is only a null check as written, since the value is thrown away and $thisType is returned. !== null would read better, or return the resolved type for the file and drop the $this check.

The isFunctionSupported point from the earlier review still stands.

Tests for a bound file helper and for one static closure case would be worth having here, neither is covered at the moment.

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.

3 participants