[BUG][JAVA] Add @Nullable to resttemplate api method parameters - #24970
SubhamAshok wants to merge 2 commits into
Conversation
Fixes OpenAPITools#23656 by using nullableArgumentForApi in resttemplate api.mustache. This ensures optional and nullable operation parameters are annotated with @nullable when useJspecify is enabled, matching webclient and restclient.
|
please update the samples to fix https://github.com/OpenAPITools/openapi-generator/actions/runs/35515230440/job/106090355898?pr=24970 when you've time. |
|
@wing328 Thanks, I have updated the resttemplate samples. |
There was a problem hiding this comment.
1 issue found across 29 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/petstore/java/resttemplate/src/main/java/org/openapitools/client/api/FakeApi.java">
<violation number="1" location="samples/client/petstore/java/resttemplate/src/main/java/org/openapitools/client/api/FakeApi.java:459">
P2: `testBodyWithBinary`/`testBodyWithBinaryWithHttpInfo` annotate the required `body` parameter `@javax.annotation.Nullable`, but the generated body immediately throws `HttpClientErrorException` when `body == null` ("verify the required parameter 'body' is set"). The Javadoc also marks it `(required)`. Marking a parameter nullable while the method rejects null at runtime misleads the static analyzers this PR targets (under `@NullMarked`, callers may pass null and only fail at runtime). The spec schema is nullable, so the required-null-check and the annotation are inconsistent; decide one: emit `@Nullable` and skip the required-null-check for schema-nullable required params, or keep the null check and emit `@Nonnull`. Note the same generated output already exists in the webclient/restclient samples, so the root cause is in the shared `nullableArgumentForApi`/`nullable_var_annotations` logic that this PR adopts for resttemplate.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * @throws RestClientException if an error occurs while attempting to invoke the API | ||
| */ | ||
| public void testBodyWithBinary(File body) throws RestClientException { | ||
| public void testBodyWithBinary(@javax.annotation.Nullable File body) throws RestClientException { |
There was a problem hiding this comment.
P2: testBodyWithBinary/testBodyWithBinaryWithHttpInfo annotate the required body parameter @javax.annotation.Nullable, but the generated body immediately throws HttpClientErrorException when body == null ("verify the required parameter 'body' is set"). The Javadoc also marks it (required). Marking a parameter nullable while the method rejects null at runtime misleads the static analyzers this PR targets (under @NullMarked, callers may pass null and only fail at runtime). The spec schema is nullable, so the required-null-check and the annotation are inconsistent; decide one: emit @Nullable and skip the required-null-check for schema-nullable required params, or keep the null check and emit @Nonnull. Note the same generated output already exists in the webclient/restclient samples, so the root cause is in the shared nullableArgumentForApi/nullable_var_annotations logic that this PR adopts for resttemplate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/java/resttemplate/src/main/java/org/openapitools/client/api/FakeApi.java, line 459:
<comment>`testBodyWithBinary`/`testBodyWithBinaryWithHttpInfo` annotate the required `body` parameter `@javax.annotation.Nullable`, but the generated body immediately throws `HttpClientErrorException` when `body == null` ("verify the required parameter 'body' is set"). The Javadoc also marks it `(required)`. Marking a parameter nullable while the method rejects null at runtime misleads the static analyzers this PR targets (under `@NullMarked`, callers may pass null and only fail at runtime). The spec schema is nullable, so the required-null-check and the annotation are inconsistent; decide one: emit `@Nullable` and skip the required-null-check for schema-nullable required params, or keep the null check and emit `@Nonnull`. Note the same generated output already exists in the webclient/restclient samples, so the root cause is in the shared `nullableArgumentForApi`/`nullable_var_annotations` logic that this PR adopts for resttemplate.</comment>
<file context>
@@ -456,7 +456,7 @@ public ResponseEntity<Void> testAdditionalPropertiesReferenceWithHttpInfo(Map<St
* @throws RestClientException if an error occurs while attempting to invoke the API
*/
- public void testBodyWithBinary(File body) throws RestClientException {
+ public void testBodyWithBinary(@javax.annotation.Nullable File body) throws RestClientException {
testBodyWithBinaryWithHttpInfo(body);
}
</file context>
PR checklist
JavaClientCodegenTest).Description
When
useJspecify=true, package-info files are marked with@NullMarked. For theresttemplatelibrary, API operation methods previously emitted raw parameter types without@Nullable, causing static analysis tools to flag optional parameters as non-null.This change updates
resttemplate/api.mustacheto use{{>nullableArgumentForApi}}, matchingwebclientandrestclient. It also enables the corresponding assertions inJavaClientCodegenTestand updates the sample output.Fixes #23656.
Summary by cubic
Adds
@Nonnulland@Nullableannotations toresttemplateAPI method parameters so required and optional parameters are explicit to static analysis tools. Previously generated methods had no nullability annotations, which made optional parameters look non-null under JSpecify@NullMarkedpackages.api.mustachenow uses the same nullable argument partial aswebclientandrestclient. Fixes #23656.JavaClientCodegenTestforresttemplate.resttemplate-springBoot4-jackson3-jspecifysample.Written for commit 69f0fd4. Summary will update on new commits.