Skip to content

Use self:: over $this-> for PHPUnit assertions#4654

Closed
staabm wants to merge 2 commits intophpstan:2.1.xfrom
staabm:this-self
Closed

Use self:: over $this-> for PHPUnit assertions#4654
staabm wants to merge 2 commits intophpstan:2.1.xfrom
staabm:this-self

Conversation

@staabm
Copy link
Contributor

@staabm staabm commented Dec 23, 2025 •
edited
Loading

with static calls assertion-calls do not affect remembered non-static properties.

in addition assertion methods are declared static so we don't need to process ignored errors for these lines

before PR

➜  phpstan-src git:(2.1.x) ✗ hyperfine 'bin/phpstan analyze tests/PHPStan/Analyser/AnalyserIntegrationTest.php' --runs=5
Benchmark 1: bin/phpstan analyze tests/PHPStan/Analyser/AnalyserIntegrationTest.php
  Time (mean ± σ):      2.537 s ±  0.047 s    [User: 2.224 s, System: 0.279 s]
  Range (min … max):    2.508 s …  2.620 s    5 runs

after PR

➜  phpstan-src git:(this-self) ✗ hyperfine 'bin/phpstan analyze tests/PHPStan/Analyser/AnalyserIntegrationTest.php' --runs=5
Benchmark 1: bin/phpstan analyze tests/PHPStan/Analyser/AnalyserIntegrationTest.php
  Time (mean ± σ):      2.526 s ±  0.016 s    [User: 2.220 s, System: 0.281 s]
  Range (min … max):    2.513 s …  2.554 s    5 runs

@staabm
Use self:: over $this-> for PHPUnit assertions
17027e6 
static calls can be analyzed faster by PHPStan (less scope managment required)
@staabm
cs
95fe618
@ondrejmirtes
Copy link
Member

ondrejmirtes commented Dec 23, 2025

I kind of disagree with this one. PHPUnit documentation allows both, and because of years of muscle memory I still use $this-> myself.

If anything we should have ignoreErrors entry with reportUnmatched: false in phpstan-phpunit so that people don't have to ignore this on their own. Or an IgnoreErrorsExtension.

@talkinnl
Copy link

talkinnl commented Dec 23, 2025

I kind of disagree with this one. PHPUnit documentation allows both, and because of years of muscle memory I still use $this-> myself.

Of course, this topic is subjective, and it may be a clash of project vs library code style... but I think this PR helps aligning with the more common test code style in the community, so less friction or surprising. :)

@staabm staabm closed this Dec 23, 2025
@staabm staabm deleted the this-self branch December 23, 2025 13:09
@staabm staabm mentioned this pull request Dec 23, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@staabm @ondrejmirtes @talkinnl