Skip to content

Commit 147793a

Browse files
dupdobrouke-broersmaCopilot
authored
fix: Handle regex parsing timeout on comments (#3350)
* fix: handle regex parsing timeout on comments * chore: improve log message * feat: improve support for multiple Stryker comments * fix: logging template error in CommentParser.cs * misc: revert some stupid changes * Update src/Stryker.Core/Stryker.Core/Mutants/CsharpNodeOrchestrators/CommentParser.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Stryker.Core/Stryker.Core/Mutants/CsharpNodeOrchestrators/CommentParser.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * chore: address copilot comments --------- Co-authored-by: Rouke Broersma <mobrockers@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 0aac685 commit 147793a

File tree

4 files changed

+43
-36
lines changed

4 files changed

+43
-36
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ public static void ShouldBeSemantically(this SyntaxTree actual, SyntaxTree expec
3838

3939
Console.WriteLine(string.Join(Environment.NewLine, diff));
4040

41+
throw new ShouldAssertException("The actual syntax tree is not equivalent to the expected syntax tree. See the differences above.");
4142
}
4243
}
4344

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ void MethodA() {if(StrykerNamespace.MutantControl.IsActive(0)){}else{
8484
[TestMethod]
8585
public void ShouldMutatePatterns()
8686
{
87-
87+
8888
var source = @"public void Test()
8989
{
9090
Console.WriteLine(new[] { 1, 2, 3, 4 } is [>= 0, .., 2 or 4]);
@@ -94,10 +94,10 @@ public void ShouldMutatePatterns()
9494
{ if(StrykerNamespace.MutantControl.IsActive(0)) {}else{
9595
if(StrykerNamespace.MutantControl.IsActive(1)){;}else{Console.WriteLine((StrykerNamespace.MutantControl.IsActive(2)?new[] { 1, 2, 3, 4 } is not [>= 0, .., 2 or 4]:(StrykerNamespace.MutantControl.IsActive(5)?new[] { 1, 2, 3, 4 } is [>= 0, .., 2 and 4]:(StrykerNamespace.MutantControl.IsActive(4)?new[] { 1, 2, 3, 4 } is [< 0, .., 2 or 4]:(StrykerNamespace.MutantControl.IsActive(3)?new[] { 1, 2, 3, 4 } is [> 0, .., 2 or 4]:new[] { 1, 2, 3, 4 } is [>= 0, .., 2 or 4])))));}
9696
}
97-
}
97+
}
9898
";
9999
ShouldMutateSourceInClassToExpected(source, expected);
100-
100+
101101
}
102102

103103
[TestMethod]
@@ -955,7 +955,7 @@ public void ShouldNotMutateTopLevelStatementsIfDisabledByComment()
955955
";
956956

957957
ShouldMutateSourceToExpected(source, expected);
958-
958+
959959
}
960960

961961
[TestMethod]
@@ -1486,16 +1486,15 @@ public void ShouldControlLocalDeclarationMutationAtTheBlockLevel()
14861486
int mag = (int)Math.Log(value, 1024);
14871487
decimal adjustedSize = (decimal)value / (1L << (mag * 10));
14881488
1489-
return $""{ adjustedSize:n1} { SizeSuffixes[mag] }"";
1489+
return $""{adjustedSize:n1} {SizeSuffixes[mag]}"";
14901490
14911491
}
14921492
else {
1493-
string[] SizeSuffixes = { (StrykerNamespace.MutantControl.IsActive(2) ? """" : ""bytes""), (StrykerNamespace.MutantControl.IsActive(3) ? """" : ""KB""), (StrykerNamespace.MutantControl.IsActive(4) ? """" : ""MB""), (StrykerNamespace.MutantControl.IsActive(5) ? """" : ""GB"") };
1494-
1495-
int mag = (int)(StrykerNamespace.MutantControl.IsActive(7) ? Math.Pow(value, 1024) : (StrykerNamespace.MutantControl.IsActive(6) ? Math.Exp(value, 1024) : Math.Log(value, 1024)));
1496-
decimal adjustedSize = (StrykerNamespace.MutantControl.IsActive(8) ? (decimal)value * (1L << (mag * 10)) : (decimal)value / ((StrykerNamespace.MutantControl.IsActive(9) ? 1L >> (mag * 10) : 1L << ((StrykerNamespace.MutantControl.IsActive(10) ? mag / 10 : mag * 10)))));
1493+
string[] SizeSuffixes = { (StrykerNamespace.MutantControl.IsActive(2)?"""":""bytes""), (StrykerNamespace.MutantControl.IsActive(3)?"""":""KB""), (StrykerNamespace.MutantControl.IsActive(4)?"""":""MB""), (StrykerNamespace.MutantControl.IsActive(5)?"""":""GB"" ) };
14971494
1498-
return (StrykerNamespace.MutantControl.IsActive(11)?$"""":$""{ adjustedSize:n1} {SizeSuffixes[mag]}"");
1495+
int mag = (int)Math.Log(value, 1024);
1496+
decimal adjustedSize = (StrykerNamespace.MutantControl.IsActive(6)?(decimal)value * (1L << (mag * 10)):(decimal)value / ((StrykerNamespace.MutantControl.IsActive(8)?1L >>> (mag * 10):(StrykerNamespace.MutantControl.IsActive(7)?1L >> (mag * 10):1L << ((StrykerNamespace.MutantControl.IsActive(9)?mag / 10:mag * 10))))));
1497+
return (StrykerNamespace.MutantControl.IsActive(10)?$"""":$""{ adjustedSize:n1} {SizeSuffixes[mag]}"");
14991498
}
15001499
}
15011500
return default(string);}";

‎src/Stryker.Core/Stryker.Core.UnitTest/Mutants/StrykerCommentTests.cs‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55

66
namespace Stryker.Core.UnitTest.Mutants;
77

8-
internal class StrykerCommentTests : MutantOrchestratorTestsBase
8+
[TestClass]
9+
public class StrykerCommentTests : MutantOrchestratorTestsBase
910
{
1011
[TestMethod]
1112
public void ShouldNotMutateIfDisabledByComment()
@@ -28,7 +29,7 @@ public void ShouldNotMutateIfDisabledByComment()
2829
source = @"public void SomeMethod() {
2930
var x = 0;
3031
{
31-
// Stryker disable all
32+
// Stryker disable all
3233
x++;
3334
}
3435
x/=2;
@@ -50,15 +51,15 @@ public void ShouldNotMutateIfDisabledByMultilineComment()
5051
{
5152
var source = @"public void SomeMethod() {
5253
var x = 0;
53-
if (condition && other) /* Stryker disable once all */ {
54+
if (condition && other) /* Stryker disable once all */ {
5455
x++;
5556
x*=2;
5657
}
5758
x/=2;
5859
}";
5960
var expected = @"public void SomeMethod() {if(StrykerNamespace.MutantControl.IsActive(0)){}else{
6061
var x = 0;
61-
if ((StrykerNamespace.MutantControl.IsActive(2)?!(condition && other):(StrykerNamespace.MutantControl.IsActive(1)?condition || other:condition && other))) /* Stryker disable once all */ {
62+
if ((StrykerNamespace.MutantControl.IsActive(2)?!(condition && other):(StrykerNamespace.MutantControl.IsActive(1)?condition || other:condition && other))) /* Stryker disable once all */ {
6263
x++;
6364
x*=2;
6465
}
@@ -97,7 +98,7 @@ public void ShouldNotMutateIfDisabledByMultilineComment()
9798
@"if ((StrykerNamespace.MutantControl.IsActive(1)?!(cond):cond)) // Stryker disable once all
9899
x++;")]
99100
[DataRow("if (/* Stryker disable once all*/cond) x++;", "if (/* Stryker disable once all*/cond) if(StrykerNamespace.MutantControl.IsActive(2)){;}else{if(StrykerNamespace.MutantControl.IsActive(3)){x--;}else{x++;}}")]
100-
101+
101102
public void ShouldNotMutateDependingOnWhereMultilineCommentIs(string source, string expected)
102103
{
103104
// must call reset as MsTest reuse test instance on/datarow
@@ -146,17 +147,17 @@ public void ShouldMutateIfInvalidComment()
146147
}
147148

148149
[TestMethod]
149-
public void ShouldOnlyUseFirstComment()
150+
public void ShouldUseEveryComment()
150151
{
151152
// enabling Stryker comment should have no impact
152153
var source = @"public void SomeMethod() {
153-
/* Stryker disable once all */
154-
// Stryker restore all
154+
/* Stryker restore all */
155+
// Stryker disable all
155156
x++;
156157
}";
157158
var expected = @"public void SomeMethod() {if(StrykerNamespace.MutantControl.IsActive(0)){}else{
158-
/* Stryker disable once all */
159-
// Stryker restore all
159+
/* Stryker restore all */
160+
// Stryker disable all
160161
x++;
161162
}}";
162163

‎src/Stryker.Core/Stryker.Core/Mutants/CsharpNodeOrchestrators/CommentParser.cs‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Diagnostics.CodeAnalysis;
23
using System.Linq;
34
using System.Text;
45
using System.Text.RegularExpressions;
@@ -69,33 +70,38 @@ private static MutationContext ParseStrykerComment(MutationContext context, Matc
6970

7071
public static MutationContext ParseNodeLeadingComments(SyntaxNode node, MutationContext context)
7172
{
72-
var processedComments = node.GetFirstToken(true).GetPreviousToken(true).TrailingTrivia
73-
.Union(node.GetLeadingTrivia())
74-
.Where(t => t.IsKind(SyntaxKind.MultiLineCommentTrivia) || t.IsKind(SyntaxKind.SingleLineCommentTrivia))
75-
.Select(t => (ProcessComment(node, context, t.ToString()), t)).Where(t => t.Item1 != null).ToList();
73+
var comments = node.GetFirstToken(true).GetPreviousToken(true)
74+
.TrailingTrivia.Union(node.GetLeadingTrivia())
75+
.Where(t => t.IsKind(SyntaxKind.MultiLineCommentTrivia) || t.IsKind(SyntaxKind.SingleLineCommentTrivia)).ToList();
76+
var result = comments.Aggregate(context, (current, t) => ProcessComment(node, current, t.ToString()));
7677

77-
if (processedComments.Count == 0)
78+
return result;
79+
}
80+
81+
[ExcludeFromCodeCoverage(Justification = "Difficult to test timeouts")]
82+
private static MutationContext ProcessComment(SyntaxNode node, MutationContext context, string commentTrivia)
83+
{
84+
try
7885
{
79-
return context;
86+
return InterpretStrykerComment(node, context, commentTrivia);
8087
}
81-
if (processedComments.Count > 1)
88+
catch (RegexMatchTimeoutException exception)
8289
{
83-
var errorMessage = new StringBuilder().Append(
84-
$"Multiple Stryker comments at {node.GetLocation().GetMappedLineSpan().StartLinePosition}, {node.SyntaxTree.FilePath}. Only the first one will be used");
85-
errorMessage.Append(string.Join(Environment.NewLine, processedComments.Select(c => c.Item2.ToString())));
86-
Logger.LogWarning(errorMessage.ToString());
87-
90+
Logger.LogWarning(exception,
91+
"Parsing Stryker comments at {StartLinePosition}, {FilePath} took too long to parse and was ignored. Comment: {Comment}",
92+
node.GetLocation().GetMappedLineSpan().StartLinePosition,
93+
node.SyntaxTree.FilePath, commentTrivia);
94+
return context;
8895
}
89-
return processedComments[0].Item1;
9096
}
9197

92-
private static MutationContext ProcessComment(SyntaxNode node, MutationContext context, string commentTrivia)
98+
private static MutationContext InterpretStrykerComment(SyntaxNode node, MutationContext context, string commentTrivia)
9399
{
94100
// perform a quick pattern check to see if it is a 'Stryker comment'
95101
var strykerCommentMatch = Pattern.Match(commentTrivia);
96102
if (!strykerCommentMatch.Success)
97103
{
98-
return null;
104+
return context;
99105
}
100106

101107
// now we can extract actual command
@@ -113,6 +119,6 @@ private static MutationContext ProcessComment(SyntaxNode node, MutationContext c
113119
"Invalid Stryker comments at {Position}, {FilePath}.",
114120
node.GetLocation().GetMappedLineSpan().StartLinePosition,
115121
node.SyntaxTree.FilePath);
116-
return null;
122+
return context;
117123
}
118124
}

0 commit comments

Comments
 (0)