Allow to skip running PHPStan for particular mutators #2136
Conversation
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.
ba188b9 to
a4c41cf
Compare
sounds like it might make sense to ignore some errors via phpstan-identifier via phpstan config. maybe we even need such a "phpstan-ignore-list" per mutator, to make sure PHPStan does not kill errors by mistake. |
idea here was not to run PHPStan at all (in this particular case), because running it even for 1-file project takes at least 1 second (it's super slow comparing to PHPUnit). I don't say that it excludes your idea, just saying that instead of running PHPStan and disabling this check we can not run it. But this needs to be tested. Can we only ignore PHPStan identifiers by adding them to temp config file or do you know how to do it by CLI? I didn't find any CLI option for this.
Interesting, will keep an eye on it, analyzing results. |
|
I will experiment with your idea and keep this PR on hold |
I wonder whether we would get better results when then mutator would produce |
I've analyzed 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:
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.phpstan.hash.neonfile for each Mutant and disable not needed phpstan checks by specifying error identified (https://phpstan.org/blog/phpstan-1-11-errors-identifiers-phpstan-pro-reboot)