Infection: Syntax errors are not accounted for, not counted

[sanmai]

As of writing Infection does not distinguish between different types of errors, of which there are at least several groups:

• Common language errors (Error: Call to protected method)
• Memory errors (Fatal error: Allowed memory size of, see #258)
• Syntax errors (we should not have them, but how do we know if we do not account for them)
• File errors (e.g. flock(): Illegal operation argument could be a temporary glitch)
• Other errors

That's just some types that I think worth counting and reporting separately. There could be more.

First reported here.

Related: #222

[maks-rafalko]

Syntax errors (we should not have them, but how do we know if we do not account for them)

I thought PHP-Parser should always generate valid PHP code, but can't find a proof in its documentation anymore. Probably I mixed up something.

Also related to https://github.com/infection/infection/issues/222#issue-305355702 (see proposed report):

6 mutations were generated:
   5 mutants were killed:
       - 5 tests failures
       - 0 PHP errors
       - 0 time outs

So we can add those groups to the report above

[sanmai]

I thought PHP-Parser should always generate valid PHP code

Well, we can safely say that it is proven that it doesn't always generate a correct code.

Errors from mutators from #243 have:

-        $updater->getStrategy()->setPackageName(self::PACKAGE_NAME);
+        false->setPackageName(self::PACKAGE_NAME);

And that is a parse error: https://3v4l.org/kZNGg

PHP Parse error:  syntax error, unexpected '->' (T_OBJECT_OPERATOR)

Therefore we must account for syntax errors.

[theofidry]

I think we should try to be smarter and account for useless/equivalent mutants at some point.

To expand a bit: creating a mutant is relatively easy and requires little resources. However evaluating a mutant is way more expansive (we need to run the tests). There is a report somewhere mentioning about ~30% of the generated mutants are redundant hence not useful. I still need to check the effective strategies we could employ, but meanwhile checking for a PHP validity would be a good start and removing a bunch of mutants that provide no value.

[BackEndTea]

What if we could lint the 'new' PHP before writing it to a file? And if it contains a syntax error ignore it.

[maks-rafalko]

Well, as @sanmai already pointed

we should not have them (syntax errors)

My opinion is we should do our best to not allow Mutators produce invalid syntax. That's it.

If it requires a complex code - no problem, we will have to write it, but not leave mutators that produce invalid syntax (related to this). So we should fix the root of the problem rather than linting the code and ignore invalid created Mutant.

Linting the code

• requires another dependency
• slows down infection
• will lint every Mutant that has always valid syntax (e.g. === -> !==) which is useless wasting of resources

---

Created Mutant with invalid syntax must be considered as a bug in Infection.

[sanmai]

$ find src/ -type f | wc -l
190
$ time find src/ -type f -exec php -l {} \;
No syntax errors detected in src/....

real    0m10.120s
user    0m6.160s
sys 0m3.093s

That's 0.05 seconds per file on average, or 0.05 seconds per every mutation. Or 50 seconds for each 1000 mutations extra to what it currently takes.

No matter how much I like knowing exactly if we caused a syntax error (and hit a bug), considering that most often we won't be dealing with syntax errors, it isn't seemingly worth it to lint every file specifically outside of the usual testing process.

[theofidry]

@sanmai remember that the time taken for each mutation greatly differs from one project to another, and it can be hella slow.

That said I agree with the thought that if the linting process is too expansive, we're better of not doing it...

[sanmai]

Other than our own needs being addressed #301, I'm sure PHPUnit can be made to report fatal errors with some twiddling with phpunit.xml we use for our runners, and we can parse the errors just like we're doing it now. (For that we would need a "broken" mutator or two, that will always produce a code with a syntax error. Next is E2E test case with this mutator, and so on with detection.)