Try phpstan-mutant-killer-infection-runner #1505

ondrejmirtes · 2025-05-18T21:45:26Z

No description provided.

ondrejmirtes · 2025-05-18T21:45:42Z

Context: phpstan/phpstan#10966 (comment)

ondrejmirtes · 2025-05-20T14:29:13Z

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

4469 mutations were generated:
    3397 mutants were killed
       0 mutants were configured to be ignored
      10 mutants were not covered by tests
       8 covered mutants were not detected
       7 errors were encountered
       0 syntax errors were encountered
       6 time outs were encountered
    1041 mutants required more time than configured

And PHPStan's one reports:


4469 mutations were generated:
    3312 mutants were killed
       0 mutants were configured to be ignored
      10 mutants were not covered by tests
      29 covered mutants were not detected
       7 errors were encountered
       0 syntax errors were encountered
      53 time outs were encountered
    1058 mutants required more time than configured

And why Psalm's run succeeds, but PHPStan's run fails with these similar numbers.

ondrejmirtes · 2025-05-20T18:33:23Z

A nice side-effect of me optimizing PHPStan - BetterReflection is now analysed in 9 seconds instead of 28 seconds on my machine!

Ocramius · 2025-05-26T15:01:38Z

phpstan.neon

-    level: 6
+    level: 8


TIL: we were still at level 6 :O

This also explains why PHPStan + Infection didn't catch many of the mutants before, BTW, since many mutations are still valid under level 6! 👍

Ocramius · 2025-05-26T15:19:51Z

src/Reflection/ReflectionProperty.php

    /** @return array{get?: ReflectionMethod, set?: ReflectionMethod} */
    public function getHooks(): array
    {
+        // @infection-ignore-all AssignCoalesce: It's just optimization


We can possibly disable the entire mutator? It will always catch optimizations

(unless there's a way to declare these optimizations at type level, perhaps?)

Ocramius · 2025-05-27T21:14:15Z

Before considering a merge here, beware that we'd first need upstream stability for the new dependency

Ocramius · 2025-05-28T09:34:39Z

tools/composer.json

        "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"


Adding a note here: let's get a stable (even if 0.x) release out, before going with this

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.

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.

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 👍

staabm · 2025-05-29T09:21:15Z

just ran PHPStan based infection on this PR on my machine and looked at the process monitoring.
I had the impression PHPStan is running a main process and worker processes for each mutation.

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

maks-rafalko · 2025-05-29T09:35:46Z

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 had the impression PHPStan is running a main process and worker processes for each mutation.

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?

staabm · 2025-05-29T10:52:29Z

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)

staabm · 2025-05-31T10:35:18Z

does anyone involved has already a idea why/where we are slow? I am not yet sure how to measure/compare it best, as the mutation testing process takes so long to run and so many processes are spawned.

do we have some machinery in place to top level profile where we spent all the time?

ondrejmirtes · 2025-05-31T11:02:37Z

This can be profiled with Blackfire same way PHPStan usually is.

I suspect that the need to reflect a bunch of classes each time might have a play in that. Being able to cache BetterReflection objects on disk would be awesome. But we need to measure if it's really the cause.

Since both Infection and PHPStan will be using the editor mode when modifying and reanalysing files, I have great interest in making it faster, so I will eventually look into it.

staabm · 2025-05-31T18:49:24Z

had a look at the results of both tools

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?

I think these errors are suspicous:

4) /home/runner/work/BetterReflection/BetterReflection/src/Reflection/ReflectionClassConstant.php:74    [M] GreaterThan

@@ @@
         $this->docComment = GetLastDocComment::forNode($node);
         $this->attributes = ReflectionAttributeHelper::createAttributes($reflector, $this, $node->attrGroups);
         $startLine = $node->getStartLine();
-        assert($startLine > 0);
+        assert($startLine >= 0);
         $endLine = $node->getEndLine();
         assert($endLine > 0);
         $this->startLine = $startLine;


5) /home/runner/work/BetterReflection/BetterReflection/src/Reflection/ReflectionClassConstant.php:76    [M] GreaterThan

@@ @@
         $startLine = $node->getStartLine();
         assert($startLine > 0);
         $endLine = $node->getEndLine();
-        assert($endLine > 0);
+        assert($endLine >= 0);
         $this->startLine = $startLine;
         $this->endLine = $endLine;
         $this->startColumn = CalculateReflectionColumn::getStartColumn($declaringClass->getLocatedSource()->getSource(), $node);


6) /home/runner/work/BetterReflection/BetterReflection/src/Reflection/ReflectionConstant.php:87    [M] GreaterThan

@@ @@
         }
         $this->docComment = GetLastDocComment::forNode($node);
         $startLine = $node->getStartLine();
-        assert($startLine > 0);
+        assert($startLine >= 0);
         $endLine = $node->getEndLine();
         assert($endLine > 0);
         $this->startLine = $startLine;


7) /home/runner/work/BetterReflection/BetterReflection/src/Reflection/ReflectionConstant.php:89    [M] GreaterThan

@@ @@
         $startLine = $node->getStartLine();
         assert($startLine > 0);
         $endLine = $node->getEndLine();
-        assert($endLine > 0);
+        assert($endLine >= 0);
         $this->startLine = $startLine;
         $this->endLine = $endLine;
         $this->startColumn = CalculateReflectionColumn::getStartColumn($this->locatedSource->getSource(), $node);


8) /home/runner/work/BetterReflection/BetterReflection/src/Reflection/ReflectionEnumCase.php:65    [M] GreaterThan

@@ @@
         $this->attributes = ReflectionAttributeHelper::createAttributes($reflector, $this, $node->attrGroups);
         $this->docComment = GetLastDocComment::forNode($node);
         $startLine = $node->getStartLine();
-        assert($startLine > 0);
+        assert($startLine >= 0);
         $endLine = $node->getEndLine();
         assert($endLine > 0);
         $this->startLine = $startLine;


9) /home/runner/work/BetterReflection/BetterReflection/src/Reflection/ReflectionEnumCase.php:67    [M] GreaterThan

@@ @@
         $startLine = $node->getStartLine();
         assert($startLine > 0);
         $endLine = $node->getEndLine();
-        assert($endLine > 0);
+        assert($endLine >= 0);
         $this->startLine = $startLine;
         $this->endLine = $endLine;
         $this->startColumn = CalculateReflectionColumn::getStartColumn($this->enum->getLocatedSource()->getSource(), $node);

looks like PHPStan is using the wrong PHPParser sources/version .
I remember that I have provided the fix necessary for these mutants with nikic/PHP-Parser#985 and I think the reported mutatnts tell us that PHPStan is using a PHPParser version while analyzing which does not contain this PR.

I can image PHPStan might have a similar problem with BetterReflection and might not used the right sources:
tools/vendor/roave/better-reflection vs. better-reflection sources in phpstan.phar vs. better-reflection sources in src/

ondrejmirtes · 2025-05-31T18:56:08Z

We discussed these. PHPStan already knows it's -1|positive-int.

Both >= and > will eliminate -1. So both are valid code. And it's impossible to write a test against these mutants. In the future Infection might now what's going on and not generate these mutants.

ondrejmirtes · 2025-05-31T19:20:47Z

I guess you expect PHPStan to report an error for >= between -1|positive-int and 0, but it currently does not do that.

maks-rafalko · 2025-06-01T13:56:41Z

does anyone involved has already a idea why/where we are slow? I am not yet sure how to measure/compare it best, as the mutation testing process takes so long to run and so many processes are spawned.

do we have some machinery in place to top level profile where we spent all the time?

Not sure if it helps, but just in case: I've created the first implementation of Infection with PHPStan integrated natively, here: infection/infection#2098

What is interesting - we have e2e tests, where in tests/e2e/PHPStan_Integration (link) we have a small project with 1 source file and tests, where Infection can be executed with PHPStan under the hood. I've added a guide on how to run Infection with PHPStan in a description of that PR.

There are few mutant generated, so it will probably be easier to debug why it's slow. Now, with the debug logs generated (tests/e2e/PHPStan_Integration/infeciton.text.log), I see that any PHPStan process takes at least 1-3s with --threads=1. When I run it with --threads=max, it increases to smth like Elapsed time: 4 seconds (increasing in time sounds logical though).

Example of the logs generated:

3) /opt/infection/tests/e2e/PHPStan_Integration/src/SourceClass.php:21    [M] CastInt

@@ @@
         // some code to generate more mutations
         $strings = ['1'];
         $ints = array_map(function ($value): int {
-            return (int) $value;
+            return $value;
         }, $strings);
         $nonEmptyArray = ['1'];
         $nonEmptyArrayFromMethod = $this->returnNonEmptyArray();

$ '/opt/infection/tests/e2e/PHPStan_Integration/vendor/bin/phpstan' '--tmp-file=/opt/infection/tests/e2e/PHPStan_Integration/./infection/mutant.95f6dce4d49c8e07cc30dac6e64c67ca.infection.php' '--instead-of=/opt/infection/tests/e2e/PHPStan_Integration/src/SourceClass.php' '--error-format=json' '--no-progress' '-vv'
  
{"totals":{"errors":0,"file_errors":1},"files":{"/opt/infection/tests/e2e/PHPStan_Integration/src/SourceClass.php":{"errors":1,"messages":[{"message":"Anonymous function should return int but returns string.","line":18,"ignorable":true,"identifier":"return.type"}]}},"errors":[]}
  
  Note: Using configuration file /opt/infection/tests/e2e/PHPStan_Integration/phpstan.neon.
  Result cache restored. 1 file will be reanalysed.
  Result cache was not saved because of --tmp-file and --instead-of CLI options passed (editor mode).
  Elapsed time: 1.25 seconds
  Used memory: 76 MB

Look at the elapsed time - 1.25s for 1-file "project".

ondrejmirtes force-pushed the phpstan-infection branch 3 times, most recently from 5c29dde to 93d3c26 Compare May 20, 2025 17:48

kukulich force-pushed the phpstan-infection branch 3 times, most recently from b96d9b3 to b45f760 Compare May 21, 2025 07:17

ondrejmirtes force-pushed the phpstan-infection branch from b45f760 to 425a6d8 Compare May 21, 2025 20:27

kukulich force-pushed the phpstan-infection branch from 425a6d8 to a30c338 Compare May 23, 2025 07:40

Ocramius reviewed May 26, 2025

View reviewed changes

kukulich and others added 5 commits May 27, 2025 23:10

ReflectionObject constructor can be private

88b0136

Ignore some mutants

305a109

ReflectionClass test covers ReflectionObject too

58b8301

Kill some mutants

4966d4b

Try phpstan-mutant-killer-infection-runner

cf755c1

kukulich force-pushed the phpstan-infection branch from a30c338 to cf755c1 Compare May 27, 2025 21:12

kukulich changed the base branch from 6.58.x to 6.59.x May 27, 2025 21:14

Ocramius reviewed May 28, 2025

View reviewed changes

maks-rafalko mentioned this pull request Jun 3, 2025

[Demo] Use Infection with native PHPStan integration #1510

Draft

maks-rafalko mentioned this pull request Jun 12, 2025

Don't mutate instanceof into pre-existing case infection/infection#2176

Merged

Try phpstan-mutant-killer-infection-runner #1505

Are you sure you want to change the base?

Try phpstan-mutant-killer-infection-runner #1505

Uh oh!

Conversation

ondrejmirtes commented May 18, 2025

Uh oh!

ondrejmirtes commented May 18, 2025

Uh oh!

ondrejmirtes commented May 20, 2025

Uh oh!

ondrejmirtes commented May 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ocramius commented May 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

staabm commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maks-rafalko commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

staabm commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

staabm commented May 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ondrejmirtes commented May 31, 2025

Uh oh!

staabm commented May 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ondrejmirtes commented May 31, 2025

Uh oh!

ondrejmirtes commented May 31, 2025

Uh oh!

maks-rafalko commented Jun 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

staabm commented May 29, 2025 •

edited

Loading

maks-rafalko commented May 29, 2025 •

edited

Loading

staabm commented May 29, 2025 •

edited

Loading

staabm commented May 31, 2025 •

edited

Loading

staabm commented May 31, 2025 •

edited

Loading

maks-rafalko commented Jun 1, 2025 •

edited

Loading