From 8ae2fba97fe404c6ac138b17aaeb4ae32458a56e Mon Sep 17 00:00:00 2001 From: Cyrille DUPUYDAUBY Date: Fri, 1 Mar 2024 15:55:36 +0100 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Richard Werkman --- docs/technical-reference/Mutation Orchestration Design.md | 6 +++--- .../ExpressionBodiedPropertyOrchestrator.cs | 4 ++-- src/Stryker.Core/Stryker.Core/Mutants/MutationStore.cs | 6 ++++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/technical-reference/Mutation Orchestration Design.md b/docs/technical-reference/Mutation Orchestration Design.md index 1d87fecf9e..a6873b7ffb 100644 --- a/docs/technical-reference/Mutation Orchestration Design.md +++ b/docs/technical-reference/Mutation Orchestration Design.md @@ -16,7 +16,7 @@ Note that we made the creation of mutation engines (aka mutators) as easy as we Stryker relies heavily on Roslyn for C# parsing. As such, a good understanding of it is important before digging further. Roslyn precompiles any valid source file to an objects tree where each node is apart of syntax construct. Each node stores and describes the associated source code, including trivia (whitespaces and comments). The tree structure captures the code hierarchy. So you have a syntax root node (the whole file), containing a namespace node (namespace directive) which usually contains a class definition node which contains various members. The class likely contains at least one method definition, with a name, parameters and a body, made of statements. Statements contain a keyword and expressions and these contain sub expressions, potentially recursively as an expression can contain a lambda etc… As such, this syntax tree is often deep (tens, even hundreds of levels). -Each node is described via the appropriate Roslyn class, such as `InvocationExpressionSyntax` for a function call, or `ForStatementSyntax` for a _for_ loop. [Sharplab] allows you to discover the syntax tree of any provided C# source file. I strongly advise you to try and use it. Quick warning: Sharplab displays the **SyntaxKind** of the syntax constructs, which may be slightly different from the actual class used to represent it; as a reminder, **SyntaxKind** refines the type definition and enables to identify variants of some constructs, such as between `x++` and `x—-`. +Each node is described via the appropriate Roslyn class, such as `InvocationExpressionSyntax` for a function call, or `ForStatementSyntax` for a _for_ loop. [Sharplab](https://sharplab.io/) allows you to discover the syntax tree of any provided C# source file. I strongly advise you to try and use it. Quick warning: Sharplab displays the **SyntaxKind** of the syntax constructs, which may be slightly different from the actual class used to represent it; as a reminder, **SyntaxKind** refines the type definition and enables to identify variants of some constructs, such as between `x++` and `x—-`. ### Syntax Node classes There are hundreds of specific syntax nodes classes, as most C# constructs get their own class. So one can have specific access to each part of a for statement: initializers, condition, increments and statement (or block statement). This greatly simplifies reading, modifying or creating any syntax construct. @@ -55,7 +55,7 @@ To this list, I would add: throw everything in the air when facing the unexpecte # Stryker mutation design Taking into account previously listed principles, we designed the following classes/class hierarchy: 1. **Mutators**: **Generates mutated version** of syntax node. Each of them implements a specific mutation strategy. They must implement [`IMutator`] and must be stateless (they can only store their configuration/options). -2. **SyntaxNode Orchestrators**: **Orchestrates the steps required for** the mutation of a specific syntax node kind. They must implement [`INodeMutator`], and must be stateless. They do most of the grunt work +2. **SyntaxNode Orchestrators**: **Orchestrates the steps required for** the mutation of a specific syntax node kind. They must implement [`INodeMutator`], and must be stateless. They are responsible for walking through the syntax tree, and determining where mutation can be placed.``` 3. [`MutationContext`]: **Keeps track of the current state of the mutation process**. It stores mutations (via [`MutationStore`]) until they are injected in the mutated syntax tree; it also tracks which mutators have been disabled via [Stryker comments] as well as other details. 4. [`CSharpNodeOrchestrator`]: **Main entry point**. While it was doing all the work in earlier version of Stryker, it is now responsible for building the needed orchestrators, mutators, the mutant placer and keeping track of created mutations. 5. [`MutantPlacer`]: **Serves** as a single entry point to **several helper functions**, including the ones for injecting mutations. Also in charge of providing the needed information for rollback logic. @@ -90,7 +90,7 @@ As a reminder, they must be stateless, and this is a strong requirement. These c We will describe the base class here, all orchestrators will be described at the end of this document as an attempt to keep it readable. ### NodeSpecificOrchestrator -This is the base class used by all other orchestrators. It inherits from `NoOrchestratorBase` which deals with Stryker comments and is not described in this document for brevity. +This is the base class used by all other orchestrators. It inherits from `NodeOrchestratorBase` which deals with Stryker comments and is not described in this document for brevity. It implements a standardized node mutation workflow. This workflow’s steps are implemented via virtual methods so that specific orchestrators can provide the adequate implementation - `Mutate(...)`: this is the main method; it specifies the steps order: diff --git a/src/Stryker.Core/Stryker.Core/Mutants/CsharpNodeOrchestrators/ExpressionBodiedPropertyOrchestrator.cs b/src/Stryker.Core/Stryker.Core/Mutants/CsharpNodeOrchestrators/ExpressionBodiedPropertyOrchestrator.cs index 55661f38c9..e4278a260a 100644 --- a/src/Stryker.Core/Stryker.Core/Mutants/CsharpNodeOrchestrators/ExpressionBodiedPropertyOrchestrator.cs +++ b/src/Stryker.Core/Stryker.Core/Mutants/CsharpNodeOrchestrators/ExpressionBodiedPropertyOrchestrator.cs @@ -38,8 +38,8 @@ protected override PropertyDeclarationSyntax OrchestrateChildrenMutation(Propert var children = node.ReplaceNodes(node.ChildNodes(), (original, _) => { - var context1 = original == node.Initializer ? context.EnterStatic() : context; - return context1.FindHandler(original).Mutate(original, semanticModel, context1); + var determinedContext = original == node.Initializer ? context.EnterStatic() : context; + return determinedContext.FindHandler(original).Mutate(original, semanticModel, determinedContext); }); return children.WithInitializer(children.Initializer.WithValue(context.PlaceStaticContextMarker(children.Initializer.Value))); } diff --git a/src/Stryker.Core/Stryker.Core/Mutants/MutationStore.cs b/src/Stryker.Core/Stryker.Core/Mutants/MutationStore.cs index 706490af6e..43c8d5a1de 100644 --- a/src/Stryker.Core/Stryker.Core/Mutants/MutationStore.cs +++ b/src/Stryker.Core/Stryker.Core/Mutants/MutationStore.cs @@ -88,7 +88,7 @@ public void Leave() { _pendingMutations.Peek().StoreMutations(old.Store); } - else if (old.Store.Count>0) + else if (old.Store.Count > 0) { Logger.LogError("Some mutations failed to be inserted, they are dropped."); foreach (var mutant in old.Store) @@ -234,7 +234,9 @@ private sealed class PendingMutations public bool Aggregate(MutationControl control) { - if (Store.Count != 0 || Control != control) return false; + if (Store.Count != 0 || Control != control) { + return false; + } _depth++; return true; }