Conversation
📝 WalkthroughWalkthroughUpdated OAS 2.0 → OAS 3.x conversion to propagate conversion context (target version, result, path) through schema/parameter/response/item/requestBody conversions and to normalize Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #360 +/- ##
==========================================
- Coverage 84.81% 84.75% -0.06%
==========================================
Files 194 194
Lines 27345 27458 +113
==========================================
+ Hits 23192 23272 +80
- Misses 2820 2842 +22
- Partials 1333 1344 +11
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
converter/helpers.go (1)
241-253:⚠️ Potential issue | 🟡 MinorConvert the response schema once before filling
content.
convertOAS2SchemaToOAS3can now append warnings. Running it inside theproducesloop emits the same issue once per media type even though OAS 2.0 has a singleresponse.schema. A regression with twoproducesvalues would catch this.As per coding guidelines: Use generated `DeepCopy()` methods for deep copying; never use JSON marshal/unmarshal for deep copying.🔧 Suggested change
+ convertedSchema := c.convertOAS2SchemaToOAS3(response.Schema, targetVersion, result, path+".schema") for _, mediaType := range mediaTypes { + var schemaCopy *parser.Schema + if convertedSchema != nil { + schemaCopy = convertedSchema.DeepCopy() + } converted.Content[mediaType] = &parser.MediaType{ - Schema: c.convertOAS2SchemaToOAS3(response.Schema, targetVersion, result, path+".schema"), + Schema: schemaCopy, } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@converter/helpers.go` around lines 241 - 253, The conversion of an OAS2 response schema is being run inside the produces loop (see response.Schema, produces and convertOAS2SchemaToOAS3), which causes duplicate warnings for each media type; instead call convertOAS2SchemaToOAS3 once before iterating mediaTypes, store the returned parser.Schema result, then for each media type assign a deep copy (use the generated DeepCopy() method) to converted.Content[mediaType] so each media type gets its own copy without re-running conversion or using JSON marshal/unmarshal; ensure you fall back to getDefaultMediaType() when produces is empty.converter/oas2_to_oas3.go (1)
180-206:⚠️ Potential issue | 🟠 MajorDon't emit the new exclusive-bound warnings in both passes.
By the time this branch runs, the same
body/formDataparameter has already been converted once viaconvertParameters. The new OAS 3.1 checks now fire in that discarded parameter pass and then again when the source is remapped into a request body, so one malformed input produces duplicate issues. Please keep the request-body remap as the only place that emits those schema warnings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@converter/oas2_to_oas3.go` around lines 180 - 206, The duplicate "exclusive-bound" schema warnings come from validating body/formData parameters twice—once in convertParameters and again when remapping into a RequestBody—so update the earlier parameter-pass to suppress those specific schema checks for parameters with In == "body" or In == "formData" (or add a boolean flag to convertParameters to skip emitting exclusive-bound warnings for parameters that will be remapped), leaving convertOAS2RequestBody and convertOAS2FormDataToRequestBody as the sole emitters of those warnings (use addIssueWithContext only in the request-body remap code paths).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@converter/helpers.go`:
- Around line 51-52: The conversion call for parameter schemas forwards the
parent path as-is causing nested diagnostic paths like
parameters[0].properties.foo; update the call that sets converted.Schema by
appending ".schema" to the forwarded path so warnings point to
parameters[0].schema.properties.foo; specifically modify the invocation of
c.convertOAS2SchemaToOAS3 (which currently passes path) to pass path+".schema"
(keeping result.TargetOASVersion and result unchanged) so param.Schema ->
converted.Schema mapping uses the correct path.
In `@converter/oas2_to_oas3.go`:
- Around line 247-250: The loop is calling convertOAS2SchemaToOAS3 for each
media type causing duplicated warnings; instead call convertOAS2SchemaToOAS3
once (using bodyParam.Schema, result.TargetOASVersion, result, "requestBody") to
produce a single converted schema, then assign a deep copy of that converted
schema to requestBody.Content[mediaType] for each entry in consumes using the
generated DeepCopy() method on the converted schema (do not use JSON
marshal/unmarshal for copying); update the block that fills requestBody.Content
to compute convertedSchema := c.convertOAS2SchemaToOAS3(...) once and then set
requestBody.Content[mediaType].Schema = convertedSchema.DeepCopy() inside the
loop.
---
Outside diff comments:
In `@converter/helpers.go`:
- Around line 241-253: The conversion of an OAS2 response schema is being run
inside the produces loop (see response.Schema, produces and
convertOAS2SchemaToOAS3), which causes duplicate warnings for each media type;
instead call convertOAS2SchemaToOAS3 once before iterating mediaTypes, store the
returned parser.Schema result, then for each media type assign a deep copy (use
the generated DeepCopy() method) to converted.Content[mediaType] so each media
type gets its own copy without re-running conversion or using JSON
marshal/unmarshal; ensure you fall back to getDefaultMediaType() when produces
is empty.
In `@converter/oas2_to_oas3.go`:
- Around line 180-206: The duplicate "exclusive-bound" schema warnings come from
validating body/formData parameters twice—once in convertParameters and again
when remapping into a RequestBody—so update the earlier parameter-pass to
suppress those specific schema checks for parameters with In == "body" or In ==
"formData" (or add a boolean flag to convertParameters to skip emitting
exclusive-bound warnings for parameters that will be remapped), leaving
convertOAS2RequestBody and convertOAS2FormDataToRequestBody as the sole emitters
of those warnings (use addIssueWithContext only in the request-body remap code
paths).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9610f6e2-53a2-4085-b048-247c2312ab98
📒 Files selected for processing (4)
converter/helpers.goconverter/helpers_test.goconverter/oas2_to_oas3.goconverter/schema_convert.go
…semantics for OAS 3.1 targets OAS 2.0 and 3.0 use boolean exclusiveMaximum/exclusiveMinimum to modify the adjacent maximum/minimum bound. OAS 3.1 (JSON Schema 2020-12) changed these to standalone numeric values, dropping maximum/minimum entirely. The converter previously copied the boolean form regardless of target version, producing invalid OAS 3.1 output. Fix all four conversion sites: - convertOAS2ParameterToOAS3: inline parameter type/format schema - convertOAS2FormDataToRequestBody: formData parameter properties - convertOAS2ItemsToSchema: nested items arrays (new c/result/path params) - convertOAS2SchemaToOAS3: component definitions via new recursive fixSchemaExclusiveMinMaxForOAS31 walker (mirrors walkSchemaFeatures) For OAS 3.1 targets, each site now sets exclusiveMaximum to *maximum and clears maximum. For OAS 3.0 targets, boolean form is preserved. Edge case: exclusiveMaximum:true with no maximum (malformed OAS 2.0) emits a SeverityWarning to ConversionResult at all four sites rather than silently dropping the constraint. Closes #358 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
erraggy
force-pushed
the
fix/converter-exclusive-min-max-oas31
branch
from
774c912 to
cc49aac
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@converter/helpers.go`:
- Around line 70-97: The code reads result.TargetOASVersion and calls
c.addIssueWithContext(result, ...) without ensuring result is non-nil, risking a
nil-pointer panic; update the blocks handling param.ExclusiveMaximum and
param.ExclusiveMinimum to first check if result != nil before accessing
result.TargetOASVersion or invoking c.addIssueWithContext (e.g., compute
isOAS31OrLater only when result is non-nil or use a safe default, and only call
c.addIssueWithContext when result is non-nil), referencing the symbols
param.ExclusiveMaximum, param.ExclusiveMinimum, c.isOAS31OrLater,
result.TargetOASVersion, and c.addIssueWithContext to locate where to add the
guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 16039dff-5002-43d8-a3d6-50f77b65a4ae
📒 Files selected for processing (4)
converter/helpers.goconverter/helpers_test.goconverter/oas2_to_oas3.goconverter/schema_convert.go
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Fixes #358
OAS 2.0/3.0 use
exclusiveMaximum/exclusiveMinimumas booleans that modify an adjacentmaximum/minimumbound. OAS 3.1 (JSON Schema 2020-12) changed these to standalone numeric values, droppingmaximum/minimumentirely. The converter previously copied the boolean form regardless of target version, producing invalid OAS 3.1 output.exclusiveMaximumto*maximumand clearsmaximum(same for min)exclusiveMaximum: truewith nomaximum(malformed OAS 2.0) now emits aSeverityWarningtoConversionResultinstead of silently dropping the constraintScope
All four conversion sites updated:
convertOAS2ParameterToOAS3— inline parameter type/format schemahelpers.goconvertOAS2ResponseToOAS3Old— response schemas (component + per-operation)helpers.goconvertOAS2FormDataToRequestBody— formData parameter propertiesoas2_to_oas3.goconvertOAS2ItemsToSchema— nested items arraysoas2_to_oas3.goconvertOAS2SchemaToOAS3— component definitions viafixSchemaExclusiveMinMaxForOAS31schema_convert.gofixSchemaExclusiveMinMaxForOAS31is a new recursive schema walker that mirrors the structure ofwalkSchemaFeatures, visiting all nested schema locations (properties, allOf/anyOf/oneOf, items, patternProperties, $defs, if/then/else, etc.) with cycle detection via a visited map.convertOAS2ItemsToSchemagainedc *Converter, result *ConversionResult, path stringparameters — consistent with every other conversion helper in this package.Test plan
exclusiveMaximum: true + maximum: 100→exclusiveMaximum: 100,maximumremovedmaximumunchangedexclusiveMaximum: truewith nilmaximum→ warning emitted, no panictargetVersionthreaded recursivelyallOftraversal: composition schemas converted correctlyfalseboolean → clears tonil(valid OAS 3.1 no-op removal)🤖 Generated with Claude Code