Fix YAML dump incorrectly handling zero values #3
Conversation
In PHP, empty(0) and empty("0") return true, which caused the YAML
dumper to incorrectly treat zero values as empty. This resulted in:
- Arrays containing 0 being serialized without the zero value
- Scalar zero values being omitted from YAML output
Changes:
- dump(): Replace 'if ($array)' with explicit null/empty checks to
properly handle arrays that may contain zero values
- _yamlize(): Replace 'empty($value)' with 'count($value) === 0' to
only treat actually empty arrays as empty, not zero values
Added test cases:
- testDumpIntegerZero: Verify integer 0 is correctly dumped
- testDumpStringZero: Verify string '0' is correctly dumped
- testDumpAssociativeZero: Verify zero values in associative arrays
- testDumpMixedWithZero: Verify mixed arrays containing zero
Fixes wp-cli/wp-cli#6188
Summary of ChangesHello @AdityaS62, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the Spyc YAML serialization library where valid zero values, both integer '0' and string '0', were erroneously dropped from the YAML output. The fix involves refining the internal logic to distinguish actual empty states from legitimate zero values, ensuring that all data is accurately represented in the generated YAML. This change improves data integrity and aligns the library's behavior with expected YAML standards. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves an issue where zero values (0 and "0") were incorrectly handled during YAML serialization due to PHP's empty() behavior. The changes in the dump and _yamlize methods correctly ensure that zero is treated as a valid scalar value. The addition of specific regression tests for various zero-value scenarios is excellent and provides strong verification for the fix. I have one minor suggestion regarding a comment in _yamlize for improved clarity.
- dump() method: Updated comment to clarify the fix handles both:
- Scalar zero values passed directly (dump(0) or dump("0"))
- Arrays containing zero values (dump([0]))
- _yamlize() method: Updated comment to clarify this is a stylistic
consistency change, since at this point is already verified
to be an array (not a scalar zero)
1 participant
Summary
This PR fixes an issue where YAML serialization incorrectly drops or misrepresents zero (0 or "0") values.
Because PHP treats 0 and "0" as empty in certain contexts, Spyc was mistakenly omitting valid scalar zero values during YAML output.
Root Cause
In PHP:
empty( 0 ) === true
empty( "0" ) === true
Spyc relied on empty() checks when determining whether a value or array should be serialized.
As a result:
Scalar zero values were treated as empty
Arrays containing zero values could be serialized without those values
YAML output became ambiguous or incomplete
Changes Made
Logic fixes
Replaced empty() checks with explicit, value-safe conditions
Ensured zero (0 / "0") is treated as a valid scalar, not as empty
Restricted “empty” handling to:
null
truly empty arrays (count($value) === 0)
Affected methods
dump(): avoids treating arrays containing zero values as empty
_yamlize(): distinguishes zero values from empty values during serialization
Tests Added
New tests cover zero handling in multiple scenarios:
Integer zero (0)
String zero ("0")
Associative arrays containing zero values
Mixed arrays containing zero values
These tests ensure zero values are preserved correctly across YAML output.
Why This Approach
Fixes the issue at the serialization layer, where the behavior originates
Avoids altering consumer code (e.g. wp-cli formatters)
Preserves backward compatibility for non-zero values
Aligns YAML output with user expectations and CLI correctness