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

[9.x] Support Enumerable in Stringable #44012

Merged

Conversation

erikgaal
Copy link
Contributor

@erikgaal erikgaal commented Sep 5, 2022

This PR adds simple quality of life improvement which allows us to pass Enumerable objects such as Collection to the Stringable helper functions that already accept array as a parameter.

For example, in a project I'm working on, we're doing the following:

/** @var Collection<array-key, string> $months */
$months = months();

Str::remove($months->toArray(), $someString);

With this PR we can skip the ->toArray call.

/** @var Collection<array-key, string> $months */
$months = months();

Str::remove($months, $someString);

@erikgaal erikgaal force-pushed the patch-stringable-replace-enumerable branch from 64a4f93 to f6aa6c2 Compare September 5, 2022 14:48
@@ -242,12 +246,16 @@ public static function contains($haystack, $needles, $ignoreCase = false)
* Determine if a given string contains all array values.
*
* @param string $haystack
* @param string[] $needles
* @param string[]|Enumerable<array-key, string> $needles
Copy link
Contributor

Choose a reason for hiding this comment

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

@taylorotwell taylorotwell merged commit d44c345 into laravel:9.x Sep 5, 2022
@erikgaal erikgaal deleted the patch-stringable-replace-enumerable branch September 6, 2022 07:41
@erikgaal
Copy link
Contributor Author

erikgaal commented Sep 6, 2022

The code works, but PHPStan throws an error now even when passing an array. Will investigate and send a PR to fix.

https://phpstan.org/r/363ce01b-696f-4e26-8c90-ba9f93b943e8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants