Why does Stryker sometimes a non visible return default;

[dupdob]

Intro

As of today, Stryker.Net injects some non visible helpers. Those are non visible because they are not part of the mutation report, and helpers because they permit some mutations to compile properly.

To be more specific, having them prevent triggering Safe mode which would result in all mutations from one (or more) method(s) to be marked as compile error.

I opened this discussion to explain this in details and potentially discuss how this could be improved.

Safe mode and associated problems.

Stryker.Net uses a attempt and remove approach to mutation: all possible mutations are generated and injected in the (in-memory) source files and then compiled. After that, Stryker removes mutations that result in compilation error (and mark them as compilation error).

As Stryker compiles all mutations at once, it needs a way to identify which mutation triggered each compilation error. The association is made thanks to the error location: if an error happens within the mutation, the mutation is removed and voilà , the error will be gone on next build attempt.
But some errors happen outside any mutation, those are often error related to variables (use of uninitialized variables and related) or to flow control (return missing and related).
These errors are triggered by mutations impacting code flow control (if, while, for....).
As Stryker does not know which mutation triggers this kind of error, it removes every mutations from the method (or construction, function...). I understand it may be a bit drastic, but first versions of Stryker would simply stop when this happened.

C# is too smart for us

Note that this problem arises because the C# compiler does a good job at static analysis and therefore allow some constructions that would not be possible otherwise. One of the best example is return elision, such as:

public bool WaitOk()
{
  while (true)
  {
    if (inError)
    {
      return false;
    }
    if (!running)
    {
      return true;
    }
  }
  // no return here, since there is a while(true)
}

Helpers

Helpers are injected to limit the triggering of same mode. As of today, two exist:

1. Return Helper: a return default; statement is injected at the end of a method if it is not void and does not have a return as a last statement. It prevents the missing return error.
2. Out parameters helper: every out parameter is initialized to default at the start of the method. It prevents any 'out parameters value has not been set before returning' error. Those are injected in every method with parameters

Why are those invisible

As there is no sure way to detect when an error could happen, these helpers are injected everywhere they may be necessary. As they are not related to any specific mutation, they do not appear in the report.

There are side effect

The major drawback of these, is that they 'fix' mutations that would otherwise be legitimate compilation errors. And then users wonder why some mutations are reported as survived or killed when they should haven been flagged as compilation errors.

[dupdob]

How to improve on this?

One can think of several ways to improve on this, depending on the end result. Note that those are not mutually exclusive and be added next to another for a better experience at the end.

1. Transparency

The objective is to report the existence of this helpers, thus helping users understand what is actually hapenning.

1. Report helpers explicitly: this requires a modification of the report script and structure; the problem is the report will be more difficult to read.
2. Report helpers as mutations (i.e. with an expanding button). It would not alter direct readability, but users may not understand that helpers are always there

2. Inject them only when necessary.

The objective is to reduce the number of situations where helpers are injected:

1. Detect when required: inject an helper when Stryker detects an error is possible. This should be easy for return helpers (detect while(true) like constructions. Probably harder for out variables. Main problem: it may be a long chase to discover all potential error situations.
2. Inject them retroactively: inject them only when a related error happens. Main problem is identifying all error appropriate error codes, it also implies a significant redesign of Stryker internal logic, since injection happens before compilation and error fixing. Not an easy task.
3. Simply remove the helpers. Recently, the safe mode has been improved: it removes first all block mutations has those are the main cause for this type of errors.

3. Remove the needs for them

The idea here is to get read of them altogether. This would be the best experience, assuming it can be done.

1. Improve the error-mutation matching logic. We could devise some form of trial and error approach to identify mutant causing errors somewhere else in the code. For example, removing one mutant and compiling again to see if the error goes away. Probably the best result altogether, but harder to implement than it sounds and it would slow down build.
2. Improve mutators so they avoid injecting mutation when this would result in an error. It appears reasonable for missing return situations (just don't remove code when there is a return here), but it would be harder for out parameters (and uninitialized variables).

[dupdob]

What's next

I have posted this list for several reasons:

1. Provide documentation on this for when people ask bout this problem
2. Offer ideas fro people wanting to work on some improvements
3. Get feedback and ideas on how to improve on this.

[Liam-Rougoor]

Thank you for this helpful explanation! For issue #2283, I've been working on altering the block mutator to add the return default within the block if it contains a return statement. I think this will make things clear, but there is a case where this solution does cause a bad side effect. Consider the following scenario:

public bool IsAdult(int age)  
        {  
            if (age >= 18)  
            {  
                return true;  
            }  
            return false;  
        }  

Here, you would actually prefer the if-block to be removed entirely, because it would compile anyways and has a different effect than replacing the return true with a return default.
I could try adding some checks to the block mutator to change its behavior to:

1. Remove a block when it (or one of its children) does not contain a return statement.
2. Remove a block when the block (or one of its children) does contain a return statement, but the containing method has a direct return statement in it somewhere.
3. Otherwise replace the block with a block that contains return default.

Something similar might also be useful for the Statement mutator, but I haven't looked into that mutator yet. I guess that if those two mutators can deal with return statements properly, we no longer need to use the EndingReturnEngine?

[dupdob]

Thanks for getting back on this. Don't worry about the statement mutator: it does not mutate return statements.
What we want to achieve is to have as much viable mutations as possible (i.e. not compile error) without any un killable ones.
The ending return engine does a too good job for the first part of this expression and somewhat poorly on the second one.
The best strategy overall is to improve the detection of compile error mutations.
The next best one is to avoid triggering compile error that Stryker is not able is to recover from and needs safe mode.

I like your strategy, but I suggest we focus on your first point: when removing a return may cause a problematic error.
The only situation I think where this is a problem is in a infinite loop situation: either a while(true) or for(;;) and there is no return at the end of a method. Then Stryker needs to be careful when removing any 'return' (within the loop).