chore: clean up code and add comments

Author: dupdob

src/Stryker.Core/Stryker.Core.UnitTest/AssertExtensions.cs

@@ -17,10 +17,7 @@ public static class AssertionExtensions
         /// </summary>
         /// <param name="actual">Resulted code</param>
         /// <param name="expected">Expected code</param>
-        public static void ShouldBeSemantically(this string actual, string expected)
-        {
-            ShouldBeSemantically(CSharpSyntaxTree.ParseText(actual).GetRoot(), CSharpSyntaxTree.ParseText(expected).GetRoot());
-        }
+        public static void ShouldBeSemantically(this string actual, string expected) => ShouldBeSemantically(CSharpSyntaxTree.ParseText(actual), CSharpSyntaxTree.ParseText(expected));
 
         /// <summary>
         /// Compares two syntax trees and asserts equality
@@ -29,14 +26,9 @@ public static void ShouldBeSemantically(this string actual, string expected)
         /// <param name="expected">Expected code</param>
         /// <remarks>Warning: this code tries to pinpoint the first different lines, but it will work on string comparison, so it may pinpoint spaces
         /// or new line variations, hiding the real differences.</remarks>
-        public static void ShouldBeSemantically(this SyntaxNode actual, SyntaxNode expected)
+        public static void ShouldBeSemantically(this SyntaxTree actual, SyntaxTree expected)
         {
-            // for some reason, nodes can be different while being textually the same
-            if (actual.ToFullString() == expected.ToFullString())
-            {
-                return;
-            }
-            var isSame = SyntaxFactory.AreEquivalent(actual, expected);
+            var isSame = actual.IsEquivalentTo(expected);
 
             if (!isSame)
             {
@@ -63,12 +55,12 @@ public static void ShouldBeWithNewlineReplace(this string actual, string expecte
             actual.ShouldBe(replaced);
         }
 
-        public static void ShouldNotContainErrors(this SyntaxNode actual)
+        public static void ShouldNotContainErrors(this SyntaxTree actual)
         {
-            var errors = actual.SyntaxTree.GetDiagnostics().Count(x => x.Severity == DiagnosticSeverity.Error);
+            var errors = actual.GetDiagnostics().Count(x => x.Severity == DiagnosticSeverity.Error);
             if (errors > 0)
             {
-                errors.ShouldBe(0, $"Actual code has build errors!\n{actual.ToFullString()}\nerrors: {string.Join(Environment.NewLine, actual.SyntaxTree.GetDiagnostics())}");
+                errors.ShouldBe(0, $"Actual code has build errors!\n{actual.GetText()}\nerrors: {string.Join(Environment.NewLine, actual.GetDiagnostics())}");
             }
         }
 

src/Stryker.Core/Stryker.Core.UnitTest/Compiling/CSharpRollbackProcessTests.cs

@@ -119,8 +119,8 @@ public void SomeLinq()
                 helpers.Add(CSharpSyntaxTree.ParseText(code, path: name, encoding: Encoding.UTF32));
             }
 
-            var mutant = mutator.Mutate(syntaxTree.GetRoot(), null);
-            helpers.Add(mutant.SyntaxTree);
+            var mutant = mutator.Mutate(syntaxTree, null);
+            helpers.Add(mutant);
 
             var references = new List<string> {
                 typeof(object).Assembly.Location,
@@ -203,8 +203,8 @@ private void RefreshAccountNumber()
                 helpers.Add(CSharpSyntaxTree.ParseText(code, path: name, encoding: Encoding.UTF32));
             }
 
-            var mutant = mutator.Mutate(syntaxTree.GetRoot(), null);
-            helpers.Add(mutant.SyntaxTree);
+            var mutant = mutator.Mutate(syntaxTree, null);
+            helpers.Add(mutant);
 
             var references = new List<string> {
                 typeof(object).Assembly.Location,

src/Stryker.Core/Stryker.Core.UnitTest/Mutants/CsharpMutantOrchestratorTests.cs

@@ -1224,7 +1224,7 @@ public void MutationsShouldHaveLinespan()
 {
 	var test3 = 2 + 5;
 }";
-            _target.Mutate(CSharpSyntaxTree.ParseText(source).GetRoot(), null);
+            _target.Mutate(CSharpSyntaxTree.ParseText(source), null);
 
             var mutants = _target.GetLatestMutantBatch().ToList();
             mutants.Count.ShouldBe(2);
@@ -1251,7 +1251,7 @@ public static bool InvokeIfNotNull(this Action a)
 		}
 	}
 }";
-            _target.Mutate(CSharpSyntaxTree.ParseText(source).GetRoot(), null);
+            _target.Mutate(CSharpSyntaxTree.ParseText(source), null);
 
             var mutants = _target.GetLatestMutantBatch().ToList();
             mutants.Count.ShouldBe(7);
@@ -1402,6 +1402,21 @@ public void ShouldNotAddReturnDefaultToMethodsWithReturnTypeVoid()
             ShouldMutateSourceInClassToExpected(source, expected);
         }
 
+        [Theory]
+        [InlineData("{return 0;}")]
+        [InlineData("{return 1; throw Exception();}")]
+        [InlineData("{return 1;yield return 0;}")]
+        [InlineData("{return 1;yield break;}")]
+        [InlineData("{;}")]
+        public void ShouldNotAddReturnOnSomeBlocks(string code)
+        {
+            string source = @$"
+int TestMethod()
+// Stryker disable all
+{code}";
+            ShouldMutateSourceInClassToExpected(source, source);
+        }
+
         [Fact]
         public void ShouldSkipStringsInSwitchExpression()
         {

src/Stryker.Core/Stryker.Core.UnitTest/Mutants/MutantOrchestratorTestsBase.cs

@@ -26,11 +26,11 @@ public MutantOrchestratorTestsBase()
 
         protected void ShouldMutateSourceToExpected(string actual, string expected)
         {
-            var actualNode = _target.Mutate(CSharpSyntaxTree.ParseText(actual).GetRoot(), null);
-            actual = actualNode.ToFullString();
+            var actualNode = _target.Mutate(CSharpSyntaxTree.ParseText(actual), null);
+            actual = actualNode.GetRoot().ToFullString();
             actual = actual.Replace(_injector.HelperNamespace, "StrykerNamespace");
-            actualNode = CSharpSyntaxTree.ParseText(actual).GetRoot();
-            var expectedNode = CSharpSyntaxTree.ParseText(expected).GetRoot();
+            actualNode = CSharpSyntaxTree.ParseText(actual);
+            var expectedNode = CSharpSyntaxTree.ParseText(expected);
             actualNode.ShouldBeSemantically(expectedNode);
             actualNode.ShouldNotContainErrors();
         }

src/Stryker.Core/Stryker.Core.UnitTest/Mutants/MutantPlacerTests.cs

@@ -184,8 +184,8 @@ public void ShouldInjectInitializersAndRestore()
             var expected = "class Test {bool Method(out int x) {{x = default(int);}x=0;}}";
 
             CheckMutantPlacerProperlyPlaceAndRemoveHelpers<BlockSyntax>(source, expected,
-                (n) => MutantPlacer.AddDefaultInitializers(n,
-                    new[]{SyntaxFactory.Parameter(SyntaxFactory.Identifier("x")).WithType(SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword))
+                (n) => MutantPlacer.InjectOutParametersInitialization(n,
+                    new[]{SyntaxFactory.Parameter(SyntaxFactory.Identifier("x")).WithModifiers(SyntaxFactory.TokenList(new[] {SyntaxFactory.Token(SyntaxKind.OutKeyword)})).WithType(SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.IntKeyword))
                     )}));
         }
 
@@ -215,7 +215,7 @@ public void ShouldRollBackFailedConstructor()
                 OptimizationMode = OptimizationModes.CoverageBasedTest,
                 MutationLevel = MutationLevel.Complete
             });
-            var actualNode = orchestrator.Mutate(CSharpSyntaxTree.ParseText(source).GetRoot(), null);
+            var actualNode = orchestrator.Mutate(CSharpSyntaxTree.ParseText(source), null).GetRoot();
 
             var node = actualNode.DescendantNodes().First(t => t is BlockSyntax);
 
@@ -233,10 +233,10 @@ public void ShouldRollBackFailedConstructor()
             restored = MutantPlacer.RemoveMutant(node);
             actualNode = actualNode.ReplaceNode(node, restored);
 
-            var expectedNode = CSharpSyntaxTree.ParseText(source.Replace("StrykerNamespace", codeInjection.HelperNamespace)).GetRoot();
+            var expectedNode = CSharpSyntaxTree.ParseText(source.Replace("StrykerNamespace", codeInjection.HelperNamespace));
             expectedNode.ShouldNotContainErrors();
-            actualNode.ShouldBeSemantically(expectedNode);
-            actualNode.ShouldNotContainErrors();
+            actualNode.SyntaxTree.ShouldBeSemantically(expectedNode);
+            actualNode.SyntaxTree.ShouldNotContainErrors();
         }
     }
 }

src/Stryker.Core/Stryker.Core.UnitTest/MutationTest/CSharpMutationTestProcessTests.cs

@@ -78,12 +78,12 @@ public void MutateShouldWriteToDisk_IfCompilationIsSuccessful()
 
             // create mocks
             var options = new StrykerOptions();
-            var orchestratorMock = new Mock<BaseMutantOrchestrator<SyntaxNode, SemanticModel>>(MockBehavior.Strict, options);
+            var orchestratorMock = new Mock<BaseMutantOrchestrator<SyntaxTree, SemanticModel>>(MockBehavior.Strict, options);
 
             fileSystem.AddDirectory(Path.Combine(FilesystemRoot, "TestProject", "bin", "Debug", "netcoreapp2.0"));
 
             // setup mocks
-            orchestratorMock.Setup(x => x.Mutate(It.IsAny<SyntaxNode>(), It.IsAny<SemanticModel>())).Returns(CSharpSyntaxTree.ParseText(SourceFile).GetRoot());
+            orchestratorMock.Setup(x => x.Mutate(It.IsAny<SyntaxTree>(), It.IsAny<SemanticModel>())).Returns(CSharpSyntaxTree.ParseText(SourceFile));
             orchestratorMock.SetupAllProperties();
             orchestratorMock.Setup(x => x.GetLatestMutantBatch()).Returns(mockMutants);
 

src/Stryker.Core/Stryker.Core/Instrumentation/BaseEngine.cs

@@ -13,7 +13,7 @@ namespace Stryker.Core.Instrumentation;
 /// <remarks>Multiple helpers can work on the same type of constructs.</remarks>
 internal abstract class BaseEngine<T>: IInstrumentCode where T: CSharpSyntaxNode
 {
-    protected BaseEngine() => Marker = new SyntaxAnnotation(MutantPlacer.Injector, InstrumentEngineId);
+    protected BaseEngine() => Marker = MutantPlacer.RegisterEngine(this);
 
     /// <summary>
     /// Annotation to be added to the instrumented node

src/Stryker.Core/Stryker.Core/Instrumentation/DefaultInitializationEngine.cs

@@ -13,7 +13,7 @@ namespace Stryker.Core.Instrumentation;
 /// <remarks>No check is made on the visibility of the variable.</remarks>
 /// <exception cref="InvalidOperationException">If the method has no body (virtual or expression form).</exception>
 /// </summary>
-/// <remarks>Parameters/variables are added on a one by one basis but they are all removed simultaneously.</remarks>
+/// <remarks>Parameters/variables are added on a one by one basis, but they are all removed simultaneously.</remarks>
 internal class DefaultInitializationEngine : BaseEngine<BlockSyntax>
 {
     private static readonly SyntaxAnnotation BlockMarker = new("InitializationBlock");
@@ -24,14 +24,14 @@ internal class DefaultInitializationEngine : BaseEngine<BlockSyntax>
     /// <param name="body">method/accessor body where to inject the assignment</param>
     /// <param name="parameters">parameters to initialize</param>
     /// <returns>the method with a block containing initializations to default for the given variables/parameters</returns>
-    public BlockSyntax AddDefaultInitializers(BlockSyntax body, IEnumerable<ParameterSyntax> parameters)
+    public BlockSyntax InjectOutParametersInitialization(BlockSyntax body, IEnumerable<ParameterSyntax> parameters)
     {
         if (body == null)
         {
             throw new InvalidOperationException(
                 "Can't add default initializer(s) to expression bodied or virtual method.");
         }
-
+        parameters = parameters.Where(p => p.Modifiers.Any(m => m.IsKind(SyntaxKind.OutKeyword))).ToList();
         if (!parameters.Any())
         {
             return body;
@@ -42,7 +42,7 @@ public BlockSyntax AddDefaultInitializers(BlockSyntax body, IEnumerable<Paramete
         if (body.Statements.Count > 0 && body.Statements[0].HasAnnotation(BlockMarker))
         {
             // we add a new initializer, we need to get the existing ones
-            initializers = (body.Statements[0] as BlockSyntax).Statements;
+            initializers = ((BlockSyntax)body.Statements[0]).Statements;
             // we can skip the initializer helper block
             originalStatements = body.Statements.Skip(1);
         }
@@ -59,7 +59,7 @@ public BlockSyntax AddDefaultInitializers(BlockSyntax body, IEnumerable<Paramete
                 p.Type.BuildDefaultExpression())))))
             .WithAdditionalAnnotations(BlockMarker);
 
-        return body.WithStatements(new SyntaxList<StatementSyntax>(originalStatements.Prepend(initializersBlock))).WithAdditionalAnnotations(Marker);
+        return body.WithStatements(new(originalStatements.Prepend(initializersBlock))).WithAdditionalAnnotations(Marker);
     }
 
     /// <summary>

src/Stryker.Core/Stryker.Core/Instrumentation/EndingReturnEngine.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Linq;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -17,7 +16,8 @@ public BlockSyntax InjectReturn(BlockSyntax block, TypeSyntax type)
         // if we had no body or the last statement is a return, no need to add one, or this is an iterator method
         if (block == null
             || block.Statements.Count == 0
-            || block.Statements.Last().Kind() == SyntaxKind.ReturnStatement
+            || block.Statements.Last().IsKind(SyntaxKind.ReturnStatement)
+            || block.Statements.Last().IsKind(SyntaxKind.ThrowStatement)
             || type.IsVoid()
             || block.ScanChildStatements(x => x.IsKind(SyntaxKind.YieldReturnStatement) || x.IsKind(SyntaxKind.YieldBreakStatement))
             || !block.ScanChildStatements(x => x.IsKind(SyntaxKind.ReturnStatement)))

src/Stryker.Core/Stryker.Core/Instrumentation/StaticInstrumentationEngine.cs

@@ -14,11 +14,12 @@ internal class StaticInstrumentationEngine : BaseEngine<BlockSyntax>
         /// injects a 'using' block with static marker class used by coverage logic.
         /// </summary>
         /// <param name="block"></param>
+        /// <param name="injector"></param>
         /// <returns></returns>
         public BlockSyntax PlaceStaticContextMarker(BlockSyntax block, CodeInjection injector) =>
             SyntaxFactory.Block( 
                 SyntaxFactory.UsingStatement(null, injector.GetContextClassConstructor(), block)).WithAdditionalAnnotations(Marker);
- 
+    
         protected override SyntaxNode Revert(BlockSyntax node)
         {
             if ( node.Statements.Count == 1 && node.Statements[0] is UsingStatementSyntax usingStatement)

src/Stryker.Core/Stryker.Core/Mutants/BaseMutantOrchestrator.cs

@@ -6,17 +6,17 @@ namespace Stryker.Core.Mutants
 {
     public abstract class BaseMutantOrchestrator
     {
-        public readonly StrykerOptions _options;
+        public readonly StrykerOptions Options;
 
         public bool MustInjectCoverageLogic =>
-            _options != null && _options.OptimizationMode.HasFlag(OptimizationModes.CoverageBasedTest) &&
-            !_options.OptimizationMode.HasFlag(OptimizationModes.CaptureCoveragePerTest);
+            Options != null && Options.OptimizationMode.HasFlag(OptimizationModes.CoverageBasedTest) &&
+            !Options.OptimizationMode.HasFlag(OptimizationModes.CaptureCoveragePerTest);
 
         public ICollection<Mutant> Mutants { get; set; }
 
         public int MutantCount { get; set; }
 
-        protected BaseMutantOrchestrator(StrykerOptions options) => _options = options;
+        protected BaseMutantOrchestrator(StrykerOptions options) => Options = options;
 
         /// <summary>
         /// Gets the stored mutants and resets the mutant list to an empty collection
@@ -34,6 +34,7 @@ public virtual IReadOnlyCollection<Mutant> GetLatestMutantBatch()
     /// Orchestrator: to arrange or manipulate, especially by means of clever or thorough planning or maneuvering.
     /// </summary>
     /// <typeparam name="T">The type of syntax node to mutate</typeparam>
+    /// <typeparam name="TY">Associated semantic model if any</typeparam>
     public abstract class BaseMutantOrchestrator<T, TY> : BaseMutantOrchestrator
     {
         protected BaseMutantOrchestrator(StrykerOptions input) : base(input)