-
Notifications
You must be signed in to change notification settings - Fork 188
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
feat(mutators): Add default parameter mutator. #2863
Conversation
# Conflicts: # src/Stryker.Core/Stryker.Core/Mutants/CsharpMutantOrchestrator.cs
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! But we might need to think about some things before merging this
|
||
namespace Stryker.Core.Mutants; | ||
|
||
public interface ICsharpMutantOrchestrator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public interface ICsharpMutantOrchestrator | |
public interface ICSharpMutantOrchestrator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original class is also called CsharpMutantOrchestrator
..
Do you want me to keep it as is, or do you want me to rename both the interface and the class to CSharp...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be best to fix the casing
} | ||
} | ||
|
||
public override MutationLevel MutationLevel => MutationLevel.Standard; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, I would place this at the top of the class, not here. Like in our other mutator classes. And I think we should debate whether this should be a level standard mutation. This could add a lot of mutations.
public override IEnumerable<Mutation> ApplyMutations(InvocationExpressionSyntax node, SemanticModel semanticModel) | ||
{ | ||
var methodSymbol = (IMethodSymbol)semanticModel.GetSymbolInfo(node).Symbol; | ||
if (methodSymbol is null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should check whether the symbol is internal or external. We might not want to mutate optional parameters of external APIs like System.Linq
. @rouke-broersma what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how we would be able to detect that
Merging with master was a bit of a hassle, it was easier to recreate #3024 |
Closes #2848.