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

New mutator proposal: remove item from literal array #597

Closed
MidnightDesign opened this issue Jan 3, 2019 · 4 comments · Fixed by #649
Closed

New mutator proposal: remove item from literal array #597

MidnightDesign opened this issue Jan 3, 2019 · 4 comments · Fixed by #649

Comments

@MidnightDesign
Copy link
Sponsor Contributor

MidnightDesign commented Jan 3, 2019

TL;DR: A "remove item from literal array" mutator would be great.

Is your feature request related to a problem? Please describe.
I'm doing kind of a fingerprinting/checksum thing, like:

$relevantData = [$item->foo(), $item->bar()];
$fingerprint = md5(json_encode($data));

I just added a third item to the list of relevant data: [$item->foo(), $item->bar(), $item->baz()]. A little later I realized that whenever the output of baz() changes, the output of foo() changes too. So I could remove the first item and have the (functionally) same thing.

Infection didn't check if the array item was required.

Describe the solution you'd like
Infection should warn me that an element in an array literal could be removed with the tests still passing.

@localheinz
Copy link
Member

localheinz commented Feb 17, 2019

I think it is good idea - I believe the challenge here is to decide how many mutations we want to yield from such a mutator.

@maks-rafalko
Copy link
Member

maks-rafalko commented Feb 17, 2019

Since we are trying to keep balance between the number of generated mutants and the speed of infection, I would say that generating N mutations for an array with N elements is too much.

What about generating 1 mutation by default (e.g. removing the first element) and have a setting to enable N mutations if someone needs a hardcore mode? :)

{
    "mutators": {
        "RemoveArrayItem": {
             "settings": {
                 "remove": "first" (first|last|all)
             }
        }
    }
}

@MidnightDesign
Copy link
Sponsor Contributor Author

MidnightDesign commented Feb 17, 2019

Maybe something like "N mutations for an array with N elements, up to X elements. If there are more than X elements, generate only one mutation for the first element". (Where X could be configured.)

@maks-rafalko maks-rafalko added this to the 0.13.0 milestone Feb 17, 2019
@localheinz
Copy link
Member

localheinz commented Feb 17, 2019

Perhaps something like using a --level option, similar to phpstan/phpstan?

Then it wouldn't be necessary to have a complicated configuration, but something that applies to all mutators that could potentially yield more than one mutation?

Also see #636.

majkel89 added a commit to majkel89/infection that referenced this issue Mar 3, 2019
majkel89 added a commit to majkel89/infection that referenced this issue Mar 3, 2019
majkel89 added a commit to majkel89/infection that referenced this issue Mar 3, 2019
majkel89 added a commit to majkel89/infection that referenced this issue Mar 3, 2019
majkel89 added a commit to majkel89/infection that referenced this issue Mar 4, 2019
@helpr helpr bot added the Has PR label Mar 6, 2019
majkel89 added a commit to majkel89/infection that referenced this issue Mar 6, 2019
majkel89 added a commit to majkel89/infection that referenced this issue Mar 6, 2019
majkel89 pushed a commit to majkel89/infection that referenced this issue Mar 8, 2019
@ghost ghost removed the Needs Review label Mar 9, 2019
maks-rafalko pushed a commit that referenced this issue Mar 9, 2019
* #597 Array item removal mutator

* #597 fixes

* #597 add missing declere strict types

* #597 fix limit setting

* #597 add type hints

* #597 some tweaks

* #597 code style fix

* #597 phpdoc fix
@helpr helpr bot added PR merged and removed Has PR labels Mar 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants