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

Add mutator to remove shared cases #1306

Merged
merged 4 commits into from Aug 29, 2020

Conversation

Khartir
Copy link
Contributor

@Khartir Khartir commented Aug 21, 2020

This PR:

Closes #1290

Code like this:

switch ($value) {
  case 'a':
    doSomething();
    break;
  case 'b':
  case 'c':
  default:
    doSomethingElse();
    break;
}

Creates the following mutants:

switch ($value) {
  case 'a':
    doSomething();
    break;
-  case 'b':
  case 'c':
  default:
    doSomethingElse();
    break;
}
switch ($value) {
  case 'a':
    doSomething();
    break;
  case 'b':
-  case 'c':
  default:
    doSomethingElse();
    break;
}
switch ($value) {
  case 'a':
    doSomething();
    break;
  case 'b':
  case 'c':
-  default:
    doSomethingElse();
    break;
}

Copy link
Member

@maks-rafalko maks-rafalko left a comment

Choose a reason for hiding this comment

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

Thanks for working on this new Mutator! Looks good to me except minor comments above.

Question: why do we remove only shared cases? Why not to remove case by case?

src/Mutator/Removal/SharedCaseRemoval.php Outdated Show resolved Hide resolved
@maks-rafalko maks-rafalko added this to the 0.18.0 milestone Aug 25, 2020
@Khartir
Copy link
Contributor Author

Khartir commented Aug 27, 2020

@maks-rafalko I only removed shared cases because there is no way to see they are missing tests by looking at the code-coverage.
Removing a "normal" case would be like removing an entire if. I assumed that this would be better detected by the existing mutators mutating the code inside the case.
Which brings up the edge case of

case 'a':
  break;

I'm unsure about this. If you want this to remove all cases (including their body) I can do so. There should then probably also be a mutator to remove if and else.

What do you think?

Regarding the suggested changes: I'll probably look into it over the weekend.

@maks-rafalko
Copy link
Member

I'm ok with removing only shared cases :) Thanks, looking forward to getting it merged!

@sanmai sanmai self-requested a review August 29, 2020 08:13
Copy link
Member

@maks-rafalko maks-rafalko left a comment

Choose a reason for hiding this comment

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

Thank you @Khartir!

@maks-rafalko maks-rafalko merged commit 0ed4691 into infection:master Aug 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Mutator that removes cases from switch-Statements
3 participants