When trying to install on PHP 8 with Drupal 9.1-dev, I see the following errors:
Deprecated: Required parameter $bubbleable_metadata follows optional parameter $data in /var/www/html/web/modules/contrib/token/token.tokens.inc on line 427
Deprecated: Required parameter $bubbleable_metadata follows optional parameter $data in /var/www/html/web/modules/contrib/token/token.tokens.inc on line 1100
Deprecated: Required parameter $bubbleable_metadata follows optional parameter $data in /var/www/html/web/modules/contrib/token/token.tokens.inc on line 1218
Deprecated: Required parameter $bubbleable_metadata follows optional parameter $data in /var/www/html/web/modules/contrib/token/token.tokens.inc on line 1555
Use PHP 8 beta 4 to install Drupal 9.1 with the required patches (see https://github.com/hussainweb/drupal-php8-composer2)
Add token module and attempt to enable.
Since changing the parameter order is difficult, I suggest making the parameter optional and set it to an empty metadata object. This shouldn't have an impact because any code invoking the hook would be passing in the parameter right now anyway.
- Decide if there is a better approach
- Write the patch and commit
None
The function parameter $bubbleable_metadata in the function token_tokens would become optional.
None
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 3173359-6.patch | 1.99 KB | hussainweb |
Comments
Comment #2
hussainwebI did a quick test and the module at least gets installed with this patch. I'll test more later.
Comment #3
tr commentedNot really a bug - core Drupal doesn't work with PHP 8 yet, and PHP 8 hasn't been released yet and can still change, so it's hardly "wrong" that the token module has problems too. See #3109885: [meta] Ensure compatibility of Drupal 9 with PHP 8.0 (as it evolves)
Comment #4
hussainweb@TR, sure, makes sense. I selected "Bug" because even though the symptom is not present in PHP 7.x, the problem is there because it does not make sense for a required parameter to follow an optional parameter. I am not worried about what we call this though. :)
Comment #5
berdirIf it's a bug then it's a bug, doesn't matter if it core has bugs as well.
*But*, the change is wrong. The fix is to make all arguments "required". These are hooks, hooks don't have optional arguments, it's up to the caller to pass default values. See hook_tokens()/system_tokens().
Comment #6
hussainwebI agree but I went with the safer approach of making it optional (in case someone is directly calling it from outside token). I'm glad you think that this should be required instead and here's an updated patch. I am skipping the interdiff as it is a different approach.
Comment #7
berdirThe only way that would work is if you'd call that specific function directly, and calling hook functions directly is excluded from BC, so this is fine with me. Calling the hook without those arguments would fail on other implementations.
Comment #9
berdirCommitted.