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

[Mutator]: Spread operator in Array Expression - leave only the first element #784

Merged
merged 11 commits into from
Sep 17, 2019

Conversation

maks-rafalko
Copy link
Member

@maks-rafalko maks-rafalko commented Sep 13, 2019

RFC: https://wiki.php.net/rfc/spread_operator_for_array
PHP 7.4+

This mutator does the following mutations:

- $a = [...[1, 2, 3], 4];
+ $a = [[1, 2, 3][0], 4];

- $a = [...$collection, 4];
+ $a = [$collection[0], 4];

- $a = [...getCollection(), 4];
+ $a = [getCollection()[0], 4];

The only one issue I think will be with this mutator, is that it can work with any \Traversable, including \Generator, and the following mutation will be self-killed:

function arrGen() {
    for($i = 11; $i < 15; $i++) {
        yield $i;
    }
}

- $arr = [...arrGen()];
+ $arr = [arrGen()[0]];

Produces an error:

Fatal error: Uncaught Error: Cannot use object of type Generator as array in ...

What can we do?

  1. The first and the best solution is to exactly know what is the return type hint of arrGen() function and knowing that, do either arrGen()[0] for array or iterator_to_array(arrGen())[0] for iterator. However, unfortunately, Infection is not able to guess types at the moment (as for example PHPStan/Psalm do it)

  2. The second option is to always do such mutation

- $arr = [...arrGen()];
+ $_traversable = arrGen();
+ $arr = [is_array($_traversable) ? $_traversable[0] : iterator_to_array($_traversable))[0];
  1. Do not mutate variables and functions and methods, only raw arrays (the very first diff above)

Thoughts?

@sanmai
Copy link
Member

sanmai commented Sep 13, 2019

Great idea! I'm all in for the variant with an is_array check. It shall be the most useful.

@maks-rafalko
Copy link
Member Author

maks-rafalko commented Sep 13, 2019

@sanmai no problem, implemented. The new mutations are:

- $a = [...[1, 2, 3], 4];
+ $a = [[1, 2, 3][0], 4];

- $a = [...$collection, 4];
+ $a = [is_array($collection) ? $collection[0] : iterator_to_array($collection)[0], 4];

- $a = [...getCollection(), 4];
+ $a = [is_array(getCollection()) ? getCollection()[0] : iterator_to_array(getCollection())[0], 4];

```
Parse error: syntax error, unexpected '...' (T_ELLIPSIS), expecting ']'
```
```
Parse error: syntax error, unexpected '...' (T_ELLIPSIS), expecting ']'
```
@sanmai
Copy link
Member

sanmai commented Sep 14, 2019

I wish iterator_to_array would take arrays, returning arrays as they were! Oh well...

It may be better to use reset() because PHP will happily unroll non-zero indexed arrays.

$array = [1 => 'bar','baz'];
$example = [...$array, 'foo'];

@maks-rafalko
Copy link
Member Author

makes sense, updated the code

@maks-rafalko
Copy link
Member Author

maks-rafalko commented Sep 14, 2019

Unfortunately, reset(arrGen()) produces a notice:

Notice: Only variables should be passed by reference

I see only one solution - introduce a variable:

- $arr = [...arrGen()];
+ $_traversable = arrGen();
+ $arr = [is_array($_traversable) ? reset($_traversable) : reset(iterator_to_array($_traversable)));

UPD:

we can also use array_slice that makes new keys: https://3v4l.org/JKM6J

function getArray() {
    return [1 => 11, 2 => 22];
}

array_slice(getArray(), 0, 1)[0]; // 11

and the diff

- $arr = [...arrGen()];
+ $arr = [array_slice(is_array($_traversable) ? $_traversable : iterator_to_array($_traversable), 0, 1)[0]];

@maks-rafalko
Copy link
Member Author

Updated to use array_slice as discussed above. For integer keys, array_slice returns a new array with reset indexes, started from 0 and we safely using array_slice(...)[0].

If the array has string keys, it's not a problem, because spread operator can not work with string keys.

@sanmai
Copy link
Member

sanmai commented Sep 16, 2019

Yes, otherwise put you can't use a spread operator with such an array in the first place.

Do you think this can work?

$a = [[...$collection][0], 4];

@maks-rafalko
Copy link
Member Author

maks-rafalko commented Sep 16, 2019

Do you think this can work?

$a = [[...$collection][0], 4];

Not sure I got the question. If this is the original code before mutation, then the current state of PR works nice with it:

$collection = [5, 6, 7];

- $a = [[...$collection][0], 4];
+ $a = [[array_slice(is_array($collection) ? $collection : iterator_to_array($collection), 0, 1][0], 4];

which results to

[[5][0], 4]

// and then 

[5, 4]

@sanmai
Copy link
Member

sanmai commented Sep 16, 2019

I'm more about this mutation:

- $a = [...$collection, 4];
+ $a = [[...$collection][0], 4];

- $a = [...getCollection(), 4];
+ $a = [[...getCollection()][0], 4];

If it works, we should be able to cut on array_slice() and so forth.

@maks-rafalko
Copy link
Member Author

👏 awesome

Updated the code and added more tests for [...new ArrayIterator(['a', 'b', 'c'])] case from RFC

@maks-rafalko
Copy link
Member Author

Appveyor fail is unrelated, see #786

@maks-rafalko maks-rafalko added this to the 0.14.0 milestone Sep 17, 2019
@maks-rafalko maks-rafalko merged commit f4d229c into master Sep 17, 2019
@maks-rafalko maks-rafalko deleted the feature/unpack-array branch September 17, 2019 09:32
@maks-rafalko
Copy link
Member Author

Thank you @sanmai for review and ideas

@maks-rafalko maks-rafalko changed the title [RFC] Mutator: Spread operator in Array Expression - leave only the first element [Mutator]: Spread operator in Array Expression - leave only the first element Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants