Stryker Does Not Honour Static Analysis

[MelbourneDeveloper]

Describe the bug Static analysis disallows the mutation, but Stryker ignores this.

Expected behavior Stryker should not add mutations that don't compile due to static analysis via Roslyn code rules

Desktop (please complete the following information):

• OS: Windows
• Type of project .NET Core
• Framework Version 5
• Stryker Version 0.22.3 (beta)

Additional context

Here is the mutation:

But, I cannot compile this because of Roslyn's code rules. Roslyn should have squashed the mutation. But, it seems as though Stryker is not honouring the rules.

You can recreate the issue with this repo: https://github.com/MelbourneDeveloper/RestClient.Net/tree/5/develop

Commit: 4c14356e80906b8017e06c422db814109635dd6d

You can run the batch files in src\RestClient.Net.UnitTests\

[rouke-broersma]

To implement analysers we need to figure out how msbuild passes them to roslyn and recreate this. Seeing as analysers are simple csharp classes or dlls I think we can just pass them straight into the compilation either as syntax tree or as assembly reference. Probably both to cover both custom made analyzer rules in the project and also rules from nuget packages. Hopefully the analysers are exposed as msbuild properties in which case buildalyzer can probably pass them to us.

We would still perform the mutation but it would cause a compile error and we would then remove it. This is the easiest implementation.

[MelbourneDeveloper]

@rouke-broersma it seems odd that you would need to do anything at all. If you depend on Roslyn for compilation, it should fail when code rules are violated. The important thing is the analysers Nuget package.

[MelbourneDeveloper]

Philosophical point:

What is the point of mutation testing?

It's to eliminate the possibility of potential code entering the system that would destroy the fundamental correctness of the system.

Static analysis has the same end goal but achieves it through a different means. It reduces possible code paths where it is clear that the system will behave incorrectly.

For example, static analysis can determine if there is code without a null check. If static analysis catches this, there is no need for mutations because static analysis already prevents them.

[rouke-broersma]

I agree that so long as the rule is active and causes compilation errors that mutation testing does not need to point out the possible mutations as well.

[rouke-broersma]

@rouke-broersma it seems odd that you would need to do anything at all. If you depend on Roslyn for compilation, it should fail when code rules are violated. The important thing is the analysers Nuget package.

It's not so much that we depend on Roslyn, but we use the Roslyn SDK directly from code. This means we replace MSBuild as project manager and that also means that any transformations MSBuild performs before handoff to Roslyn need to be performed in stryker as well. Since MSBuild is proprietary and MSBuild internal logic is mostly trial-by-error and information in old thick books that my generation is no longer forced to read we have to reverse-engineer a lot of MSBuild logic 😅

[dupdob]

By the way, looking into this issue, I just ran into this: MSBuildWorkspace https://gist.github.com/DustinCampbell/32cd69d04ea1c08a16ae5c4cd21dd3a3

The text hints it should be easy to achieve (but still no clue on how), but at least, MSBuildWorkSpace could be an interesting options to increase Stryker compatibility with project with custom build steps.

[rouke-broersma]

Last time we looked into msbuild workspace it did not work on dotnet core which is why we don't use it. Perhaps these days it works on core.

[dupdob]

Just did a quick repro: it turns out that Buildalyzer lists analyzers separately, i.e. they are not in the 'AssemblyReferences' list.

[rouke-broersma]

Just did a quick repro: it turns out that Buildalyzer lists analyzers separately, i.e. they are not in the 'AssemblyReferences' list.

I expected as much

[MelbourneDeveloper]

Here is another scenario:

FirstOrDefault is not valid because it could return null and nullable reference types are turned on.

[MelbourneDeveloper]

The whole nullable reference types ecosystem depends on static analysis to work. If you take away static analysis, you are left with buggy code.

[rouke-broersma]

The whole nullable reference types ecosystem depends on static analysis to work. If you take away static analysis, you are left with buggy code.

Nullable reference types are not roslyn analyzer rules, they're their own compiler feature. They also don't generate errors by default. We already enable Nullable reference types during the compilation but it could be we need to extract your warnings as errors rules to be able to use them during mutation testing.

[GrahamTheCoder]

Last time we looked into msbuild workspace it did not work on dotnet core which is why we don't use it. Perhaps these days it works on core.

It does work for both, but iirc you can't build a framework assembly from a core process. I ended up with two exes, the core one delegates to the framework one when needed. Then there's a bit of assembly binding faff too. This might be a good starter: https://github.com/icsharpcode/CodeConverter/blob/1817d3a504dbd04c0bb63b225d1aa3dbb1c2bb96/CommandLine/CodeConv.Shared/MSBuildWorkspaceConverter.cs#L115

Here's the PR where I implemented using MSBuildWorkspace for a similar shape project https://github.com/icsharpcode/CodeConverter/pull/556

[dupdob]

@MelbourneDeveloper : could you check with the latest version please ? we have improved support for analyzers and this issue may be resolved.

[dupdob]

The needed feature (analyzer support) has been implemented. I close this issue for now. Feel free to reopen it if this is still a problem with latest versions (>1.5)