FilterInterface::tips return does not match core implementations

I have uploaded the patch. please review

What about the return type ? I thought that starting with D10 we were adding return type hints, are we not ? (not saying we do: I just wonder)

Otherwise LGTM.

@fgm we do for D10.

Added a return type hint. Please review.
Thanks.

@AkashKumar07 as you can see this patch doesn't pass tests. Can you fix it before we can review ?

Once you get an error, like this, iou may want to test locally on your machine before submitting the patch to drupalci, as you then see where the failure is, so you don't have to run the whole test suite, but just one test that failed on CI, and it's much faster to run locally.

Adding ? for null type hint.

Look at the results of that patch: there are more errors: https://www.drupal.org/pift-ci-job/2712812 →

You could run phpstan locally yourself to get results faster: just install phpstan-drupal, e.g. using the Extension Installer https://github.com/phpstan/extension-installer : that way you can get the same results as those given by DrupalCI on your own machine.

Actually, there are a couple strings returned too, so the patch is a bit larger.

Thanks @fgm for taking care of this

Tests are all green so adding typehint is good.

Short and sweet!

We can't add the return type hint to the interface in a minor release. See 📌 Enable existing interfaces to add return type hints with a deprecation message for implementors Active .

Also wonder, could we use Stringable instead of TranslatableMarkup here? It'd be more generic.

Thanks. I see that 📌 Enable existing interfaces to add return type hints with a deprecation message for implementors Active does not currently suggest a path forward. So what should we do here ?

• Just fix the phpdoc ? :-(
• Define and add the [#ReturnTypeHintDeprecation] ? That seems out of scope.

Regarding \Stringable, the current phpdoc incorrectly specifies string|null, so it does not even allow it, convenient though it would be. If we dig a little, the string return is only ever used in FilterHtml, with all other core implementations using either naked returns (so NULL) or TranslatableMarkup so we could just as well consider it is an error and only allow ?\TranslatableMarkup.

But since this is what we document the interface to be, we must probably assume that contrib/custom devs have looked at the core implementations and just handled the actual returns, so they also return ?\TranslatableMarkup. Should be move to \Stringable<\code>, that would be the type to add to the typed return in 11.x, and it could in the future break any contrib actually using any of the public methods on <code>?\TranslatableMarkup (because that's what core actually used) whenever someone decides to update core to return a \Stringable that is not also a \TranslatableMarkup because, hey, that's what core documents... so I think the safest PHPdoc signature would be ?\Drupal\Core\StringTranslation\TranslatableMarkup.

I removed the novice tag because that's no longer a novice question.