Fix PHP 8.4 deprecated implicit nullable parameters and use constant for Anthropic API version#40
Conversation
felixarntz
left a comment
There was a problem hiding this comment.
@JosephGabito Thank you for the PR, and great catch!
I have one point of feedback for the proposed approach. Also, there are a few other places in the codebase where a similar change would need to be made:
Felix_Arntz\AI_Services\Google\Google_AI_API_ClientFelix_Arntz\AI_Services\Services\Base\Generic_AI_API_ClientFelix_Arntz\AI_Services\Services\Service_Registration_Context
It would be great if you could align the approach throughout these classes.
Updated the Anthropic_AI_API_Client constructor to accept an optional Authentication instance. The code now checks for the presence of the authentication object before setting the custom header, improving flexibility and preventing potential errors when authentication is not provided.
Corrected the docblock for the $authentication parameter in the constructor to clarify the default value wording.
Changed the type hint for the Authentication parameter in the Google_AI_API_Client constructor to allow null values. This improves flexibility when instantiating the client without authentication.
Updated constructors in Generic_AI_API_Client and Service_Registration_Context to accept nullable Authentication parameters. This change improves type safety and clarifies that Authentication is optional.
Anthropic_AI_API_Client for PHP 8.4 CompatibilityUpdated constructors and methods across several service classes to use explicit nullable type hints (e.g., ?Request_Handler, ?Option_Encrypter, ?callable) for optional parameters. This improves type safety and clarity in the codebase.
|
Thanks for reviewing the PR. Updated both PR title and description to better align with the changes. I also did change things to other locations to better align with everything :)
Cheers |
felixarntz
left a comment
There was a problem hiding this comment.
@JosephGabito Thank you for the updates!
The PR looks great as is. Just noticed in the tests that there is one more case of inexplicit nullable parameters in the codebase, Felix_Arntz\AI_Services\Services\Util\Data_Encryption::__construct(), see https://github.com/felixarntz/ai-services/actions/runs/16105825834/job/45523564678?pr=40. I'm happy to merge this as is, but if you're willing to update that last case too, I'd appreciate it!
The other flagged case comes from the library https://github.com/felixarntz/wp-oop-plugin-lib that this plugin uses, so I'll make sure to fix those there and update!
|
Update: See felixarntz/wp-oop-plugin-lib@1fdfe74 for the upstream fix. |
Description
This PR fixes PHP 8.4 deprecated nullable parameters by replacing the unsupported
$authentication = nullpattern with proper nullable type hints (?Authentication) across all API clients and service registration context. Additionally, replaces the hardcoded Anthropic API version with a class constant for better maintainability.What Changed
Fixed PHP 8.4 deprecated nullable parameters:
Replaced
Authentication $authentication = nullwith?Authentication $authentication = nullacross all relevant files.Added Anthropic API version constant:
Defined
ANTHROPIC_API_VERSION = '2023-06-01'to replace hardcoded version string.Why This Is Safe