Skip to content

[FSSDK-12610] Fix: PHP 8.4 implicit nullable parameter deprecation warnings#296

Merged
Tamara-Barum merged 3 commits into
masterfrom
tbarum/FSSDK-12610-fix-implicit-nullable-deprecation
May 20, 2026
Merged

[FSSDK-12610] Fix: PHP 8.4 implicit nullable parameter deprecation warnings#296
Tamara-Barum merged 3 commits into
masterfrom
tbarum/FSSDK-12610-fix-implicit-nullable-deprecation

Conversation

@Tamara-Barum
Copy link
Copy Markdown
Contributor

@Tamara-Barum Tamara-Barum commented May 20, 2026

Summary

  • Fixes PHP 8.4 deprecation warnings for implicitly nullable parameter types across 7 files (14 parameters total)
  • Changes Type $param = null to ?Type $param = null in constructors and methods
  • Updates PHPDoc to reflect nullable types
  • No behavioral change — parameters already accepted null, this makes the nullable type explicit as PHP 8.4 requires

Jira

FSSDK-12610

Related

Files changed

File Parameters fixed
src/Optimizely/Optimizely.php $eventDispatcher, $logger, $errorHandler, $userProfileService, $configManager, $notificationCenter
src/Optimizely/ProjectConfigManager/HTTPProjectConfigManager.php $logger, $errorHandler, $notificationCenter
src/Optimizely/DecisionService/DecisionService.php $userProfileService
src/Optimizely/Utils/Validator.php $logger
src/Optimizely/Utils/VariableTypeUtils.php $logger
src/Optimizely/Event/Dispatcher/DefaultEventDispatcher.php $httpClient
src/Optimizely/OptimizelyConfig/OptimizelyConfigService.php $logger

Test plan

  • Run composer test on PHP 8.4 to confirm deprecation warnings are gone
  • Run full test suite to verify no regressions
  • Verify SDK works correctly on PHP 8.1+ (backward compatible per composer.json constraint)

🤖 Generated with Claude Code

…rnings

Use explicit nullable types (?Type) instead of implicit nullable (Type $param = null)
to resolve deprecation warnings on PHP 8.4.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@raju-opti raju-opti requested a review from Copilot May 20, 2026 14:26
@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 97.336%. remained the same — tbarum/FSSDK-12610-fix-implicit-nullable-deprecation into master

@coveralls
Copy link
Copy Markdown

coveralls commented May 20, 2026

Coverage Status

coverage: 97.336%. remained the same — tbarum/FSSDK-12610-fix-implicit-nullable-deprecation into master

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to eliminate PHP 8.4 deprecation warnings caused by implicitly nullable typed parameters by making nullability explicit (Type $x = null?Type $x = null) in key SDK constructors.

Changes:

  • Updated Optimizely constructor to explicitly mark optional dependencies as nullable.
  • Updated HTTPProjectConfigManager constructor to explicitly mark optional logger/error handler/notification center as nullable.
  • Updated DecisionService constructor to explicitly mark the optional user profile service as nullable.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/Optimizely/Optimizely.php Makes optional constructor dependencies explicitly nullable to avoid PHP 8.4 implicit-nullable deprecations.
src/Optimizely/ProjectConfigManager/HTTPProjectConfigManager.php Marks optional logger/error handler/notification center params as nullable for PHP 8.4 compatibility.
src/Optimizely/DecisionService/DecisionService.php Marks optional UserProfileServiceInterface constructor param as nullable for PHP 8.4 compatibility.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Optimizely/Optimizely.php
Comment thread src/Optimizely/Optimizely.php
Comment thread src/Optimizely/Optimizely.php
Comment thread src/Optimizely/DecisionService/DecisionService.php
Tamara-Barum and others added 2 commits May 20, 2026 09:39
…, VariableTypeUtils, DefaultEventDispatcher, OptimizelyConfigService

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@alexjoeyyong
Copy link
Copy Markdown

Qwen 3.6 27b had this for a review.

PR #296 Review: PHP 8.4 Implicit Nullable Parameter Deprecation Fix

Verdict: Legit and clean — safe to merge. ✅

What it does
Changes Type $param = null → ?Type $param = null across 7 files (10 parameters total) to fix PHP 8.4 deprecation
warnings for implicitly nullable parameters. This is a purely cosmetic/static analysis fix — zero behavioral change
 since all parameters already accepted null at runtime.

Copilot's 4 comments (all valid, all addressed)


#: 1
Copilot concern: PHPDoc types didn't match nullable signatures (e.g. UserProfileServiceInterface vs
?UserProfileServiceInterface)
Status: ✅ Fixed — commit 3 updates PHPDoc to ?Type
────────────────────────────────────────
#: 2
Copilot concern: PR only covered 3 files initially but other implicit nullable params existed elsewhere
(Validator.php, VariableTypeUtils.php, DefaultEventDispatcher, OptimizelyConfigService)
Status: ✅ Fixed — commit 2 expanded coverage to all remaining occurrences
────────────────────────────────────────
#: 3
Copilot concern: PR description claimed PHP 7.4+ backward compat, but composer.json requires >=8.1
Status: ⚠️ Not fixed in PR, but harmless — the PR description is just wrong, the code is fine. Explicit nullable
types (?Type) have been valid since PHP 7.1, so backward compat with 8.1+ is maintained.
────────────────────────────────────────
#: 4
Copilot concern: Same as #1, for DecisionService.php
Status: ✅ Fixed


Concerns / things to watch

 1. No tests added — This is acceptable since there's zero behavioral change. The existing test suite passing on
    PHP 8.1/8.2 (which CI confirms) is sufficient. Ideally they'd run this on PHP 8.4 too, but that's a CI config
    gap, not a PR issue.

 2. Author used Claude Co-Authored-By — All 3 commits have Co-Authored-By: Claude Opus 4.6. The author
    (Tamara-Barum) is a repo MEMBER with an @optimizely.com email, so this is an internal engineer. Legitimate.

 3. `raju-opti` approved before the fix commits — The APPROVE came after commit 1 but before commits 2 and 3
    expanded the scope. The final state is correct, but the approval predates the complete fix. Not a blocker since
     CI passed on the final commit.

 4. PR description still mentions PHP 7.4+ — Minor inaccuracy. composer.json requires >=8.1, so the "Verify SDK
    works correctly on PHP 7.4+" test plan line is stale. Doesn't affect the code.

CI status
All green: Linting ✅, Unit Tests 8.1 ✅, Unit Tests 8.2 ✅, Integration Tests ✅, Coverage ✅, Compatibility Suite
 ✅, Jira ticket check ✅.

Bottom line
Straightforward, low-risk fix for a real PHP 8.4 deprecation. Copilot caught legitimate gaps (incomplete coverage,
stale PHPDoc), the author addressed all of them in follow-up commits. No reason not to merge.

@Tamara-Barum Tamara-Barum merged commit 52b8b07 into master May 20, 2026
10 checks passed
@Tamara-Barum Tamara-Barum deleted the tbarum/FSSDK-12610-fix-implicit-nullable-deprecation branch May 20, 2026 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants