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

fix: phpdoc for repeatedfield #9783

Merged
merged 2 commits into from
Apr 18, 2022
Merged

fix: phpdoc for repeatedfield #9783

merged 2 commits into from
Apr 18, 2022

Conversation

bshaffer
Copy link
Contributor

@bshaffer bshaffer commented Apr 13, 2022

I have found two issues with PHP protobuf's RepeatedField when using static analyzers:

  1. The PHPDoc says that the first argument to offsetSet is an integer, but this value can also be null, in the case when a RepeatedField is being appended to (e.g. $repeatedField[] = $someRepeatedValue). Without marking the first argument as optional, static analyzers think that instances of RepeatedField cannot be appended to.
  2. The PHPDoc says that offsetSet accepts object and offsetGet returns object. But object excludes primitive types in PHP such as string int, bool, and array. I believe it should accept primitives, and as a result, in certain circumstances the static analyzers are failing. I've tested this with using the accepted PHP type mixed, which represents any object or primitive. The keyword only exists in PHP 8.0, but this is acceptable documentation for any version of PHP.

@perezd perezd merged commit 81e5139 into protocolbuffers:main Apr 18, 2022
@bshaffer bshaffer deleted the patch-3 branch April 19, 2022 18:24
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.

4 participants