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
Enhancement: Implement UnwrapArrayReduce mutator #533
Enhancement: Implement UnwrapArrayReduce mutator #533
Conversation
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.
π
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.
π
@sanmai as I remember, you suggested to unwrap the 3rd parameter, not the 1st.
Some thoughts about it:
It means that the result of the This, in its turn, means that unrapping the 1st argument instead of 3rd can lead to type mismatches, which will immediately lead to incorrectly killed mutant. Example: - function getSum(array $numbers): int {
- return array_reduce($numbers, function sum($carry, $item) {
- $carry += $item;
- return $carry;
- },
- 0
- );
- }
// incorrect mutation
+ function getSum(array $numbers): int {
+ return $numbers; // <---------- array instead of int
+ }
// correct mutation
+ function getSum(array $numbers): int {
+ return 0;
+ } Probably we can think about all possible cases and produce mutation depending on context, existence of the 3rd parameter, etc.? WDYT? |
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.
Really this has to be about the last argument, not about first as a whole at very least. Sorry about previous review!
I wonder where I was looking... True, unwrapping the first argument only makes hardly any sense, most of the time itβll be a definite error. On the other hand, thereβs something about the first argument... |
@borNfreee @sanmai You are right, this makes a lot of sense! |
bdc4fa5
to
038d387
Compare
@@ -63,6 +63,10 @@ | |||
return false; | |||
} | |||
|
|||
if (!array_key_exists($this->getParameterIndex(), $node->args)) { |
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.
Instead of array_key_exists()
, would you prefer me to use count()
?
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 feel like array_key_exists()
is most explicit for a generic case of this mutator.
count()
is not such, adds extra limitations.
First argument opens possibility for an additional mutator: - array_reduce($array, $func, $initial);
+ $array[0]; I guess this should be a purpose of a different PR. |
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.
π π
Thank you @localheinz |
Thank you, @BackEndTea, @borNfreee, @sanmai, and @sidz! |
This PR
UnwrapArrayReduce
mutatorRelated to #514.
πββοΈ Follows the example in #513.