Add class level mutation control (for method only)#3131
Add class level mutation control (for method only)#3131
Conversation
dupdob
force-pushed
the
memberMutantSupport
branch
from
70e3fbb to
ccb073c
Compare
dupdob
force-pushed
the
memberMutantSupport
branch
from
ccb073c to
b78fc59
Compare
|
# Conflicts: # src/Stryker.Core/Stryker.Core/Helpers/RoslynHelper.cs # src/Stryker.Core/Stryker.Core/Instrumentation/IfInstrumentationEngine.cs
There was a problem hiding this comment.
Pull request overview
This PR extends Stryker.NET's mutation logic to support method-level mutations. Instead of mutating individual expressions within a method, it introduces a new mechanism where entire methods can be mutated by creating renamed copies of the original and mutated versions, with a redirector method that dispatches to the appropriate version based on mutation activation conditions.
Changes:
- Introduces
RedirectMethodEnginefor method-level mutation instrumentation via method redirection - Adds helper methods to
RoslynHelperfor code generation support (AsBlock,WithTrailingNewLine,RemoveNamedMember) - Updates instrumentation interfaces and base classes to support removal from the entire syntax tree
- Modifies rollback process to use the new
RemoveMutationmethod for class-level removals
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Stryker.Core/Stryker.Core/Instrumentation/RedirectMethodEngine.cs | New engine that implements method-level mutation via method redirection pattern |
| src/Stryker.Core/Stryker.Core/Instrumentation/IInstrumentCode.cs | Adds RemoveInstrumentationFrom method to interface for tree-level removal |
| src/Stryker.Core/Stryker.Core/Instrumentation/BaseEngine.cs | Implements default RemoveInstrumentationFrom method |
| src/Stryker.Core/Stryker.Core/Instrumentation/IfInstrumentationEngine.cs | Refactors to use new AsBlock helper method |
| src/Stryker.Core/Stryker.Core/Mutants/MutantPlacer.cs | Adds new RemoveMutation method for removing mutations from entire tree |
| src/Stryker.Core/Stryker.Core/Mutants/CsharpNodeOrchestrators/BaseFunctionOrchestrator.cs | Implements RemoveInstrumentationFrom for function orchestrators |
| src/Stryker.Core/Stryker.Core/Mutants/MutationStore.cs | Formatting cleanup (empty line removal/addition) |
| src/Stryker.Core/Stryker.Core/Helpers/RoslynHelper.cs | Adds helper extension methods for syntax manipulation |
| src/Stryker.Core/Stryker.Core/Compiling/CSharpRollbackProcess.cs | Updates to use new RemoveMutation method |
| src/Stryker.TestRunner.VsTest.UnitTest/VsTestMockingHelper.cs | Apparent syntax error in constructor initialization |
| src/Stryker.Core/Stryker.Core.UnitTest/Instrumentation/RedirectMethodEngineShould.cs | New test file for RedirectMethodEngine functionality |
| src/Stryker.Core/Stryker.Core.UnitTest/AssertExtensions.cs | Improves error message formatting in test assertions |
| src/Stryker.Core/Stryker.Core.UnitTest/Mutants/MutantOrchestratorTestsBase.cs | Adds unnecessary cast operation |
| src/Stryker.TestRunner.VsTest.UnitTest/Stryker.TestRunner.VsTest.UnitTest.csproj | Adds compile exclusions for test resources |
| src/Stryker.Core/Stryker.Core.UnitTest/Stryker.Core.UnitTest.csproj | Reorganizes test resource file handling with different copy settings |
src/Stryker.Core/Stryker.Core.UnitTest/Mutants/MutantOrchestratorTestsBase.cs
Show resolved
Hide resolved
| { | ||
| if (!originalClass.Contains(originalMethod)) | ||
| { | ||
| throw new ArgumentException($"Syntax tree does not contains {originalMethod.Identifier}.", nameof(originalMethod)); |
There was a problem hiding this comment.
Grammatical error in exception message: "does not contains" should be "does not contain"
| throw new ArgumentException($"Syntax tree does not contains {originalMethod.Identifier}.", nameof(originalMethod)); | |
| throw new ArgumentException($"Syntax tree does not contain {originalMethod.Identifier}.", nameof(originalMethod)); |
| public static SyntaxNode RemoveMutation(SyntaxNode nodeToRemove) | ||
| { | ||
| var annotatedNode = nodeToRemove.GetAnnotatedNodes(Injector).FirstOrDefault(); | ||
| if (annotatedNode != null) | ||
| { | ||
| var id = annotatedNode.GetAnnotations(Injector).First().Data; | ||
| if (!string.IsNullOrEmpty(id)) | ||
| { | ||
| return instrumentEngines[id].engine.RemoveInstrumentationFrom(nodeToRemove.SyntaxTree.GetRoot(), annotatedNode); | ||
| } | ||
| } | ||
| throw new InvalidOperationException($"Unable to find an engine to remove injection from this node: '{nodeToRemove}'"); | ||
| } |
There was a problem hiding this comment.
Potential logic issue: The method always returns the entire syntax tree root from RemoveInstrumentationFrom, but the calling code in CSharpRollbackProcess.cs is tracking nodes and expects to update them incrementally. The old RemoveMutant method would return a node that could be tracked and replaced, but this new method returns the entire tree root, which may break the node tracking logic. Consider whether the assignment trackedTree = MutantPlacer.RemoveMutation(nodeToRemove); should instead be using a pattern that preserves the tracked tree structure.
| public ClassDeclarationSyntax InjectRedirect(ClassDeclarationSyntax originalClass, | ||
| ExpressionSyntax condition, | ||
| MethodDeclarationSyntax originalMethod, | ||
| MethodDeclarationSyntax mutatedMethod) |
There was a problem hiding this comment.
Missing XML documentation: This public method lacks XML documentation. For consistency with other public methods in similar classes (e.g., InjectIf in IfInstrumentationEngine, PlaceWithConditionalExpression in ConditionalInstrumentationEngine), add XML comments describing the parameters and return value. Document what InjectRedirect does: it creates a method redirect pattern where the original method is renamed, a mutated version is added, and a dispatcher method with the original name routes calls based on a condition.
| public static ClassDeclarationSyntax RemoveNamedMember(this ClassDeclarationSyntax classNode, string memberName) => | ||
| classNode.RemoveNode(classNode.Members.First( m => m switch | ||
| { | ||
| MethodDeclarationSyntax method => method.Identifier.ToString() == memberName, | ||
| PropertyDeclarationSyntax field => field.Identifier.ToString() == memberName, | ||
| _ => false | ||
| }), SyntaxRemoveOptions.KeepNoTrivia); |
There was a problem hiding this comment.
Potential null reference exception: If no member with the specified name is found, First() will throw an InvalidOperationException. Consider using FirstOrDefault() and handling the null case, or adding a more descriptive exception message that explains which member was not found.
| public static ClassDeclarationSyntax RemoveNamedMember(this ClassDeclarationSyntax classNode, string memberName) => | |
| classNode.RemoveNode(classNode.Members.First( m => m switch | |
| { | |
| MethodDeclarationSyntax method => method.Identifier.ToString() == memberName, | |
| PropertyDeclarationSyntax field => field.Identifier.ToString() == memberName, | |
| _ => false | |
| }), SyntaxRemoveOptions.KeepNoTrivia); | |
| public static ClassDeclarationSyntax RemoveNamedMember(this ClassDeclarationSyntax classNode, string memberName) | |
| { | |
| var memberToRemove = classNode.Members.FirstOrDefault(m => m switch | |
| { | |
| MethodDeclarationSyntax method => method.Identifier.ToString() == memberName, | |
| PropertyDeclarationSyntax field => field.Identifier.ToString() == memberName, | |
| _ => false | |
| }); | |
| if (memberToRemove is null) | |
| { | |
| throw new InvalidOperationException($"Member '{memberName}' was not found in class '{classNode.Identifier}'."); | |
| } | |
| return classNode.RemoveNode(memberToRemove, SyntaxRemoveOptions.KeepNoTrivia); | |
| } |
|
This is an extension of the mutation logic design to support mutations for whole methods.