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

consumption of method_signature annotation doesn't account for dot-notation access of nested fields for a required field #684

Open
noahdietz opened this issue Feb 14, 2024 · 1 comment
Labels
good first issue This issue is a good place to started contributing to this repository. priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@noahdietz
Copy link
Collaborator

The following code:

if (count($methodSignatureArguments) !== $requiredFields->count()) {
throw new \LogicException(sprintf(
'missing method signature arguments (Expected "%s", found %s)',
$methodSignature,
implode(',', $requiredFields->map(fn ($f) => $f->name)->toArray()) ?: 'none'
));
}

Does not account for a signature like "foo.a,foo.b" where foo is a google.api.field_behavior = REQUIRED field. The presence of foo.a should be sufficient.

Furthermore, we need to make sure that build fragment generation can handle a dot-notation signature!

@noahdietz noahdietz added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. priority: p3 Desirable enhancement or fix. May not be included in next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Feb 14, 2024
@noahdietz
Copy link
Collaborator Author

Note: the only example of this atm is an api in private preview, so testing of the generator with that proto uncovered this.

Technically, nested fields in signatures is allowed per AIP-4232

image

@noahdietz noahdietz added the good first issue This issue is a good place to started contributing to this repository. label Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue This issue is a good place to started contributing to this repository. priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

1 participant