Skip to content

[12.x] Typed getters for Arr helper #55567

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

Merged
merged 6 commits into from
Apr 29, 2025

Conversation

tibbsa
Copy link
Contributor

@tibbsa tibbsa commented Apr 26, 2025

Adds typed getter helpers (Arr::string(), Arr::integer(), Arr::float(), Arr::boolean(), Arr::array()) as also exist on the Config facade, in an effort to make static analysis easier when using the Arr::get() helper (including tests).

Documentation PR: laravel/docs#10354

@tibbsa tibbsa changed the title Typed getters for Arr helper [12.x] Typed getters for Arr helper Apr 26, 2025
@shaedrich
Copy link
Contributor

Can't these methods be put into a trait which just calls the implementing class' get() method so we don't have to duplicate (most of) the logic for these two (and possible more) classes (in the future)?

@tibbsa
Copy link
Contributor Author

tibbsa commented Apr 26, 2025

@shaedrich

Can't these methods be put into a trait which just calls the implementing class' get() method so we don't have to duplicate (most of) the logic for these two (and possible more) classes (in the future)?

Possibly. It's a little icky because the exception error message specifically reference the 'type' that is being accessed (e.g. 'configuration keys' vs 'array keys'). Solving that either means changing the exception errors even on the Config class (possibly breaking), or adding a helper function that each using class would have to implement to identify the 'type' of content being accessed.

If the thought was that this could be used elsewhere, it might be worth doing. An earlier attempt at making wide "Typeable" objects didn't go anywhere though (#51302), so I kept this as simple and to-the-point as possible.

@shaedrich
Copy link
Contributor

shaedrich commented Apr 26, 2025

Ah, I didn't know the Typeable PR—thanks for linking it 👍🏻

Yeah, what you mentioned makes this possibly tricky—perhaps too tricky to be a good and feasible alternative

@taylorotwell taylorotwell merged commit 7f5c15f into laravel:12.x Apr 29, 2025
40 checks passed
@decadence
Copy link
Contributor

Maybe it makes sense to make this available for Collection too 🤔

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.

4 participants