Unexpected "syntax errors were encountered" after updating to 0.25.0

[kubawerlos]

Question	Answer
Infection version	0.25.0
Test Framework version	PHPUnit 9.5.9
PHP version	8.0.10
Platform	Ubuntu
Github Repo	https://github.com/kubawerlos/php-cs-fixer-custom-fixers

phpunit.xml

https://github.com/kubawerlos/php-cs-fixer-custom-fixers/blob/main/phpunit.xml

Output with issue

  917 mutations were generated:
       872 mutants were killed
         0 mutants were not covered by tests
         0 covered mutants were not detected
        44 errors were encountered
         1 syntax errors were encountered
         0 time outs were encountered
         0 mutants required more time than configured
  
  Metrics:
           Mutation Score Indicator (MSI): 99%
           Mutation Code Coverage: 100%
           Covered Code MSI: 99%
  
  Please note that some mutants will inevitably be harmless (i.e. false positives).
  Escaped mutants:
  ================
  
  Timed Out mutants:
  ==================
  
  Skipped mutants:
  ================

Mutator UnwrapStrReplace after updating from 0.24.0 to 0.25.0 started to make Infection failing with "syntax errors were encountered".

See the changes in this PR: kubawerlos/php-cs-fixer-custom-fixers#595 - code \str_replace("\n", "\n//", $codeToCommentOut); was fine on Infection 0.24.0 and on 0.25.0 it fails (see the failing job).

There are others str_replace calls which are mutated and are fine.

Very similar case for substr - started to report "syntax errors were encountered" with unwrapping mutator and only for some cases.

I've tried to investigate it a bit:

• I've tried to see the mutated code, but without any success (I didn't try much, only with --show-mutations)
• also all the cases I've tried to add to tests/phpunit/Mutator/Unwrap/UnwrapStrReplaceTest.php were passing

[maks-rafalko]

I've tried to see the mutated code, but without any success (I didn't try much, only with --show-mutations)

See #1564 (comment), it explains how to work with the logs and show more detailed info.

I've run your repository with additional --log-verbosity=all --debug options and here what it shows:

Syntax Errors mutants:
======================

1) /tmp/php-cs-fixer-custom-fixers/src/Fixer/CommentedOutFunctionFixer.php:236    [M] UnwrapStrReplace

--- Original
+++ New
@@ @@
             /** @var string $prefix */
             $prefix = Preg::replace('/(^|\\R)(\\h*$)/D', '$1//$2', $beforeStartToken->getContent());
         }
-        $codeToCommentOut = $prefix . \str_replace("\n", "\n//", $codeToCommentOut);
+        $codeToCommentOut = $prefix . $codeToCommentOut;
         if ($tokens->offsetExists($endIndex + 1)) {
             /** @var Token $afterEndToken */
             $afterEndToken = $tokens[$endIndex + 1];

$ '/tmp/php-cs-fixer-custom-fixers/vendor/phpunit/phpunit/phpunit' '--configuration' '/tmp/infection/phpunitConfiguration.fcf5379bb2a903d3ca0614b010ea0f36.infection.xml'
  PHPUnit 9.5.9 by Sebastian Bergmann and contributors.
  
  .......................E
  
  Time: 00:00.045, Memory: 10.00 MB
  
  There was 1 error:
  
  1) Tests\Fixer\CommentedOutFunctionFixerTest::testFix with data set "multiline call" ('<?php\n//                var_...      ', '<?php\n                var_du...      ')
  ParseError: syntax error, unexpected token ","
  
  /tmp/php-cs-fixer-custom-fixers/dev-tools/vendor/friendsofphp/php-cs-fixer/src/Tokenizer/Tokens.php:983
  /tmp/php-cs-fixer-custom-fixers/dev-tools/vendor/friendsofphp/php-cs-fixer/src/Tokenizer/Tokens.php:196
  /tmp/php-cs-fixer-custom-fixers/src/Fixer/CommentedOutFunctionFixer.php:189
  /tmp/php-cs-fixer-custom-fixers/src/Fixer/CommentedOutFunctionFixer.php:142
  /tmp/php-cs-fixer-custom-fixers/src/Fixer/CommentedOutFunctionFixer.php:96
  /tmp/php-cs-fixer-custom-fixers/tests/Fixer/AbstractFixerTestCase.php:144
  /tmp/php-cs-fixer-custom-fixers/tests/Fixer/CommentedOutFunctionFixerTest.php:43
  
  ERRORS!
  Tests: 24, Assertions: 194, Errors: 1.

So it looks like it is your code produces syntax error, not Infection (seems like code is not correctly fixed by CommentedOutFunctionFixer)

You can read this topic #1566 and this comment in particular to get an idea how to kill such mutant: #1566 (reply in thread)

[maks-rafalko]

Feel free to share your findings and reopen if you still think this is an Infection bug.

For now, it looks like the change in the source code of the fixer produces incorrect behavior in your code.

[sanmai]

Looks like a bug to me, TBH. Reason: if we can't tell if something is a legitimate syntax error caused by Infection, we shouldn't report it as such.

Alternatively, we could leave syntax errors to be reported only during debugging. This should go inline with our desire to help hackers to debug their custom mutators.

[maks-rafalko]

If we look into this example, #1566 (reply in thread) I can't say that reporting syntax errors caused NOT by Infection is useless.

This gives for the end developer information that some mutants will always lead to fatal (syntax) errors, without any ability to be escaped.

Example:

imagine we are writing code evaluator with the very simple logic:

function evaluateCode(string $expression): void
{
    eval($expression . ';');
}

and Infection generates the following mutants

function evaluateCode(string $expression): void
{
-    eval($expression . ';');
+    eval($expression);
}

function evaluateCode(string $expression): void
{
-    eval($expression . ';');
+    eval(';' . $expression);
}

then, this application will always lead to syntax errors for those mutants: https://3v4l.org/JB7Hu

Do we need to waste time generating them? IMO they are useless mutations that should be ignored, and displaying syntax errors with X to the output will notify developers about such cases.

[sanmai]

That's right. Shouldn't we then re-brand this feature as usable by end users instead of just developers?

I mean, who could've known we'll find infection mutating code to cause actual syntax errors! Yes, it is fairly obvious in retrospect, but still.

[kubawerlos]

There are cases when after mutation code is still valid syntax and the error happens. Consider this code (of course it's simplification of the case that lead me to this report):

        $code = '<?php echo 42;X';
        $code = \substr($code, 0, -1);
        $tokens = \token_get_all($code, \TOKEN_PARSE);

Mutator UnwrapSubstr will make:

        $code = '<?php echo 42;X';
        $code = $code;
        $tokens = \token_get_all($code, \TOKEN_PARSE);

So I'd expect the mutant to be considered killed if there is a test for it or escaped if there is no test.

I've pushed my test to https://github.com/werlos/php-cs-fixer-custom-fixers/tree/infection-example in cases anyone wants to experiment.

[maks-rafalko]

Released https://github.com/infection/infection/releases/tag/0.25.1

[kubawerlos]

Thank you @maks-rafalko, works like a charm.

[maks-rafalko]

Cool.

Thanks @sanmai for fixing it.

[sanmai]

You too @kubawerlos thanks for getting light on the problem.