Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixing protobuf mergeFromJsonString stub #4840

Open
wants to merge 2 commits into
base: v5
Choose a base branch
from

Conversation

brettmc
Copy link
Contributor

@brettmc brettmc commented Feb 29, 2024

since protobuf v3.13 (protocolbuffers/protobuf#7695 about 4 years ago) Google\Protobuf\Internal\Message::mergeFromJsonString has an optional second boolean parameter.

since protobuf v3.13 (protocolbuffers/protobuf#7695 about 4 years ago) Google\Protobuf\Internal\Message::mergeFromJsonString has an optional second boolean parameter.
the parameter has a different name in native vs extension :(
@brettmc
Copy link
Contributor Author

brettmc commented Feb 29, 2024

Currently, when running phan on php 8.0+ when supplying the second parameter and using the protobuf extension (but not the native library), this results in a PhanParamTooManyInternal Call with 2 arg(s) to \Google\Protobuf\Internal\Message::mergeFromJsonString($data) which only takes 1 arg(s). This is an ArgumentCountError for internal functions in PHP 8.0+.

LMK if this needs a test. It looks like that would require adding the protobuf extension into the dockerfile that the tests are run against (which would impact build times).

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.

None yet

1 participant