InstanceOf_ mutator

[sanmai]

• Doc PR: InstanceOf_ mutator site#164

Quite simple one. Replaces:

$foo instanceof Anything

With either true or false.

• When true we assume the value always the of same type, and the check is redundant.
• When false we assume the testing isn't thorough.

Report for this mutator:

338 mutations were generated:
     316 mutants were killed
       0 mutants were not covered by tests
      22 covered mutants were not detected
       0 errors were encountered
       0 time outs were encountered

[maks-rafalko]

A couple of questions:

1. How a developer should kill it? Or is this mutator should show the redundant check (like public -> protected indicating that $a instance of $b is not needed? Can we add such info to description of the mutator?

2. I would suggest instead of producing 2 mutations (true, false) do the following

   -$a instance of $b
   +! $a instance of $b

   Why? Because producing 2 mutations (true, false) duplicates original value (being it true of false). With the suggested above mutation, we will have only one new mutation - the opposite to the original. What do you think?

[sanmai]

How a developer should kill it?

By ensuring this check is actually required by using appropriately typed arguments, or by removing the check altogether. I think we have to come up with a style of explanation before writing any of them since stating the obvious might not be as useful.

I we'd go with the negation approach you suggest, we'll have this:

169 mutations were generated:
     169 mutants were killed

Not to say it's bad, but not as nearly as lucrative as in the current implementation.

Consider this escaping mutation in ReflectionVisitor.php:

--- Original
+++ New
@@ @@
         if ($node instanceof Node\Stmt\ClassMethod) {
             return true;
         }
-        if ($node instanceof Node\Expr\Closure) {
+        if (false) {
             return true;
         }
         return false;

Are we really testing this method receives Node\Expr\Closure on input? How can this escape? I just verified it is indeed the case the test does not notice this change.

And why do test notice if we negate this comparison?..

[theofidry]

I think canMutate() should be placed before mutate(), but since it's hardly something we can enforce don't mind me if you don't agree with that

[sanmai]

Well, I took another mutator for a template, and that's how it was there.

[theofidry]

I thought the CS fixer would pick that up...

[sanmai]

It doesn't fix fixtures for some reason I didn't yet have time to investigate.