Cast attr to string to prevent a PHP deprecation warning in 8.1

[beryllium]

I received this Deprecation notice when running unit tests for sculpin under PHP 8.1:

PHP Deprecated:  preg_match_all():
  Passing null to parameter #2 ($subject) of type string is deprecated
  in sculpin/vendor/michelf/php-markdown/Michelf/MarkdownExtra.php
  on line 252

I solved it in the leanest way possible, which was to cast $attr to string. Let me know if you would prefer a different solution.

And thanks for building this library! I've been quite happy with it.

[DivineDominion]

@michelf Would this be a reasonable fix? I get this warning all over the place, too 😅

[michelf]

It works. But there's a fast path at the beginning of the function:

if (empty($attr) && !$defaultIdValue && empty($classes)) {
   return "";
}

I think it'd make more sense to check for null there, after all we're already checking for an empty value there.

[beryllium]

So instead of

if (empty($attr) && !$defaultIdValue && empty($classes)) {

You're thinking of something like:

if (null === $attr || (empty($attr) && !$defaultIdValue && empty($classes))) {

... or is there a simpler implementation that would provide the desired logic?

[michelf]

Ah, I was wrong here. empty already checks for null in that condition. The issue happens when this if evaluates to false because of the other two conditions. That's when preg_match is fed with a null value.

So I think the best fix would be to change $attr to an empty string when null. Casting works, but I'd favor $attr ?? "" in the call to preg_match: A cast would have the downside of not complaining if the function is fed with some nonsense value (an object, an array, etc.)