Support treat warnings as errors while compiling

[adamnova]

Null coalescing mutation is not killed when nullability is enforced.

public class Model
    {
        public static Model Create(string? nullable)
        {
            return new Model()
            {
                NotNullable = nullable ?? string.Empty // The actual code
                NotNullable = nullable // the survived mutation which does not actually compile if left like this in the code
            };
        }
        public string NotNullable { get; init; } = string.Empty;
    }

edit:// Updated text, because I made some mistake during initial testing which made me think that it was caused by warnings as errors, after further retesting I found out this was not the case.

[rouke-broersma]

We probably don't copy the treat warnings as errors config from msbuild to roslyn in Stryker.

[adamnova]

That would make sense, since the first Stryker build (handled by MSBuild, not sure how mutations are handled internally by Stryker) failed properly with error CS8601: Possible null reference assignment. when I removed the ?? string.Empty.

[dupdob]

I think Stryker should adhere to this option (warning as errors): it would mark more mutants as CompileError, but it makes sense.

[adamnova]

I've played a little more with my repro project and first I could only run into this problem once I enabled treat warnings as errors and changed the error to warning. But now it happens even when I get rid of it. Are there any hidden caches or something? I tried clearing everything to double check that it still reports this mutation and it does

If I change the code to this mutation it won't compile which imo means it should not be a mutant. I am including my very small repro project. If anyone could try it and see if they get the same issue. It's basically the function I posted + a test.

NullableMutationRepro.zip

[rouke-broersma]

@adamnova there's no reason for that code to not compile when treat warnings as errors is not turned on though? In fact I'm your repro it is turned on, in the editor config.

[adamnova]

I changed the severity of nullability to error as well. I was isolating that one variable (as in if its really caused by treat warnings as errors) and found out it might not be the one I thought.

[adamnova]

I have updated the title, because the I was able to replicate it without the treat as warnings and errors in a new project made from scratch. I must've made some mistake during my initial testing, that killed the mutant. I have also tested it on a different machine, to make sure there is nothing specific to mine and same issue.

It does not make sense to me to have mutants that are not allowed by the compiler to consider them as survived. Even though enforceability of nullability can be sometimes questionable, but in this case we are testing for a bug, that can never actually happen.

[rouke-broersma]

@adamnova I am really struggling to understand your scenario. The code you posted is valid and will compile. See:

Warning, not error. If you enable treat warning as error for this rule, then it will not compile. We agree that we should support this. However you seem to suggest that this should fail to compile in any scenario, which I cannot reproduce and I also do not understand why it would do so anyway.

[adamnova]

It would not compile because the severity of that rule is set to an error. This is done by setting dotnet_diagnostic.CS8601.severity = error in .editorconfig.

Does it really compile for you when treat warnings as errors is set to false? I downloaded the zip and tried it myself by only removing ?? string.empty and it does not compile for me.

Perhaps your VS ignores the .editorconfig?

[rouke-broersma]

dotnet_diagnostic.CS8601.severity = error is the same as treating that warning as error just configured in a different way. The reason it doesn't compile is extra config that we do not currently support. In a normal scenario (no treat warning as error/editorconfig) this would compile fine. Yes we need to support this but

I was able to replicate it without the treat warnings as errors

Is not correct. That's the part I was confused about in our message.

[adamnova]

I see, sorry for the confusion. I considered them separately because one is done through .editorconfig and the other one is project configuration. So it was possible these were 2 separate cases and one could be more general than the other, or maybe they don't overlap at all. I am not sure exactly how this is handled internally.

And yes, without any configuration changes the project will compile just fine in which case, mutant is absolutely valid.