Deprecate passing non-string parameters to \Drupal\Core\Utility\Token::scan then implement typehint

Thank you for this patch, this issue was making me crazier than usual.

Spoke too soon. I applied the patch but when I go the dev page for node tokens it still kicks with an error this time. Time to go back to digging.

I also encountered this issue in multiple scenarios and first I tried creating/applying patches for each module that uses Token and was having this issue but in the end got to the conclusion that the best would be to fix the problem at the root and not in every place that method is used.
I tried the fix from the pull request from this issue and it fixed our problem. Before that I dag a lot to find where the scan() or replace() methods were being used and to prevent "NULL" from being passed but couldn't get to the bottom of all. So then I tried to fix it at the root and it worked fine and didn't create any other issue. I Added the fix from the PR also as patch so that people can easily apply it until this gets merged.
I believe that it would be much better to get this fixed in core and not in every other module where it is being used.

The Token::scan() API documentation specifically states that the first argument should be a string, not a NULL, so it should be up to other code to make sure the argument isn't a NULL.

So really this is a symptom of another bug, that token_default_tokens_alter() in the Token module sometimes sends NULL to the token replacement service. In my case:

1. content type A has entity reference field to content type B
2. use token in metatag module to output [field_reference_example:entity:url] from content type A
3. open example of content type A where reference field is empty
4. error shown

\Drupal::service('token_default.manager')->replaceMissingTokensWithDefaults(...) possibly returns NULL replacements. Patch attached, but possibly this is still just solving a symptom rather than the problem.

If the others who experienced this issue can also confirm that you have Token contrib module in place, I think we can safely move this issue over to Token.

Apologies, its Token Default not Token that was the cause of the problem for me. I created an issue for that here https://www.drupal.org/project/token_default/issues/3320681 - if others can confirm that that also stops the php notice for them, we can close this issue.

I agree that this issue originates elsewhere but don't close this issue, we can use it to implement strong typing on the scan() function. We'll need:

1. A patch for Drupal 10 which catches non-string parameters, triggers a drupal deprecation warning and replaces NULL with an empty string
2. A patch for Drupal 11 which sets a string type on the $text parameter and has no deprecation warning or replacement

This might qualify as a task though, rather than a bug, I'll ask in #bugsmash.

Verdict's in, moving to task.

It would seem I was wrong about a patch for Drupal 11, I'm not sure how that is being handled.

Here's a patch adding the deprecation. I don't *think* this needs a test but I could be wrong.

Oops, bad copy/paste in previous patch. Let's try that again.

LGTM Thanks @darvanen.

and replaces NULL with an empty string

Is this not relevant anymore? Seems like it would be needed?

Thanks @acbramley, I did forget that, I also completely forgot the change record, here's a draft.

Oops, this should work better. I have tested casting NULL to a string and it does work.

Believe the change record needs to be in the trigger_error no?

Good point, the example I copied from didn't have one but you're right, that should be there.

That was quick! Looks good to me.

We should be adding test coverage of this deprecation because it contains some logic. I looked for existing test coverage of Token::scan() and I found core/modules/system/tests/src/Functional/System/TokenScanTest.php which is a pointless functional test - it can be a unit test - and I found a unit test \Drupal\Tests\Core\Utility\TokenTest - which sets up a token service for us to use in testing - so I moved everything there.

Tested the Patch on #22 working fine for me, D10

Committed f2e1dcd and pushed to 10.1.x. Thanks!

@catch do we need to adjust 11.x now that it is available? Or is that something that happens later?

Oh I see it's just an issue queue placeholder. How *do* we ensure 11 gets updated?