I’m getting php:S3358 “Ternary operators should not be nested” on a line that chains (nests really isn’t even the right word) several shorthand ternary and null coalescing operators. In my specific case, I’m trying to extract an api key from various sources and use the first one that is set, according to some priority. My code looks like this:
$api_key = $arg_key ?: $request_key ?: $_COOKIE['api_key'] ?? '';
This is, in my view, absolutely easy to understand, assuming the reader knows what the operators mean (which is a fair assumption, otherwise the rule should be “Do not use operators someone might not know”):
"Set the variable named api_key to the first valid value you find in the following list:
- $arg_key (valid = not falsey)
- $request_key (valid = not falsey)
- $_COOKIES[‘api_key’] (valid = not null)
- default: empty string"
The compliant code according to the rule would look like this:
$api_key = $_COOKIES['api_key'] ?? '';
$api_key = $request_key ?: $api_key;
$api_key = $arg_key ?: $api_key;
which reverses the priority in order to avoid a potential Notice: undefined index, making the priority less clear. Alternatively, one could use an if-chain like so:
$api_key = '';
if ($arg_key) {
$api_key = $arg_key;
} elseif ($request_key) {
$api_key = $request_key;
} elseif (isset($_COOKIES['api_key'])) {
$api_key = $_COOKIES['api_key'];
}
Those can also be combined in an inconsistent mess of styles. Either way, the compliant code is significantly more verbose and complex, while arguably actually more difficult to understand. Shorthand operator chaining is as clear as it gets when a variable should be set to the first valid value out of a selection of more than 2. This gets more expressed the larger the list of potential values becomes.
I would thus like to request the following feature/change:
Restrict rule php:S3358 to real ternary operators (a ? b : c), excluding the shorthand ternary operator (a ?: b) and the null coalescing operator (a ?? b).
I really don’t want to disable the rule because it’s a very good rule and makes a lot of sense as long as it flags actual (non-shorthand) ternary operators being nested inside one another. But for chaining shorthand operators in sequence, it’s counter-productive since that should actually be encouraged.
