[12.x] Typed getters for Env helper #55658
Conversation
src/Illuminate/Support/Env.php
Outdated
| if (! is_string($value)) { | ||
| throw new InvalidArgumentException( | ||
| sprintf('Environment value for key [%s] must be a string, %s given.', $key, gettype($value)) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Would it make sense to move these to a helper like
<?php
namespace Illuminate\Support\Typed;
if (! function_exists('check_type')) {
/**
* @param 'string'|'int'|'integer'|'long'|'bool'|'boolean'|'float'|'double'|'real' $type
* @throws \InvalidArgumentException
*/
function check_type(mixed $value, callable $checkFn, string $type, string $key, string $group = 'variable'): mixed
{
throw_unless(
$checkFn($value),
InvalidArgumentException::class,
sprintf('%s value for key [%s] must be a %s, %s given.',
$group,
$key,
$type,
gettype($value)),
),
);
return $value;
}
}
if (! function_exists('check_string')) {
function check_string(mixed $value, string $key, string $group = 'variable'): string
{
return check_type($value, is_string(...), 'string', $key, $group);
}
}or even
<?php
namespace Illuminate\Support\Typed;
use InvalidArgumentException;
if (! function_exists('check_type')) {
/**
* @param 'string'|'int'|'integer'|'long'|'bool'|'boolean'|'float'|'double'|'real' $type
* @throws \InvalidArgumentException
*/
function check_type(mixed $value, string $type, string $key, string $group = 'variable'): mixed
{
$isValid = match ($type) {
'string' => is_string($value),
'int', 'integer', 'long' => is_int($value),
'bool', 'boolean' => is_array($value),
'float', 'double', 'real' => is_float($value),
default => throw new InvalidArgumentException('Type "'. $type . '" is not supported. Use one of: string, int, integer, long, bool, boolean, float, double, real')
};
throw_unless(
$isValid,
InvalidArgumentException::class,
sprintf('%s value for key [%s] must be a %s, %s given.',
$group,
$key,
$type,
gettype($value)),
),
);
return $value;
}
}This could be reused for the array getters (#55567) as well as those for the config (#50140)
There was a problem hiding this comment.
Great idea! However, instead of is_int, maybe it’s better to use filter_var with FILTER_VALIDATE_INT?
Since values from .env (even if you set BANK_NUMBER=12345) are always strings, FILTER_VALIDATE_INT will properly check if the value can actually be cast to an integer. Same applies to is_float - it might be better to use FILTER_VALIDATE_FLOAT in this case.
There was a problem hiding this comment.
Thanks for adding it—love it 😍 👍🏻
src/Illuminate/Support/Env.php
Outdated
| $value = Env::get($key, $default); | ||
|
|
||
| if (! is_string($value)) { | ||
| throw new InvalidArgumentException( | ||
| sprintf('Environment value for key [%s] must be a string, %s given.', $key, gettype($value)) | ||
| ); | ||
| } | ||
| check_type($value, 'string', $key, 'Environment'); | ||
|
|
||
| return $value; |
There was a problem hiding this comment.
You could also simplify this even further:
| $value = Env::get($key, $default); | |
| if (! is_string($value)) { | |
| throw new InvalidArgumentException( | |
| sprintf('Environment value for key [%s] must be a string, %s given.', $key, gettype($value)) | |
| ); | |
| } | |
| check_type($value, 'string', $key, 'Environment'); | |
| return $value; | |
| return check_type($value, 'string', $key, 'Environment'); |
or
| $value = Env::get($key, $default); | |
| if (! is_string($value)) { | |
| throw new InvalidArgumentException( | |
| sprintf('Environment value for key [%s] must be a string, %s given.', $key, gettype($value)) | |
| ); | |
| } | |
| check_type($value, 'string', $key, 'Environment'); | |
| return $value; | |
| return check_type( | |
| Env::get($key, $default), | |
| 'string', | |
| $key, | |
| 'Environment' | |
| 8000 | ); |
There was a problem hiding this comment.
Thank you for the code review, this really looks cleaner.
src/Illuminate/Support/Env.php
Outdated
| @@ -216,19 +200,15 @@ public static function array(string $key, $default = null): array | |||
| { | |||
| $value = Env::get($key, $default); | |||
|
|
|||
| if ($value === null) { | |||
| if (is_null($value)) { | |||
There was a problem hiding this comment.
Is there a reason, you changed to a function call?
There was a problem hiding this comment.
That was my mistake. I’ll revert it to the previous version, thanks for catching that.
src/Illuminate/Support/Env.php
Outdated
| if ($value === null) { | ||
| return []; | ||
| } | ||
|
|
||
| if (is_string($value)) { | ||
| return array_map('trim', explode(',', $value)); | ||
| } | ||
|
|
||
| check_type($value, 'array', $key, 'Environment'); | ||
|
|
||
| return $value; | ||
| return check_type($value, 'array', $key, 'Environment'); |
There was a problem hiding this comment.
If you want, even this can be shortened:
| if ($value === null) { | |
| return []; | |
| } | |
| if (is_string($value)) { | |
| return array_map('trim', explode(',', $value)); | |
| } | |
| check_type($value, 'array', $key, 'Environment'); | |
| return $value; | |
| return check_type($value, 'array', $key, 'Environment'); | |
| return match (true) { | |
| $value === null => [], | |
| is_string($value) => array_map('trim', explode(',', $value)), | |
| default => check_type($value, 'array', $key, 'Environment'), | |
| }; |
|
Thanks for your pull request to Laravel! Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include. If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions! |
Added methods
Env::string(string $key, $default = null): stringEnv::integer(string $key, $default = null): intEnv::float(string $key, $default = null): floatEnv::boolean(string $key, $default = null): boolEnv::array(string $key, $default = null): arrayAs exist on the
Configfacede andArrhelper.