-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
[R] optionally skip parsing responses to R6 objects #22705
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
base: master
Are you sure you want to change the base?
[R] optionally skip parsing responses to R6 objects #22705
Conversation
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.
24 issues found across 19 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/R/store_api.R">
<violation number="1" location="samples/client/petstore/R/R/store_api.R:300">
P2: New `parse` argument inserted before `...` in public `GetInventoryWithHttpInfo` breaks positional callers and can error when `if (!parse)` negates a non-logical value.</violation>
<violation number="2" location="samples/client/petstore/R/R/store_api.R:414">
P2: `parse` added before `...` in `GetOrderByIdWithHttpInfo` rebinds positional `...` arguments and can error when negated.</violation>
<violation number="3" location="samples/client/petstore/R/R/store_api.R:554">
P2: `PlaceOrderWithHttpInfo` now places `parse` before `...`, breaking positional callers and risking errors at `if (!parse)`.</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/pet_api.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/pet_api.R:418">
P2: Adding `parse` before `...` in the public `_with_http_info` signature breaks positional callers by rebinding the third argument to `parse`, introducing a backward-incompatible API change.</violation>
</file>
<file name="samples/client/echo_api/r/R/body_api.R">
<violation number="1" location="samples/client/echo_api/r/R/body_api.R:205">
P2: Backward-incompatible: `parse` added before `...` in `WithHttpInfo` methods, breaking positional `...` calls and contradicting non-positional intent.</violation>
<violation number="2" location="samples/client/echo_api/r/R/body_api.R:243">
P2: Binary response corrupted when parse=FALSE: converting image/gif body to text drops raw bytes</violation>
</file>
<file name="samples/client/petstore/R-httr2/R/pet_api.R">
<violation number="1" location="samples/client/petstore/R-httr2/R/pet_api.R:398">
P1: Unnamed `...` args now collide with `parse` in wrapper call, causing runtime error "formal argument 'parse' matched by multiple actual arguments" when extra args are passed</violation>
</file>
<file name="samples/client/petstore/R/R/pet_api.R">
<violation number="1" location="samples/client/petstore/R/R/pet_api.R:418">
P2: `parse` added before `...` in `*WithHttpInfo` breaks existing positional `...` calls and can error in `if (!parse)`</violation>
</file>
<file name="samples/client/petstore/R/R/user_api.R">
<violation number="1" location="samples/client/petstore/R/R/user_api.R:629">
P2: `parse` parameter added for void endpoints but ignored: success responses still force `content <- NULL`, so `parse=FALSE` cannot return raw text as documented</violation>
<violation number="2" location="samples/client/petstore/R/R/user_api.R:763">
P2: `parse` added before `...` in GetUserByNameWithHttpInfo enables positional binding and breaks existing positional `...` callers</violation>
<violation number="3" location="samples/client/petstore/R/R/user_api.R:893">
P2: `parse` added before `...` in LoginUserWithHttpInfo enables positional binding and breaks existing positional `...` callers</violation>
</file>
<file name="samples/client/petstore/R-httr2/R/fake_api.R">
<violation number="1" location="samples/client/petstore/R-httr2/R/fake_api.R:215">
P2: `parse` added before `...` changes positional argument binding for `_with_http_info`, breaking previous calls that passed extra args positionally and causing `!parse` to error.</violation>
<violation number="2" location="samples/client/petstore/R-httr2/R/fake_api.R:344">
P2: `parse` placed before `...` in `_with_http_info` alters positional binding and can make previous positional extra args hit `!parse`, causing runtime errors.</violation>
</file>
<file name="samples/client/petstore/R-httr2/R/store_api.R">
<violation number="1" location="samples/client/petstore/R-httr2/R/store_api.R:300">
P1: Adding `parse` before `...` in `get_inventory_with_http_info` changes positional binding and can break existing callers that passed request options positionally via `...`.</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/fake_api.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/fake_api.R:215">
P2: parse is placed before ... in add_pet_optional_with_http_info, making parse positionally matchable and breaking positional callers that previously targeted ...</violation>
<violation number="2" location="samples/client/petstore/R-httr2-wrapper/R/fake_api.R:344">
P2: parse precedes ... in fake_data_file_with_http_info, allowing positional capture of parse and breaking positional callers intended for ...</violation>
</file>
<file name="samples/client/petstore/R/R/fake_api.R">
<violation number="1" location="samples/client/petstore/R/R/fake_api.R:215">
P2: New `parse` argument before `...` breaks positional callers: third argument now binds to `parse`, not `...`, changing behavior/causing errors.</violation>
<violation number="2" location="samples/client/petstore/R/R/fake_api.R:344">
P2: `parse` added before `...` intercepts positional `...` arguments, breaking existing callers and causing runtime errors/behavior changes.</violation>
</file>
<file name="samples/client/echo_api/r/R/form_api.R">
<violation number="1" location="samples/client/echo_api/r/R/form_api.R:345">
P2: New `parse` argument for `TestFormOneofWithHttpInfo` is missing from the roxygen `@param` list, resulting in undocumented formal argument warnings.</violation>
</file>
<file name="samples/client/echo_api/r/R/path_api.R">
<violation number="1" location="samples/client/echo_api/r/R/path_api.R:91">
P1: New `parse` parameter is placed before `...`, so unnamed extra args bind to `parse` instead of `...`, breaking existing `...` forwarding and causing runtime errors when `if (!parse)` is evaluated on non-logical values.</violation>
</file>
<file name="samples/client/echo_api/r/R/header_api.R">
<violation number="1" location="samples/client/echo_api/r/R/header_api.R:94">
P2: New `parse` parameter added before `...` in `WithHttpInfo` can rebind existing positional arguments, unlike the wrapper which forces `parse` to be named-only.</violation>
</file>
<file name="samples/client/echo_api/r/R/query_api.R">
<violation number="1" location="samples/client/echo_api/r/R/query_api.R:230">
P2: New `parse` argument is placed before `...`, so unnamed extra args bind to `parse` positionally and break `...` forwarding (can error on `!parse`).</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/user_api.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/user_api.R:763">
P2: `parse` inserted before `...` in `_with_http_info` signature makes positional `...` arguments bind to `parse`, breaking existing callers</violation>
<violation number="2" location="samples/client/petstore/R-httr2-wrapper/R/user_api.R:893">
P2: `parse` added before `...` in `login_user_with_http_info` causes positional `...` args to bind to `parse`, breaking existing calls</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| #' @return character | ||
| TestAuthHttpBasic = function(data_file = NULL, ...) { | ||
| local_var_response <- self$TestAuthHttpBasicWithHttpInfo(data_file = data_file, ...) | ||
| TestAuthHttpBasic = function(data_file = NULL, ..., .parse = TRUE) { |
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.
This follows the ..., .variable = some-default pattern common for forcing named parameters when overriding defaults. E.g., see https://purrr.tidyverse.org/reference/map_if.html
I opted for .parse over parse to avoid potential conflicts with endpoint arguments since R supports variable names that begin with a period.
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.
10 issues found across 19 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_api.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/pet_api.R:552">
P2: `.parse` parameter is documented but ignored for void endpoints; even with `.parse = FALSE`, successful responses return NULL content instead of raw text.</violation>
</file>
<file name="samples/client/echo_api/r/R/body_api.R">
<violation number="1" location="samples/client/echo_api/r/R/body_api.R:205">
P2: `.parse` added to *WithHttpInfo but roxygen docs not updated (param/return drift)</violation>
</file>
<file name="samples/client/petstore/R/R/store_api.R">
<violation number="1" location="samples/client/petstore/R/R/store_api.R:192">
P1: .parse parameter on DeleteOrder is unused; success always nulls content, so parse=FALSE cannot return raw response</violation>
</file>
<file name="samples/client/petstore/R/R/fake_api.R">
<violation number="1" location="samples/client/petstore/R/R/fake_api.R:477">
P2: .parse parameter added for this void endpoint is ignored; success responses still set content to NULL, so `.parse = FALSE` cannot return unparsed text as documented.</violation>
</file>
<file name="samples/client/petstore/R-httr2/R/pet_api.R">
<violation number="1" location="samples/client/petstore/R-httr2/R/pet_api.R:552">
P2: .parse flag is accepted but ignored for delete_pet_with_http_info; 2xx responses always null content so callers cannot obtain unparsed text as documented.</violation>
<violation number="2" location="samples/client/petstore/R-httr2/R/pet_api.R:1491">
P2: .parse flag is ignored in update_pet_with_form_with_http_info; 2xx responses always drop content, so `.parse = FALSE` cannot return the raw text response.</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/user_api.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/user_api.R:301">
P2: .parse parameter is added/documented but ignored for 2xx void endpoints; success bodies are forced to NULL so `.parse = FALSE` cannot return unparsed text.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/r/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/r/api.mustache:573">
P2: `.parse` flag is ignored for primitive-return endpoints; despite being documented for all operations, the switch only runs for non-primitive return types so callers cannot get raw response text for primitives.</violation>
</file>
<file name="samples/client/petstore/R-httr2/R/user_api.R">
<violation number="1" location="samples/client/petstore/R-httr2/R/user_api.R:301">
P2: .parse flag is added/documented but ignored; successful void responses always set content to NULL so `.parse=FALSE` cannot return raw text as promised.</violation>
</file>
<file name="samples/client/petstore/R/R/user_api.R">
<violation number="1" location="samples/client/petstore/R/R/user_api.R:301">
P2: `.parse` parameter is ignored; 2xx responses still set content to NULL so `.parse = FALSE` cannot return unparsed text</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
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.
18 issues found across 19 files (changes from recent commits).
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/echo_api/r/R/header_api.R">
<violation number="1" location="samples/client/echo_api/r/R/header_api.R:162">
P2: `data_file` is no longer written when `.parse = FALSE` because the early return now occurs before `WriteFile`, regressing the documented save-to-file behavior.</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/store_api.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/store_api.R:341">
P2: `data_file` is no longer written when `.parse = FALSE` because the early return happens before `WriteFile`, breaking the advertised file-saving behavior</violation>
</file>
<file name="samples/client/petstore/R/R/user_api.R">
<violation number="1" location="samples/client/petstore/R/R/user_api.R:816">
P2: `data_file` is silently ignored when `.parse = FALSE` because the function returns before `WriteFile` is called (behavior regression).</violation>
</file>
<file name="samples/client/petstore/R-httr2/R/pet_api.R">
<violation number="1" location="samples/client/petstore/R-httr2/R/pet_api.R:480">
P2: `data_file` is no longer written when `.parse = FALSE` because the early return happens before `WriteFile`, regressing the documented file-save option.</violation>
</file>
<file name="samples/client/echo_api/r/R/form_api.R">
<violation number="1" location="samples/client/echo_api/r/R/form_api.R:175">
P2: `data_file` is ignored when `.parse = FALSE` because the early return happens before `WriteFile`, regressing file output for raw responses.</violation>
</file>
<file name="samples/client/echo_api/r/R/query_api.R">
<violation number="1" location="samples/client/echo_api/r/R/query_api.R:284">
P2: `data_file` is never written when `.parse = FALSE` due to early return before `WriteFile`, regressing the documented save-to-file behavior.</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/fake_api.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/fake_api.R:271">
P2: `data_file` is no longer written when `.parse = FALSE` because `WriteFile` was moved after the early return, so requested files are silently ignored.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/r/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/r/api.mustache:564">
P2: .parse short-circuit returns before WriteFile, so data_file is never saved when parsing is disabled</violation>
</file>
<file name="samples/client/petstore/R-httr2/R/store_api.R">
<violation number="1" location="samples/client/petstore/R-httr2/R/store_api.R:341">
P1: `data_file` is ignored when `.parse = FALSE` because the early return now occurs before `WriteFile`, regressing file saving for raw responses.</violation>
</file>
<file name="samples/client/echo_api/r/R/body_api.R">
<violation number="1" location="samples/client/echo_api/r/R/body_api.R:243">
P1: `data_file` no longer writes when `.parse=FALSE` due to early return before `WriteFile` (regression)</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/pet_api.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/pet_api.R:480">
P2: `data_file` is skipped when `.parse = FALSE` because `WriteFile` is after the early return.</violation>
</file>
<file name="samples/client/petstore/R/R/store_api.R">
<violation number="1" location="samples/client/petstore/R/R/store_api.R:341">
P2: data_file is no longer written when .parse = FALSE because WriteFile was moved after the early return</violation>
</file>
<file name="samples/client/petstore/R-httr2-wrapper/R/user_api.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/user_api.R:816">
P2: data_file not written when .parse=FALSE due to early return before WriteFile (regression from prior behavior)</violation>
</file>
<file name="samples/client/petstore/R-httr2/R/user_api.R">
<violation number="1" location="samples/client/petstore/R-httr2/R/user_api.R:816">
P2: login_user_with_http_info now returns early when `.parse = FALSE`, bypassing WriteFile so data_file output is lost (regression).</violation>
</file>
<file name="samples/client/echo_api/r/R/path_api.R">
<violation number="1" location="samples/client/echo_api/r/R/path_api.R:178">
P2: `data_file` is never written when `.parse = FALSE` because `WriteFile` moved below the early return, causing a regression for callers who request raw responses but still want the file saved.</violation>
</file>
<file name="samples/client/echo_api/r/R/auth_api.R">
<violation number="1" location="samples/client/echo_api/r/R/auth_api.R:141">
P2: Bearer endpoint no longer writes data_file when .parse=FALSE due to return before WriteFile</violation>
</file>
<file name="samples/client/petstore/R/R/pet_api.R">
<violation number="1" location="samples/client/petstore/R/R/pet_api.R:480">
P2: `data_file` is never written when `.parse = FALSE` because the code returns before `WriteFile`, regressing the file-save option.</violation>
</file>
<file name="samples/client/petstore/R-httr2/R/fake_api.R">
<violation number="1" location="samples/client/petstore/R-httr2/R/fake_api.R:270">
P2: data_file is ignored when .parse = FALSE due to early return before WriteFile, preventing the documented file save</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
thanks for the PR
maybe I miss it, what exactly is the name of the option to skip parsing responses to R6 objects? |
@Ramanth, @saigiridhar21, @wing328
This PR adds an option in the R client to return the REST response as text. This can be useful in cases where the end-user intent is to coerce data into an R
listordata.frameanyway, in which case parsing to R6 objects (which involves multiple hops between text andlist/data.framerepresentations of the data) can be unnecessary compute.The feature is off by default, so expected behavior is unchanged. It will only kick in if the user adds an explicit
parse = FALSEargument to an endpoint method call. I also positioned the argument after the ellipsis argument (...) to so that this is only available when using a named argument, not positionally.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
Add an optional .parse flag to R client endpoints that return data to allow returning raw responses and reduce overhead. Default behavior is unchanged.
Written for commit 122acd2. Summary will update on new commits.