Try phpstan-mutant-killer-infection-runner #1505
Conversation
|
Context: phpstan/phpstan#10966 (comment) |
|
@Ocramius Can you please check the reported mutants in the PHPStan run to see whether they're valuable and whether you expect them to be killed by static analysis? What I don't get is that the Psalm run reports: And PHPStan's one reports: And why Psalm's run succeeds, but PHPStan's run fails with these similar numbers. |
ondrejmirtes
force-pushed
the
phpstan-infection
branch
3 times, most recently
from
5c29dde to
93d3c26
Compare
|
A nice side-effect of me optimizing PHPStan - BetterReflection is now analysed in 9 seconds instead of 28 seconds on my machine! |
kukulich
force-pushed
the
phpstan-infection
branch
3 times, most recently
from
b96d9b3 to
b45f760
Compare
ondrejmirtes
force-pushed
the
phpstan-infection
branch
from
b45f760 to
425a6d8
Compare
kukulich
force-pushed
the
phpstan-infection
branch
from
425a6d8 to
a30c338
Compare
phpstan.neon
Outdated
| level: 6 | ||
| level: 8 |
There was a problem hiding this comment.
TIL: we were still at level 6 :O
There was a problem hiding this comment.
This also explains why PHPStan + Infection didn't catch many of the mutants before, BTW, since many mutations are still valid under level 6! 👍
| @@ -621,6 +622,7 @@ public function getHook(ReflectionPropertyHookType $hookType): ReflectionMethod| | |||
| /** @return array{get?: ReflectionMethod, set?: ReflectionMethod} */ | |||
| public function getHooks(): array | |||
| { | |||
| // @infection-ignore-all AssignCoalesce: It's just optimization | |||
There was a problem hiding this comment.
We can possibly disable the entire mutator? It will always catch optimizations
There was a problem hiding this comment.
(unless there's a way to declare these optimizations at type level, perhaps?)
kukulich
force-pushed
the
phpstan-infection
branch
from
a30c338 to
cf755c1
Compare
|
Before considering a merge here, beware that we'd first need upstream stability for the new dependency |
| @@ -5,7 +5,8 @@ | |||
| "phpstan/phpstan-phpunit": "^2.0.6", | |||
| "vimeo/psalm": "^6.11.0", | |||
| "roave/backward-compatibility-check": "^8.13.0", | |||
| "roave/infection-static-analysis-plugin": "^1.37.0" | |||
| "roave/infection-static-analysis-plugin": "^1.38.0", | |||
| "phpstan/mutant-killer-infection-runner": "1.0.x-dev" | |||
There was a problem hiding this comment.
Adding a note here: let's get a stable (even if 0.x) release out, before going with this
There was a problem hiding this comment.
I'm not probably going to release this package, there's a bug that causes PHPStan not be called for some mutants and I don't know why. I talked to Maks at PHPers over the weekend and the official support in Infection should land very soon.
We will continue working on this, mostly for performance improvements, but there's no reason to delay the feature in Infection itself.
There was a problem hiding this comment.
offtop:
I talked to Maks at PHPers over the weekend and the official support in Infection should land very soon.
confirm, I do my best on bringing it to Infection natively, I think by the end of the week or early next week we will have at least opened PR with draft implementation.
Unfortunately, to support static analysis, I have to prepare other things to allow proper parallelization of PHPStan processes, so some architecture changes are required on Infection side first, thus it's not quick.
There was a problem hiding this comment.
I think by the end of the week or early next week we will have at least opened PR with draft implementation.
Just to be clear: nobody is chasing anybody :-)
What matters most is that there's a plan 👍
|
just ran PHPStan based infection on this PR on my machine and looked at the process monitoring. 
My gut feeling is we maybe can speedup the overall process by not using a worker, but run the analysis in the main process per mutation to reduce process spawning overhead |
could you please explain what do you mean here? I was told by @ondrejmirtes that we only want to run PHPStan as a CLI tool, not as a PHP code calls like Psalm's plugin is done. So we have no other way than running it as another process, or did I miss something?
I'm not sure if we are on the same page on how it's currently working, but: either in this plugin, and in Infection native implementation (which is WIP) - Infection is a "process orchestrator", so it creates Mutants (temp files) and runs PHPStan against them in different processes. But seems like I misunderstand your message. Do you mean that PHPStan itself create sub-processes when it is run as a sub-process? |
Yes. In case we can assume that most of the mutations don't lead to phpstan needs to re-analyze hundreds of files, it might be faster to run multiple mutations in parallel and let phpstan run in single-process mode (we need to measure why we currently are way slower than psalm in the github action job for infection analysis) |
No description provided.