Skip to content

fix(generator): use json_name when available#12940

Open
diegomarquezp wants to merge 7 commits intomainfrom
biglake-p1
Open

fix(generator): use json_name when available#12940
diegomarquezp wants to merge 7 commits intomainfrom
biglake-p1

Conversation

@diegomarquezp
Copy link
Copy Markdown
Contributor

b/505851871

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for the json_name protobuf option within the GAPIC generator for Java REST clients, ensuring that HTTP query parameters correctly use custom JSON names, such as those in kebab-case. The changes involve updating the Field and HttpBinding models to store the JSON name and modifying the HttpRuleParser to extract and propagate this information. Review feedback highlights that certain ternary checks for jsonName are redundant because the protobuf descriptor provides a default value, and suggests using the diamond operator for ArrayList instantiation to avoid raw types.

I am having trouble creating individual review comments. Click here to see my feedback.

sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposer.java (929-931)

medium

The fieldDescriptor.getJsonName() method (used in Parser.java to populate the Field model) always returns a non-empty string, as it defaults to the lower-camel-case version of the field name if the json_name option is not explicitly set in the proto. Consequently, httpBindingFieldName.jsonName() is unlikely to be null here, making this ternary operation and the fallback to JavaStyle.toLowerCamelCase redundant. If the intention is to use the generator's custom camel-casing logic for fields without an explicit json_name, the parser should be updated to only set jsonName when the option is present.

References
  1. Avoid adding defensive null checks for values that are guaranteed to be non-null by design, as this can hide invariant breaks.

sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java (150)

medium

Avoid using raw types. Use the diamond operator <> for the ArrayList instantiation.

      List<String> jsonNameParts = new ArrayList<>();

sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java (163-166)

medium

As noted in other files, field.jsonName() is expected to be non-null and non-empty because it is populated from fieldDescriptor.getJsonName(), which provides a default camel-case name if the json_name option is missing. This ternary check and the fallback to JavaStyle.toLowerCamelCase appear to be unreachable code under the current parsing logic.

References
  1. Avoid adding defensive null checks for values that are guaranteed to be non-null by design, as this can hide invariant breaks.

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed for 'gapic-generator-java-root'

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@sonarqubecloud
Copy link
Copy Markdown

@diegomarquezp diegomarquezp marked this pull request as ready for review April 27, 2026 20:55
@diegomarquezp diegomarquezp requested a review from a team as a code owner April 27, 2026 20:55
Copy link
Copy Markdown
Contributor

@blakeli0 blakeli0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending discussion of b/505851871#comment4

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.

2 participants