diff --git a/docparse/docparse.go b/docparse/docparse.go index 6f8661f..30f443d 100644 --- a/docparse/docparse.go +++ b/docparse/docparse.go @@ -51,6 +51,13 @@ type Config struct { StructTag string MapTypes map[string]string MapFormats map[string]string + + // InferRequired marks response/body struct fields as required when the + // Go type implies presence: non-pointer fields without `omitempty` in + // the struct tag and without an explicit `{optional}` doc tag. Path, + // query, and form parameters are unaffected (they have their own + // required handling). + InferRequired bool } // DefaultResponse references. diff --git a/docparse/jsonschema.go b/docparse/jsonschema.go index bc8fa49..9f0e40f 100644 --- a/docparse/jsonschema.go +++ b/docparse/jsonschema.go @@ -6,6 +6,7 @@ import ( "go/ast" "os" "path/filepath" + "reflect" "strconv" "strings" @@ -77,6 +78,11 @@ func structToSchema(prog *Program, name, tagName string, ref Reference) (*Schema if !sliceutil.Contains([]string{"path", "query", "form"}, ref.Context) { fixRequired(schema, prop) + + if prog.Config.InferRequired && isInferredRequired(p.KindField, tagName) && + !sliceutil.Contains(schema.Required, name) { + schema.Required = append(schema.Required, name) + } } if prop == nil { @@ -91,6 +97,39 @@ func structToSchema(prog *Program, name, tagName string, ref Reference) (*Schema return schema, nil } +// isInferredRequired reports whether a struct field should be auto-marked as +// required when Config.InferRequired is enabled. A field is considered +// required unless it is a pointer, has `omitempty` in its struct tag, or +// has an explicit `{optional}` doc tag. Embedded fields (no field name) +// are never inferred — embedding is a structural concern, not a contract. +func isInferredRequired(f *ast.Field, tagName string) bool { + if f == nil || len(f.Names) == 0 { + return false + } + if _, ok := f.Type.(*ast.StarExpr); ok { + return false + } + if f.Tag != nil { + tag := reflect.StructTag(strings.Trim(f.Tag.Value, "`")).Get(tagName) + for _, opt := range strings.Split(tag, ",")[1:] { + if strings.TrimSpace(opt) == "omitempty" { + return false + } + } + } + + var doc string + if f.Doc != nil { + doc = f.Doc.Text() + } else if f.Comment != nil { + doc = f.Comment.Text() + } + if hasTag(doc, paramOptional) { + return false + } + return true +} + // The required tags are added to the property itself, rather than to the // parent. So fix that by moving it from "prop" to "parent". // diff --git a/docparse/jsonschema_test.go b/docparse/jsonschema_test.go index 80f5d4b..464a2aa 100644 --- a/docparse/jsonschema_test.go +++ b/docparse/jsonschema_test.go @@ -4,6 +4,8 @@ import ( "fmt" "go/ast" "go/build" + "go/parser" + "go/token" "testing" "github.com/teamwork/test/diff" @@ -210,3 +212,37 @@ func TestFieldToProperty(t *testing.T) { } }) } + +func TestIsInferredRequired(t *testing.T) { + cases := []struct { + name string + src string + want bool + }{ + {"non-pointer no tag", `struct{ F string }`, true}, + {"non-pointer json tag", "struct{ F string `json:\"f\"` }", true}, + {"pointer", `struct{ F *string }`, false}, + {"omitempty", "struct{ F string `json:\"f,omitempty\"` }", false}, + {"omitempty with whitespace", "struct{ F string `json:\"f, omitempty\"` }", false}, + {"other tag option not omitempty", "struct{ F string `json:\"f,string\"` }", true}, + {"explicit optional doc", "struct{\n// {optional}\nF string\n}", false}, + {"explicit required on pointer doc", "struct{\n// {required}\nF *string\n}", false}, + {"embedded", `struct{ string }`, false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fset := token.NewFileSet() + src := "package p\ntype T " + tc.src + file, err := parser.ParseFile(fset, "in.go", src, parser.ParseComments) + if err != nil { + t.Fatalf("parse: %v", err) + } + st := file.Decls[0].(*ast.GenDecl).Specs[0].(*ast.TypeSpec).Type.(*ast.StructType) + got := isInferredRequired(st.Fields.List[0], "json") + if got != tc.want { + t.Errorf("got %v, want %v", got, tc.want) + } + }) + } +} diff --git a/testdata/openapi2/src/infer-required/in.go b/testdata/openapi2/src/infer-required/in.go new file mode 100644 index 0000000..d2cbc4b --- /dev/null +++ b/testdata/openapi2/src/infer-required/in.go @@ -0,0 +1,37 @@ +package infer_required + +// Currency exercises every inference branch in one struct. +type Currency struct { + // ID is required by default (non-pointer, no omitempty). + ID int64 `json:"id"` + // Name is required by default. + Name string `json:"name"` + // Code is required by default. + Code string `json:"code"` + // Symbol is a pointer, so it is not inferred as required. + Symbol *string `json:"symbol"` + // DecimalPoints is a pointer. + DecimalPoints *int64 `json:"decimalPoints"` + // Description has omitempty, so it is not required. + Description string `json:"description,omitempty"` + // Notes is explicitly marked optional via doc tag. {optional} + Notes string `json:"notes"` + // LegacyRequired exercises the explicit doc tag on a pointer field. {required} + LegacyRequired *string `json:"legacyRequired"` +} + +// CurrencyResponse wraps a currency. +type CurrencyResponse struct { + Currency Currency `json:"currency"` +} + +// CurrencyQuery is a query-parameter struct; required inference must not +// apply here (path/query/form params have their own required handling). +type CurrencyQuery struct { + Code string `json:"code"` +} + +// GET /currencies/{id}.json get a currency +// +// Query: CurrencyQuery +// Response 200 (application/json): CurrencyResponse diff --git a/testdata/openapi2/src/infer-required/test.conf b/testdata/openapi2/src/infer-required/test.conf new file mode 100644 index 0000000..d9e7808 --- /dev/null +++ b/testdata/openapi2/src/infer-required/test.conf @@ -0,0 +1,2 @@ +# Enable Go-type-based required inference for response/body schemas. +infer-required true diff --git a/testdata/openapi2/src/infer-required/want.yaml b/testdata/openapi2/src/infer-required/want.yaml new file mode 100644 index 0000000..4f86e9f --- /dev/null +++ b/testdata/openapi2/src/infer-required/want.yaml @@ -0,0 +1,79 @@ +swagger: "2.0" +info: + title: x + version: x +consumes: + - application/json +produces: + - application/json +tags: + - name: a + - name: currency + - name: get +paths: + /currencies/{id}.json: + get: + operationId: GET_currencies_{id}.json + tags: + - get + - a + - currency + produces: + - application/json + parameters: + - name: Code + in: query + type: string + - name: id + in: path + type: integer + required: true + responses: + 200: + description: 200 OK + schema: + $ref: '#/definitions/infer-required.CurrencyResponse' +definitions: + infer-required.Currency: + title: Currency + description: Currency exercises every inference branch in one struct. + type: object + required: + - id + - name + - code + - legacyRequired + properties: + code: + description: Code is required by default. + type: string + decimalPoints: + description: DecimalPoints is a pointer. + type: integer + description: + description: Description has omitempty, so it is not required. + type: string + id: + description: ID is required by default (non-pointer, no omitempty). + type: integer + legacyRequired: + description: LegacyRequired exercises the explicit doc tag on a pointer field. + type: string + name: + description: Name is required by default. + type: string + notes: + description: Notes is explicitly marked optional via doc tag. + type: string + symbol: + description: Symbol is a pointer, so it is not inferred as required. + type: string + infer-required.CurrencyResponse: + title: CurrencyResponse + description: CurrencyResponse wraps a currency. + type: object + required: + - currency + properties: + currency: + $ref: '#/definitions/infer-required.Currency'