Skip to content

fix: shorten generic type definition names in JSON schema #57

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.

Sign up for GitHub

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
akfaiz merged 3 commits into from
Apr 15, 2026
Merged

fix: shorten generic type definition names in JSON schema #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2169330
fix: shorten generic type definition names in JSON schema
akfaiz Apr 15, 2026
51e0c38
feat: adjust test case for generic response
akfaiz Apr 15, 2026
4e666ed
feat: add nested generic case
akfaiz Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions jsonschema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,25 @@ package spec

import (
"fmt"
"reflect"
"regexp"
"strings"

"github.com/oaswrap/spec/internal/debuglog"
"github.com/oaswrap/spec/openapi"
"github.com/swaggest/jsonschema-go"
)

var genericInstRe = regexp.MustCompile(`^(\w+)\[(.+)\]$`)

func getJSONSchemaOpts(cfg *openapi.ReflectorConfig, logger *debuglog.Logger) []func(*jsonschema.ReflectContext) {
var opts []func(*jsonschema.ReflectContext)

if cfg == nil {
opts = append(opts, jsonschema.InterceptDefName(shortenGenericName))
Copy link
Copy Markdown

@llamapreview llamapreview Bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 | Confidence: High

The shortenGenericName interceptor is added in two separate code paths (when cfg is nil and when it's not). This duplication is unnecessary and increases the risk of the two code paths diverging in the future. The logic should be consolidated. The simplest fix is to remove the early return for cfg == nil (as suggested in Finding 1 to fix the breaking change) and let the main flow handle the addition, ensuring consistency.

return opts
Comment on lines 16 to +21
Copy link
Copy Markdown

@llamapreview llamapreview Bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 | Confidence: High

The PR introduces a breaking change in behavior when cfg.ReflectorConfig is nil. Previously, if the config was nil, no JSON schema options (including interceptors) were applied. Now, the shortenGenericName interceptor is always added when cfg == nil. This changes the default output schema for all users not explicitly setting a ReflectorConfig. Since the related_context (reflector3.go and reflector31.go) shows getJSONSchemaOpts is called and its results are applied to reflector.DefaultOptions regardless of the config's presence, this unconditionally changes the public API's schema generation output for those users, a breaking change not indicated in the PR's "Breaking change" checkbox.

Suggested change
func getJSONSchemaOpts(cfg *openapi.ReflectorConfig, logger *debuglog.Logger) []func(*jsonschema.ReflectContext) {
var opts []func(*jsonschema.ReflectContext)
if cfg == nil {
opts = append(opts, jsonschema.InterceptDefName(shortenGenericName))
return opts
func getJSONSchemaOpts(cfg *openapi.ReflectorConfig, logger *debuglog.Logger) []func(*jsonschema.ReflectContext) {
var opts []func(*jsonschema.ReflectContext)
if cfg == nil {
// Do NOT add default interceptors to preserve existing behavior when no config is provided.
return opts
}

}

if cfg.InlineRefs {
opts = append(opts, jsonschema.InlineRefs)
logger.Printf("set inline references to true")
Expand All @@ -27,6 +37,7 @@ func getJSONSchemaOpts(cfg *openapi.ReflectorConfig, logger *debuglog.Logger) []
opts = append(opts, jsonschema.StripDefinitionNamePrefix(cfg.StripDefNamePrefix...))
logger.LogAction("set strip definition name prefix", fmt.Sprintf("%v", cfg.StripDefNamePrefix))
}
opts = append(opts, jsonschema.InterceptDefName(shortenGenericName))
if cfg.InterceptDefNameFunc != nil {
opts = append(opts, jsonschema.InterceptDefName(cfg.InterceptDefNameFunc))
logger.Printf("set custom intercept definition name function")
Comment on lines 39 to 43
Copy link
Copy Markdown

@llamapreview llamapreview Bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 | Confidence: High

The PR's logic creates a conflict when a user has configured a custom InterceptDefNameFunc. The jsonschema-go library's InterceptDefName option sets a single function; the second append overrides the first. Therefore, when a custom function is provided, the new shortenGenericName feature is silently disabled. This violates the principle of least surprise and undermines the feature's stated goal of being a default improvement. Users expecting the shortening behavior alongside their custom logic will not get it, and the log message "set custom intercept definition name function" misleadingly implies addition, not replacement.

Code Suggestion:

// Define a composite interceptor that applies shortening, then custom logic if provided.
compositeInterceptor := func(t reflect.Type, defaultDefName string) string {
	shortName := shortenGenericName(t, defaultDefName)
	if cfg.InterceptDefNameFunc != nil {
		return cfg.InterceptDefNameFunc(t, shortName) // Custom func receives the shortened name.
	}
	return shortName
}
opts = append(opts, jsonschema.InterceptDefName(compositeInterceptor))
if cfg.InterceptDefNameFunc != nil {
	logger.Printf("set custom intercept definition name function")
}

Expand Down Expand Up @@ -63,3 +74,36 @@ func getJSONSchemaOpts(cfg *openapi.ReflectorConfig, logger *debuglog.Logger) []

return opts
}

// shortenGenericName converts "Page[some/pkg.Item]" to "PageItem".
func shortenGenericName(t reflect.Type, defaultDefName string) string {
m := genericInstRe.FindStringSubmatch(t.Name())
if m == nil {
return defaultDefName
}
// Use the container name from defaultDefName, which already has the package
// prefix applied and StripDefinitionNamePrefix already run — so the result
// is consistent with how non-generic struct names are generated.
containerName := m[1]
if before, _, found := strings.Cut(defaultDefName, "["); found {
containerName = before
}
args := strings.Split(m[2], ", ")
result := containerName
var sb strings.Builder
for _, arg := range args {
arg = strings.TrimPrefix(arg, "*")
var suffixSb strings.Builder
for strings.HasPrefix(arg, "[]") {
suffixSb.WriteString("List")
arg = arg[2:]
}
arg = strings.TrimPrefix(arg, "*")
if i := strings.LastIndex(arg, "."); i >= 0 {
arg = arg[i+1:]
}
sb.WriteString(arg + suffixSb.String())
}
result += sb.String()
return result
}
11 changes: 5 additions & 6 deletions reflector3.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,12 @@ func newReflector3(cfg *openapi.Config, logger *debuglog.Logger) reflector {

var parameterTagMapping map[openapi.ParameterIn]string

// Custom options for JSON schema generation
if cfg.ReflectorConfig != nil {
jsonSchemaOpts := getJSONSchemaOpts(cfg.ReflectorConfig, logger)
if len(jsonSchemaOpts) > 0 {
reflector.DefaultOptions = append(reflector.DefaultOptions, jsonSchemaOpts...)
}
jsonSchemaOpts := getJSONSchemaOpts(cfg.ReflectorConfig, logger)
if len(jsonSchemaOpts) > 0 {
reflector.DefaultOptions = append(reflector.DefaultOptions, jsonSchemaOpts...)
}

if cfg.ReflectorConfig != nil {
for _, opt := range cfg.ReflectorConfig.TypeMappings {
reflector.AddTypeMapping(opt.Src, opt.Dst)
logger.LogAction("add type mapping", fmt.Sprintf("%T -> %T", opt.Src, opt.Dst))
Expand Down
11 changes: 5 additions & 6 deletions reflector31.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,12 @@ func newReflector31(cfg *openapi.Config, logger *debuglog.Logger) reflector {

var parameterTagMapping map[openapi.ParameterIn]string

// Custom options for JSON schema generation
if cfg.ReflectorConfig != nil {
jsonSchemaOpts := getJSONSchemaOpts(cfg.ReflectorConfig, logger)
if len(jsonSchemaOpts) > 0 {
reflector.DefaultOptions = append(reflector.DefaultOptions, jsonSchemaOpts...)
}
jsonSchemaOpts := getJSONSchemaOpts(cfg.ReflectorConfig, logger)
if len(jsonSchemaOpts) > 0 {
reflector.DefaultOptions = append(reflector.DefaultOptions, jsonSchemaOpts...)
}

if cfg.ReflectorConfig != nil {
for _, opt := range cfg.ReflectorConfig.TypeMappings {
reflector.AddTypeMapping(opt.Src, opt.Dst)
logger.LogAction("add type mapping", fmt.Sprintf("%T -> %T", opt.Src, opt.Dst))
Expand Down
37 changes: 37 additions & 0 deletions router_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ func TestRouter(t *testing.T) {
Name: "Authentication",
Description: "Operations related to user authentication",
}),
option.WithReflectorConfig(
option.TypeMapping(NullString{}, new(string)),
option.TypeMapping(NullTime{}, new(time.Time)),
),
option.WithSecurity("bearerAuth", option.SecurityHTTPBearer("Bearer")),
},
setup: func(r spec.Router) {
r.Post("/login",
Expand All @@ -213,6 +218,38 @@ func TestRouter(t *testing.T) {
option.Request(new(LoginRequest)),
option.Response(200, new(Response[Token])),
)
r.Get("/user",
option.OperationID("getUserProfile"),
option.Summary("Get User Profile"),
option.Description("This operation retrieves the authenticated user's profile."),
option.Tags("Authentication"),
option.Security("bearerAuth"),
option.Response(200, new(Response[User])),
)
r.Get("/users",
option.OperationID("getUsers"),
option.Summary("Get Users"),
option.Description("This operation retrieves a list of users."),
option.Tags("Authentication"),
option.Security("bearerAuth"),
option.Response(200, new(Response[[]User])),
)
r.Get("/nested-generic-user",
option.OperationID("getNestedGenericUser"),
option.Summary("Get Nested Generic User"),
option.Description("This operation retrieves a nested generic user."),
option.Tags("Authentication"),
option.Security("bearerAuth"),
option.Response(200, new(Response[Response[User]])),
)
r.Get("/nested-generic-users",
option.OperationID("getNestedGenericUsers"),
option.Summary("Get Nested Generic Users"),
option.Description("This operation retrieves a nested generic users."),
option.Tags("Authentication"),
option.Security("bearerAuth"),
option.Response(200, new(Response[Response[[]User]])),
)
},
},
{
Expand Down
4 changes: 2 additions & 2 deletions testdata/all_operation_options_3.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ paths:
content:
application/json:
schema:
$ref: '#/components/schemas/SpecTestResponseGithubComOaswrapSpecTestUser'
$ref: '#/components/schemas/SpecTestResponseUser'
description: Response body for operation options
security:
- apiKey: []
Expand All @@ -45,7 +45,7 @@ components:
type: object
SpecTestNullTime:
type: object
SpecTestResponseGithubComOaswrapSpecTestUser:
SpecTestResponseUser:
properties:
data:
$ref: '#/components/schemas/SpecTestUser'
Expand Down
4 changes: 2 additions & 2 deletions testdata/all_operation_options_31.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ paths:
content:
application/json:
schema:
$ref: '#/components/schemas/SpecTestResponseGithubComOaswrapSpecTestUser'
$ref: '#/components/schemas/SpecTestResponseUser'
description: Response body for operation options
security:
- apiKey: []
Expand All @@ -47,7 +47,7 @@ components:
type: object
SpecTestNullTime:
type: object
SpecTestResponseGithubComOaswrapSpecTestUser:
SpecTestResponseUser:
properties:
data:
$ref: '#/components/schemas/SpecTestUser'
Expand Down
125 changes: 123 additions & 2 deletions testdata/generic_response_3.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,75 @@ paths:
content:
application/json:
schema:
$ref: '#/components/schemas/SpecTestResponseGithubComOaswrapSpecTestToken'
$ref: '#/components/schemas/SpecTestResponseToken'
description: OK
summary: User Login
tags:
- Authentication
/nested-generic-user:
get:
description: This operation retrieves a nested generic user.
operationId: getNestedGenericUser
responses:
"200":
content:
application/json:
schema:
$ref: '#/components/schemas/SpecTestResponseUserType2'
description: OK
security:
- bearerAuth: []
summary: Get Nested Generic User
tags:
- Authentication
/nested-generic-users:
get:
description: This operation retrieves a nested generic users.
operationId: getNestedGenericUsers
responses:
"200":
content:
application/json:
schema:
$ref: '#/components/schemas/SpecTestResponseUserType3'
description: OK
security:
- bearerAuth: []
summary: Get Nested Generic Users
tags:
- Authentication
/user:
get:
description: This operation retrieves the authenticated user's profile.
operationId: getUserProfile
responses:
"200":
content:
application/json:
schema:
$ref: '#/components/schemas/SpecTestResponseUser'
description: OK
security:
- bearerAuth: []
summary: Get User Profile
tags:
- Authentication
/users:
get:
description: This operation retrieves a list of users.
operationId: getUsers
responses:
"200":
content:
application/json:
schema:
$ref: '#/components/schemas/SpecTestResponseUserList'
description: OK
security:
- bearerAuth: []
summary: Get Users
tags:
- Authentication
components:
schemas:
SpecTestLoginRequest:
Expand All @@ -40,17 +104,74 @@ components:
- username
- password
type: object
SpecTestResponseGithubComOaswrapSpecTestToken:
SpecTestResponseToken:
properties:
data:
$ref: '#/components/schemas/SpecTestToken'
status:
example: 200
type: integer
type: object
SpecTestResponseUser:
properties:
data:
$ref: '#/components/schemas/SpecTestUser'
status:
example: 200
type: integer
type: object
SpecTestResponseUserList:
properties:
data:
items:
$ref: '#/components/schemas/SpecTestUser'
nullable: true
type: array
status:
example: 200
type: integer
type: object
SpecTestResponseUserType2:
properties:
data:
$ref: '#/components/schemas/SpecTestResponseUser'
status:
example: 200
type: integer
type: object
SpecTestResponseUserType3:
properties:
data:
$ref: '#/components/schemas/SpecTestResponseUserList'
status:
example: 200
type: integer
type: object
SpecTestToken:
properties:
token:
example: abc123
type: string
type: object
SpecTestUser:
properties:
age:
nullable: true
type: integer
created_at:
format: date-time
type: string
email:
type: string
id:
type: integer
updated_at:
format: date-time
type: string
username:
type: string
type: object
securitySchemes:
bearerAuth:
scheme: Bearer
type: http
Loading
Loading