[Demo] Use Infection with native PHPStan integration#1510
Draft
maks-rafalko wants to merge 4 commits intoRoave:6.60.xfrom
Draft
[Demo] Use Infection with native PHPStan integration#1510maks-rafalko wants to merge 4 commits intoRoave:6.60.xfrom
maks-rafalko wants to merge 4 commits intoRoave:6.60.xfrom
Conversation
Author
|
Please run GH actions. (working example is on my fork: maks-rafalko#2) |
This was referenced Jun 3, 2025
Ocramius
reviewed
Jun 4, 2025
maks-rafalko
added a commit
to infection/infection
that referenced
this pull request
Jun 7, 2025
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.
1 task
maks-rafalko
added a commit
to infection/infection
that referenced
this pull request
Jun 7, 2025
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.
Author
|
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% |
Contributor
as long as the PR has a merge conflict with the target branch no github actions will start |
20f6d34 to
c3f3808
Compare
Author
|
looks like it requires it since I'm not a contributor |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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=phpstanand 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-errorfrom 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