Problem/Motivation
\Drupal\Core\Authentication\AuthenticationCollectorInterface::getProvider() has documentation to say $provider_id should be a string but sometimes a NULL is used which causes a deprecation for PHP 8.5.
Steps to reproduce
Proposed resolution
1. Add typehint to interface
2. Leave code in \Drupal\Core\Authentication\AuthenticationCollector::getProvider alone so if contrib of custom calls with NULL a deprecation is thrown on PHP 8.5.
3. Fix code so it no longer calls it with NULL.
Remaining tasks
User interface changes
None
Introduced terminology
N/a
API changes
Typehint added to conform to docs to \Drupal\Core\Authentication\AuthenticationCollectorInterface::getProvider - should add a follow-up to add typehint to concrete implementation in 12.0
Data model changes
None
Release notes snippet
N/a
Issue fork drupal-3554714
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3554714-add-string-typehint
changes, plain diff MR !13602
Comments
Comment #2
alexpottComment #4
alexpottComment #5
andypostLooks ok but not clear why only interface updated, it's already API change in interface so implementation could be updated same time
ref https://git.drupalcode.org/project/drupal/-/merge_requests/13602/diffs?c...
Comment #6
andypostComment #7
alexpott@andypost - no adding the typehint to the interface is fine and will not break anything. Adding the typehint to the implementation will break any calls in contrib or custom which pass a NULL. If we leave as is it'll trigger a deprecation in PHP 8.5 telling people to fix their code. In Drupal 12 we can add the typehint and then any code that call it with a never will trigger an exception. That way we get a continuous upgrade path.
Comment #8
smustgrave commented@alexpott with regards to your last comment does that need to happen before 12?
Comment #9
alexpott@smustgrave No it needs to happen in 12.x once it is open.
Comment #10
smustgrave commentedThanks for the quick reply! In that case I believe this one should be good to go. If I jumped the gun apologize.
Comment #14
catchCommitted/pushed to 11.x and cherry-picked to 11.3.x, thanks!