-
Notifications
You must be signed in to change notification settings - Fork 136
[Demo] Use Infection with native PHPStan integration #1510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 6.60.x
Are you sure you want to change the base?
Conversation
Please run GH actions. (working example is on my fork: maks-rafalko#2) |
I've analuzed 200+ mutations in BetterReflection (Roave/BetterReflection#1510) and there are many cases where PHPStan kills a mutant while it should not - those are false-positives (meaning we get killed mutant but in fact it can be escaped/not well testes), for example: ```diff @@ @@ if ($reflection instanceof ReflectionFunction && $this->containsLine($reflection, $lineNumber)) { return $reflection; } - if ($reflection instanceof ReflectionConstant && $this->containsLine($reflection, $lineNumber)) { + if (true && $this->containsLine($reflection, $lineNumber)) { return $reflection; } } $ '/app/vendor/bin/phpstan' '--tmp-file=/tmp/infection/mutant.a99f5bbe9bbbb1514da2bdbe5e315d5c.infection.php' '--instead-of=/app/src/Util/FindReflectionOnLine.php' '--error-format=json' '--no-progress' '-vv' PHPStan error: Left side of && is always true. ``` In PHPStan, it always produces "Left|Right side of && is always true" or "If condition is always true." See https://phpstan.org/r/a9736cca-dc3a-4033-8f2a-80dbc13951b9 So it doesn't make sense for `InstanceOf_` mutator to try to kill it by PHPStan, because it will always be killed while tests can be with the whole.
I've analuzed 200+ mutations in BetterReflection (Roave/BetterReflection#1510) and there are many cases where PHPStan kills a mutant while it should not - those are false-positives (meaning we get killed mutant but in fact it can be escaped/not well testes), for example: ```diff @@ @@ if ($reflection instanceof ReflectionFunction && $this->containsLine($reflection, $lineNumber)) { return $reflection; } - if ($reflection instanceof ReflectionConstant && $this->containsLine($reflection, $lineNumber)) { + if (true && $this->containsLine($reflection, $lineNumber)) { return $reflection; } } $ '/app/vendor/bin/phpstan' '--tmp-file=/tmp/infection/mutant.a99f5bbe9bbbb1514da2bdbe5e315d5c.infection.php' '--instead-of=/app/src/Util/FindReflectionOnLine.php' '--error-format=json' '--no-progress' '-vv' PHPStan error: Left side of && is always true. ``` In PHPStan, it always produces "Left|Right side of && is always true" or "If condition is always true." See https://phpstan.org/r/a9736cca-dc3a-4033-8f2a-80dbc13951b9 So it doesn't make sense for `InstanceOf_` mutator to try to kill it by PHPStan, because it will always be killed while tests can be with the whole.
GH actions are not executed automatically :( Now (among tens of other improvements) the output shows killed mutants by PHPStan by marking them with [...]
.................A.............AAM.A....A......... (3650 / 4409)
.......A......M.MM.AA..A.......A...A.........A.... (3700 / 4409)
......M.AAA...A.................A.A.M..MA....A.... (3750 / 4409)
.................A.......A.....................M.. (3800 / 4409)
.AA...A..A...............A.AAA......A............E (3850 / 4409)
E..A..........A................................... (3900 / 4409)
..........................A....................... (3950 / 4409)
..............................T................... (4000 / 4409)
.................................................. (4050 / 4409)
.........T........................................ (4100 / 4409)
.................................................. (4150 / 4409)
T..............E..................AA.............. (4200 / 4409)
.........M................................A....... (4250 / 4409)
.....................................A..........E. (4300 / 4409)
E.E.............................................AA (4350 / 4409)
....................A.............A............... (4400 / 4409)
......A.A (4409 / 4409)
4409 mutations were generated:
3660 mutants were killed by Test Framework
+ 212 mutants were killed by Static Analysis
0 mutants were configured to be ignored
11 mutants were not covered by tests
47 covered mutants were not detected
7 errors were encountered
0 syntax errors were encountered
9 time outs were encountered
463 mutants required more time than configured
Metrics:
Mutation Score Indicator (MSI): 98%
Mutation Code Coverage: 99%
Covered Code MSI: 98% |
as long as the PR has a merge conflict with the target branch no github actions will start |
20f6d34
to
c3f3808
Compare
looks like it requires it since I'm not a contributor |
This is not intended to be merged (at least for now), more like to have a discussion and to compare with #1505
This PR:
infection --static-analysis-tool=phpstan
and it will do the job. So it's opt-in, by default PHPStan is not usedroave/infection-static-analysis-plugin/
andphpstan/mutant-killer-infection-runner
? Infection runs PHPStan processes in parallel (depending on--threads
) while those plugins run Psalm/PHPStan processes sequentially because of the nature of the the "hack" used there.There is still too much work to do, here are the major issues for now:
--stop-on-first-error
from PHPStan (because it doesn't exist yet xD)For example:
^ PHPStan always kills this Mutant, but in reality the tests can easily have a whole here, not testing properly the left side of this condition. I think on Infection side we need to mark some mutators (mutator operators) to not require static analysis, and just finish if they are escaped, not wasting time with static analysis.
cc @ondrejmirtes @staabm