Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integer mutators #2968

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2,609 changes: 1,445 additions & 1,164 deletions src/Stryker.Core/Stryker.Core.UnitTest/Mutants/CsharpMutantOrchestratorTests.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
using Microsoft.CodeAnalysis.CSharp;
using Shouldly;
using Stryker.Core.Mutators;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Xunit;

namespace Stryker.Core.UnitTest.Mutators;

public class IntegerNegationMutatorTests : TestBase
{
[Fact]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have recently migrated all unit tests from xunit to mstest, sorry! Could you migrate your unit tests to mstest?

public void ShouldBeMutationLevelStandard()
{
var target = new IntegerNegationMutator();
target.MutationLevel.ShouldBe(MutationLevel.Standard);
}

[Theory]
[InlineData(10, -10)]
[InlineData(-10, 10)]
public void ShouldMutate(int original, int expected)
{
var target = new IntegerNegationMutator();
var parent = SyntaxFactory.EqualsValueClause(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(original)));

var result = target.ApplyMutations(parent.DescendantNodes().First() as LiteralExpressionSyntax, null).ToList();

var mutation = result.ShouldHaveSingleItem();

mutation.ReplacementNode.ShouldBeOfType<LiteralExpressionSyntax>()
.Token.Value.ShouldBe(expected);
mutation.DisplayName.ShouldBe("Number negation mutation");
}

[Theory]
[InlineData(SyntaxKind.StringLiteralExpression)]
[InlineData(SyntaxKind.CharacterLiteralExpression)]
[InlineData(SyntaxKind.NullLiteralExpression)]
[InlineData(SyntaxKind.DefaultLiteralExpression)]
[InlineData(SyntaxKind.TrueLiteralExpression)]
[InlineData(SyntaxKind.FalseLiteralExpression)]
public void ShouldNotMutate(SyntaxKind original)
{
var target = new IntegerNegationMutator();

var result = target.ApplyMutations(SyntaxFactory.LiteralExpression(original), null).ToList();

Assert.Empty(result);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
using Microsoft.CodeAnalysis.CSharp;
using Shouldly;
using Stryker.Core.Mutators;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Xunit;

namespace Stryker.Core.UnitTest.Mutators;

public class IntegerNullificationMutatorTests : TestBase
{
[Fact]
public void ShouldBeMutationLevelStandard()
{
var target = new IntegerNullificationMutator();
target.MutationLevel.ShouldBe(MutationLevel.Standard);
}

[Theory]
[InlineData(10, 0)]
[InlineData(-10, 0)]
public void ShouldMutate(int original, int expected)
{
var target = new IntegerNullificationMutator();

var parent = SyntaxFactory.EqualsValueClause(SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(original)));

var result = target.ApplyMutations(parent.DescendantNodes().First() as LiteralExpressionSyntax, null).ToList();

var mutation = result.ShouldHaveSingleItem();

mutation.ReplacementNode.ShouldBeOfType<LiteralExpressionSyntax>()
.Token.Value.ShouldBe(expected);
mutation.DisplayName.ShouldBe("Integer nullification mutation");
}

[Theory]
[InlineData("array[1]")]
public void ShouldNotMutate(string expression)
{
var target = new IntegerNullificationMutator();

var parent = SyntaxFactory.ParseExpression(expression);
var child = parent.DescendantNodes(_ => true).OfType<LiteralExpressionSyntax>().FirstOrDefault();


var result = target.ApplyMutations(child, null).ToList();

Assert.Empty(result);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public void ShouldValidateExcludedMutation()

var ex = Should.Throw<InputException>(() => target.Validate<Mutator>());

ex.Message.ShouldBe($"Invalid excluded mutation (gibberish). The excluded mutations options are [Statement, Arithmetic, Block, Equality, Boolean, Logical, Assignment, Unary, Update, Checked, Linq, String, Bitwise, Initializer, Regex, NullCoalescing, Math, StringMethod, Conditional]");
ex.Message.ShouldBe($"Invalid excluded mutation (gibberish). The excluded mutations options are [Statement, Arithmetic, Block, Equality, Boolean, Logical, Assignment, Unary, Update, Checked, Linq, String, Bitwise, Initializer, Regex, NullCoalescing, Math, StringMethod, Conditional, Number]");
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ private static List<IMutator> DefaultMutatorList() =>
new MathMutator(),
new SwitchExpressionMutator(),
new IsPatternExpressionMutator(),
new StringMethodMutator()
new StringMethodMutator(),
new IntegerNullificationMutator(),
new IntegerNegationMutator()
];

private IEnumerable<IMutator> Mutators { get; }
Expand Down
36 changes: 36 additions & 0 deletions src/Stryker.Core/Stryker.Core/Mutators/IntegerNegationMutator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
using System.Collections.Generic;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Stryker.Core.Mutants;

namespace Stryker.Core.Mutators;

public class IntegerNegationMutator : MutatorBase<LiteralExpressionSyntax>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why having two separate mutators?

{
public override MutationLevel MutationLevel => MutationLevel.Standard;

public override IEnumerable<Mutation> ApplyMutations(LiteralExpressionSyntax node, SemanticModel semanticModel)
{
if (!IsNumberLiteral(node) || node.Token.Value is not (int currentValue and not 0))
{
yield break;
}

var replacementValue = -currentValue;
yield return new Mutation
{
OriginalNode = node,
ReplacementNode = SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(replacementValue)),
DisplayName = "Number negation mutation",
Type = Mutator.Number
};
}

private static bool IsNumberLiteral(LiteralExpressionSyntax node)
{
var kind = node.Kind();
return kind == SyntaxKind.NumericLiteralExpression && node.Parent is EqualsValueClauseSyntax;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why limiting this mutation to assignment ? it seems that it could be uses within any expression (excluding indexed access [...] ). It should at least also support method arguments.

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using System.Collections.Generic;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Stryker.Core.Mutants;

namespace Stryker.Core.Mutators;

public class IntegerNullificationMutator : MutatorBase<LiteralExpressionSyntax>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why having two separate mutators ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured they can possibly be conflicting, and so I thought it'd be a good idea to add them separately.
When mutating var x = 10 for instance, would this be var x = 0? var x = -10?
Maybe I'm all wrong about this and it doesn't work this way at all: if this is not an issue it could be a single mutator.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stryker's mutation orchestration takes care of this. A mutator needs only to generate a mutated version of some syntax construct.
Using separate mutators allows the users to fine grain the mutation they want, assuming each mutator uses a different Type (not the case here). Having multiple mutators sharing the same type may be interesting for code clarity when some mutators are complex (not the case here)

{
public override MutationLevel MutationLevel => MutationLevel.Standard;

public override IEnumerable<Mutation> ApplyMutations(LiteralExpressionSyntax node, SemanticModel semanticModel)
{
if (IsNumberLiteral(node) && node.Token.Value is int value && value != 0)
{
yield return new Mutation
{
OriginalNode = node,
ReplacementNode = SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(0)),
DisplayName = "Integer nullification mutation",
Type = Mutator.Number
};
}
}

private static bool IsNumberLiteral(LiteralExpressionSyntax node)
{
var kind = node.Kind();
return kind == SyntaxKind.NumericLiteralExpression && node.Parent is EqualsValueClauseSyntax;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why limiting this mutation to assignment ? it seems that it could be uses within any expression. It should at least also support method arguments.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great idea! initially, I didn't have a constraint on the node parent at all so all occurrences would be mutated. I then constrained it to at least work for the example in the original issue, and I kinda left it open for suggestions if other places should receive this mutation as well

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

limitation should only be placed if one detect an actual issue. But most (if not all) risky constructs, such as constants or attributes, are already identified and dealt with.

}
}
4 changes: 3 additions & 1 deletion src/Stryker.Core/Stryker.Core/Mutators/Mutator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ public enum Mutator
[MutatorDescription("String Method")]
StringMethod,
[MutatorDescription("Conditional operators")]
Conditional
Conditional,
[MutatorDescription("Number literals")]
Number
}

public static class EnumExtension
Expand Down
Loading