-
Notifications
You must be signed in to change notification settings - Fork 32
add formats to openapi3 numeric types #151
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,22 @@ | |
| r := &Reflector{} | ||
| r.SpecEns() | ||
|
|
||
| r.DefaultOptions = append(r.DefaultOptions, jsonschema.InterceptSchema(func(params jsonschema.InterceptSchemaParams) (stop bool, err error) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2 | Confidence: High The change adds format annotations for numeric types in OpenAPI 3.0 schemas, aligning with OpenAPI specification standards. While this improves specification compliance, the implementation unconditionally sets formats without checking if they were already defined. This could override custom formats set by users or other interceptors in the chain. The change follows the same pattern as the OpenAPI 3.1 implementation (#140), but lacks a guard condition to preserve existing formats. Code Suggestion: switch params.Value.Kind() {
case reflect.Int64:
if params.Schema.Format == nil {
params.Schema.WithFormat("int64")
}
// ... other cases |
||
| // See https://spec.openapis.org/oas/v3.0.0.html#data-types. | ||
| switch params.Value.Kind() { //nolint // Not all kinds have formats defined. | ||
| case reflect.Int64: | ||
| params.Schema.WithFormat("int64") | ||
| case reflect.Int32: | ||
| params.Schema.WithFormat("int32") | ||
| case reflect.Float32: | ||
| params.Schema.WithFormat("float") | ||
| case reflect.Float64: | ||
| params.Schema.WithFormat("double") | ||
|
Comment on lines
+36
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2 | Confidence: Medium The format names "float" and "double" align with OpenAPI 3.0 specification, but the code lacks inline comments explaining that these correspond to IEEE 754 single and double precision formats. Adding brief specification references would improve clarity for maintainers. Code Suggestion: case reflect.Float32:
params.Schema.WithFormat("float") // IEEE 754 single precision
case reflect.Float64:
params.Schema.WithFormat("double") // IEEE 754 double precision |
||
| } | ||
|
|
||
| return false, nil | ||
| })) | ||
|
|
||
| return r | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1086,7 +1086,7 @@ func TestReflector_AddOperation_defName(t *testing.T) { | |
| "paths":{ | ||
| "/foo":{ | ||
| "post":{ | ||
| "parameters":[{"name":"bar","in":"header","schema":{"type":"number"}}], | ||
| "parameters":[{"name":"bar","in":"header","schema":{"type":"number","format":"double"}}], | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2 | Confidence: High The test update correctly validates the new format behavior, but only covers the |
||
| "requestBody":{ | ||
| "content":{ | ||
| "application/json":{"schema":{"$ref":"#/components/schemas/Openapi3TestReqJSON"}}, | ||
|
|
||
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.
P2 | Confidence: High
The example update demonstrates the new format annotation in generated documentation, improving example accuracy. This serves as effective documentation for users expecting format annotations in their OpenAPI specs.