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

Allow missing elements when destructuring arrays in foreach loops #1880

Merged

Conversation

MidnightDesign
Copy link
Contributor

@MidnightDesign MidnightDesign commented Oct 9, 2023

This PR:

  • Covered by tests

This fixes a bug where Infection threw an exception when generating mutations for a codebase containing <?php foreach ($items as [, $value]) { }.

The issue was an assertion that checked whether all elements in Array_#$items were non-null.

I'm not sure this is the ideal solution for this problem. Maybe we do want to generate mutants in this position. But for now, it will at least no longer throw.

If I just removed the assertion and widened the argument type on getItemsIndexes, then it would generate a mutant removing the empty first element, but I'm unsure whether that's an interesting mutation to have.

Assert::allNotNull($node->items);
$parent = $node->getAttribute('parent');
// Don't mutate destructured values in foreach loops
if ($parent instanceof Node\Stmt\Foreach_ && $parent->valueVar === $node) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be in the canMutate() method below? Since there we are deciding whether to mutate Node or not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I totally overlooked that method. An update is on its way.

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 @MidnightDesign

@maks-rafalko maks-rafalko merged commit 2789fdd into infection:master Oct 9, 2023
40 checks passed
@MidnightDesign MidnightDesign deleted the destructuring-missing-element branch October 9, 2023 12:08
@maks-rafalko
Copy link
Member

Released https://github.com/infection/infection/releases/tag/0.27.4

github-merge-queue bot referenced this pull request in Lendable/clock Oct 9, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [infection/infection](https://togithub.com/infection/infection) |
`^0.27.3` -> `^0.27.4` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/infection%2finfection/0.27.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/infection%2finfection/0.27.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/infection%2finfection/0.27.3/0.27.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/infection%2finfection/0.27.3/0.27.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [php-cs-fixer/shim](https://togithub.com/PHP-CS-Fixer/shim) |
`^3.34.0` -> `^3.34.1` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/php-cs-fixer%2fshim/3.34.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/php-cs-fixer%2fshim/3.34.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/php-cs-fixer%2fshim/3.34.0/3.34.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/php-cs-fixer%2fshim/3.34.0/3.34.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [phpstan/phpstan](https://togithub.com/phpstan/phpstan) | `^1.10.37`
-> `^1.10.38` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/phpstan%2fphpstan/1.10.38?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/phpstan%2fphpstan/1.10.38?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/phpstan%2fphpstan/1.10.37/1.10.38?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/phpstan%2fphpstan/1.10.37/1.10.38?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [phpunit/phpunit](https://phpunit.de/)
([source](https://togithub.com/sebastianbergmann/phpunit)) | `^10.3.5`
-> `^10.4.1` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/phpunit%2fphpunit/10.4.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/phpunit%2fphpunit/10.4.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/phpunit%2fphpunit/10.3.5/10.4.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/phpunit%2fphpunit/10.3.5/10.4.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>infection/infection (infection/infection)</summary>

###
[`v0.27.4`](https://togithub.com/infection/infection/releases/tag/0.27.4):
Allow missing elements when destructuring arrays in foreach loops

[Compare
Source](https://togithub.com/infection/infection/compare/0.27.3...0.27.4)

**Fixed:**

- Allow missing elements when destructuring arrays in foreach loops by
[@&#8203;MidnightDesign](https://togithub.com/MidnightDesign) in
[https://github.com/infection/infection/pull/1880](https://togithub.com/infection/infection/pull/1880)

**Full Changelog**:
infection/infection@0.27.3...0.27.4

</details>

<details>
<summary>PHP-CS-Fixer/shim (php-cs-fixer/shim)</summary>

###
[`v3.34.1`](https://togithub.com/PHP-CS-Fixer/shim/compare/v3.34.0...v3.34.1)

[Compare
Source](https://togithub.com/PHP-CS-Fixer/shim/compare/v3.34.0...v3.34.1)

</details>

<details>
<summary>phpstan/phpstan (phpstan/phpstan)</summary>

###
[`v1.10.38`](https://togithub.com/phpstan/phpstan/releases/tag/1.10.38)

[Compare
Source](https://togithub.com/phpstan/phpstan/compare/1.10.37...1.10.38)

# Improvements 🔧

- Remove function signatures that were removed in PHP 8.0
([#&#8203;2662](https://togithub.com/phpstan/phpstan-src/pull/2662)),
[#&#8203;9954](https://togithub.com/phpstan/phpstan/issues/9954), thanks
[@&#8203;Th3Cod3](https://togithub.com/Th3Cod3)!

# Bugfixes 🐛

-   Update BetterReflection
- Fix for non-backed enums
(ondrejmirtes/BetterReflection@4dd0f2b)
- Do not invalidate types passed by value
(phpstan/phpstan-src@0b8dca7),
[https://github.com/phpstan/phpstan/discussions/9927](https://togithub.com/phpstan/phpstan/discussions/9927)
- Do not influence types of arguments before the function is called
(phpstan/phpstan-src@c45d42d)
- Fixed collapsing of constant arrays
(phpstan/phpstan-src@746de74),
[#&#8203;9985](https://togithub.com/phpstan/phpstan/issues/9985)

# Internals 🔍

- Lazier type-resolving in `isset` checks
([#&#8203;2664](https://togithub.com/phpstan/phpstan-src/pull/2664)),
thanks [@&#8203;staabm](https://togithub.com/staabm)!
- Refactor to invalidate args inside `processArgs`
(phpstan/phpstan-src@52eb6f8)
- Merge array types a bit later
(phpstan/phpstan-src@adbc35a)

</details>

<details>
<summary>sebastianbergmann/phpunit (phpunit/phpunit)</summary>

###
[`v10.4.1`](https://togithub.com/sebastianbergmann/phpunit/compare/10.4.0...10.4.1)

[Compare
Source](https://togithub.com/sebastianbergmann/phpunit/compare/10.4.0...10.4.1)

###
[`v10.4.0`](https://togithub.com/sebastianbergmann/phpunit/compare/10.3.5...10.4.0)

[Compare
Source](https://togithub.com/sebastianbergmann/phpunit/compare/10.3.5...10.4.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/Lendable/clock).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4wLjMiLCJ1cGRhdGVkSW5WZXIiOiIzNy44LjEiLCJ0YXJnZXRCcmFuY2giOiJtYXN0ZXIifQ==-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Ben Challis <ben@lendable.co.uk>
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.

2 participants