-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
[R] avoid to-JSON issues when R6 classes contain lists of R6 classes #22828
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
Merged
+304
−18
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
14 issues found across 75 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="samples/client/petstore/R-httr2-wrapper/R/pet_map.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/pet_map.R:84">
P2: Recursive conversion helpers are added but never used; additional_properties still serialized as raw R6 objects so nested R6 values remain unconverted.</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/dummy_model.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/dummy_model.R:85">
P2: Added nested-R6 helper is never used, so toSimpleType()/toJSONString still pass nested R6 values through unchanged and the intended serialization fix is not applied.</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/basque_pig.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/basque_pig.R:98">
P2: New nested-R6 helper added but never used; additional_properties still serialize raw R6 objects and can fail JSON conversion</violation>
</file>
<file name="samples/client/petstore/R/R/zebra.R">
<violation number="1" location="samples/client/petstore/R/R/zebra.R:101">
P2: Added nested-R6 conversion helper is unused; nested R6 values in additional_properties are still passed directly to jsonlite::toJSON, leaving serialization failures unresolved.</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/allof_tag_api_response.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/allof_tag_api_response.R:137">
P2: extractSimpleType/hasNestedR6 were added but never used by toSimpleType() or toJSONString(), so additional_properties are still serialized without conversion and nested R6 values remain unhandled.</violation>
</file>
<file name="samples/client/petstore/R/R/dog.R">
<violation number="1" location="samples/client/petstore/R/R/dog.R:112">
P2: Nested R6 conversion helpers are unused, so toJSON still receives raw R6 objects and may fail for additional_properties</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/animal.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/animal.R:100">
P2: New nested-R6 conversion helper is added but never used; additional_properties still pass raw R6 objects to jsonlite::toJSON, defeating the intended nested-R6 fix.</violation>
</file>
<file name="samples/client/petstore/R/R/whale.R">
<violation number="1" location="samples/client/petstore/R/R/whale.R:111">
P2: New nested-R6 helpers are added but never used; `toSimpleType()`/`toJSONString()` still emit raw `additional_properties`, so nested R6 objects remain unconverted and the intended JSON fix is ineffective.</violation>
</file>
<file name="samples/client/petstore/R/R/date.R">
<violation number="1" location="samples/client/petstore/R/R/date.R:115">
P2: New nested-R6 conversion helper is unused; additional_properties still pass R6 values straight to jsonlite::toJSON, so nested R6 in additional properties will still fail serialization and the helper becomes dead code.</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/format_test.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/format_test.R:258">
P2: New nested-R6 conversion helpers are added but never invoked by `toSimpleType()`/`toJSONString()`, so nested R6 values are still serialized raw and the intended JSON fix isn’t applied in this class.</violation>
</file>
<file name="samples/client/petstore/R/R/special.R">
<violation number="1" location="samples/client/petstore/R/R/special.R:165">
P2: Added extractSimpleType()/hasNestedR6 helpers are never invoked in Special, so serialization still copies additional_properties verbatim and the new recursive conversion logic is unused.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/r/modelGeneric.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/r/modelGeneric.mustache:276">
P2: New public method names can collide with schema property names, causing field/method overwrite in generated R6 models when a schema property is named `extractSimpleType` or `hasNestedR6`.</violation>
</file>
<file name="samples/client/petstore/R/R/model_api_response.R">
<violation number="1" location="samples/client/petstore/R/R/model_api_response.R:111">
P2: `extractSimpleType()`/`hasNestedR6()` are added but never used by `toSimpleType()`/`toJSONString()`, so nested R6 values still reach `jsonlite::toJSON()` unconverted. The new helpers are effectively dead code and do not address the to-JSON issue.</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/cat.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/cat.R:112">
P2: New nested-R6 conversion helpers are never used in Cat serialization methods, so additional_properties are still passed through unchanged and the added helper has no effect.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
samples/client/petstore/R-httr2-wrapper/R/allof_tag_api_response.R
Outdated
Show resolved
Hide resolved
Member
|
thanks for the PR let's give it a try |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@Ramanth, @saigiridhar21, @wing328
This is a follow-up to #22697.
In #22697 I improved handling for deserializing cases where there is a list of R6 objects nested in a field of a parent R6 object. This PR handles converting an R6 object back to text (e.g., for printing to the console or generating a JSON representation of the object). Existing code did not properly handle the case and would throw an error when
$toSimpleType()was invoked onxwhenxwas a list (of R6 objects). This PR will recursively delve into the fields of an R6 object and invoke$toSimpleType()on nested R6 objects only after ensuring an object is an R6 object.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 JSON serialization in the R client when models contain nested lists/maps of R6 objects. Adds a recursive extractor so only R6 instances call toSimpleType, preventing errors when printing or generating JSON.
Bug Fixes
Refactors
Written for commit 9327b21. Summary will update on new commits.