Skip to content

Comments

Add option for PHP to emit default values for JSON.#23985

Closed
SpencerMalone wants to merge 1 commit intoprotocolbuffers:mainfrom
SpencerMalone:php-default-value-option
Closed

Add option for PHP to emit default values for JSON.#23985
SpencerMalone wants to merge 1 commit intoprotocolbuffers:mainfrom
SpencerMalone:php-default-value-option

Conversation

@SpencerMalone
Copy link
Contributor

@SpencerMalone SpencerMalone commented Oct 15, 2025

This is a reopening of #6035, where it was requested that someone contribute a PR. I chose the option name EMIT_DEFAULTS to match the ruby client and existing upd option. I tried to be relatively comprehensive with the tests, but let me know if there's anything stylistically or logically you want changed.

Locally the tests run happily:

php % git rev-parse HEAD 

7cb3b2d9f60bdffce69c01143ca15c46d2eaa900
php % php -dextension=ext/google/protobuf/modules/protobuf.so -d error_reporting="E_ALL & ~E_DEPRECATED" vendor/bin/phpunit --bootstrap tests/force_c_ext.php tests/EncodeDecodeTest.php
PHPUnit 8.5.26 #StandWithUkraine

...............................................................  63 / 147 ( 42%)
............................................................... 126 / 147 ( 85%)
.....................                                           147 / 147 (100%)

Time: 49 ms, Memory: 6.00 MB

OK (147 tests, 1150 assertions)

@SpencerMalone SpencerMalone force-pushed the php-default-value-option branch 2 times, most recently from dcc8b6b to 7cb3b2d Compare October 15, 2025 15:35
@SpencerMalone SpencerMalone marked this pull request as ready for review October 15, 2025 15:37
@SpencerMalone SpencerMalone requested a review from a team as a code owner October 15, 2025 15:37
@SpencerMalone SpencerMalone requested review from bshaffer and removed request for a team October 15, 2025 15:37
@zhangskz zhangskz added php 🅰️ safe for tests Mark a commit as safe to run presubmits over labels Oct 16, 2025
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Oct 16, 2025
@zhangskz
Copy link
Member

I think you handled this right already from initial review, but just an FYI that the language in the guidance for option to emit default values has since been updated the original issue was opened to be more precise about explicit vs implicit presence, outside of proto3.

https://protobuf.dev/programming-guides/json/#json-options

Always emit fields without presence: Fields that don’t support presence and that have their default value are omitted by default in JSON output (for example, an implicit presence integer with a 0 value, implicit presence string fields that are empty strings, and empty repeated and map fields). An implementation may provide an option to override this behavior and output fields with their default values.

@SpencerMalone SpencerMalone force-pushed the php-default-value-option branch from 7cb3b2d to 6d8bc74 Compare October 16, 2025 05:21
@zhangskz zhangskz added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Oct 17, 2025
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Oct 17, 2025
@zhangskz
Copy link
Member

This roughly lgtm, but @mkruskal-google to review for PHP.

@SpencerMalone
Copy link
Contributor Author

Thanks y'all!

@SpencerMalone
Copy link
Contributor Author

What is the process for getting this cut into a release? Do I need to do anything further?

@zhangskz
Copy link
Member

Looks like this missed the branch cut for our 33.x release, but should get picked up for our v34.0 release in Q1 which we'll be starting RC's for soon.

@SpencerMalone
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants