Skip to content

Account for optional keys in get_flag_value() return type - #368

Merged
swissspidy merged 1 commit into
mainfrom
fix-get-flag-value-optional-keys
Oct 6, 2026
Merged

swissspidy merged 1 commit into
mainfrom
fix-get-flag-value-optional-keys

Conversation

@swissspidy

@swissspidy swissspidy commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

When $assoc_args is an array shape, GetFlagValueFunctionDynamicReturnTypeExtension returned the key's value type even if the key is optional, dropping the default. For example, with

/** @param array{'link-type'?: 'json'|'xml'} $assoc_args */
$link_type = Utils\get_flag_value( $assoc_args, 'link-type' );

$link_type was inferred as 'json'|'xml' instead of 'json'|'xml'|null, so null !== $link_type checks get reported as always true. This currently breaks PHPStan on wp-cli/embed-command main (run).

Changes:

  • For an optional key, union the value type with the default type.
  • Since get_flag_value() uses isset(), a nullable value also falls back to the default, so null is removed from the value type and the default added.
  • Added asserts to tests/data/get_flag_value.php covering optional keys, an explicit default, and a nullable value; they fail without the fix.

Verified locally with PHPStan 2.3.0: composer phpunit -- --filter TestDynamicReturnTypeExtension, composer phpstan, composer phpcs and composer lint all pass. embed-command main analyses cleanly with this extension.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HsH4o5pKmm1Qd5cqwAm7vB


Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved return type inference for flag lookups with optional or nullable values, including when a default value is provided.

When `$assoc_args` is an array shape and the flag is an optional key,
the default is returned if the key is missing. Likewise, since
get_flag_value() uses isset(), a nullable value also falls back to the
default. Include the default type in both cases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HsH4o5pKmm1Qd5cqwAm7vB
Copilot AI balanced review requested due to automatic review settings October 6, 2026 08:19
@swissspidy
swissspidy requested a review from a team as a code owner October 6, 2026 08:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8ccb31bb-9fea-4283-85e2-43ec1238e62f
📥 Commits

Reviewing files that changed from the base of the PR and between ce989aa and 4d8198d.

📒 Files selected for processing (2)
  • src/PHPStan/GetFlagValueFunctionDynamicReturnTypeExtension.php
  • tests/data/get_flag_value.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The return type extension now accounts for optional keys in constant arrays. Tests cover optional and nullable values, required values, and calls with and without defaults.

Changes

Flag value inference

Layer / File(s) Summary
Flag value inference and tests
src/PHPStan/GetFlagValueFunctionDynamicReturnTypeExtension.php, tests/data/get_flag_value.php
The extension returns the default when a key is missing. For a required non-null value, it returns that value’s type. For an optional or nullable value, it combines the non-null value type with the default. Tests check inferred types for these cases.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: brianhenryie

Merge Risk: ⚪ Minimal · up to 4d819

The change addresses optional and nullable flag-value inference. The PR reports passing checks on PHPStan 2.3.0, while compatibility with the also-allowed 2.2.14 remains unconfirmed; no concrete failure is established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: accounting for optional keys when inferring the return type of get_flag_value().
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...GetFlagValueFunctionDynamicReturnTypeExtension.php 0.00% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

@swissspidy swissspidy added this to the 5.3.4 milestone Oct 6, 2026
@swissspidy
swissspidy merged commit 5a0df94 into main Oct 6, 2026
70 of 71 checks passed
@swissspidy
swissspidy deleted the fix-get-flag-value-optional-keys branch October 6, 2026 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated-pr bug Something isn't working scope:testing Related to testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants