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.

Sign up for GitHub

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

Jump to bottom

test: regression test for mapping custom simple collections #1899

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Open

test: regression test for mapping custom simple collections #1899

wants to merge 1 commit into from

Conversation

erikgaal
Copy link
Contributor

@erikgaal erikgaal commented Apr 2, 2024 •
edited
Loading

  • Added or updated tests
  • Documented user facing changes

Since 2.9.1 a change was introduced to Collections where calling map or mapWithKeys on a custom \Illuminate\Support\Collection would make PHPStan report returning a basic Collection, even though the custom Collection was actually returned.

On a79b46b9 running this testcase results in the following failure:

1) Tests\Type\CollectionDynamicReturnTypeExtensionsTest::testFileAsserts with data set "/Users/erikgaal/Dev/larastan/tests/Type/data/collection-generic-static-methods-l948.php:204" ('type', '/Users/erikgaal/Dev/larastan...48.php', 'App\FavoritesCollection', 'Illuminate\Support\Collection...rites>', 204)
Expected type App\FavoritesCollection, got type Illuminate\Support\Collection<(int|string), App\ValueObjects\Favorites> in /Users/erik.gaal/Dev/larastan/tests/Type/data/collection-generic-static-methods-l948.php on line 204.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'App\FavoritesCollection'
+'Illuminate\Support\Collection<(int|string), App\ValueObjects\Favorites>'

Changes

This has since been fixed in trunk, but no tests exist for this scenario. This PR adds regression tests for these two scenario's.

Breaking changes

None.

@canvural
Copy link
Collaborator

Hi,

If you can update the PR with the latest changes we might be able to see if the checks are passing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@erikgaal @canvural