Skip to content
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

Introduce a new command-line parameter for "generate spec" allowing to set "x-nullable: true" by default for each Go field having a pointer type and not having JSON 'omitempty' option set #3128

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Introduce a new command-line parameter for "generate spec" allowing to set "x-nullable: true" by default for each Go field having a pointer type and not having JSON 'omitempty' option set #3128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
20fa9d7
Introduce a new command-line parameter for "generate spec" allowing t…
zenovich Aug 12, 2024
52c2a82
Merge branch 'master' into pointers_nullable_by_default
zenovich Sep 23, 2024
f02fa0a
Merge branch 'master' into pointers_nullable_by_default
zenovich Oct 16, 2024
00e1aca
Merge branch 'master' into pointers_nullable_by_default
zenovich Nov 9, 2024
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
24 changes: 13 additions & 11 deletions cmd/swagger/commands/generate/spec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,18 @@

// SpecFile command to generate a swagger spec from a go application
type SpecFile struct {
WorkDir string `long:"work-dir" short:"w" description:"the base path to use" default:"."`
BuildTags string `long:"tags" short:"t" description:"build tags" default:""`
ScanModels bool `long:"scan-models" short:"m" description:"includes models that were annotated with 'swagger:model'"`
Compact bool `long:"compact" description:"when present, doesn't prettify the json"`
Output flags.Filename `long:"output" short:"o" description:"the file to write to"`
Input flags.Filename `long:"input" short:"i" description:"an input swagger file with which to merge"`
Include []string `long:"include" short:"c" description:"include packages matching pattern"`
Exclude []string `long:"exclude" short:"x" description:"exclude packages matching pattern"`
IncludeTags []string `long:"include-tag" short:"" description:"include routes having specified tags (can be specified many times)"`
ExcludeTags []string `long:"exclude-tag" short:"" description:"exclude routes having specified tags (can be specified many times)"`
ExcludeDeps bool `long:"exclude-deps" short:"" description:"exclude all dependencies of project"`
WorkDir string `long:"work-dir" short:"w" description:"the base path to use" default:"."`
BuildTags string `long:"tags" short:"t" description:"build tags" default:""`
ScanModels bool `long:"scan-models" short:"m" description:"includes models that were annotated with 'swagger:model'"`
Compact bool `long:"compact" description:"when present, doesn't prettify the json"`
Output flags.Filename `long:"output" short:"o" description:"the file to write to"`
Input flags.Filename `long:"input" short:"i" description:"an input swagger file with which to merge"`
Include []string `long:"include" short:"c" description:"include packages matching pattern"`
Exclude []string `long:"exclude" short:"x" description:"exclude packages matching pattern"`
IncludeTags []string `long:"include-tag" short:"" description:"include routes having specified tags (can be specified many times)"`
ExcludeTags []string `long:"exclude-tag" short:"" description:"exclude routes having specified tags (can be specified many times)"`
ExcludeDeps bool `long:"exclude-deps" short:"" description:"exclude all dependencies of project"`
SetXNullableForPointers bool `long:"nullable-pointers" short:"n" description:"set x-nullable extension to true automatically for fields of pointer types without 'omitempty'"`
}

// Execute runs this command
Expand All @@ -65,6 +66,7 @@
opts.IncludeTags = s.IncludeTags
opts.ExcludeTags = s.ExcludeTags
opts.ExcludeDeps = s.ExcludeDeps
opts.SetXNullableForPointers = s.SetXNullableForPointers

Check warning on line 69 in cmd/swagger/commands/generate/spec.go

View check run for this annotation

Codecov / codecov/patch

cmd/swagger/commands/generate/spec.go#L69 

Added line #L69 was not covered by tests
swspec, err := codescan.Run(&opts)
if err != nil {
return err
Expand Down
32 changes: 32 additions & 0 deletions cmd/swagger/commands/generate/spec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"os"
"testing"

"github.com/stretchr/testify/require"

"github.com/go-swagger/go-swagger/codescan"

"github.com/jessevdk/go-flags"
Expand Down Expand Up @@ -34,6 +36,36 @@ func TestSpecFileExecute(t *testing.T) {
}
}

func TestSpecFileExecuteRespectsSetXNullableForPointersOption(t *testing.T) {
outputFileName := "spec.json"
spec := &SpecFile{
WorkDir: "../../../../fixtures/enhancements/pointers-nullable-by-default",
Output: flags.Filename(outputFileName),
ScanModels: true,
SetXNullableForPointers: true,
}

defer func() { _ = os.Remove(outputFileName) }()

err := spec.Execute(nil)
require.NoError(t, err)

data, err := os.ReadFile(outputFileName)
require.NoError(t, err)

var got map[string]interface{}
err = json.Unmarshal(data, &got)
require.NoError(t, err)

require.Len(t, got["definitions"], 2)
require.Contains(t, got["definitions"], "Item")
itemDefinition := got["definitions"].(map[string]interface{})["Item"].(map[string]interface{})
require.Contains(t, itemDefinition["properties"], "Value1")
value1Property := itemDefinition["properties"].(map[string]interface{})["Value1"].(map[string]interface{})
require.Contains(t, value1Property, "x-nullable")
assert.Equal(t, true, value1Property["x-nullable"])
}

func TestGenerateJSONSpec(t *testing.T) {
opts := codescan.Options{
WorkDir: basePath,
Expand Down
1 change: 1 addition & 0 deletions cmd/swagger/completion/swagger.zsh-completion
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ __generate_spec() {
'(-h,--help)'{-h,--help}'[Print help message]' \
'(-w,--work-dir)'{-w,--work-dir}"[the base path to use (default: .)]:work-dir" \
'(-m,--scan-models)'{-m,--scan-models}"[includes models that were annotated with 'swagger:model']" \
'(-n,--nullable-pointers)'{-n,--nullable-pointers}"[set x-nullable extension to true automatically for fields of pointer types without 'omitempty']" \
"(--compact)--compact[when present, doesn't prettify the the json]" \
'(-o,--output)'{-o,--output}"[the file to write to]:output" \
'(-i,--input)'{-i,--input}"[an input swagger file with which to merge]:input" && return 0
Expand Down
69 changes: 36 additions & 33 deletions codescan/application.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,17 @@

// Options for the scanner
type Options struct {
Packages []string
InputSpec *spec.Swagger
ScanModels bool
WorkDir string
BuildTags string
ExcludeDeps bool
Include []string
Exclude []string
IncludeTags []string
ExcludeTags []string
Packages []string
InputSpec *spec.Swagger
ScanModels bool
WorkDir string
BuildTags string
ExcludeDeps bool
Include []string
Exclude []string
IncludeTags []string
ExcludeTags []string
SetXNullableForPointers bool
}

type scanCtx struct {
Expand Down Expand Up @@ -94,7 +95,7 @@

app, err := newTypeIndex(pkgs, opts.ExcludeDeps,
sliceToSet(opts.IncludeTags), sliceToSet(opts.ExcludeTags),
opts.Include, opts.Exclude)
opts.Include, opts.Exclude, opts.SetXNullableForPointers)

Check warning on line 98 in codescan/application.go

View check run for this annotation

Codecov / codecov/patch

codescan/application.go#L98 

Added line #L98 was not covered by tests
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -418,16 +419,17 @@
return list, descList, true
}

func newTypeIndex(pkgs []*packages.Package, excludeDeps bool, includeTags, excludeTags map[string]bool, includePkgs, excludePkgs []string) (*typeIndex, error) {
func newTypeIndex(pkgs []*packages.Package, excludeDeps bool, includeTags, excludeTags map[string]bool, includePkgs, excludePkgs []string, setXNullableForPointers bool) (*typeIndex, error) {

Check warning on line 422 in codescan/application.go

View check run for this annotation

Codecov / codecov/patch

codescan/application.go#L422 

Added line #L422 was not covered by tests
ac := &typeIndex{
AllPackages: make(map[string]*packages.Package),
Models: make(map[*ast.Ident]*entityDecl),
ExtraModels: make(map[*ast.Ident]*entityDecl),
excludeDeps: excludeDeps,
includeTags: includeTags,
excludeTags: excludeTags,
includePkgs: includePkgs,
excludePkgs: excludePkgs,
AllPackages: make(map[string]*packages.Package),
Models: make(map[*ast.Ident]*entityDecl),
ExtraModels: make(map[*ast.Ident]*entityDecl),
excludeDeps: excludeDeps,
includeTags: includeTags,
excludeTags: excludeTags,
includePkgs: includePkgs,
excludePkgs: excludePkgs,
setXNullableForPointers: setXNullableForPointers,

Check warning on line 432 in codescan/application.go

View check run for this annotation

Codecov / codecov/patch

codescan/application.go#L424-L432 

Added lines #L424 - L432 were not covered by tests
}
if err := ac.build(pkgs); err != nil {
return nil, err
Expand All @@ -436,19 +438,20 @@
}

type typeIndex struct {
AllPackages map[string]*packages.Package
Models map[*ast.Ident]*entityDecl
ExtraModels map[*ast.Ident]*entityDecl
Meta []metaSection
Routes []parsedPathContent
Operations []parsedPathContent
Parameters []*entityDecl
Responses []*entityDecl
excludeDeps bool
includeTags map[string]bool
excludeTags map[string]bool
includePkgs []string
excludePkgs []string
AllPackages map[string]*packages.Package
Models map[*ast.Ident]*entityDecl
ExtraModels map[*ast.Ident]*entityDecl
Meta []metaSection
Routes []parsedPathContent
Operations []parsedPathContent
Parameters []*entityDecl
Responses []*entityDecl
excludeDeps bool
includeTags map[string]bool
excludeTags map[string]bool
includePkgs []string
excludePkgs []string
setXNullableForPointers bool

Check warning on line 454 in codescan/application.go

View check run for this annotation

Codecov / codecov/patch

codescan/application.go#L454 

Added line #L454 was not covered by tests
}

func (a *typeIndex) build(pkgs []*packages.Package) error {
Expand Down
2 changes: 1 addition & 1 deletion codescan/parameters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@
continue
}

name, ignore, _, err := parseJSONTag(afld)
name, ignore, _, _, err := parseJSONTag(afld)

Check warning on line 342 in codescan/parameters.go

View check run for this annotation

Codecov / codecov/patch

codescan/parameters.go#L342 

Added line #L342 was not covered by tests
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion codescan/responses.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@
continue
}

name, ignore, _, err := parseJSONTag(afld)
name, ignore, _, _, err := parseJSONTag(afld)

Check warning on line 336 in codescan/responses.go

View check run for this annotation

Codecov / codecov/patch

codescan/responses.go#L336 

Added line #L336 was not covered by tests
if err != nil {
return err
}
Expand Down
33 changes: 24 additions & 9 deletions codescan/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,12 @@
ps.AddExtension("x-go-name", fld.Name())
}

if s.ctx.app.setXNullableForPointers {
if _, isPointer := fld.Type().(*types.Signature).Results().At(0).Type().(*types.Pointer); isPointer && (ps.Extensions == nil || (ps.Extensions["x-nullable"] == nil && ps.Extensions["x-isnullable"] == nil)) {

Check warning on line 653 in codescan/schema.go

View check run for this annotation

Codecov / codecov/patch

codescan/schema.go#L652-L653 

Added lines #L652 - L653 were not covered by tests
ps.AddExtension("x-nullable", true)
}
}

Check warning on line 657 in codescan/schema.go

View check run for this annotation

Codecov / codecov/patch

codescan/schema.go#L656-L657 

Added lines #L656 - L657 were not covered by tests
seen[name] = fld.Name()
tgt.Properties[name] = ps
}
Expand Down Expand Up @@ -714,7 +720,7 @@
continue
}

_, ignore, _, err := parseJSONTag(afld)
_, ignore, _, _, err := parseJSONTag(afld)
if err != nil {
return err
}
Expand Down Expand Up @@ -814,7 +820,7 @@
continue
}

name, ignore, isString, err := parseJSONTag(afld)
name, ignore, isString, omitEmpty, err := parseJSONTag(afld)

Check warning on line 823 in codescan/schema.go

View check run for this annotation

Codecov / codecov/patch

codescan/schema.go#L823 

Added line #L823 was not covered by tests
if err != nil {
return err
}
Expand Down Expand Up @@ -851,6 +857,13 @@
addExtension(&ps.VendorExtensible, "x-go-name", fld.Name())
}

if s.ctx.app.setXNullableForPointers {
if _, isPointer := fld.Type().(*types.Pointer); isPointer && !omitEmpty &&
(ps.Extensions == nil || (ps.Extensions["x-nullable"] == nil && ps.Extensions["x-isnullable"] == nil)) {
ps.AddExtension("x-nullable", true)
}
}

Check warning on line 866 in codescan/schema.go

View check run for this annotation

Codecov / codecov/patch

codescan/schema.go#L862-L866 

Added lines #L862 - L866 were not covered by tests
// we have 2 cases:
// 1. field with different name override tag
// 2. field with different name removes tag
Expand Down Expand Up @@ -1104,17 +1117,17 @@
return t[0]
}

func parseJSONTag(field *ast.Field) (name string, ignore bool, isString bool, err error) {
func parseJSONTag(field *ast.Field) (name string, ignore, isString, omitEmpty bool, err error) {

Check warning on line 1120 in codescan/schema.go

View check run for this annotation

Codecov / codecov/patch

codescan/schema.go#L1120 

Added line #L1120 was not covered by tests
if len(field.Names) > 0 {
name = field.Names[0].Name
}
if field.Tag == nil || len(strings.TrimSpace(field.Tag.Value)) == 0 {
return name, false, false, nil
return name, false, false, false, nil

Check warning on line 1125 in codescan/schema.go

View check run for this annotation

Codecov / codecov/patch

codescan/schema.go#L1125 

Added line #L1125 was not covered by tests
}

tv, err := strconv.Unquote(field.Tag.Value)
if err != nil {
return name, false, false, err
return name, false, false, false, err

Check warning on line 1130 in codescan/schema.go

View check run for this annotation

Codecov / codecov/patch

codescan/schema.go#L1130 

Added line #L1130 was not covered by tests
}

if strings.TrimSpace(tv) != "" {
Expand All @@ -1127,16 +1140,18 @@
isString = isFieldStringable(field.Type)
}

omitEmpty = jsonParts.Contain("omitempty")

switch jsonParts.Name() {
case "-":
return name, true, isString, nil
return name, true, isString, omitEmpty, nil

Check warning on line 1147 in codescan/schema.go

View check run for this annotation

Codecov / codecov/patch

codescan/schema.go#L1147 

Added line #L1147 was not covered by tests
case "":
return name, false, isString, nil
return name, false, isString, omitEmpty, nil
default:
return jsonParts.Name(), false, isString, nil
return jsonParts.Name(), false, isString, omitEmpty, nil

Check warning on line 1151 in codescan/schema.go

View check run for this annotation

Codecov / codecov/patch

codescan/schema.go#L1151 

Added line #L1151 was not covered by tests
}
}
return name, false, false, nil
return name, false, false, false, nil

Check warning on line 1154 in codescan/schema.go

View check run for this annotation

Codecov / codecov/patch

codescan/schema.go#L1154 

Added line #L1154 was not covered by tests
}

// isFieldStringable check if the field type is a scalar. If the field type is
Expand Down
70 changes: 70 additions & 0 deletions codescan/schema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,76 @@ func TestEmbeddedAllOf(t *testing.T) {
assertProperty(t, &asch, "string", "cat", "", "Cat")
}

func TestPointersAreNullableByDefaultWhenSetXNullableForPointersIsSet(t *testing.T) {
assertModel := func(sctx *scanCtx, packagePath, modelName string) {
decl, _ := sctx.FindDecl(packagePath, modelName)
require.NotNil(t, decl)
prs := &schemaBuilder{
ctx: sctx,
decl: decl,
}
models := make(map[string]spec.Schema)
require.NoError(t, prs.Build(models))

schema := models[modelName]
require.Len(t, schema.Properties, 5)

require.Contains(t, schema.Properties, "Value1")
assert.Equal(t, true, schema.Properties["Value1"].Extensions["x-nullable"])
require.Contains(t, schema.Properties, "Value2")
assert.NotContains(t, schema.Properties["Value2"].Extensions, "x-nullable")
require.Contains(t, schema.Properties, "Value3")
assert.Equal(t, false, schema.Properties["Value3"].Extensions["x-nullable"])
require.Contains(t, schema.Properties, "Value4")
assert.NotContains(t, schema.Properties["Value4"].Extensions, "x-nullable")
assert.Equal(t, false, schema.Properties["Value4"].Extensions["x-isnullable"])
require.Contains(t, schema.Properties, "Value5")
assert.NotContains(t, schema.Properties["Value5"].Extensions, "x-nullable")
}

packagePath := "github.com/go-swagger/go-swagger/fixtures/enhancements/pointers-nullable-by-default"
sctx, err := newScanCtx(&Options{Packages: []string{packagePath}, SetXNullableForPointers: true})
require.NoError(t, err)

assertModel(sctx, packagePath, "Item")
assertModel(sctx, packagePath, "ItemInterface")
}

func TestPointersAreNotNullableByDefaultWhenSetXNullableForPointersIsNotSet(t *testing.T) {
assertModel := func(sctx *scanCtx, packagePath, modelName string) {
decl, _ := sctx.FindDecl(packagePath, modelName)
require.NotNil(t, decl)
prs := &schemaBuilder{
ctx: sctx,
decl: decl,
}
models := make(map[string]spec.Schema)
require.NoError(t, prs.Build(models))

schema := models[modelName]
require.Len(t, schema.Properties, 5)

require.Contains(t, schema.Properties, "Value1")
assert.NotContains(t, schema.Properties["Value1"].Extensions, "x-nullable")
require.Contains(t, schema.Properties, "Value2")
assert.NotContains(t, schema.Properties["Value2"].Extensions, "x-nullable")
require.Contains(t, schema.Properties, "Value3")
assert.Equal(t, false, schema.Properties["Value3"].Extensions["x-nullable"])
require.Contains(t, schema.Properties, "Value4")
assert.NotContains(t, schema.Properties["Value4"].Extensions, "x-nullable")
assert.Equal(t, false, schema.Properties["Value4"].Extensions["x-isnullable"])
require.Contains(t, schema.Properties, "Value5")
assert.NotContains(t, schema.Properties["Value5"].Extensions, "x-nullable")
}

packagePath := "github.com/go-swagger/go-swagger/fixtures/enhancements/pointers-nullable-by-default"
sctx, err := newScanCtx(&Options{Packages: []string{packagePath}})
require.NoError(t, err)

assertModel(sctx, packagePath, "Item")
assertModel(sctx, packagePath, "ItemInterface")
}

func TestSwaggerTypeNamed(t *testing.T) {
sctx := loadClassificationPkgsCtx(t)
decl := getClassificationModel(sctx, "NamedWithType")
Expand Down
1 change: 1 addition & 0 deletions docs/generate-spec/_index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Help Options:
--include-tag= include routes having specified tags (can be specified many times)
--exclude-tag= exclude routes having specified tags (can be specified many times)
--exclude-deps exclude all dependencies of project
-n, --nullable-pointers set x-nullable extension to true automatically for fields of pointer types without 'omitempty'
```

See code annotation rules [here](../reference/annotations)
Loading
Loading