Problem/Motivation

Some parameters are nullable but not explicitly marked as nullable.

On PHP 8.4 that will yield warnings like:

Implicitly marking parameter $user as nullable is deprecated, the explicit nullable type must be used instead OpenIDConnectAccountsForm.php:124

Proposed resolution

Apply attached patch to 8.x-1.x branch.

CommentFileSizeAuthor
#2 openid_connect-explicitly_mark_parameters_as_nullable-3568983-2.patch3.08 KBarnested

Issue fork openid_connect-3568983

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

arnested created an issue. See original summary.

arnested’s picture

arnested commented
Status: Active » Needs review
StatusFileSize
new openid_connect-explicitly_mark_parameters_as_nullable-3568983-2.patch3.08 KB
clairero’s picture

clairero commented

We're using version 1.5.0 of the module. After switching to PHP 8.4, we also encountered deprecation warnings. I applied and tested the patch and can confirm it works well - the warnings are no longer appearing.

clairero’s picture

clairero commented
Status: Needs review » Reviewed & tested by the community

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

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented

Full disclosure: @clairero and I are working on the same project.

Comment #3 describes testing. I also reviewed the changes in the patch, and they look good to me. +1 for RTBC.

I checked https://www.php.net/manual/en/language.types.declarations.php, and "Support for nullable types has been added" in PHP 7.1.0, so that should not be a problem.

The changes in the patch have already been made on the 3.x branch, in #3433705: Automated Drupal 11 compatibility fixes for openid_connect and #3452009: Cleanup CI pipeline to ensure all validations pass.. We might also add return type declarations to match the 3.x branch, but I think it is OK to make just the changes that fix actual problems. Exception: src/OpenIDConnectAuthmap.php was removed in #3030596: Compatibility with externalauth , and the changes here were never made in that file on the 3.x branch.

I opened a merge request based on the patch from #2.

benjifisher opened merge request !199