Problem/Motivation

In D11.3.2 with PHP 8.4.17 I get this warning:
Deprecated function: Drupal\typogrify\SmartyPants::smartNumbers(): Implicitly marking parameter $ctx as nullable is deprecated, the explicit nullable type must be used instead in include() (line 576 of /xxx/drupal/vendor/composer/ClassLoader.php)

CommentFileSizeAuthor
#2 typogrify-3569369-1-php84-nullable-deprecation.patch2.57 KBnickolaj

Issue fork typogrify-3569369

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:

About issue forks

Comments

promes created an issue. See original summary.

nickolaj’s picture

nickolaj
Primary language Ukrainian
commented
Status: Active » Needs review
StatusFileSize
new typogrify-3569369-1-php84-nullable-deprecation.patch2.57 KB

Added explicit nullable type hints (`?string`) to all parameters in the `SmartyPants` class that have `NULL` as their default values.

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

benjifisher opened merge request !16

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented
Status: Needs review » Needs work

@promes, @nickolaj:

Thanks for reporting and working on this issue!

I made a merge request (MR) using the patch from #2. (I copied the extended commit message, but I am not sure it will be preserved when I merge the changes.) I notice a few problems when reviewing the changes:

When I search for = NULL, I find one more function that should be updated: i18nQuotes(). (There are also two functions where the nullable object is already declared as nullable: smartNumbers() and smartAbbreviation().) Or is there some reason not to update i18nQuotes()?

In process(), the variable $ctx is used as an array, despite the @param comment. So it should be declared as ?array and not ?string. I think that is the only function where it is used, but I think we should always declare it as ?array for consistency.

Finally, when we update the type declaration in a function declaration, I would like to update the @param comment to match. That is, I would like to see @param string|null $attr and @param array|null $ctx.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented
Status: Needs work » Reviewed & tested by the community

I apologize for being impatient, but I decided to fix this myself (and then review it myself).

The automated tests catch some of the problems I noticed when reviewing the code. After my update, the tests pass.

I plan to fix this issue and then make a new release of the module.

  • benjifisher committed 12b76df7 on 8.x-1.x 
    fix: #3569369 Implicitly marking parameter $ctx as nullable is...
benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.