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

Open
wants to merge 5 commits into
base: 12.x
Choose a base branch
from

Conversation

tibbsa
Copy link

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

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

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

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.

2 participants