[JAVA](bugfix) indentation error for java client generator#23228
Conversation
There was a problem hiding this comment.
15 issues found across 103 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
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/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/RFC3339DateFormat.java">
<violation number="1" location="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/RFC3339DateFormat.java:41">
P2: `parse(String)` delegates to the non-throwing ParsePosition parser and may return `null` instead of signaling parse failure with `ParseException`.</violation>
</file>
<file name="samples/client/others/java/restclient-sealedInterface/README.md">
<violation number="1" location="samples/client/others/java/restclient-sealedInterface/README.md:101">
P2: Generated README sample catches `HttpStatusCodeException` without importing it, making the documented example uncompilable as-is.</violation>
</file>
<file name="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/RFC3339JavaTimeModule.java">
<violation number="1" location="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/RFC3339JavaTimeModule.java:32">
P1: Deserializers are added after `super.setupModule(context)`, so custom RFC3339 deserializers are not registered during module setup.</violation>
</file>
<file name="samples/client/others/java/restclient-sealedInterface/.travis.yml">
<violation number="1" location="samples/client/others/java/restclient-sealedInterface/.travis.yml:8">
P1: Travis CI JDK matrix is incompatible with the sample’s Java 17+ build requirements, so CI does not validate the intended feature level.</violation>
</file>
<file name="samples/client/others/java/restclient-sealedInterface/docs/FooApi.md">
<violation number="1" location="samples/client/others/java/restclient-sealedInterface/docs/FooApi.md:99">
P2: Java docs example uses `List<FooRefOrValue>` without importing `java.util.List`, so the sample does not compile as copied.</violation>
</file>
<file name="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/auth/HttpBearerAuth.java">
<violation number="1" location="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/auth/HttpBearerAuth.java:36">
P2: `getBearerToken()` can throw `NullPointerException` when `tokenSupplier` is unset/null, despite the class otherwise supporting absent bearer tokens.</violation>
</file>
<file name="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/ApiClient.java">
<violation number="1" location="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/ApiClient.java:470">
P2: JSON query collection serialization collapses collections into a single string, making MULTI unreachable and producing invalid JSON arrays for non-comma separators.</violation>
<violation number="2" location="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/ApiClient.java:720">
P2: Cookies are added as separate `Cookie` headers for request and default maps, causing duplicate cookie values and broken override semantics.</violation>
</file>
<file name="samples/client/others/java/restclient-sealedInterface/pom.xml">
<violation number="1" location="samples/client/others/java/restclient-sealedInterface/pom.xml:49">
P2: The enforced minimum Maven version (2.2.0) is far below the minimum required by the declared plugin versions, so the project advertises unsupported Maven runtimes.</violation>
</file>
<file name="samples/client/others/java/restclient-sealedInterface/git_push.sh">
<violation number="1" location="samples/client/others/java/restclient-sealedInterface/git_push.sh:48">
P2: Personal access token is embedded in the remote URL, causing it to be persisted in `.git/config` and increasing credential leakage risk.</violation>
<violation number="2" location="samples/client/others/java/restclient-sealedInterface/git_push.sh:53">
P2: `git_push.sh` hardcodes `master` for pull/push, which breaks for repos using `main` or another default branch.</violation>
<violation number="3" location="samples/client/others/java/restclient-sealedInterface/git_push.sh:57">
P2: `git push` failure is masked by the `grep` pipeline, so the script can report success when push actually failed.</violation>
</file>
<file name="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/model/Extensible.java">
<violation number="1" location="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/model/Extensible.java:37">
P2: `final` is being emitted for a non-sealed model (`Extensible`), which can unintentionally prevent subclassing and indicates over-broad sealed-template logic.</violation>
</file>
<file name="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/ServerConfiguration.java">
<violation number="1" location="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/ServerConfiguration.java:48">
P2: `URL()` can throw `NullPointerException` because `variables` is not null-guarded before iterating `this.variables.entrySet()`.</violation>
<violation number="2" location="samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/ServerConfiguration.java:55">
P1: `URL(Map<String,String>)` can throw `NullPointerException` because it dereferences nullable `serverVariable.enumValues` (and accepts null override values) without validation.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| @Override | ||
| public void setupModule(SetupContext context) { | ||
| super.setupModule(context); |
There was a problem hiding this comment.
P1: Deserializers are added after super.setupModule(context), so custom RFC3339 deserializers are not registered during module setup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/RFC3339JavaTimeModule.java, line 32:
<comment>Deserializers are added after `super.setupModule(context)`, so custom RFC3339 deserializers are not registered during module setup.</comment>
<file context>
@@ -0,0 +1,39 @@
+
+ @Override
+ public void setupModule(SetupContext context) {
+ super.setupModule(context);
+
+ addDeserializer(Instant.class, RFC3339InstantDeserializer.INSTANT);
</file context>
| # | ||
| language: java | ||
| jdk: | ||
| - openjdk12 |
There was a problem hiding this comment.
P1: Travis CI JDK matrix is incompatible with the sample’s Java 17+ build requirements, so CI does not validate the intended feature level.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/restclient-sealedInterface/.travis.yml, line 8:
<comment>Travis CI JDK matrix is incompatible with the sample’s Java 17+ build requirements, so CI does not validate the intended feature level.</comment>
<file context>
@@ -0,0 +1,22 @@
+#
+language: java
+jdk:
+ - openjdk12
+ - openjdk11
+ - openjdk10
</file context>
|
|
||
| if (variables != null && variables.containsKey(name)) { | ||
| value = variables.get(name); | ||
| if (serverVariable.enumValues.size() > 0 && !serverVariable.enumValues.contains(value)) { |
There was a problem hiding this comment.
P1: URL(Map<String,String>) can throw NullPointerException because it dereferences nullable serverVariable.enumValues (and accepts null override values) without validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/ServerConfiguration.java, line 55:
<comment>`URL(Map<String,String>)` can throw `NullPointerException` because it dereferences nullable `serverVariable.enumValues` (and accepts null override values) without validation.</comment>
<file context>
@@ -0,0 +1,72 @@
+
+ if (variables != null && variables.containsKey(name)) {
+ value = variables.get(name);
+ if (serverVariable.enumValues.size() > 0 && !serverVariable.enumValues.contains(value)) {
+ throw new IllegalArgumentException("The variable " + name + " in the server URL has invalid value " + value + ".");
+ }
</file context>
|
|
||
| @Override | ||
| public Date parse(String source) { | ||
| return parse(source, new ParsePosition(0)); |
There was a problem hiding this comment.
P2: parse(String) delegates to the non-throwing ParsePosition parser and may return null instead of signaling parse failure with ParseException.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/RFC3339DateFormat.java, line 41:
<comment>`parse(String)` delegates to the non-throwing ParsePosition parser and may return `null` instead of signaling parse failure with `ParseException`.</comment>
<file context>
@@ -0,0 +1,58 @@
+
+ @Override
+ public Date parse(String source) {
+ return parse(source, new ParsePosition(0));
+ }
+
</file context>
| try { | ||
| Bar result = apiInstance.createBar(barCreate); | ||
| System.out.println(result); | ||
| } catch (HttpStatusCodeException e) { |
There was a problem hiding this comment.
P2: Generated README sample catches HttpStatusCodeException without importing it, making the documented example uncompilable as-is.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/restclient-sealedInterface/README.md, line 101:
<comment>Generated README sample catches `HttpStatusCodeException` without importing it, making the documented example uncompilable as-is.</comment>
<file context>
@@ -0,0 +1,162 @@
+ try {
+ Bar result = apiInstance.createBar(barCreate);
+ System.out.println(result);
+ } catch (HttpStatusCodeException e) {
+ System.err.println("Exception when calling BarApi#createBar");
+ System.err.println("Status code: " + e.getStatusCode().value());
</file context>
|
|
||
| fi | ||
|
|
||
| git pull origin master |
There was a problem hiding this comment.
P2: git_push.sh hardcodes master for pull/push, which breaks for repos using main or another default branch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/restclient-sealedInterface/git_push.sh, line 53:
<comment>`git_push.sh` hardcodes `master` for pull/push, which breaks for repos using `main` or another default branch.</comment>
<file context>
@@ -0,0 +1,57 @@
+
+fi
+
+git pull origin master
+
+# Pushes (Forces) the changes in the local repository up to the remote repository
</file context>
| echo "[INFO] \$GIT_TOKEN (environment variable) is not set. Using the git credential in your environment." | ||
| git remote add origin https://${git_host}/${git_user_id}/${git_repo_id}.git | ||
| else | ||
| git remote add origin https://${git_user_id}:"${GIT_TOKEN}"@${git_host}/${git_user_id}/${git_repo_id}.git |
There was a problem hiding this comment.
P2: Personal access token is embedded in the remote URL, causing it to be persisted in .git/config and increasing credential leakage risk.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/restclient-sealedInterface/git_push.sh, line 48:
<comment>Personal access token is embedded in the remote URL, causing it to be persisted in `.git/config` and increasing credential leakage risk.</comment>
<file context>
@@ -0,0 +1,57 @@
+ echo "[INFO] \$GIT_TOKEN (environment variable) is not set. Using the git credential in your environment."
+ git remote add origin https://${git_host}/${git_user_id}/${git_repo_id}.git
+ else
+ git remote add origin https://${git_user_id}:"${GIT_TOKEN}"@${git_host}/${git_user_id}/${git_repo_id}.git
+ fi
+
</file context>
|
|
||
| # Pushes (Forces) the changes in the local repository up to the remote repository | ||
| echo "Git pushing to https://${git_host}/${git_user_id}/${git_repo_id}.git" | ||
| git push origin master 2>&1 | grep -v 'To https' |
There was a problem hiding this comment.
P2: git push failure is masked by the grep pipeline, so the script can report success when push actually failed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/restclient-sealedInterface/git_push.sh, line 57:
<comment>`git push` failure is masked by the `grep` pipeline, so the script can report success when push actually failed.</comment>
<file context>
@@ -0,0 +1,57 @@
+
+# Pushes (Forces) the changes in the local repository up to the remote repository
+echo "Git pushing to https://${git_host}/${git_user_id}/${git_repo_id}.git"
+git push origin master 2>&1 | grep -v 'To https'
</file context>
| Extensible.JSON_PROPERTY_AT_TYPE | ||
| }) | ||
| @jakarta.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", comments = "Generator version: 7.21.0-SNAPSHOT") | ||
| public final class Extensible implements Serializable { |
There was a problem hiding this comment.
P2: final is being emitted for a non-sealed model (Extensible), which can unintentionally prevent subclassing and indicates over-broad sealed-template logic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/model/Extensible.java, line 37:
<comment>`final` is being emitted for a non-sealed model (`Extensible`), which can unintentionally prevent subclassing and indicates over-broad sealed-template logic.</comment>
<file context>
@@ -0,0 +1,173 @@
+ Extensible.JSON_PROPERTY_AT_TYPE
+})
+@jakarta.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", comments = "Generator version: 7.21.0-SNAPSHOT")
+public final class Extensible implements Serializable {
+ private static final long serialVersionUID = 1L;
+
</file context>
| String url = this.URL; | ||
|
|
||
| // go through variables and replace placeholders | ||
| for (Map.Entry<String, ServerVariable> variable: this.variables.entrySet()) { |
There was a problem hiding this comment.
P2: URL() can throw NullPointerException because variables is not null-guarded before iterating this.variables.entrySet().
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/restclient-sealedInterface/src/main/java/org/openapitools/client/ServerConfiguration.java, line 48:
<comment>`URL()` can throw `NullPointerException` because `variables` is not null-guarded before iterating `this.variables.entrySet()`.</comment>
<file context>
@@ -0,0 +1,72 @@
+ String url = this.URL;
+
+ // go through variables and replace placeholders
+ for (Map.Entry<String, ServerVariable> variable: this.variables.entrySet()) {
+ String name = variable.getKey();
+ ServerVariable serverVariable = variable.getValue();
</file context>
There was a problem hiding this comment.
1 issue found across 1 file (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=".github/workflows/samples-java-client-jdk11.yaml">
<violation number="1" location=".github/workflows/samples-java-client-jdk11.yaml:27">
P2: Workflow trigger includes `restclient-sealedInterface`, but matrix omits that sample, so JDK11 CI can pass without building the changed sample.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
dce114c to
67d7bb1
Compare
|
since the output only works with JDK17+, please add the new folder in https://github.com/OpenAPITools/openapi-generator/blob/master/.github/workflows/samples-java-client-jdk17.yaml instead |
|
https://github.com/OpenAPITools/openapi-generator/actions/runs/23043644077/job/66928932845?pr=23228 please update the samples again |
67d7bb1 to
88da441
Compare
88da441 to
f2a3be9
Compare
updated |
|
thanks for the PR which has been merged |
When enabling serializableModel, useOneOfInterfaces and useSealedOneOfInterfaces, 2 issues we're found:
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)Summary by cubic
Fixes spacing and duplicate modifier issues in generated Java oneOf sealed interfaces and adds a
restclientsealed-interface sample wired into the JDK17 samples workflow, with Maven CI on Java 17/21 and an executable Gradle wrapper for Gradle builds.extendslist beforepermitsinJava/oneof_interface.mustache.finalonly once inJava/sealed.mustachewhen nopermitsare present (apply to the first implemented interface only).gradlewexecutable in the new sample to ensure Gradle builds run in CI.Written for commit f4fe3d1. Summary will update on new commits.