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

feat(mutators): Add default parameter mutator. #2863

Closed
wants to merge 3 commits into from

Conversation

Liam-Rougoor
Copy link
Contributor

Closes #2848.

Copy link

sonarcloud bot commented Mar 1, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
1 New Minor Issues (required ≤ 0)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

Copy link
Member

@richardwerkman richardwerkman left a 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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public interface ICsharpMutantOrchestrator
public interface ICSharpMutantOrchestrator

Copy link
Contributor Author

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...?

Copy link
Member

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;
Copy link
Member

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)
Copy link
Member

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?

Copy link
Member

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

@Liam-Rougoor
Copy link
Contributor Author

Merging with master was a bit of a hassle, it was easier to recreate #3024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mutator for default arguments.
3 participants