Skip to content

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

Closed
Liam-Rougoor wants to merge 3 commits intomasterfrom
default-parameter-mutator
Closed

feat(mutators): Add default parameter mutator.#2863
Liam-Rougoor wants to merge 3 commits intomasterfrom
default-parameter-mutator

Conversation

@Liam-Rougoor
Copy link
Copy Markdown
Contributor

Closes #2848.

# Conflicts:
#	src/Stryker.Core/Stryker.Core/Mutants/CsharpMutantOrchestrator.cs
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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