Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Part of #3220021: [meta] Ensure compatibility of Drupal 9 with PHP 8.1 (as it evolves)
Steps to reproduce
php -r 'var_dump(explode("|", NULL));'
PHP Deprecated: explode(): Passing null to parameter #2 ($string) of type string is deprecated in Command line code on line 1
Deprecated: explode(): Passing null to parameter #2 ($string) of type string is deprecated in Command line code on line 1
array(1) {
[0]=>
string(0) ""
}
Proposed resolution
Ensure the result of getRequirement() is not passed to a string function when it is NULL.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#11 | 3239553-11.patch | 3.06 KB | alexpott |
#11 | 2-11-interdiff.txt | 2.59 KB | alexpott |
Comments
Comment #2
andypost@alexpott the interdiff against META
Comment #3
daffie CreditAttribution: daffie commentedCan we change this line for better readability to:
To me the added variable "$no_route_format" just does not improve the overall readability.
Comment #4
andypostThis variable used to prevent calling
$route->hasRequirement('_format')
twiceComment #5
daffie CreditAttribution: daffie commentedAll that the method call does is (from vendor/symfony/routing/Route.php):
To me the added variable does not increase the overall readability.
There is still one call the the method that can be replaced, if you want to replace them all:
Comment #6
andypost@daffie this is different route
$view_route
, the patch replaceing only calls to$route->hasRequirement('_format')
Comment #7
daffie CreditAttribution: daffie commented@andypost: You are right.
The patch looks good to me.
Comment #8
larowlanI think this would read better if the variable was $has_route_format, i.e. we can keep both ternaries in the same order - default then fallback, rather than having them one each way.
Then we can just use !$has_route_format in the if statement
As it stands, it took a few passes to comprehend what was happening
Thoughts?
Comment #9
alexpottPlease do all the \Symfony\Component\Routing\Route::getRequirement() fixes in #3220021: [meta] Ensure compatibility of Drupal 9 with PHP 8.1 (as it evolves) in one issue.
Comment #10
alexpottAlso the use of
$no_route_format
and then!$no_route_format
for double negatives is not at all readable. The original change in #3220021: [meta] Ensure compatibility of Drupal 9 with PHP 8.1 (as it evolves) was better.Comment #11
alexpottHere's the work as was from #3220021: [meta] Ensure compatibility of Drupal 9 with PHP 8.1 (as it evolves) plus the other changes due to
getRequirement()
returning NULL.Comment #12
alexpottComment #13
andypostGreat to fix all places same time!
This is what I used to try prevent - readability of ! always hard
Comment #15
catchCommitted cd6b58e and pushed to 9.3.x. Thanks!