Skip to content

[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

Draft
wants to merge 4 commits into
base: 6.60.x
Choose a base branch
from

Conversation

maks-rafalko
Copy link

@maks-rafalko maks-rafalko commented Jun 3, 2025

This is not intended to be merged (at least for now), more like to have a discussion and to compare with #1505


This PR:

  • uses Infection with native integration of PHPStan. Now, we can just run infection --static-analysis-tool=phpstan and it will do the job. So it's opt-in, by default PHPStan is not used
  • what is the difference between native integration and tools like roave/infection-static-analysis-plugin/ and phpstan/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:

  • it's slow. For some (not yet known) reason PHPStan process takes at least 1s even for 1-file project
  • we don't use --stop-on-first-error from PHPStan (because it doesn't exist yet xD)
  • many of the Mutants are killed by PHPStan when they shouldn't

For example:

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

^ 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

@maks-rafalko
Copy link
Author

Please run GH actions. (working example is on my fork: maks-rafalko#2)

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.
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.
@maks-rafalko
Copy link
Author

maks-rafalko commented Jun 10, 2025

GH actions are not executed automatically :(

Now (among tens of other improvements) the output shows killed mutants by PHPStan by marking them with A so it looks like this:

[...]

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

@staabm
Copy link
Contributor

staabm commented Jun 10, 2025

GH actions are not executed automatically :(

as long as the PR has a merge conflict with the target branch no github actions will start

@maks-rafalko maks-rafalko force-pushed the feature/infection-phpstan branch from 20f6d34 to c3f3808 Compare June 10, 2025 16:45
@maks-rafalko
Copy link
Author

looks like it requires it since I'm not a contributor

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