Try phpstan-mutant-killer-infection-runner

[ondrejmirtes]

trafficstars

[ondrejmirtes]

Context: https://github.com/phpstan/phpstan/discussions/10966#discussioncomment-13186740

[ondrejmirtes]

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

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

[Ocramius]

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

[staabm]

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]

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]

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]

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]

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]

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 https://github.com/nikic/PHP-Parser/pull/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]

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]

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]

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: https://github.com/infection/infection/pull/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".