Skip to content

Conversation

@blobaugh
Copy link
Contributor

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:

  • 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.

Description

Documentation

Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.

[ ] Yes

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

@blobaugh blobaugh requested a review from a team as a code owner November 25, 2025 14:55
@blobaugh blobaugh requested a review from dandorman November 25, 2025 14:55
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 25, 2025

Greptile Overview

Greptile Summary

This PR systematically resolves PHP 8.4 deprecation warnings by adding explicit nullable type hints (?string, ?array, ?bool) to all optional parameters with null defaults across the SDK.

Key Changes:

  • Updated 100+ optional parameters across all main SDK classes (UserManagement, Organizations, DirectorySync, SSO, MFA, AuditLogs, Portal)
  • Fixed interface/implementation signature mismatch in RequestClientInterface and CurlRequestClient
  • Added explicit type hints to exception constructors
  • Added 7 new tests verifying null parameter handling works correctly

Implementation Quality:

  • Changes are mechanical and consistent - all implicitly nullable parameters now have explicit ?type declarations
  • Required parameters (without defaults) intentionally left without type hints to avoid breaking changes
  • Tests confirm backward compatibility is maintained
  • All 167 tests pass according to PR description

Confidence Score: 5/5

  • This PR is completely safe to merge - it only adds type hints without changing behavior
  • Perfect score because: (1) changes are purely additive type hints with no logic modifications, (2) fixes a real PHP 8.4 deprecation issue, (3) comprehensive test coverage added for null parameter handling, (4) backward compatible with PHP 7.3+, (5) follows consistent pattern across entire codebase
  • No files require special attention - all changes follow the same safe pattern

Important Files Changed

File Analysis

Filename Score Overview
lib/UserManagement.php 5/5 Added explicit nullable type hints (?string, ?array, ?bool) to 60+ optional parameters across all methods - changes are mechanical and correct
lib/Organizations.php 5/5 Added explicit nullable type hints to optional parameters in listOrganizations, createOrganization, and updateOrganization methods
lib/DirectorySync.php 5/5 Added explicit nullable string type hints to optional parameters in listDirectories, listGroups, and listUsers methods
lib/SSO.php 5/5 Added nullable string type hints to optional parameters in getAuthorizationUrl and listConnections methods
lib/RequestClient/RequestClientInterface.php 5/5 Updated interface signature to add explicit nullable array type hints with default null values for headers and params
lib/RequestClient/CurlRequestClient.php 5/5 Updated implementation to match interface with nullable array type hints for headers and params parameters
tests/WorkOS/UserManagementTest.php 5/5 Added 4 new tests to verify null parameter handling: testUpdateUserWithNullOptionalParams, testEnrollAuthFactorWithNullOptionalParams, testCreateUserWithNullOptionalParams, testUpdateOrganizationMembershipWithNullRoleSlug

Sequence Diagram

sequenceDiagram
    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
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a 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

Edit Code Review Agent Settings | Greptile

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.
@blobaugh blobaugh force-pushed the fix-php-deprecation-notices branch from 560a945 to be83f58 Compare November 25, 2025 15:35
Copy link
Member

@nicknisi nicknisi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@nicknisi nicknisi merged commit 2714471 into workos:main Nov 25, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

PHP 8.4 Deprecation: Implicitly marking parameter as nullable is deprecated

2 participants