-
Notifications
You must be signed in to change notification settings - Fork 17
Fix PHP 8.4 deprecation: Add explicit nullable type hints #309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Greptile OverviewGreptile SummaryThis PR systematically resolves PHP 8.4 deprecation warnings by adding explicit nullable type hints ( Key Changes:
Implementation Quality:
Confidence Score: 5/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant App as PHP Application
participant SDK as WorkOS SDK Methods
participant Client as Client::request()
participant Interface as RequestClientInterface
participant Curl as CurlRequestClient
participant API as WorkOS API
Note over App,SDK: Before PHP 8.4: Implicit nullable params
App->>SDK: createUser(email, null, null, ...)
Note over SDK: function createUser($email, $password = null)
Note over SDK: ⚠️ PHP 8.4 Deprecation Warning
Note over App,SDK: After Fix: Explicit nullable type hints
App->>SDK: createUser(email, null, null, ...)
Note over SDK: function createUser($email, ?string $password = null)
SDK->>Client: request(POST, path, null, params, true)
Note over Client: function request($method, $path, ?array $headers = null, ?array $params = null)
Client->>Interface: request(POST, url, null, params)
Note over Interface: Interface signature matches implementation
Interface->>Curl: request(POST, url, null, params)
Note over Curl: function request(..., ?array $headers = null, ?array $params = null)
Curl->>API: HTTP POST with JSON body
API-->>Curl: Response
Curl-->>Client: [result, headers, statusCode]
Client-->>SDK: Response data
SDK-->>App: Resource\User object
Note over App,API: ✅ No deprecation warnings in PHP 8.4
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
16 files reviewed, no comments
Resolves workos#296 PHP 8.4 deprecates implicitly nullable parameters (e.g., `string $param = null`). This change adds explicit nullable type hints (`?string $param = null`) across the SDK to eliminate deprecation warnings while maintaining backward compatibility with PHP 7.3+. Changes: - Added explicit nullable type hints to all nullable parameters in: - UserManagement, AuditLogs, Organizations, SSO, MFA, Portal, DirectorySync - Client, CurlRequestClient, WebhookResponse - Exception classes (GenericException, BaseRequestException) - Updated RequestClientInterface to match implementation signature - Added tests to verify null parameter handling and backward compatibility - Fixed test expectations to match actual implementation behavior All 167 tests pass. No breaking changes - only type hints added.
Take particular note of the changes to $roleSlugs. There was a mismatch between the phpdoc, tests, and api docs. This commit went with the array due to both the test and api doc specifying the array type. The php doc was updated to reflect the array change.
560a945 to
be83f58
Compare
nicknisi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Resolves #296
PHP 8.4 deprecates implicitly nullable parameters (e.g.,
string $param = null). This change adds explicit nullable type hints (?string $param = null) across the SDK to eliminate deprecation warnings while maintaining backward compatibility with PHP 7.3+.Changes:
All 167 tests pass. No breaking changes - only type hints added.
Description
Documentation
Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.
If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.