Problem/Motivation

I have a piece of content that requires a time format to be entered within square brackets. Examples are [16:00] and [12:00].

This is being recognised as a token with a type of int, however Drupal\Core\Utility\Token->generate() requires the type to be a string.

Steps to reproduce

* Install Drupal 10.1.x
* Install and enable the token_filter module.
* Go to Configuration -> Content authoring -> Text formats and editors. Proceed to Basic HTML and tick "Replaces global and entity tokens with their values".
* Create a basic page with some content including [16:00] and [12:00].
* Save the page and view it.
* In your IDE anywhere within Drupal\Core\Utility\Token->generate(), set a breakpoint for your debugger.
* You'll note that the type is set as int. Example: `$type = {int} 16`.
* The expected result should be as a string. Example: `$type = "16"`.

Proposed resolution

In the generate() method of Drupal\Core\Utility\Token, set the type declaration as a string (as expected by the function).

Remaining tasks

* Create a failing test.
* Create patch.
* Ensure test is passing.

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

Issue fork drupal-3450635

Command icon 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:

Comments

hoffismo created an issue. See original summary.

hoffismo’s picture

Issue summary: View changes
hoffismo’s picture

StatusFileSize
new703 bytes

Adding initial patch but requires tests.

cilefen’s picture

Use a pull request instead.

immaculatexavier made their first commit to this issue’s fork.

immaculatexavier’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Postponed (maintainer needs more info)

The steps appear to require contrib modules. Is this reproducible in core?

hoffismo’s picture

Issue summary: View changes
StatusFileSize
new786 bytes

Thanks @smustgrave. I've updated the issue and provided a MR. I'm able to reproduce this with Core and the Token Filter module. It's TokenFilter Filter plugin directly calls Drupal\Core\Utility\Token->replace(). The generate() method is expecting a string, however it's receiving an int with the example I provided in the issue description.

larowlan’s picture

Status: Postponed (maintainer needs more info) » Needs review
larowlan’s picture

larowlan changed the visibility of the branch 3450635-numeric-content-inside to hidden.

larowlan’s picture

Status: Needs review » Needs work

@hoffismo can you provide the stack trace of the error you're seeing here?

I'm guessing you've got a hook_tokens somewhere that is using strict types and that's the issue - core isn't strict-typed yet.

We can't make the current change in the MR as it has BC implications.

The correct way to do this would be to check the type of the $type and if it's not a string, then trigger a deprecation error and cast it.

That would allow callers to be notified and updated before the next major.

However, I think the fact its getting called with an integer type might be a mistake - I think the correct fix would likely be in `\Drupal\Core\Utility\Token::doReplace` where it is iterating

foreach ($text_tokens as $type => $tokens) {
      $replacements += $this->generate($type, $tokens, $data, $options, $bubbleable_metadata);
      if (!empty($options['clear'])) {
        $replacements += array_fill_keys($tokens, '');
      }
    }

I think here $type should be checked against valid types.

E.g something like

$info = $this->getInfo();
foreach (array_intersect_key($text_tokens, $info)  as $type => $tokens) {

That should prevent it even being called with `16`

hoffismo’s picture

StatusFileSize
new6.96 KB

Thanks @larowlan. I've attached a stack trace. Yes that's correct, we do have a hook_tokens() in a custom module declaring the $type as string. I can confirm removing that does work to align it with core, since it's not strict typed as yet.

larowlan’s picture

Thanks @hoffismo - I think that probably unblocks you for now too 👍️

quietone’s picture

Version: 10.1.x-dev » 11.x-dev

This needs to be committed to 11.x (which is main) first and then applied to branches.

mstrelan’s picture

Coming here from #3479519: Incorrect type hinting in the smart_trim_tokens function. I think we should be able to use string as a type hint, but the preg_match_all in \Drupal\Core\Utility\Token::scan finds ints and floats too. Should core cast these to string?

dpi’s picture

Also reported at #3534857: scheduled_transitions_tokens expects string but can receive int, but it seems like it should be fixed in core.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.