stryker-mutator / stryker-net

Mutation testing for .NET core and .NET framework!
https://stryker-mutator.io
Apache License 2.0
1.78k stars 187 forks source link

Reduce safe mode situtations #1953

Open dupdob opened 2 years ago

dupdob commented 2 years ago

Is your feature request related to a problem? Please describe. Stryker often triggers safe mode due to some uninitialized variable. This is a problem because it is displayed as such, but more important, it removes 'useful' mutations along the way.

Describe the solution you'd like Ideally, Stryker should be able to remove the guilty mutation(s), leaving the other alone. But I have identified several interesting alternatives:

  1. Block & statement mutators could skip block/statement with assignment. This may be a bit drastic.
  2. Block & statement mutators could replace assignment statements with assignment to default. This requires some analysis but appears to be interesting.
  3. Stryker could recognize Use of unassigned local variable... errors and injects the needed assignment to default value. A bit trickier than the previous one, but avoid the systematic and probably often useless injection of assignments.
  4. Stryker could recognize Use of unassigned local variable... errors and silently trigger safe mode for those situations.

I think solution 1 and 2 are the most interesting, but I would love to have comments and suggestions to help identify the good approach to be implemented.

Additional context I am the one who added 'Safe mode' to Stryker.

rouke-broersma commented 2 years ago

I agree that 1 and 2 are the most interesting where 2 would be the best case scenario. Avoiding creating compile errors where possible seems favorable over catching compile errors and fixing afterwards such as with rollback or 3 and 4.

richardwerkman commented 2 years ago

I think we might have another option. Let's take a look at the code (from #1923)

        public static decimal CalculateValue(bool a, decimal b, decimal c)
        {
            decimal result;

            if (a)
            {
                result = b;
            }
            else
            {
                result = c;
            }

            return result;
        }

If we could initialize any declaration like decimal result = default; the error Use of unassigned local variable can never happen. And we could still place the block and statement mutations like we do now

dupdob commented 2 years ago

yes, but that may be painful to roll back when necessary. I was thinking about this

       public static decimal CalculateValue(bool a, decimal b, decimal c)
        {
            decimal result;

            if (a)
             if (Stryker.IsActive(1))
            {
                result = default;
            }
            else
            {
                result = b;
            }
            else if (Stryker.IsActive(2))
            {
                result = default;
            }
            else
            {
                result = c;
            }
            return result;
        }

(indenting sucks, but you get the idea)

richardwerkman commented 2 years ago

Why do you think we would need to rollback the default assignment? It should not affect runtime as all code paths should override the value. Except for our own mutation paths but they would result in compile errors otherwise.

I just feel like our solutions both result in the same behaviour but yours could be more work?

dupdob commented 2 years ago

We always need rollback, just in case: some syntax form may raise problems we did not anticipate, or a newer C# may offer some conflicting new syntax. here, we would need to be able to rollback each initialization. It can be done, of course.