From 4ea0db4b8f1934fe4033a66efa23ca0c553c9b5c Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Fri, 14 Aug 2026 09:51:07 +0200 Subject: [PATCH 1/8] refact(genspec-wasi): declare every scan option once, in one table The flag-to-option mapping was written per command, and was partial in a different way each time: genspec-wasi covered every boolean knob but no filter and no naming option, genspec-tui covered a hand-picked subset, and neither guard could see what the other was missing. An option added to the library reached whichever command somebody remembered. It now lives in internal/cliopts, as tables keyed by the field each entry writes to, with one guard asserting that no value-typed field of the options is left unreachable from a command line. Commands register the whole surface and add only what is their own: where to write, in what format, how loud. genspec-wasi consequently gains -include, -exclude, -include-tags, -exclude-tags, -name-from-tags, -name-concat-budget, -goflags, -gowork and -goexperiment, which it could not set before. Nothing is renamed and nothing is dropped, and the document it produces for an unchanged invocation is byte-identical. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- cmd/genspec-wasi/main.go | 61 +--- cmd/genspec-wasi/options.go | 136 ------- cmd/genspec-wasi/options_test.go | 111 ------ internal/cliopts/cliopts.go | 333 ++++++++++++++++++ internal/cliopts/cliopts_test.go | 282 +++++++++++++++ internal/cliopts/doc.go | 29 ++ internal/cliopts/errors.go | 13 + .../cliopts}/exec_native.go | 2 +- .../cliopts}/exec_wasm.go | 2 +- internal/cliopts/loader.go | 40 +++ internal/cliopts/split.go | 36 ++ 11 files changed, 747 insertions(+), 298 deletions(-) delete mode 100644 cmd/genspec-wasi/options.go delete mode 100644 cmd/genspec-wasi/options_test.go create mode 100644 internal/cliopts/cliopts.go create mode 100644 internal/cliopts/cliopts_test.go create mode 100644 internal/cliopts/doc.go create mode 100644 internal/cliopts/errors.go rename {cmd/genspec-wasi => internal/cliopts}/exec_native.go (93%) rename {cmd/genspec-wasi => internal/cliopts}/exec_wasm.go (95%) create mode 100644 internal/cliopts/loader.go create mode 100644 internal/cliopts/split.go diff --git a/cmd/genspec-wasi/main.go b/cmd/genspec-wasi/main.go index 53072864..77566976 100644 --- a/cmd/genspec-wasi/main.go +++ b/cmd/genspec-wasi/main.go @@ -39,6 +39,7 @@ import ( "path/filepath" "github.com/go-openapi/codescan" + "github.com/go-openapi/codescan/internal/cliopts" "github.com/go-openapi/codescan/internal/exportdata" ) @@ -55,12 +56,10 @@ func main() { type config struct { set *flag.FlagSet - workdir *string - bools map[string]*bool - buildTags *string - goos *string - goarch *string - loader *string + // scan is every knob the library takes, declared once in internal/cliopts and shared with the + // other commands. What follows is this command's own: where the document goes, and in what shape. + scan *cliopts.Values + exportData *string format *string output *string @@ -70,14 +69,8 @@ type config struct { func registerFlags(fs *flag.FlagSet) *config { return &config{ - set: fs, - workdir: fs.String("workdir", ".", "directory the scan runs from; patterns are relative to it"), - bools: registerBools(fs), - buildTags: fs.String("build-tags", "", "comma-separated go build tags to apply while loading"), - goos: fs.String("goos", "", "GOOS the scanned code is built for (default: this machine's)"), - goarch: fs.String("goarch", "", "GOARCH the scanned code is built for (default: this machine's)"), - loader: fs.String("loader", "auto", - `package loader: "go" runs go list, "own" needs no toolchain, "auto" picks own where there is no exec`), + set: fs, + scan: cliopts.Register(fs), exportData: fs.String("export-data", "", "directory or .zip of pre-computed export data for dependencies (see hack/genexportdata):\n"+ "full fidelity, none of the cost of type-checking them from source"), @@ -144,18 +137,10 @@ func resolveFormat(format string) (bool, error) { } func (c *config) options(patterns []string, stderr io.Writer, sink *collector) (*codescan.Options, error) { - if len(patterns) == 0 { - patterns = []string{"./..."} - } - - opts := &codescan.Options{ - Packages: patterns, - WorkDir: *c.workdir, - BuildTags: *c.buildTags, - GOOS: *c.goos, - GOARCH: *c.goarch, + opts := &codescan.Options{Packages: cliopts.Patterns(patterns)} + if err := c.scan.Apply(opts); err != nil { + return nil, err } - applyBools(opts, c.bools) switch { case *c.exportData != "": @@ -171,16 +156,11 @@ func (c *config) options(patterns []string, stderr io.Writer, sink *collector) ( } } - useOwn, err := resolveLoader(*c.loader) - if err != nil { - return nil, err - } - opts.ToolchainFreeLoader = useOwn - if useOwn { + if opts.ToolchainFreeLoader { // The loader reads the host filesystem directly, so -workdir has to name a place that exists // there rather than a place relative to wherever the process happens to be. Under WASI it never // is: a guest starts at "/", which is not a preopen, so only an absolute path names a mount. - abs, err := absolutePath(*c.workdir) + abs, err := absolutePath(opts.WorkDir) if err != nil { return nil, err } @@ -203,23 +183,6 @@ func (c *config) options(patterns []string, stderr io.Writer, sink *collector) ( return opts, nil } -// resolveLoader reports whether to use codescan's own loader. -// -// "auto" asks whether this build can start a subprocess at all: on wasm it cannot, so `go list` is -// not an option and the choice makes itself. -func resolveLoader(mode string) (bool, error) { - switch mode { - case "own": - return true, nil - case "go": - return false, nil - case "auto": - return !canExec(), nil - default: - return false, fmt.Errorf("%w: -loader %q is not one of go, own, auto", errBadFlag, mode) - } -} - // absolutePath resolves p against the working directory, in the host's own notion of a path. // // Absolute means what the host means by it, not "starts with a slash": a Windows -workdir starts diff --git a/cmd/genspec-wasi/options.go b/cmd/genspec-wasi/options.go deleted file mode 100644 index d9717d56..00000000 --- a/cmd/genspec-wasi/options.go +++ /dev/null @@ -1,136 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers -// SPDX-License-Identifier: Apache-2.0 - -package main - -import ( - "flag" - - "github.com/go-openapi/codescan" -) - -// boolOptions is every boolean knob on codescan.Options that this command exposes, in the order the -// help text lists them. -// -// A table rather than a field per flag: the option set grows, and a caller that cannot see a knob has -// no way to reach it. TestFlagsCoverEveryBoolOption fails when a new one lands without a decision -// here, so the gap is caught at the source rather than by a user meeting -// "flag provided but not defined". -// -// Flag names are the field name in kebab-case, without exception - guessing a shorter spelling is how -// a caller ends up guessing wrong. -var boolOptions = []boolOption{ //nolint:gochecknoglobals // the flag table, read once at startup - { - "scan-models", true, "also emit definitions for swagger:model types", - func(o *codescan.Options) *bool { return &o.ScanModels }, - }, - { - "prune-unused-models", false, "drop discovered models nothing references (needs -scan-models)", - func(o *codescan.Options) *bool { return &o.PruneUnusedModels }, - }, - { - "exclude-deps", false, "scan only the packages matched by the patterns, never their dependencies", - func(o *codescan.Options) *bool { return &o.ExcludeDeps }, - }, - - { - "ref-aliases", false, "aliases produce a $ref instead of being expanded", - func(o *codescan.Options) *bool { return &o.RefAliases }, - }, - { - "transparent-aliases", false, "aliases dissolve entirely, never producing a definition", - func(o *codescan.Options) *bool { return &o.TransparentAliases }, - }, - - { - "emit-ref-siblings", false, "emit a $ref'd field's description and extensions beside the $ref", - func(o *codescan.Options) *bool { return &o.EmitRefSiblings }, - }, - { - "skip-all-of-compounding", false, "never wrap a $ref in an allOf compound", - func(o *codescan.Options) *bool { return &o.SkipAllOfCompounding }, - }, - { - "default-all-of-for-embeds", false, "render a plain struct embed as allOf composition", - func(o *codescan.Options) *bool { return &o.DefaultAllOfForEmbeds }, - }, - - { - "set-x-nullable-for-pointers", false, "mark pointer fields x-nullable", - func(o *codescan.Options) *bool { return &o.SetXNullableForPointers }, - }, - { - "skip-extensions", false, "omit the x-go-* vendor extensions", - func(o *codescan.Options) *bool { return &o.SkipExtensions }, - }, - { - "emit-x-go-type", false, "stamp x-go-type on every emitted definition", - func(o *codescan.Options) *bool { return &o.EmitXGoType }, - }, - - { - "single-line-comment-as-description", false, "a one-line doc comment is a description, never a title", - func(o *codescan.Options) *bool { return &o.SingleLineCommentAsDescription }, - }, - { - "after-decl-comments", false, "accept annotations inside or after a declaration", - func(o *codescan.Options) *bool { return &o.AfterDeclComments }, - }, - { - "clean-go-doc", false, "rewrite godoc-only syntax carried into a title or description", - func(o *codescan.Options) *bool { return &o.CleanGoDoc }, - }, - { - "skip-enum-descriptions", false, "keep the enum const-name mapping off the description", - func(o *codescan.Options) *bool { return &o.SkipEnumDescriptions }, - }, - - { - "skip-jsonify-interface-methods", false, "emit interface-method names verbatim", - func(o *codescan.Options) *bool { return &o.SkipJSONifyInterfaceMethods }, - }, - { - "emit-hierarchical-names", false, "emit over-budget collision groups as nested definitions", - func(o *codescan.Options) *bool { return &o.EmitHierarchicalNames }, - }, - - { - "stub-stdlib", false, - "synthesize standard-library types instead of reading GOROOT: no Go installation and no module\n" + - "cache needed, much smaller footprint, but not failsafe -- see the package documentation", - func(o *codescan.Options) *bool { return &o.StubStdlib }, - }, - { - "skip-compiled-dependencies", false, - "read every dependency from source rather than taking its types from the compiler's export\n" + - "data: the spec is the same either way, but this is much slower on a warm cache and much\n" + - "faster on a cold one. Native builds only -- it needs -loader=go, which WebAssembly cannot run", - func(o *codescan.Options) *bool { return &o.SkipCompiledDependencies }, - }, -} - -type boolOption struct { - name string - value bool - help string - field func(*codescan.Options) *bool -} - -// registerBools declares every boolean flag and returns where the parsed values land. -func registerBools(fs *flag.FlagSet) map[string]*bool { - parsed := make(map[string]*bool, len(boolOptions)) - for _, opt := range boolOptions { - parsed[opt.name] = fs.Bool(opt.name, opt.value, opt.help) - } - - return parsed -} - -// applyBools copies the parsed values onto the options the scan runs with. -func applyBools(opts *codescan.Options, parsed map[string]*bool) { - for _, opt := range boolOptions { - if got, ok := parsed[opt.name]; ok { - *opt.field(opts) = *got - } - } -} diff --git a/cmd/genspec-wasi/options_test.go b/cmd/genspec-wasi/options_test.go deleted file mode 100644 index 9da45019..00000000 --- a/cmd/genspec-wasi/options_test.go +++ /dev/null @@ -1,111 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers -// SPDX-License-Identifier: Apache-2.0 - -package main - -import ( - "flag" - "io" - "reflect" - "testing" - - "github.com/go-openapi/codescan" - "github.com/go-openapi/testify/v2/assert" - "github.com/go-openapi/testify/v2/require" -) - -// boolsNotOnCLI are the boolean options deliberately without a flag, with the reason. -var boolsNotOnCLI = map[string]string{ //nolint:gochecknoglobals // table for the drift guard - "DescWithRef": "deprecated in favour of EmitRefSiblings", - "Debug": "deprecated no-op; the stderr logger was retired", - "ToolchainFreeLoader": "reached through -loader, which also has an auto mode a boolean cannot state", -} - -// TestFlagsCoverEveryBoolOption is what stops a knob being unreachable from the command line. -// -// A caller cannot use an option that has no flag, and finds out by meeting "flag provided but not -// defined" - after writing something against a surface that was never there. Fail here instead. -// -// Coverage is decided by which field each entry actually writes to, not by deriving a name from the -// field: a mechanical rendering cannot know that JSONify is one word and HTTPServer is two. -func TestFlagsCoverEveryBoolOption(t *testing.T) { - t.Parallel() - - covered := map[string]string{} - for _, opt := range boolOptions { - covered[fieldWrittenBy(t, opt)] = opt.name - } - - typ := reflect.TypeFor[codescan.Options]() - for i := range typ.NumField() { - f := typ.Field(i) - if !f.IsExported() || f.Type.Kind() != reflect.Bool { - continue - } - if _, excused := boolsNotOnCLI[f.Name]; excused { - continue - } - - assert.NotEmpty(t, covered[f.Name], - "codescan.Options.%s is reachable from no flag. Add an entry to boolOptions, or excuse it "+ - "in boolsNotOnCLI with a reason.", f.Name) - } -} - -// fieldWrittenBy reports which field of codescan.Options an entry's setter targets, by writing -// through it and seeing what moved. -func fieldWrittenBy(t *testing.T, opt boolOption) string { - t.Helper() - - var opts codescan.Options - *opt.field(&opts) = true - - typ := reflect.TypeFor[codescan.Options]() - value := reflect.ValueOf(opts) - for i := range typ.NumField() { - if typ.Field(i).Type.Kind() == reflect.Bool && value.Field(i).Bool() { - return typ.Field(i).Name - } - } - require.Fail(t, "flag "+opt.name+" writes to no field of codescan.Options") - - return "" -} - -// TestBoolFlagTableIsCurrent catches two flags sharing a name, and two flags writing to the same -// field - either leaves one of them silently doing nothing. -func TestBoolFlagTableIsCurrent(t *testing.T) { - t.Parallel() - - names := map[string]bool{} - fields := map[string]string{} - - for _, opt := range boolOptions { - require.False(t, names[opt.name], "flag %q is declared twice", opt.name) - names[opt.name] = true - - field := fieldWrittenBy(t, opt) - require.Empty(t, fields[field], - "flags %q and %q both write to Options.%s", fields[field], opt.name, field) - fields[field] = opt.name - } -} - -func TestBoolFlagsRegisterAndApply(t *testing.T) { - t.Parallel() - - fs := flag.NewFlagSet("genspec-wasi", flag.ContinueOnError) - fs.SetOutput(io.Discard) - parsed := registerBools(fs) - - // scan-models defaults true, so turning it off proves the explicit -name=false form works - the - // reason booleans are rendered that way rather than as bare switches. - require.NoError(t, fs.Parse([]string{"-prune-unused-models=true", "-scan-models=false"})) - - var opts codescan.Options - applyBools(&opts, parsed) - - assert.True(t, opts.PruneUnusedModels) - assert.False(t, opts.ScanModels) - assert.False(t, opts.SkipExtensions, "an untouched flag must leave its field alone") -} diff --git a/internal/cliopts/cliopts.go b/internal/cliopts/cliopts.go new file mode 100644 index 00000000..b668e60a --- /dev/null +++ b/internal/cliopts/cliopts.go @@ -0,0 +1,333 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliopts + +import ( + "flag" + + "github.com/go-openapi/codescan/internal/scanner" +) + +// Options is the scan configuration these flags write to. +// +// It is codescan.Options under its internal name - the public type is an alias of this one - so this +// package depends downward on the scanner rather than back on the root package. A caller holding a +// *codescan.Options may pass it here unconverted: they are the same type. +type Options = scanner.Options + +// option is one flag over a value-typed field: what it is called, what it defaults to, what it says +// for itself, and where it lands. +// +// The setter is the entry's identity. Naming the field in a string instead would let a rename pass +// the compiler and leave the flag writing nowhere, and it is what lets the coverage guard ask which +// field an entry actually touches rather than deriving a name from a name. +type option[T any] struct { + name string + value T + help string + field func(*Options) *T +} + +// boolOptions is every boolean knob, grouped as the help text lists them. +// +// Defaults are the library's own (false) except where both existing commands already chose +// otherwise: -scan-models is on, because a command asked to produce a specification and handed a +// package of annotated models should produce their definitions. +var boolOptions = []option[bool]{ //nolint:gochecknoglobals // the flag table, read once at startup + { + "scan-models", true, "also emit definitions for swagger:model types", + func(o *Options) *bool { return &o.ScanModels }, + }, + { + "prune-unused-models", false, "drop discovered models nothing references (needs -scan-models)", + func(o *Options) *bool { return &o.PruneUnusedModels }, + }, + { + "exclude-deps", false, "scan only the packages matched by the patterns, never their dependencies", + func(o *Options) *bool { return &o.ExcludeDeps }, + }, + + { + "ref-aliases", false, "aliases produce a $ref instead of being expanded", + func(o *Options) *bool { return &o.RefAliases }, + }, + { + "transparent-aliases", false, "aliases dissolve entirely, never producing a definition", + func(o *Options) *bool { return &o.TransparentAliases }, + }, + + { + "emit-ref-siblings", false, "emit a $ref'd field's description and extensions beside the $ref", + func(o *Options) *bool { return &o.EmitRefSiblings }, + }, + { + "skip-all-of-compounding", false, "never wrap a $ref in an allOf compound", + func(o *Options) *bool { return &o.SkipAllOfCompounding }, + }, + { + "default-all-of-for-embeds", false, "render a plain struct embed as allOf composition", + func(o *Options) *bool { return &o.DefaultAllOfForEmbeds }, + }, + + { + "set-x-nullable-for-pointers", false, "mark pointer fields x-nullable", + func(o *Options) *bool { return &o.SetXNullableForPointers }, + }, + { + "skip-extensions", false, "omit the x-go-* vendor extensions", + func(o *Options) *bool { return &o.SkipExtensions }, + }, + { + "emit-x-go-type", false, "stamp x-go-type on every emitted definition", + func(o *Options) *bool { return &o.EmitXGoType }, + }, + + { + "single-line-comment-as-description", false, "a one-line doc comment is a description, never a title", + func(o *Options) *bool { return &o.SingleLineCommentAsDescription }, + }, + { + "after-decl-comments", false, "accept annotations inside or after a declaration", + func(o *Options) *bool { return &o.AfterDeclComments }, + }, + { + "clean-go-doc", false, "rewrite godoc-only syntax carried into a title or description", + func(o *Options) *bool { return &o.CleanGoDoc }, + }, + { + "skip-enum-descriptions", false, "keep the enum const-name mapping off the description", + func(o *Options) *bool { return &o.SkipEnumDescriptions }, + }, + + { + "skip-jsonify-interface-methods", false, "emit interface-method names verbatim", + func(o *Options) *bool { return &o.SkipJSONifyInterfaceMethods }, + }, + { + "emit-hierarchical-names", false, "emit over-budget collision groups as nested definitions", + func(o *Options) *bool { return &o.EmitHierarchicalNames }, + }, + + { + "stub-stdlib", false, + "synthesize standard-library types instead of reading GOROOT: no Go installation and no module\n" + + "cache needed, much smaller footprint, but not failsafe -- see the package documentation", + func(o *Options) *bool { return &o.StubStdlib }, + }, + { + "skip-compiled-dependencies", false, + "read every dependency from source rather than taking its types from the compiler's export\n" + + "data: the spec is the same either way, but this is much slower on a warm cache and much\n" + + "faster on a cold one. Native builds only -- it needs -loader=go, which WebAssembly cannot run", + func(o *Options) *bool { return &o.SkipCompiledDependencies }, + }, +} + +// stringOptions is every knob taking one string. +// +// The go environment lives here rather than being inherited, so that a scan is reproducible: each of +// these decides what gets built, and so what the specification says. +var stringOptions = []option[string]{ //nolint:gochecknoglobals // the flag table, read once at startup + { + "workdir", ".", "directory the scan runs from; patterns are relative to it", + func(o *Options) *string { return &o.WorkDir }, + }, + { + "build-tags", "", "comma-separated go build tags to apply while loading", + func(o *Options) *string { return &o.BuildTags }, + }, + + { + "goos", "", "GOOS the scanned code is built for (default: this machine's)", + func(o *Options) *string { return &o.GOOS }, + }, + { + "goarch", "", "GOARCH the scanned code is built for (default: this machine's)", + func(o *Options) *string { return &o.GOARCH }, + }, + { + "goflags", "", + `default go command flags, as GOFLAGS (e.g. "-tags=integration"); -build-tags wins`, + func(o *Options) *string { return &o.GOFLAGS }, + }, + { + "gowork", "", + `workspace selection, as GOWORK: "off" to disable, a path to a go.work, empty to search upwards`, + func(o *Options) *string { return &o.GOWORK }, + }, + { + "goexperiment", "", + `toolchain experiments to enable, as GOEXPERIMENT (e.g. "jsonv2")`, + func(o *Options) *string { return &o.GOEXPERIMENT }, + }, +} + +// floatOptions is every knob taking one number. +var floatOptions = []option[float64]{ //nolint:gochecknoglobals // the flag table, read once at startup + { + "name-concat-budget", 0, + "readability cutoff for collision-renaming by concatenation (0 = codescan's default of 0.65)", + func(o *Options) *float64 { return &o.NameConcatBudget }, + }, +} + +// listOption is a flag taking a comma-separated list. +// +// threeWay says the field distinguishes "not given" from "given as empty". Where it is false the two +// collapse onto nil, which is what the scanner reads as "no filter" - so passing -include= is the +// same as not passing it at all, and means what it looks like it means. +type listOption struct { + name string + help string + field func(*Options) *[]string + threeWay bool +} + +// listOptions is every knob taking a list. +var listOptions = []listOption{ //nolint:gochecknoglobals // the flag table, read once at startup + { + name: "include", + help: "comma-separated patterns; only matching packages are scanned", + field: func(o *Options) *[]string { return &o.Include }, + }, + { + name: "exclude", + help: "comma-separated patterns; matching packages are skipped", + field: func(o *Options) *[]string { return &o.Exclude }, + }, + { + name: "include-tags", + help: "comma-separated swagger tags; only matching operations are emitted", + field: func(o *Options) *[]string { return &o.IncludeTags }, + }, + { + name: "exclude-tags", + help: "comma-separated swagger tags; matching operations are skipped", + field: func(o *Options) *[]string { return &o.ExcludeTags }, + }, + { + name: "name-from-tags", + help: `ordered struct tags a field's name derives from (default "json"; pass empty to use the Go` + "\n" + + "field name)", + field: func(o *Options) *[]string { return &o.NameFromTags }, + threeWay: true, + }, +} + +// Values is where a parsed command line lands, before [Values.Apply] copies it onto the options. +// +// Lists are held as the raw string the caller typed: splitting at Apply time is what leaves the +// three-way cases able to tell an empty list from an absent one. +type Values struct { + set *flag.FlagSet + + bools map[string]*bool + strings map[string]*string + floats map[string]*float64 + lists map[string]*string + loader *string +} + +// Register declares every flag on fs and returns where the parsed values will land. +// +// The whole surface is registered: a command that hides a knob only makes it unreachable, and the +// caller finds out by meeting "flag provided but not defined" after writing something against a +// surface that was never there. +func Register(fs *flag.FlagSet) *Values { + values := &Values{ + set: fs, + bools: make(map[string]*bool, len(boolOptions)), + strings: make(map[string]*string, len(stringOptions)), + floats: make(map[string]*float64, len(floatOptions)), + lists: make(map[string]*string, len(listOptions)), + } + + for _, opt := range boolOptions { + values.bools[opt.name] = fs.Bool(opt.name, opt.value, opt.help) + } + for _, opt := range stringOptions { + values.strings[opt.name] = fs.String(opt.name, opt.value, opt.help) + } + for _, opt := range floatOptions { + values.floats[opt.name] = fs.Float64(opt.name, opt.value, opt.help) + } + for _, opt := range listOptions { + values.lists[opt.name] = fs.String(opt.name, "", opt.help) + } + values.loader = fs.String(loaderFlag, loaderAuto, loaderHelp) + + return values +} + +// Apply copies what was parsed onto the options a scan runs with. +// +// It fails only on a flag whose value is not one of a closed set (-loader): everything else was +// already validated by the flag package when it parsed. +func (v *Values) Apply(opts *Options) error { + for _, opt := range boolOptions { + if got, ok := v.bools[opt.name]; ok { + *opt.field(opts) = *got + } + } + for _, opt := range stringOptions { + if got, ok := v.strings[opt.name]; ok { + *opt.field(opts) = *got + } + } + for _, opt := range floatOptions { + if got, ok := v.floats[opt.name]; ok { + *opt.field(opts) = *got + } + } + for _, opt := range listOptions { + got, ok := v.lists[opt.name] + if !ok { + continue + } + *opt.field(opts) = v.list(opt, *got) + } + + own, err := resolveLoader(*v.loader) + if err != nil { + return err + } + opts.ToolchainFreeLoader = own + + return nil +} + +// list reads one list flag, honouring the three-way contract where the field has one. +// +// - nil (unset) keeps whatever the library does by default +// - an empty but non-nil slice is an explicit "consult nothing" +// +// Collapsing those two would make -name-from-tags= silently mean the opposite of what it says: not +// "use the Go field name" but "use the json tag". +func (v *Values) list(opt listOption, raw string) []string { + if !opt.threeWay { + return SplitList(raw) + } + + if !v.passed(opt.name) { + return nil + } + if parsed := SplitList(raw); parsed != nil { + return parsed + } + + return []string{} +} + +// passed reports whether the named flag appeared on the command line, as opposed to holding its +// default. +func (v *Values) passed(name string) bool { + seen := false + v.set.Visit(func(f *flag.Flag) { + if f.Name == name { + seen = true + } + }) + + return seen +} diff --git a/internal/cliopts/cliopts_test.go b/internal/cliopts/cliopts_test.go new file mode 100644 index 00000000..e4e240f8 --- /dev/null +++ b/internal/cliopts/cliopts_test.go @@ -0,0 +1,282 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliopts + +import ( + "flag" + "io" + "reflect" + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +// notOnTheCommandLine are the value-typed options deliberately without a flag, with the reason. +// +// Everything else must be reachable. An entry here is a decision, and reading it should be enough to +// judge whether it still holds. +var notOnTheCommandLine = map[string]string{ //nolint:gochecknoglobals // table for the drift guard + "Packages": "positional arguments, so that `genspec ./api/...` reads like every other Go command", + "DescWithRef": "deprecated in favour of EmitRefSiblings", + "Debug": "deprecated no-op; the stderr logger was retired", +} + +// coveredByLoader is the option the -loader flag discharges. +// +// It is a boolean the tables cannot carry, because the choice has three answers and the useful +// default is the third one. TestLoaderWritesTheToolchainFreeOption is what proves this excuse. +const coveredByLoader = "ToolchainFreeLoader" + +// TestFlagsCoverEveryValueTypedOption is what stops a knob being unreachable from the command line. +// +// A caller cannot use an option that has no flag, and finds out by meeting "flag provided but not +// defined" - after writing something against a surface that was never there. Fail here instead, on +// the pull request that adds the option. +func TestFlagsCoverEveryValueTypedOption(t *testing.T) { + t.Parallel() + + covered := coveredFields(t) + covered[coveredByLoader] = loaderFlag + + typ := reflect.TypeFor[Options]() + for i := range typ.NumField() { + f := typ.Field(i) + if !f.IsExported() || !isValueTyped(f.Type) { + continue + } + if _, excused := notOnTheCommandLine[f.Name]; excused { + continue + } + + assert.NotEmptyf(t, covered[f.Name], + "codescan.Options.%s is reachable from no flag. Add an entry to the tables in cliopts.go, "+ + "or excuse it in notOnTheCommandLine with a reason.", f.Name) + } +} + +// TestFlagTablesAreCurrent catches two flags sharing a name, and two flags writing to the same field +// - either leaves one of them silently doing nothing. +func TestFlagTablesAreCurrent(t *testing.T) { + t.Parallel() + + names := map[string]bool{loaderFlag: true} + fields := map[string]string{} + + for field, name := range coveredFields(t) { + require.Falsef(t, names[name], "flag %q is declared twice", name) + names[name] = true + + require.Emptyf(t, fields[field], "flags %q and %q both write to Options.%s", fields[field], name, field) + fields[field] = name + } +} + +// TestExcusedOptionsStillExist keeps notOnTheCommandLine honest. +// +// An excuse for a field that has since been renamed or removed reads as a decision that was made, +// and silently stops guarding anything. +func TestExcusedOptionsStillExist(t *testing.T) { + t.Parallel() + + typ := reflect.TypeFor[Options]() + for name := range notOnTheCommandLine { + _, ok := typ.FieldByName(name) + assert.Truef(t, ok, "notOnTheCommandLine excuses %s, which codescan.Options no longer has", name) + } +} + +func TestDefaults(t *testing.T) { + t.Parallel() + + opts := parse(t) + + assert.Equal(t, ".", opts.WorkDir) + assert.True(t, opts.ScanModels, "a command asked for a specification should emit the models it finds") + assert.False(t, opts.PruneUnusedModels) + assert.Empty(t, opts.BuildTags) + assert.Zero(t, opts.NameConcatBudget, "0 is what the library reads as its own default of 0.65") + assert.Nil(t, opts.Include) + assert.Nil(t, opts.NameFromTags) + assert.Equal(t, !canExec(), opts.ToolchainFreeLoader, "-loader defaults to auto") +} + +func TestParseEveryKind(t *testing.T) { + t.Parallel() + + opts := parse(t, + "-scan-models=false", + "-prune-unused-models", + "-workdir=/src", + "-goexperiment=jsonv2", + "-name-concat-budget=0.8", + "-include=a/..., b/... ", + "-exclude-tags=internal", + "-loader=own", + ) + + assert.False(t, opts.ScanModels, "the explicit -name=false form is why booleans are not bare switches") + assert.True(t, opts.PruneUnusedModels) + assert.Equal(t, "/src", opts.WorkDir) + assert.Equal(t, "jsonv2", opts.GOEXPERIMENT) + assert.InDelta(t, 0.8, opts.NameConcatBudget, 1e-9) + assert.Equal(t, []string{"a/...", "b/..."}, opts.Include, "entries are trimmed, empties dropped") + assert.Equal(t, []string{"internal"}, opts.ExcludeTags) + assert.True(t, opts.ToolchainFreeLoader) + assert.False(t, opts.SkipExtensions, "an untouched flag must leave its field alone") +} + +// TestNameFromTagsIsThreeWay pins the one option where an empty value is not the same as no value. +func TestNameFromTagsIsThreeWay(t *testing.T) { + t.Parallel() + + t.Run("unset means the library's default", func(t *testing.T) { + t.Parallel() + + assert.Nil(t, parse(t).NameFromTags) + }) + + t.Run("empty means the Go field name", func(t *testing.T) { + t.Parallel() + + got := parse(t, "-name-from-tags=").NameFromTags + require.NotNil(t, got, "an explicit empty list must not collapse onto nil, which means json") + assert.Empty(t, got) + }) + + t.Run("a list is the list", func(t *testing.T) { + t.Parallel() + + assert.Equal(t, []string{"form", "json"}, parse(t, "-name-from-tags=form,json").NameFromTags) + }) +} + +// TestFiltersAreNotThreeWay states the other half of the contract: everywhere else, empty and absent +// are the same thing, because nil is already what "no filter" means. +func TestFiltersAreNotThreeWay(t *testing.T) { + t.Parallel() + + assert.Nil(t, parse(t, "-include=").Include) + assert.Nil(t, parse(t, "-include= , ").Include) +} + +func TestLoaderWritesTheToolchainFreeOption(t *testing.T) { + t.Parallel() + + assert.True(t, parse(t, "-loader=own").ToolchainFreeLoader) + assert.False(t, parse(t, "-loader=go").ToolchainFreeLoader) + assert.Equal(t, !canExec(), parse(t, "-loader=auto").ToolchainFreeLoader) +} + +func TestLoaderRefusesAnythingElse(t *testing.T) { + t.Parallel() + + fs := flag.NewFlagSet("test", flag.ContinueOnError) + fs.SetOutput(io.Discard) + values := Register(fs) + require.NoError(t, fs.Parse([]string{"-loader=cargo"})) + + err := values.Apply(&Options{}) + + require.ErrorIs(t, err, ErrBadFlag) + assert.Contains(t, err.Error(), "cargo") +} + +func TestPatterns(t *testing.T) { + t.Parallel() + + assert.Equal(t, []string{DefaultPatterns}, Patterns(nil)) + assert.Equal(t, []string{"./api/..."}, Patterns([]string{"./api/..."})) +} + +func TestSplitList(t *testing.T) { + t.Parallel() + + assert.Nil(t, SplitList("")) + assert.Nil(t, SplitList(" , ")) + assert.Equal(t, []string{"a", "b"}, SplitList(" a , b ")) +} + +// parse registers the surface, parses argv and returns the options it produced. +func parse(t *testing.T, argv ...string) Options { + t.Helper() + + fs := flag.NewFlagSet("test", flag.ContinueOnError) + fs.SetOutput(io.Discard) + values := Register(fs) + require.NoError(t, fs.Parse(argv)) + + var opts Options + require.NoError(t, values.Apply(&opts)) + + return opts +} + +// coveredFields reports which field of the options each flag writes to, keyed by field name. +// +// It asks by writing a sentinel through the setter and seeing what moved, rather than deriving a +// field name from a flag name: a mechanical rendering cannot know that JSONify is one word where +// HTTPServer is two, and a rename would leave the derivation agreeing with itself. +func coveredFields(t *testing.T) map[string]string { + t.Helper() + + covered := make(map[string]string) + for _, opt := range boolOptions { + covered[fieldWrittenBy(t, opt.name, func(o *Options) { *opt.field(o) = true })] = opt.name + } + for _, opt := range stringOptions { + covered[fieldWrittenBy(t, opt.name, func(o *Options) { *opt.field(o) = sentinel })] = opt.name + } + for _, opt := range floatOptions { + covered[fieldWrittenBy(t, opt.name, func(o *Options) { *opt.field(o) = sentinelFloat })] = opt.name + } + for _, opt := range listOptions { + covered[fieldWrittenBy(t, opt.name, func(o *Options) { *opt.field(o) = []string{sentinel} })] = opt.name + } + + return covered +} + +const ( + sentinel = "cliopts sentinel" + sentinelFloat = 42.5 +) + +// fieldWrittenBy runs write against a zero Options and reports the single field that changed. +func fieldWrittenBy(t *testing.T, name string, write func(*Options)) string { + t.Helper() + + var opts Options + write(&opts) + + var moved string + typ := reflect.TypeFor[Options]() + value := reflect.ValueOf(opts) + for i := range typ.NumField() { + f := typ.Field(i) + if !f.IsExported() || !isValueTyped(f.Type) || value.Field(i).IsZero() { + continue + } + require.Emptyf(t, moved, "flag %q writes to both Options.%s and Options.%s", name, moved, f.Name) + moved = f.Name + } + require.NotEmptyf(t, moved, "flag %q writes to no field of codescan.Options", name) + + return moved +} + +// isValueTyped reports whether a field is of a kind the tables can carry. +// +// The rest - a filesystem to read, a specification to merge, a callback - is not something a command +// line can state, and is the command's business rather than this package's. +func isValueTyped(typ reflect.Type) bool { + switch typ.Kind() { + case reflect.Bool, reflect.String, reflect.Float64: + return true + case reflect.Slice: + return typ.Elem().Kind() == reflect.String + default: + return false + } +} diff --git a/internal/cliopts/doc.go b/internal/cliopts/doc.go new file mode 100644 index 00000000..ddbb4c7f --- /dev/null +++ b/internal/cliopts/doc.go @@ -0,0 +1,29 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +// Package cliopts declares codescan's option surface once, as flags. +// +// Every command that scans - genspec, genspec-wasi - needs the same thing: a [flag.FlagSet] carrying +// one flag per knob on codescan.Options, and a way to copy what was parsed onto the options a scan +// runs with. Written per command, that mapping is partial in a different way each time, and a knob +// added to the library reaches whichever command somebody remembered. +// +// So it is written here instead, as tables keyed by the field each entry writes to, with one guard +// (see the package tests) asserting that no value-typed field of the options is left unreachable. +// A command registers the whole surface and adds only what is its own - where to write, what format, +// how loud. +// +// # Naming +// +// A flag is the kebab-case of the field it writes, without exception: SkipJSONifyInterfaceMethods is +// -skip-jsonify-interface-methods. Guessing a shorter spelling is how a caller ends up guessing +// wrong, and no rule can derive that JSONify is one word where HTTPServer is two - which is also why +// coverage is decided by writing through a setter and seeing what moved, never by mangling a name. +// +// # What is not here +// +// Options that are not values: the packages to scan (positional arguments), a filesystem or export +// data to read (the command opens a path), a specification to merge into (the command loads it), and +// the diagnostic and provenance callbacks. Those are the command's business; the tests carry the +// list, with a reason each. +package cliopts diff --git a/internal/cliopts/errors.go b/internal/cliopts/errors.go new file mode 100644 index 00000000..94f0fba7 --- /dev/null +++ b/internal/cliopts/errors.go @@ -0,0 +1,13 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliopts + +import "errors" + +// ErrBadFlag is a flag whose value is not one of the ones it accepts. +// +// A sentinel rather than a formatted string at each site: a caller - a test, or a command deciding +// an exit code - can then ask which kind of refusal it met without matching on prose that is free to +// change. +var ErrBadFlag = errors.New("bad flag") diff --git a/cmd/genspec-wasi/exec_native.go b/internal/cliopts/exec_native.go similarity index 93% rename from cmd/genspec-wasi/exec_native.go rename to internal/cliopts/exec_native.go index 65f14c10..1bb433b9 100644 --- a/cmd/genspec-wasi/exec_native.go +++ b/internal/cliopts/exec_native.go @@ -3,7 +3,7 @@ //go:build !wasm -package main +package cliopts // canExec reports whether this build can start a subprocess. func canExec() bool { return true } diff --git a/cmd/genspec-wasi/exec_wasm.go b/internal/cliopts/exec_wasm.go similarity index 95% rename from cmd/genspec-wasi/exec_wasm.go rename to internal/cliopts/exec_wasm.go index cae36117..bafdd569 100644 --- a/cmd/genspec-wasi/exec_wasm.go +++ b/internal/cliopts/exec_wasm.go @@ -3,7 +3,7 @@ //go:build wasm -package main +package cliopts // canExec reports whether this build can start a subprocess. // diff --git a/internal/cliopts/loader.go b/internal/cliopts/loader.go new file mode 100644 index 00000000..9e80d275 --- /dev/null +++ b/internal/cliopts/loader.go @@ -0,0 +1,40 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliopts + +import "fmt" + +// The package loader, as a choice of three rather than the boolean the library takes. +// +// ToolchainFreeLoader is a bool, but the useful third answer - "whichever one can run here" - is not +// expressible as one, and it is the answer a caller wants by default. So the flag states the choice +// and [resolveLoader] reduces it, which is also where the boolean is discharged from the coverage +// guard's point of view. +const ( + loaderFlag = "loader" + + loaderGo = "go" // packages.Load, which runs `go list` + loaderOwn = "own" // codescan's own loader: no toolchain, no subprocess + loaderAuto = "auto" // own where there is no exec, go otherwise + + loaderHelp = `package loader: "go" runs go list, "own" needs no toolchain, "auto" picks own where there is no exec` +) + +// resolveLoader reports whether to use codescan's own loader. +// +// "auto" asks whether this build can start a subprocess at all: on wasm it cannot, so `go list` is +// not an option and the choice makes itself. +func resolveLoader(mode string) (bool, error) { + switch mode { + case loaderOwn: + return true, nil + case loaderGo: + return false, nil + case loaderAuto: + return !canExec(), nil + default: + return false, fmt.Errorf("%w: -%s %q is not one of %s, %s, %s", + ErrBadFlag, loaderFlag, mode, loaderGo, loaderOwn, loaderAuto) + } +} diff --git a/internal/cliopts/split.go b/internal/cliopts/split.go new file mode 100644 index 00000000..2334cc25 --- /dev/null +++ b/internal/cliopts/split.go @@ -0,0 +1,36 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliopts + +import "strings" + +// DefaultPatterns is what a command scans when the caller names nothing. +const DefaultPatterns = "./..." + +// SplitList parses a comma-separated flag into trimmed, non-empty entries. +// +// It returns nil when there is nothing usable - nil being what the scanner reads as "no filter". +func SplitList(s string) []string { + var out []string + for p := range strings.SplitSeq(s, ",") { + if p = strings.TrimSpace(p); p != "" { + out = append(out, p) + } + } + + return out +} + +// Patterns is the packages to scan: what the caller named, or everything under the working +// directory. +// +// Positional rather than a flag, so that `genspec ./api/...` reads the way every other Go command +// does - which is also why it cannot be a table entry. +func Patterns(args []string) []string { + if len(args) > 0 { + return args + } + + return []string{DefaultPatterns} +} From d313aeaf11e97cb4b9e5d13b9548a55d88d3a9ba Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Fri, 14 Aug 2026 13:29:13 +0200 Subject: [PATCH 2/8] feat(genspec): add a standalone command-line spec generator genspec scans annotated Go source and writes the Swagger 2.0 document it describes. It is the ordinary native command: every knob the library takes is a flag, the document goes to standard output or to -output, and what the scan observed goes to standard error. Beyond what genspec-wasi already does headlessly, it writes YAML as readily as JSON, merges its discoveries into an existing document, and checks what it produced: - -format json|yaml|auto, where auto reads the extension of -output, so -output swagger.yaml is enough to get YAML; - -input merges into an existing document, for everything a scanner cannot know - the host, the security definitions, a hand-written path; - -validate reports what is wrong with the document against the Swagger 2.0 schema, after writing it, so an invalid one is still there to read; - diagnostics are colored when stderr is a terminal, honouring NO_COLOR and TERM=dumb, with -quiet, -verbose and -color to say otherwise; - -fail-on gives the exit status a threshold, covering what -validate found as well as what the scan observed, since a reader meets them as one stream. It defaults to never. The exit status distinguishes a failed scan, an unusable command line, a tripped threshold and an invalid document, so a pipeline can tell them apart. Its own module, as genspec-tui is, so that a colored logger and a YAML writer stay out of the library's dependencies. The library is required at a version that will carry internal/cliopts, where the shared flag surface lives; until that release the module builds through the workspace, which is how CI tests it. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- .claude/CLAUDE.md | 10 ++ .gitignore | 1 + README.md | 24 ++- cmd/genspec/README.md | 111 ++++++++++++++ cmd/genspec/diagnostics.go | 228 ++++++++++++++++++++++++++++ cmd/genspec/diagnostics_test.go | 195 ++++++++++++++++++++++++ cmd/genspec/doc.go | 36 +++++ cmd/genspec/errors.go | 53 +++++++ cmd/genspec/flags.go | 154 +++++++++++++++++++ cmd/genspec/flags_test.go | 131 ++++++++++++++++ cmd/genspec/go.mod | 46 ++++++ cmd/genspec/go.sum | 84 ++++++++++ cmd/genspec/input.go | 38 +++++ cmd/genspec/main.go | 145 ++++++++++++++++++ cmd/genspec/main_test.go | 261 ++++++++++++++++++++++++++++++++ cmd/genspec/output.go | 77 ++++++++++ cmd/genspec/validate.go | 72 +++++++++ go.work | 1 + 18 files changed, 1664 insertions(+), 3 deletions(-) create mode 100644 cmd/genspec/README.md create mode 100644 cmd/genspec/diagnostics.go create mode 100644 cmd/genspec/diagnostics_test.go create mode 100644 cmd/genspec/doc.go create mode 100644 cmd/genspec/errors.go create mode 100644 cmd/genspec/flags.go create mode 100644 cmd/genspec/flags_test.go create mode 100644 cmd/genspec/go.mod create mode 100644 cmd/genspec/go.sum create mode 100644 cmd/genspec/input.go create mode 100644 cmd/genspec/main.go create mode 100644 cmd/genspec/main_test.go create mode 100644 cmd/genspec/output.go create mode 100644 cmd/genspec/validate.go diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 2d92ff10..e10ac526 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -42,6 +42,16 @@ to builders without direct coupling. | `provenance.go` | `Provenance` — ties a spec JSON pointer to the source position of the Go construct that produced it; emitted via `Options.OnProvenance` (cross-ref linker, source side) | | `classify/` | Classification predicates usable from both scanner and builders (e.g. `IsAllowedExtension`) | +### `internal/cliopts/` — the command-line option surface + +Every knob on `codescan.Options` as a flag, declared once and shared by the commands (`cmd/genspec`, +`cmd/genspec-wasi`) so a flag means the same thing whichever one you reach for. Entries are keyed by +the field's *setter*, never by a name derived from the field, and a guard in the package tests fails +when a value-typed option lands with no flag (or excuse). Flag names are the kebab-case of the field +without exception. `-loader` (`go|own|auto`) is the tri-state that discharges `ToolchainFreeLoader`. +Options that are not values — `FS`, `ExportData`, `InputSpec`, the callbacks — are the command's +business, not the table's. + ### `internal/packages/` — toolchain-free package loader (experimental) Owns **both** ways of resolving a package graph, behind one `Loader`: delegate to diff --git a/.gitignore b/.gitignore index 1286f7a8..4374ae89 100644 --- a/.gitignore +++ b/.gitignore @@ -36,6 +36,7 @@ internal/exportdata/exportdata.zip # stray binaries from "go build ./hack/..." /genexportdata +/genspec /genspec-wasi /loader-benchmark diff --git a/README.md b/README.md index 4f7f60ca..230f0fb5 100644 --- a/README.md +++ b/README.md @@ -68,9 +68,27 @@ diagnostics and spec validation — is on the doc site: ## Generate a spec from the command line -`genspec-wasi` is the headless counterpart: it writes the specification to standard output and takes no -dependency beyond the library, so it also cross-compiles to WebAssembly and runs under a WASI -runtime with no Go toolchain installed. +`genspec` is the headless counterpart: it writes the specification to standard output, or to the file +`-output` names, and reports what the scan observed as colored diagnostics on standard error. + +```cmd +go install github.com/go-openapi/codescan/cmd/genspec@latest +``` + +```cmd +genspec -workdir [my source location] ./... +``` + +Every option the library takes is a flag. Beyond those, it writes YAML as readily as JSON +(`-output swagger.yaml` is enough), merges its discoveries into an existing document with `-input`, +and checks what it produced with `-validate`. Its exit status says which of those went wrong. + +See [cmd/genspec/README.md](cmd/genspec/README.md). + +### Where there is no Go toolchain + +`genspec-wasi` runs the same scan taking no dependency beyond the library, so it cross-compiles to +WebAssembly and runs under a WASI runtime with no Go toolchain installed and no subprocess. ```cmd go install github.com/go-openapi/codescan/cmd/genspec-wasi@latest diff --git a/cmd/genspec/README.md b/cmd/genspec/README.md new file mode 100644 index 00000000..f9364cf5 --- /dev/null +++ b/cmd/genspec/README.md @@ -0,0 +1,111 @@ + + +# genspec + +Point it at annotated Go source, get a Swagger 2.0 document. + +```sh +go install github.com/go-openapi/codescan/cmd/genspec@latest + +# scan the module in the current directory, write the document to standard output +genspec + +# somewhere else, narrowed, to a file, and checked +genspec -workdir ../my-api -output swagger.yaml -validate ./internal/api/... +``` + +This is the ordinary native command. Everything the library can be told is a +flag, the document goes to standard output or to `-output`, and what the scan +observed goes to standard error as colored diagnostics. + +## Which one to reach for + +Three commands run the same scan. The question is about the machine, not about +the specification: + +| Command | Use it when | +|---------|-------------| +| **`genspec`** | you are on a normal machine and want a specification | +| [`genspec-wasi`](../genspec-wasi/README.md) | there is no Go toolchain, no subprocess, or you are running under WebAssembly — it takes no dependency beyond the library. It also speaks a machine-readable envelope (`-format=json`) carrying diagnostics and cross-references | +| [`genspec-tui`](../genspec-tui/README.md) | you are working *on* the annotations and want the source and the document side by side, live | + +They share their flag surface: `internal/cliopts` declares every knob the +library takes, once, so `-name-from-tags` means the same thing whichever one you +reach for. A guard there fails the build when an option lands with no flag. + +## Output + +`-output` names the file, or `-` for standard output (the default). `-format` +is `json`, `yaml`, or `auto` — which reads the extension of `-output` and writes +JSON when that says nothing. So the common cases need no `-format` at all: + +```sh +genspec -output swagger.yaml # YAML, because of the name +genspec -output swagger.json # JSON +genspec > swagger.json # JSON +genspec -compact # JSON with no indentation +``` + +YAML is derived from the JSON rendering, which costs key order: the document +comes out alphabetical rather than in the order the spec types declare. Same +information, different diff against a hand-written file. + +`-input` merges the scan's discoveries into an existing document — the place for +everything a scanner cannot know, such as the host, the security definitions, or +a hand-written path the annotations do not describe. + +## Diagnostics + +Everything the scan observed is reported on standard error, colored when that is +a terminal. Nothing is written there when there is nothing to say. + +| Flag | Meaning | +|------|---------| +| `-quiet` | say nothing at all | +| `-verbose` | also report hints, which are muted by default | +| `-color` | `auto` (a terminal), `always`, `never`. `auto` honours `NO_COLOR` and `TERM=dumb` | +| `-validate` | check the document against the Swagger 2.0 schema and report what is wrong with it | +| `-fail-on` | exit non-zero when something reaches this severity: `error`, `warning`, or `never` | + +`-fail-on` covers what `-validate` found as well as what the scan observed: they +reach the reader as one stream, so a threshold that saw only half of it would be +a trap rather than a policy. It defaults to `never`, because a scan that emits +warnings is the ordinary case, and a command that failed the build over one +would mostly teach people to stop reading them. + +## Exit status + +A specification is written whenever one could be produced, so a non-zero status +describes what was wrong with it rather than meaning nothing came out. + +| Status | Meaning | +|--------|---------| +| 0 | the scan produced a document, and nothing asked for more | +| 1 | the scan failed | +| 2 | the command line does not make sense | +| 3 | what was reported reached the severity `-fail-on` names | +| 4 | `-validate` found the document invalid | + +`-validate` finding the document invalid outranks `-fail-on`: it is the more +specific answer. + +## Scan options + +`genspec -h` lists them all. They fall into families: + +- **what to scan** — `-workdir`, the positional package patterns, `-build-tags`, + `-include` / `-exclude`, `-include-tags` / `-exclude-tags`, `-exclude-deps` +- **what to build it as** — `-goos`, `-goarch`, `-goflags`, `-gowork`, + `-goexperiment`. Each changes what gets compiled, and so what the document + says; each is a flag rather than inherited state, so a scan is reproducible +- **how to load it** — `-loader`, `-stub-stdlib`, `-skip-compiled-dependencies` +- **what to emit** — `-scan-models`, `-prune-unused-models`, the alias and + `allOf` knobs, `-skip-extensions`, the naming knobs, the doc-comment knobs + +They are the library's own options under their own names: a flag is the +kebab-case of the field it writes, without exception. See the [package +documentation](https://pkg.go.dev/github.com/go-openapi/codescan#Options) for +what each one means. diff --git a/cmd/genspec/diagnostics.go b/cmd/genspec/diagnostics.go new file mode 100644 index 00000000..aac1ba11 --- /dev/null +++ b/cmd/genspec/diagnostics.go @@ -0,0 +1,228 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "fmt" + "io" + "log/slog" + "os" + "path/filepath" + + "github.com/SladkyCitron/slogcolor" + + "github.com/go-openapi/codescan" +) + +// reporter is where everything the scan observed goes. +// +// codescan never writes to standard error itself - every observation arrives through a callback - so +// this is the only thing standing between a scan and a silent one. It is also what decides the exit +// status: a caller running this in a pipeline wants to be told, and a summary a human reads is not +// something a pipeline can act on. +type reporter struct { + logger *slog.Logger + + // root is what positions are reported relative to, so that a diagnostic names the file the way + // the caller would. + root string + + // hints says whether hints are reported. They are muted by default: they are the scanner + // thinking aloud - a model it discovered, a name it reduced - and there are a great many. + hints bool + + // threshold is the least serious severity that still fails the command, when there is one. + threshold codescan.Severity + failing bool + + // seen counts every diagnostic by severity, muted ones included: what was suppressed was still + // observed, and the summary saying so is how a caller learns -verbose has something to show. + seen map[codescan.Severity]int + + // tripped records that something reached the threshold, so that the whole scan is reported + // before the command exits on it rather than at the first one. + tripped bool +} + +// newReporter wires a sink for the scan's diagnostics. +func newReporter(cfg *config, stderr io.Writer) (*reporter, error) { + threshold, failing, err := resolveFailOn(*cfg.failOn) + if err != nil { + return nil, err + } + + colorize, err := resolveColor(*cfg.color, stderr) + if err != nil { + return nil, err + } + + return &reporter{ + logger: logger(stderr, colorize, *cfg.quiet), + hints: *cfg.verbose, + threshold: threshold, + failing: failing, + seen: make(map[codescan.Severity]int), + }, nil +} + +// onDiagnostic is what [codescan.Options.OnDiagnostic] is given. +func (r *reporter) onDiagnostic(diag codescan.Diagnostic) { + r.record(diag.Severity) + + if diag.Severity == codescan.SeverityHint && !r.hints { + return + } + + r.log(diag.Severity, diag.Message, + slog.String("code", string(diag.Code)), + slog.String("file", r.relative(diag.Pos.Filename)), + slog.Int("line", diag.Pos.Line), + slog.Int("column", diag.Pos.Column), + ) +} + +// record tallies one finding and decides whether it fails the command. +// +// Everything the command reports goes through here - what the scan observed and what -validate +// found - because a reader meets them as one stream, in one set of colours, and a threshold that saw +// only half of it would be a trap rather than a policy. +// +// A severity is more serious the lower it sorts, so "at least as serious as the threshold" reads +// backwards. Counted even when the finding itself is muted: what makes a command fail should not +// depend on how loud it was asked to be. +func (r *reporter) record(severity codescan.Severity) { + r.seen[severity]++ + + if r.failing && severity <= r.threshold { + r.tripped = true + } +} + +// log writes one line at the level a severity maps onto. +func (r *reporter) log(sev codescan.Severity, msg string, attrs ...any) { + switch sev { + case codescan.SeverityError: + r.logger.Error(msg, attrs...) + case codescan.SeverityWarning: + r.logger.Warn(msg, attrs...) + case codescan.SeverityHint: + r.logger.Info(msg, attrs...) + default: + r.logger.Info(msg, attrs...) + } +} + +// relative renders a position's file the way the caller named it. +// +// An absolute path is what the scanner records, and what it means is right - but it is not what +// somebody standing in their own module wants to read, nor what an editor can be handed. Where the +// two cannot be related, the absolute path is still better than nothing. +func (r *reporter) relative(filename string) string { + if filename == "" || r.root == "" { + return filename + } + + rel, err := filepath.Rel(r.root, filename) + if err != nil { + return filename + } + + return rel +} + +// summarize reports what the command observed in all, and whether that is a failure. +// +// The counts include what was muted. A scan whose hints were suppressed says how many there were, +// which is the only way a caller finds out that -verbose has anything to add. +func (r *reporter) summarize() error { + counted := []struct { + severity codescan.Severity + label string + }{ + {codescan.SeverityError, "errors"}, + {codescan.SeverityWarning, "warnings"}, + {codescan.SeverityHint, "hints"}, + } + + attrs := make([]any, 0, len(counted)) + total := 0 + for _, c := range counted { + if n := r.seen[c.severity]; n > 0 { + attrs = append(attrs, slog.Int(c.label, n)) + total += n + } + } + + if total > 0 { + r.logger.Info("finished", attrs...) + } + + if r.tripped { + return fmt.Errorf("%w (%s)", errDiagnostics, r.threshold) + } + + return nil +} + +// logger builds the sink the reporter writes through. +func logger(stderr io.Writer, colorize, quiet bool) *slog.Logger { + if quiet { + return slog.New(discardHandler{}) + } + + opts := *slogcolor.DefaultOptions + opts.NoColor = !colorize + // A timestamp on every line of a scan that takes a second, and the name of the file inside this + // command that happened to call the logger, are both noise: what a reader wants located is the + // diagnostic's own position, which travels as an attribute. + opts.NoTime = true + opts.SrcFileMode = slogcolor.Nop + + return slog.New(slogcolor.NewHandler(stderr, &opts)) +} + +// resolveColor decides whether to colorize, from what was asked and where it is writing. +func resolveColor(mode string, stderr io.Writer) (bool, error) { + switch mode { + case colorAlways: + return true, nil + case colorNever: + return false, nil + case colorAuto: + return isTerminal(stderr) && os.Getenv("NO_COLOR") == "" && os.Getenv("TERM") != "dumb", nil + default: + return false, fmt.Errorf("%w: -color %q is not one of %s, %s, %s", + errUsage, mode, colorAlways, colorNever, colorAuto) + } +} + +// isTerminal reports whether w is something a person is looking at. +// +// A character device is the stdlib's own answer to that question, and it is enough here: the cost of +// being wrong is escape codes in a file, which is exactly what -color=never is for. +func isTerminal(w io.Writer) bool { + file, ok := w.(*os.File) + if !ok { + return false + } + + info, err := file.Stat() + if err != nil { + return false + } + + return info.Mode()&os.ModeCharDevice != 0 +} + +// discardHandler is what -quiet installs. +// +// A handler that refuses every level, rather than a writer pointed at io.Discard: the difference is +// that nothing is formatted at all, and formatting is most of the cost of a diagnostic nobody reads. +type discardHandler struct{} + +func (discardHandler) Enabled(context.Context, slog.Level) bool { return false } +func (discardHandler) Handle(context.Context, slog.Record) error { return nil } +func (h discardHandler) WithAttrs([]slog.Attr) slog.Handler { return h } +func (h discardHandler) WithGroup(string) slog.Handler { return h } diff --git a/cmd/genspec/diagnostics_test.go b/cmd/genspec/diagnostics_test.go new file mode 100644 index 00000000..a233d375 --- /dev/null +++ b/cmd/genspec/diagnostics_test.go @@ -0,0 +1,195 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "go/token" + "os" + "path/filepath" + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" + + "github.com/go-openapi/codescan" +) + +// reporterFor builds a reporter writing into a buffer, as the flags given would. +func reporterFor(t *testing.T, argv ...string) (*reporter, *bytes.Buffer) { + t.Helper() + + fs := parseInto(t, argv...) + var buf bytes.Buffer + + report, err := newReporter(fs, &buf) + require.NoError(t, err) + + return report, &buf +} + +func diagnostic(severity codescan.Severity, message string) codescan.Diagnostic { + return codescan.Diagnostic{ + Severity: severity, + Code: "scan.test", + Message: message, + Pos: token.Position{Filename: "/src/api/models.go", Line: 7, Column: 3}, + } +} + +func TestReporterWritesWhatItIsGiven(t *testing.T) { + t.Parallel() + + report, out := reporterFor(t, "-color=never") + report.onDiagnostic(diagnostic(codescan.SeverityWarning, "something to say")) + + assert.Contains(t, out.String(), "something to say") + assert.Contains(t, out.String(), "WARN") + assert.Contains(t, out.String(), "code=scan.test") + assert.Contains(t, out.String(), "line=7") +} + +// TestReporterMutesHintsButCountsThem is why the summary exists: a scan can be quiet and still have +// had a great deal to say, and the count is the only hint that -verbose would show it. +func TestReporterMutesHintsButCountsThem(t *testing.T) { + t.Parallel() + + report, out := reporterFor(t, "-color=never") + report.onDiagnostic(diagnostic(codescan.SeverityHint, "thinking aloud")) + + assert.NotContains(t, out.String(), "thinking aloud") + + require.NoError(t, report.summarize()) + assert.Contains(t, out.String(), "hints=1") +} + +func TestReporterVerboseShowsHints(t *testing.T) { + t.Parallel() + + report, out := reporterFor(t, "-color=never", "-verbose") + report.onDiagnostic(diagnostic(codescan.SeverityHint, "thinking aloud")) + + assert.Contains(t, out.String(), "thinking aloud") +} + +func TestReporterQuietWritesNothingAtAll(t *testing.T) { + t.Parallel() + + report, out := reporterFor(t, "-quiet", "-verbose") + report.onDiagnostic(diagnostic(codescan.SeverityError, "a real problem")) + _ = report.summarize() + + assert.Empty(t, out.String()) +} + +func TestReporterThreshold(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + failOn string + severity codescan.Severity + fails bool + }{ + {"never fails on nothing", "never", codescan.SeverityError, false}, + {"error fails on an error", "error", codescan.SeverityError, true}, + {"error ignores a warning", "error", codescan.SeverityWarning, false}, + {"warning fails on a warning", "warning", codescan.SeverityWarning, true}, + {"warning fails on an error too", "warning", codescan.SeverityError, true}, + {"warning ignores a hint", "warning", codescan.SeverityHint, false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + report, _ := reporterFor(t, "-color=never", "-fail-on="+tc.failOn) + report.onDiagnostic(diagnostic(tc.severity, "a finding")) + + err := report.summarize() + if !tc.fails { + require.NoError(t, err) + + return + } + require.ErrorIs(t, err, errDiagnostics) + }) + } +} + +// TestReporterTripsOnAMutedDiagnostic states that policy and volume are separate: -quiet asks for +// silence, not for a different answer. +func TestReporterTripsOnAMutedDiagnostic(t *testing.T) { + t.Parallel() + + report, _ := reporterFor(t, "-quiet", "-fail-on=warning") + report.onDiagnostic(diagnostic(codescan.SeverityWarning, "muted, but still true")) + + require.ErrorIs(t, report.summarize(), errDiagnostics) +} + +func TestReporterSaysNothingWhenThereIsNothingToSay(t *testing.T) { + t.Parallel() + + report, out := reporterFor(t, "-color=never") + + require.NoError(t, report.summarize()) + assert.Empty(t, out.String(), "a summary of nothing is noise") +} + +func TestReporterRelative(t *testing.T) { + t.Parallel() + + report := &reporter{root: filepath.FromSlash("/src")} + + assert.Equal(t, filepath.FromSlash("api/models.go"), + report.relative(filepath.FromSlash("/src/api/models.go"))) + assert.Empty(t, report.relative(""), "a diagnostic about no file in particular stays that way") + + unrelated := &reporter{} + assert.Equal(t, filepath.FromSlash("/src/api/models.go"), + unrelated.relative(filepath.FromSlash("/src/api/models.go")), + "with no root to relate it to, the absolute path is still better than nothing") +} + +func TestResolveColor(t *testing.T) { + t.Parallel() + + always, err := resolveColor(colorAlways, &bytes.Buffer{}) + require.NoError(t, err) + assert.True(t, always, "a buffer is not a terminal, and -color=always does not ask") + + never, err := resolveColor(colorNever, os.Stderr) + require.NoError(t, err) + assert.False(t, never) + + _, err = resolveColor("sometimes", &bytes.Buffer{}) + require.ErrorIs(t, err, errUsage) +} + +// TestResolveColorAutoOnSomethingThatIsNotATerminal covers the case every pipeline meets: a +// redirected stream gets no escape codes without anybody having to say so. +func TestResolveColorAutoOnSomethingThatIsNotATerminal(t *testing.T) { + t.Parallel() + + auto, err := resolveColor(colorAuto, &bytes.Buffer{}) + require.NoError(t, err) + assert.False(t, auto) + + file, err := os.CreateTemp(t.TempDir(), "out") + require.NoError(t, err) + defer func() { _ = file.Close() }() + + auto, err = resolveColor(colorAuto, file) + require.NoError(t, err) + assert.False(t, auto, "a regular file is not a terminal either") +} + +func TestResolveColorAutoRespectsNoColor(t *testing.T) { + t.Setenv("NO_COLOR", "1") + + // Not on a terminal to begin with, so this states the environment is consulted at all rather than + // that it wins - which is the most this can check without one. + auto, err := resolveColor(colorAuto, &bytes.Buffer{}) + require.NoError(t, err) + assert.False(t, auto) +} diff --git a/cmd/genspec/doc.go b/cmd/genspec/doc.go new file mode 100644 index 00000000..d145fab6 --- /dev/null +++ b/cmd/genspec/doc.go @@ -0,0 +1,36 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +// Command genspec scans annotated Go source and writes the Swagger 2.0 specification it describes. +// +// go install github.com/go-openapi/codescan/cmd/genspec@latest +// genspec -workdir ./api ./... +// +// It is the ordinary, native way to run codescan: everything the library can be told is a flag, the +// document goes to standard output or to -output, and what the scan observed goes to standard error +// as colored diagnostics. See -h for the whole surface. +// +// # The family +// +// Three commands run the same scan, and which one to reach for is a question about the machine +// rather than about the specification: +// +// genspec here. Native, colored diagnostics, YAML, merges into an existing document, +// validates what it produced +// genspec-wasi the same scan with no dependency beyond the library, so it cross-compiles to +// WebAssembly and runs under a WASI runtime with no Go toolchain installed. It also +// speaks a machine-readable envelope (-format=json) carrying diagnostics and +// cross-references +// genspec-tui the same scan, live, with the source and the document side by side +// +// # Exit status +// +// A specification is written whenever one could be produced, so a non-zero status describes what was +// wrong with it rather than meaning nothing came out: +// +// 0 the scan produced a document, and nothing asked for more +// 1 the scan failed +// 2 the command line does not make sense +// 3 what was reported reached the severity -fail-on names +// 4 -validate found the document invalid +package main diff --git a/cmd/genspec/errors.go b/cmd/genspec/errors.go new file mode 100644 index 00000000..53554326 --- /dev/null +++ b/cmd/genspec/errors.go @@ -0,0 +1,53 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "errors" + + "github.com/go-openapi/codescan/internal/cliopts" +) + +// What the command refuses, and what it reports about a document it did produce. +// +// Sentinels rather than a formatted string at each site: the exit status is decided by asking which +// kind of refusal was met, so a shell can tell "the scan failed" from "the scan is fine and you +// asked me to be strict about it" without matching on prose that is free to change. +var ( + // errUsage is a command line that does not make sense. + errUsage = errors.New("bad usage") + // errDiagnostics is a run whose findings reached the severity -fail-on names. + errDiagnostics = errors.New("reported findings reached the -fail-on threshold") + // errInvalidSpec is a document that -validate found invalid. + errInvalidSpec = errors.New("the specification is not valid") +) + +// Exit statuses. See the package documentation. +const ( + exitOK = 0 + exitFailed = 1 + exitUsage = 2 + exitDiagnostics = 3 + exitInvalidSpec = 4 +) + +// exitStatus maps what run reported onto what the shell sees. +// +// Anything unrecognised is a failed scan: the specific statuses are promises about states the +// command distinguishes on purpose, and a new kind of error is not one of them until it is added +// here. +func exitStatus(err error) int { + switch { + case err == nil: + return exitOK + case errors.Is(err, errUsage), errors.Is(err, cliopts.ErrBadFlag): + return exitUsage + case errors.Is(err, errDiagnostics): + return exitDiagnostics + case errors.Is(err, errInvalidSpec): + return exitInvalidSpec + default: + return exitFailed + } +} diff --git a/cmd/genspec/flags.go b/cmd/genspec/flags.go new file mode 100644 index 00000000..58353c0c --- /dev/null +++ b/cmd/genspec/flags.go @@ -0,0 +1,154 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "flag" + "fmt" + "io" + "strings" + + "github.com/go-openapi/codescan" + "github.com/go-openapi/codescan/internal/cliopts" +) + +// What -format can be told. "auto" reads the extension of -output, and writes JSON when that says +// nothing - which is what a document going to standard output does. +const ( + formatAuto = "auto" + formatJSON = "json" + formatYAML = "yaml" +) + +// What -color can be told. "auto" is the only one that looks at where it is writing. +const ( + colorAuto = "auto" + colorAlways = "always" + colorNever = "never" +) + +// What -fail-on can be told: the least serious diagnostic that still makes the command exit +// non-zero. +// +// "never" is the default because a scan that emits warnings is the ordinary case - a codebase with +// nothing to say about it is the exception - and a command that failed the build over one would +// mostly teach people to stop reading it. +const ( + failNever = "never" + failError = "error" + failWarning = "warning" +) + +// config is the whole command line: the library's own knobs, plus this command's. +type config struct { + set *flag.FlagSet + + // scan is every knob the library takes, declared in internal/cliopts and shared with the other + // commands, so that a flag means the same thing whichever one you reach for. + scan *cliopts.Values + + input *string + output *string + format *string + compact *bool + validate *bool + + color *string + quiet *bool + verbose *bool + failOn *string + + version *bool +} + +func registerFlags(fs *flag.FlagSet) *config { + return &config{ + set: fs, + scan: cliopts.Register(fs), + + input: fs.String("input", "", + "a specification to merge the scan's discoveries into, as JSON or YAML"), + output: fs.String("output", "-", `write the specification here ("-" for standard output)`), + format: fs.String("format", formatAuto, + `what to write: "json", "yaml", or "auto" to read the extension of -output`), + compact: fs.Bool("compact", false, "write the document without indentation"), + validate: fs.Bool("validate", false, + "check the document against the Swagger 2.0 schema and report what is wrong with it"), + + color: fs.String("color", colorAuto, + `colored diagnostics: "always", "never", or "auto" for whenever stderr is a terminal`), + quiet: fs.Bool("quiet", false, "say nothing on standard error"), + verbose: fs.Bool("verbose", false, "also report hints, which are muted by default"), + failOn: fs.String("fail-on", failNever, + `exit non-zero when anything reported - by the scan or by -validate - reaches this`+"\n"+ + `severity: "error", "warning", or "never"`), + + version: fs.Bool("version", false, "print the version and exit"), + } +} + +// usage is what -h prints. +// +// Written out rather than left to the flag package's default, because the first thing a reader needs +// is that the packages are positional and that the document goes to standard output - neither of +// which is a flag, so neither would appear. +func usage(fs *flag.FlagSet, stderr io.Writer) func() { + return func() { + fmt.Fprintln(stderr, "usage: genspec [flags] [packages...]") + fmt.Fprintln(stderr, "\nScans annotated Go source and writes the Swagger 2.0 specification it describes.") + fmt.Fprintln(stderr, "\nThe packages are Go patterns, resolved against -workdir. Naming none scans "+ + cliopts.DefaultPatterns) + fmt.Fprintln(stderr, "The document goes to standard output unless -output names a file; what the scan") + fmt.Fprintln(stderr, "observed goes to standard error.") + fmt.Fprintln(stderr, "\nFlags:") + fs.PrintDefaults() + } +} + +// resolveFormat decides what to write, from what was asked and where it is going. +// +// The extension is consulted only where nothing was said: a caller who writes -format=json to a file +// called spec.yaml has said something contradictory on purpose, and the flag is the one that says it +// out loud. +func resolveFormat(format, output string) (string, error) { + switch format { + case formatJSON, formatYAML: + return format, nil + case formatAuto: + if isYAMLName(output) { + return formatYAML, nil + } + + return formatJSON, nil + default: + return "", fmt.Errorf("%w: -format %q is not one of %s, %s, %s", + errUsage, format, formatJSON, formatYAML, formatAuto) + } +} + +// isYAMLName reports whether a path names a YAML file. +func isYAMLName(path string) bool { + lower := strings.ToLower(path) + + return strings.HasSuffix(lower, ".yaml") || strings.HasSuffix(lower, ".yml") +} + +// resolveFailOn reads the threshold as the severity it names. +// +// Reported as a [codescan.Severity] and a flag saying whether there is a threshold at all, rather +// than as a severity meaning "none": the enum belongs to the scanner, and inventing a member of it +// here would be inventing a diagnostic that cannot happen. +func resolveFailOn(failOn string) (codescan.Severity, bool, error) { + switch failOn { + case failNever: + return 0, false, nil + case failError: + return codescan.SeverityError, true, nil + case failWarning: + return codescan.SeverityWarning, true, nil + default: + return 0, false, fmt.Errorf("%w: -fail-on %q is not one of %s, %s, %s", + errUsage, failOn, failError, failWarning, failNever) + } +} diff --git a/cmd/genspec/flags_test.go b/cmd/genspec/flags_test.go new file mode 100644 index 00000000..e6889743 --- /dev/null +++ b/cmd/genspec/flags_test.go @@ -0,0 +1,131 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "flag" + "io" + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" + + "github.com/go-openapi/codescan" +) + +// parseInto registers the command line and parses argv into it. +func parseInto(t *testing.T, argv ...string) *config { + t.Helper() + + fs := flag.NewFlagSet("genspec", flag.ContinueOnError) + fs.SetOutput(io.Discard) + + cfg := registerFlags(fs) + require.NoError(t, fs.Parse(argv)) + + return cfg +} + +func TestResolveFormat(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + format string + output string + want string + }{ + {"json is json", formatJSON, "-", formatJSON}, + {"yaml is yaml", formatYAML, "-", formatYAML}, + {"auto to standard output is json", formatAuto, "-", formatJSON}, + {"auto reads a .yaml extension", formatAuto, "spec.yaml", formatYAML}, + {"auto reads a .yml extension", formatAuto, "spec.yml", formatYAML}, + {"auto reads it whatever its case", formatAuto, "SPEC.YAML", formatYAML}, + {"auto falls back to json", formatAuto, "spec.json", formatJSON}, + {"auto on a file with no extension is json", formatAuto, "spec", formatJSON}, + {"an explicit format wins over the name", formatJSON, "spec.yaml", formatJSON}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := resolveFormat(tc.format, tc.output) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestResolveFormatRefusesAnythingElse(t *testing.T) { + t.Parallel() + + _, err := resolveFormat("toml", "-") + + require.ErrorIs(t, err, errUsage) + assert.Contains(t, err.Error(), "toml") +} + +func TestResolveFailOn(t *testing.T) { + t.Parallel() + + _, failing, err := resolveFailOn(failNever) + require.NoError(t, err) + assert.False(t, failing, "the default lets a scan report whatever it likes and still succeed") + + severity, failing, err := resolveFailOn(failError) + require.NoError(t, err) + assert.True(t, failing) + assert.Equal(t, codescan.SeverityError, severity) + + severity, failing, err = resolveFailOn(failWarning) + require.NoError(t, err) + assert.True(t, failing) + assert.Equal(t, codescan.SeverityWarning, severity) + + _, _, err = resolveFailOn("hint") + require.ErrorIs(t, err, errUsage, "hints are not a threshold: a scan that fails on them fails always") +} + +// TestFlagsReachTheLibrary states the point of sharing the table with the other commands: what the +// library takes is registered here too, and lands where it says. +func TestFlagsReachTheLibrary(t *testing.T) { + t.Parallel() + + cfg := parseInto(t, + "-workdir=/src", "-scan-models=false", "-exclude-tags=internal", "-emit-x-go-type", + ) + + var opts codescan.Options + require.NoError(t, cfg.scan.Apply(&opts)) + + assert.Equal(t, "/src", opts.WorkDir) + assert.False(t, opts.ScanModels) + assert.Equal(t, []string{"internal"}, opts.ExcludeTags) + assert.True(t, opts.EmitXGoType) +} + +func TestOptionsRefusesAnUnknownLoader(t *testing.T) { + t.Parallel() + + cfg := parseInto(t, "-loader=cargo") + + _, err := cfg.options(nil, &reporter{}) + + require.Error(t, err) + assert.Equal(t, exitUsage, exitStatus(err), "a flag value that does not exist is a usage error") +} + +// TestOptionsDefaultsToEverythingUnderTheWorkingDirectory covers the argument that is not a flag. +func TestOptionsDefaultsToEverythingUnderTheWorkingDirectory(t *testing.T) { + t.Parallel() + + cfg := parseInto(t) + + opts, err := cfg.options(nil, &reporter{}) + require.NoError(t, err) + assert.Equal(t, []string{"./..."}, opts.Packages) + + opts, err = cfg.options([]string{"./api/..."}, &reporter{}) + require.NoError(t, err) + assert.Equal(t, []string{"./api/..."}, opts.Packages) +} diff --git a/cmd/genspec/go.mod b/cmd/genspec/go.mod new file mode 100644 index 00000000..5f57131c --- /dev/null +++ b/cmd/genspec/go.mod @@ -0,0 +1,46 @@ +module github.com/go-openapi/codescan/cmd/genspec + +// The library is required at the version that will carry internal/cliopts, which this command reads +// its flag surface from. Until codescan releases it, this module builds through the workspace at the +// repository root - which is how CI tests it - and `go install .../cmd/genspec@latest` will resolve +// once that release is out. +go 1.25.0 + +require ( + github.com/SladkyCitron/slogcolor v1.9.0 + github.com/go-openapi/codescan v0.36.3 + github.com/go-openapi/loads v0.25.0 + github.com/go-openapi/spec v0.22.9 + github.com/go-openapi/strfmt v0.27.0 + github.com/go-openapi/validate v0.26.3 + go.yaml.in/yaml/v3 v3.0.5 +) + +require ( + github.com/fatih/color v1.16.0 // indirect + github.com/go-openapi/analysis v0.26.0 // indirect + github.com/go-openapi/errors v0.22.8 // indirect + github.com/go-openapi/jsonpointer v1.0.0 // indirect + github.com/go-openapi/jsonreference v1.0.0 // indirect + github.com/go-openapi/swag/conv v0.28.0 // indirect + github.com/go-openapi/swag/fileutils v0.28.0 // indirect + github.com/go-openapi/swag/jsonutils v0.28.0 // indirect + github.com/go-openapi/swag/loading v0.28.0 // indirect + github.com/go-openapi/swag/mangling v0.28.0 // indirect + github.com/go-openapi/swag/pools v0.28.0 // indirect + github.com/go-openapi/swag/stringutils v0.28.0 // indirect + github.com/go-openapi/swag/typeutils v0.28.0 // indirect + github.com/go-openapi/swag/yamlutils v0.28.0 // indirect + github.com/go-viper/mapstructure/v2 v2.5.0 // indirect + github.com/google/go-cmp v0.7.0 // indirect + github.com/google/uuid v1.6.0 // indirect + github.com/mattn/go-colorable v0.1.13 // indirect + github.com/mattn/go-isatty v0.0.24 // indirect + github.com/oklog/ulid/v2 v2.1.2 // indirect + golang.org/x/mod v0.38.0 // indirect + golang.org/x/net v0.57.0 // indirect + golang.org/x/sync v0.22.0 // indirect + golang.org/x/sys v0.47.0 // indirect + golang.org/x/text v0.40.0 // indirect + golang.org/x/tools v0.48.0 // indirect +) diff --git a/cmd/genspec/go.sum b/cmd/genspec/go.sum new file mode 100644 index 00000000..3c03e96a --- /dev/null +++ b/cmd/genspec/go.sum @@ -0,0 +1,84 @@ +github.com/SladkyCitron/slogcolor v1.9.0 h1:fr4LeG+T6pZ1RYNNOP925jaBYrTdpA6BLzGy29yd4vI= +github.com/SladkyCitron/slogcolor v1.9.0/go.mod h1:ft8LEVIl4isUkebakhv+ngNXJjWBumnwhXfxTLApf3M= +github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= +github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= +github.com/go-openapi/analysis v0.26.0 h1:1xECln1iMMmQnTjgcknC1vi1hA4KISt6IHpSwnqcuwI= +github.com/go-openapi/analysis v0.26.0/go.mod h1:40gERFi/2dyXA1FaqRRLxkv1IlC6X+GPDNd1xrYAjZE= +github.com/go-openapi/codescan v0.36.3 h1:acI//gm/PfYaM6GI5k3+qiXGHZ2vbZOjSG3RidLgua0= +github.com/go-openapi/codescan v0.36.3/go.mod h1:n0z3IJDm3ysnon+bLfBbHoy0WrRzlBE0LfeuAxGEnuM= +github.com/go-openapi/errors v0.22.8 h1:oP7sW7TWc3wFFjrzzj0nI83H2qMBkNjNfSd+XRejk/I= +github.com/go-openapi/errors v0.22.8/go.mod h1:BuUoHcYrU6E7V9gfj1I5wLQqgtIHnup/alXZ8KdgQ0w= +github.com/go-openapi/jsonpointer v1.0.0 h1:kR9tHqY0CtZaOPVFm622dPVNhrvYpwr4uCxgL3h1H8s= +github.com/go-openapi/jsonpointer v1.0.0/go.mod h1:Z3rw7dWu1p9IgitXCFamSlA5lmDiklEB6vkaxcNZW5Y= +github.com/go-openapi/jsonreference v1.0.0 h1:jlmTr6torcd1YgDQvSfNmRtKzYDO4FGBkrAdlAVWnpY= +github.com/go-openapi/jsonreference v1.0.0/go.mod h1:jtwdyGbJk0Xhe5Y+rwtglQP6Sb1WZST4rT32LWB+sv0= +github.com/go-openapi/loads v0.25.0 h1:74Bc2snfaVlsHzwdQj/3gsA9XJz3daXTJVs+4ZaK7jI= +github.com/go-openapi/loads v0.25.0/go.mod h1:JFBw4SIB9+PTIFHDfcXuSSy5h6aWzjtUCrPYyx3qWU8= +github.com/go-openapi/spec v0.22.9 h1:/vKIFDcGKp0ktZWGbym/tJEWbk6/XOEmAVU0kqKMH+w= +github.com/go-openapi/spec v0.22.9/go.mod h1:b/mNUYIOQOyIiUzUzXEE8xzyZqf93KvM9hQGP91yfl0= +github.com/go-openapi/strfmt v0.27.0 h1:kbcTeaD9TXuXD0hhMXzuYa1sdTo6+dWGvwjW93E80IM= +github.com/go-openapi/strfmt v0.27.0/go.mod h1:s/qhDqfY72irigXUGJmtgid2Rm+3tnz3k8hZaRmvWYc= +github.com/go-openapi/swag/conv v0.28.0 h1:GtqqbyFe7vR5Y7ehxG9W6/OvrSFdf1OLeTGp40TqxH8= +github.com/go-openapi/swag/conv v0.28.0/go.mod h1:mbUE+mzctnhxi864m0Q07SpN8OowD9JhxmxuYvZZD/k= +github.com/go-openapi/swag/fileutils v0.28.0 h1:Z04XWQD7R8Eq+7GnOrjovBxPPmZzsS4gt2H2GPGIViU= +github.com/go-openapi/swag/fileutils v0.28.0/go.mod h1:VvJFZLTZS0AI854gEQz5tk7dBESdLjiNUMSZ/th2ry8= +github.com/go-openapi/swag/jsonutils v0.28.0 h1:YIch6FwO7RXzeAnbO8Tu7dWBZeUEH+4nA0HXltVTnv4= +github.com/go-openapi/swag/jsonutils v0.28.0/go.mod h1:CYM3WlTUcagR2ZoHdz54di/cbBqt82tuxuXgAjxw+mg= +github.com/go-openapi/swag/jsonutils/fixtures_test v0.28.0 h1:qV+VVUAx5Oro8WjVWpZeql7YReTKhT4smR4zhcOQZr0= +github.com/go-openapi/swag/jsonutils/fixtures_test v0.28.0/go.mod h1:mofwUWx70wvskwESqRJ//k/9kURmCgyJl5m5Ppoh5kY= +github.com/go-openapi/swag/loading v0.28.0 h1:td8QZdZC9MIYGGSnSPKShKiK22I2tU5UQvuUhIBPRLU= +github.com/go-openapi/swag/loading v0.28.0/go.mod h1:rXB0QiQX5mMveXEA7ouM4KiiM9jVJe4K6BVbwhD1M4k= +github.com/go-openapi/swag/mangling v0.28.0 h1:pH8eyeNO9SLYsTMWJrurnNfKmDa28XrlA+HePVD53VM= +github.com/go-openapi/swag/mangling v0.28.0/go.mod h1:jtBE2+V+3pILxOR7Vgce+Cwp6A2PgZbvVqfNntbVs0w= +github.com/go-openapi/swag/pools v0.28.0 h1:HPMZWSAfce3rdVTFcjFiCIBtDg9h4x2QlRrHipwhxeU= +github.com/go-openapi/swag/pools v0.28.0/go.mod h1:kVQefhSK5RWuRe7BXsL8htgBPAMpN7HDGpGEknqugeE= +github.com/go-openapi/swag/stringutils v0.28.0 h1:ixsc9iYgDPubHL/8nSkbnryEHpD2VRlBMLKpQyPXcDU= +github.com/go-openapi/swag/stringutils v0.28.0/go.mod h1:lzRN95CxXmA03XcDWHLOb6nOMcxCqR5rGY0lOgsfRoM= +github.com/go-openapi/swag/typeutils v0.28.0 h1:nRBKSBXjDgf01VDPB3fWeD9nQuhCOVeIYAkUx2tbkyY= +github.com/go-openapi/swag/typeutils v0.28.0/go.mod h1:Srm0xFNRZ1Y+vCxJclo5qzx8aj+1pAKda/YfFPrG0dQ= +github.com/go-openapi/swag/yamlutils v0.28.0 h1:TV3JXH6DS46KUroDtMLAYHGkdWf5VDq3wVWFirmzROY= +github.com/go-openapi/swag/yamlutils v0.28.0/go.mod h1:x0q/yndZHEgk9Rx3DyDqzFUmHy55KTvIZldvF2dTJXs= +github.com/go-openapi/testify/enable/yaml/v2 v2.6.0 h1:gGHwAJ0R/5jU8BEGDbfRNR3hL68dAVi84WuOApp29B0= +github.com/go-openapi/testify/enable/yaml/v2 v2.6.0/go.mod h1:tY+St1SGq4NFl0QIqdTY4aEdbChAHxhyB77XQi9iJCo= +github.com/go-openapi/testify/v2 v2.6.0 h1:5PKH2HE7YJ/LuRPQGvSxBRlFXNQhSetBLlGAgUEu3ug= +github.com/go-openapi/testify/v2 v2.6.0/go.mod h1:SgsVHtfooshd0tublTtJ50FPKhujf47YRqauXXOUxfw= +github.com/go-openapi/validate v0.26.3 h1:OkfZgLvLDnGP2hrRGD+42WBiPWWkoHomTJ+IVI+KaDc= +github.com/go-openapi/validate v0.26.3/go.mod h1:7DOOa4raU6NRe7A8VQSKbm3VcuUIioREYHFt+er9Sk8= +github.com/go-viper/mapstructure/v2 v2.5.0 h1:vM5IJoUAy3d7zRSVtIwQgBj7BiWtMPfmPEgAXnvj1Ro= +github.com/go-viper/mapstructure/v2 v2.5.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= +github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= +github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= +github.com/mattn/go-isatty v0.0.24 h1:tGZZoVgT/KiqK1c8ocVLeDS8BSWMRd47J3Lbz7vsReI= +github.com/mattn/go-isatty v0.0.24/go.mod h1:nMCL3Zebbrt45jsMDgnfIwz6ydEQApk5oEI3HqDio6A= +github.com/oklog/ulid/v2 v2.1.2 h1:IEclFb9JNvzYA6MW2SCxbLzcHTVsfqm3PrqGQJH5zec= +github.com/oklog/ulid/v2 v2.1.2/go.mod h1:rcEKHmBBKfef9DhnvX7y1HZBYxjXb0cP5ExxNsTT1QQ= +github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30 h1:BHT1/DKsYDGkUgQ2jmMaozVcdk+sVfz0+1ZJq4zkWgw= +github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= +github.com/yuin/goldmark v1.4.13 h1:fVcFKWvrslecOb/tg+Cc05dkeYx540o0FuFt3nUVDoE= +github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= +go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= +go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= +golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= +golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= +golang.org/x/mod v0.38.0 h1:MECBjubtXD7yj4HrhIUcywNaGeNVUdfVnxmPajOk4yk= +golang.org/x/mod v0.38.0/go.mod h1:V6Xz0pq8TQ3dGqVQ1FVHuelZpAL0uNhSkk9ogYP3c40= +golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= +golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU= +golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= +golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/telemetry v0.0.0-20260708182218-49f421fb7959 h1:RJhm5l6Fo4rmEIcndxDllNhhf/fAx8qIm4t6A7vpm2A= +golang.org/x/telemetry v0.0.0-20260708182218-49f421fb7959/go.mod h1:LV7u5Oco+Z/g6XI7PqN+EUUUGGkEcmB1uj2ceI0fOVg= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= +golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= +golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= +golang.org/x/tools v0.48.0 h1:3+hClM1aLL5mjMKm5ovokw9epgRXPuu2tILgismM6RE= +golang.org/x/tools v0.48.0/go.mod h1:08xX0orndb/F7jJxGDicx061tyd5pcMto75YMAXr6lk= diff --git a/cmd/genspec/input.go b/cmd/genspec/input.go new file mode 100644 index 00000000..db29f4b3 --- /dev/null +++ b/cmd/genspec/input.go @@ -0,0 +1,38 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "fmt" + "os" + + "github.com/go-openapi/loads" + "github.com/go-openapi/spec" +) + +// loadInput reads the specification the scan's discoveries are merged into. +// +// What the scan finds is written on top of this document rather than beside it, so it is the place +// for everything a scanner cannot know: the host, the security definitions, a hand-written path the +// annotations do not describe. +func loadInput(path string) (*spec.Swagger, error) { + if path == "" { + return nil, nil //nolint:nilnil // no input is not an input that is empty: the scan starts from nothing + } + + info, err := os.Stat(path) + if err != nil { + return nil, fmt.Errorf("cannot read -input: %w", err) + } + if info.IsDir() { + return nil, fmt.Errorf("%w: -input %q is a directory, not a specification", errUsage, path) + } + + document, err := loads.Spec(path) + if err != nil { + return nil, fmt.Errorf("cannot load -input %q: %w", path, err) + } + + return document.Spec(), nil +} diff --git a/cmd/genspec/main.go b/cmd/genspec/main.go new file mode 100644 index 00000000..27716878 --- /dev/null +++ b/cmd/genspec/main.go @@ -0,0 +1,145 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "errors" + "flag" + "fmt" + "io" + "os" + "path/filepath" + "runtime/debug" + + "github.com/go-openapi/codescan" + "github.com/go-openapi/codescan/internal/cliopts" +) + +func main() { + err := run(os.Args[1:], os.Stdout, os.Stderr) + if err != nil && !errors.Is(err, flag.ErrHelp) { + fmt.Fprintln(os.Stderr, "genspec:", err) + } + + if status := exitStatus(err); status != exitOK { + os.Exit(status) + } +} + +// run is the whole command, with the process left outside. +// +// Everything it touches arrives as an argument, so a test drives the real entry point rather than a +// rehearsal of it - which is the only way the flag surface, the exit statuses and what lands on +// which stream are checked at all. +func run(argv []string, stdout, stderr io.Writer) error { + fs := flag.NewFlagSet("genspec", flag.ContinueOnError) + fs.SetOutput(stderr) + fs.Usage = usage(fs, stderr) + + cfg := registerFlags(fs) + if err := fs.Parse(argv); err != nil { + if errors.Is(err, flag.ErrHelp) { + return err + } + + return fmt.Errorf("%w: %w", errUsage, err) + } + + if *cfg.version { + fmt.Fprintln(stdout, version()) + + return nil + } + + format, err := resolveFormat(*cfg.format, *cfg.output) + if err != nil { + return err + } + + report, err := newReporter(cfg, stderr) + if err != nil { + return err + } + + opts, err := cfg.options(fs.Args(), report) + if err != nil { + return err + } + + doc, err := codescan.Run(opts) + if err != nil { + return err + } + + rendered, asJSON, err := render(doc, format, *cfg.compact) + if err != nil { + return err + } + + if err := write(rendered, *cfg.output, stdout); err != nil { + return err + } + + // Validation comes after the document is written, so that an invalid one is still there to be + // read. Being told what is wrong with a specification you cannot look at is not much of a report. + var invalid error + if *cfg.validate { + invalid = checkValid(asJSON, report) + } + + if tripped := report.summarize(); invalid == nil { + return tripped + } + + return invalid +} + +// options assembles what the scan runs with. +func (c *config) options(patterns []string, report *reporter) (*codescan.Options, error) { + opts := &codescan.Options{Packages: cliopts.Patterns(patterns)} + if err := c.scan.Apply(opts); err != nil { + return nil, err + } + + input, err := loadInput(*c.input) + if err != nil { + return nil, err + } + opts.InputSpec = input + + // Positions are reported relative to where the scan ran, which has to be settled before anything + // is reported - and resolved, since -workdir is free to be relative and a diagnostic is not. + root, err := filepath.Abs(opts.WorkDir) + if err != nil { + return nil, fmt.Errorf("cannot resolve -workdir %q: %w", opts.WorkDir, err) + } + report.root = root + + opts.OnDiagnostic = report.onDiagnostic + + return opts, nil +} + +// version reports what this build is, as the module system recorded it. +// +// A binary installed with `go install ...@latest` carries its version; one built from a working copy +// carries the revision instead, and says so rather than claiming a release it is not. +func version() string { + info, ok := debug.ReadBuildInfo() + if !ok { + return "genspec (unknown version)" + } + + if v := info.Main.Version; v != "" && v != "(devel)" { + return "genspec " + v + } + + for _, setting := range info.Settings { + if setting.Key == "vcs.revision" { + return "genspec (devel, " + setting.Value + ")" + } + } + + return "genspec (devel)" +} diff --git a/cmd/genspec/main_test.go b/cmd/genspec/main_test.go new file mode 100644 index 00000000..470b51d0 --- /dev/null +++ b/cmd/genspec/main_test.go @@ -0,0 +1,261 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "encoding/json" + "errors" + "flag" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +// errUnrecognised stands for an error this command knows nothing about, which is the case the exit +// status has to have an answer for. +var errUnrecognised = errors.New("something else") + +// petstore is the corpus's complete example: a meta block, paths, parameters, responses and models. +// A document produced from it exercises every part of this command that is not about flags. +func petstore(t *testing.T) string { + t.Helper() + + return filepath.Join(fixtures(t), "goparsing", "petstore") +} + +// unannotated is a fixture with no swagger:meta block, so the document it produces has no info +// section - which is what makes it invalid on its own, and what makes an -input overlay visible. +func unannotated(t *testing.T) string { + t.Helper() + + return filepath.Join(fixtures(t), "enhancements", "additional-properties") +} + +// fixtures locates the repository's corpus from this file, rather than from the working directory, +// which `go test` is free to set to wherever the package lives. +func fixtures(t *testing.T) string { + t.Helper() + + _, thisFile, _, ok := runtime.Caller(0) + require.True(t, ok, "cannot resolve this test's own path") + + return filepath.Clean(filepath.Join(filepath.Dir(thisFile), "..", "..", "fixtures")) +} + +// exec runs the command as the process would, and reports what it wrote and what it returned. +func exec(t *testing.T, argv ...string) (stdout, stderr string, err error) { + t.Helper() + + var out, errs bytes.Buffer + err = run(argv, &out, &errs) + + return out.String(), errs.String(), err +} + +// scanned is exec on a fixture, decoded, for the tests that care about the document rather than +// about the bytes. +func scanned(t *testing.T, dir string, argv ...string) map[string]any { + t.Helper() + + // Flags first, patterns last: everything after the first positional argument is a pattern, which + // is the flag package's rule and not one this command is free to soften. + argv = append([]string{"-workdir", dir, "-quiet"}, argv...) + + stdout, _, err := exec(t, append(argv, "./...")...) + require.NoError(t, err) + + var document map[string]any + require.NoError(t, json.Unmarshal([]byte(stdout), &document)) + + return document +} + +func TestRunWritesTheDocumentToStandardOutput(t *testing.T) { + t.Parallel() + + document := scanned(t, petstore(t)) + + assert.Equal(t, "2.0", document["swagger"]) + assert.NotEmpty(t, document["paths"], "the petstore describes paths") + assert.NotEmpty(t, document["definitions"], "and the models they carry") +} + +func TestRunWritesToAFile(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "swagger.json") + + stdout, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-output", path, "./...") + require.NoError(t, err) + assert.Empty(t, stdout, "the document went to the file, so nothing goes to standard output") + + written, err := os.ReadFile(path) + require.NoError(t, err) + + var document map[string]any + require.NoError(t, json.Unmarshal(written, &document)) + assert.Equal(t, "2.0", document["swagger"]) +} + +// TestRunInfersYAMLFromTheOutputName is the half of -format=auto that a caller never states: naming +// a file spec.yaml and getting JSON in it would be a surprise, and the extension is the only place +// the intention was written down. +func TestRunInfersYAMLFromTheOutputName(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "swagger.yaml") + + _, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-output", path, "./...") + require.NoError(t, err) + + written, err := os.ReadFile(path) + require.NoError(t, err) + + assert.True(t, strings.HasPrefix(string(written), "basePath:"), + "a YAML document, not a JSON one: %.40s", written) +} + +func TestRunCompactDropsTheIndentation(t *testing.T) { + t.Parallel() + + stdout, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-compact", "./...") + require.NoError(t, err) + + assert.NotContains(t, stdout, "\n ", "compact means one line, not a differently indented one") + assert.True(t, json.Valid([]byte(stdout))) +} + +// TestRunOverlaysTheInputSpecification covers what -input is for: everything a scanner cannot know. +// The fixture has no meta block, so an info section in the document can only have come from the +// input, and definitions can only have come from the scan. +func TestRunOverlaysTheInputSpecification(t *testing.T) { + t.Parallel() + + base := filepath.Join(t.TempDir(), "base.json") + require.NoError(t, os.WriteFile(base, []byte( + `{"swagger":"2.0","host":"example.com","info":{"title":"Written by hand","version":"1.0"}}`, + ), 0o600)) + + document := scanned(t, unannotated(t), "-input", base) + + assert.Equal(t, "example.com", document["host"], "the input's own content survives the merge") + assert.NotEmpty(t, document["definitions"], "and the scan's discoveries are written on top of it") +} + +func TestRunRefusesADirectoryAsInput(t *testing.T) { + t.Parallel() + + _, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-input", t.TempDir(), "./...") + + require.ErrorIs(t, err, errUsage) + assert.Equal(t, exitUsage, exitStatus(err)) +} + +// TestRunValidateFailsOnAnInvalidDocument is the contract of -validate: the document is still +// written, and the status says what is wrong with it. +func TestRunValidateFailsOnAnInvalidDocument(t *testing.T) { + t.Parallel() + + stdout, stderr, err := exec(t, "-workdir", unannotated(t), "-validate", "-color=never", "./...") + + require.ErrorIs(t, err, errInvalidSpec) + assert.Equal(t, exitInvalidSpec, exitStatus(err)) + assert.True(t, json.Valid([]byte(stdout)), "the document is written even when it does not validate") + assert.Contains(t, stderr, "info in body is required", "and what is wrong with it is reported") +} + +func TestRunFailsOnTheThresholdItWasGiven(t *testing.T) { + t.Parallel() + + _, _, err := exec(t, "-workdir", unannotated(t), "-fail-on=warning", "-color=never", "./...") + + require.ErrorIs(t, err, errDiagnostics) + assert.Equal(t, exitDiagnostics, exitStatus(err)) +} + +// TestRunIsSilentByDefaultOnACleanScan states what a command in a pipeline can count on: nothing on +// standard error unless there is something to say. +func TestRunIsSilentByDefaultOnACleanScan(t *testing.T) { + t.Parallel() + + _, stderr, err := exec(t, "-workdir", petstore(t), "-color=never", "./...") + + require.NoError(t, err) + assert.Empty(t, stderr) +} + +func TestRunQuietSaysNothing(t *testing.T) { + t.Parallel() + + stdout, stderr, err := exec(t, "-workdir", unannotated(t), "-quiet", "-verbose", "./...") + + require.NoError(t, err) + assert.NotEmpty(t, stdout, "-quiet is about the diagnostics, not about the document") + assert.Empty(t, stderr) +} + +func TestRunReportsPositionsRelativeToTheWorkingDirectory(t *testing.T) { + t.Parallel() + + _, stderr, err := exec(t, "-workdir", unannotated(t), "-color=never", "./...") + + require.NoError(t, err) + require.NotEmpty(t, stderr, "this fixture has something to say about itself") + assert.Contains(t, stderr, "file=api.go", "a path the caller could open, not an absolute one") +} + +func TestRunPrintsItsVersion(t *testing.T) { + t.Parallel() + + stdout, _, err := exec(t, "-version") + + require.NoError(t, err) + assert.Contains(t, stdout, "genspec") +} + +// TestRunHelpIsNotAFailure keeps -h at status 0. flag reports it as an error, and treating it as one +// makes a command that cannot be asked what it does without failing. +func TestRunHelpIsNotAFailure(t *testing.T) { + t.Parallel() + + _, stderr, err := exec(t, "-h") + + require.ErrorIs(t, err, flag.ErrHelp) + assert.Contains(t, stderr, "usage: genspec") + assert.Contains(t, stderr, "packages are Go patterns") +} + +func TestRunRejectsAnUnknownFlag(t *testing.T) { + t.Parallel() + + _, _, err := exec(t, "-not-a-flag") + + require.ErrorIs(t, err, errUsage) + assert.Equal(t, exitUsage, exitStatus(err)) +} + +func TestRunRejectsAnUnreadableWorkdir(t *testing.T) { + t.Parallel() + + _, _, err := exec(t, "-workdir", filepath.Join(t.TempDir(), "nowhere"), "-quiet", "./...") + + require.Error(t, err) + assert.Equal(t, exitFailed, exitStatus(err)) +} + +func TestExitStatus(t *testing.T) { + t.Parallel() + + assert.Equal(t, exitOK, exitStatus(nil)) + assert.Equal(t, exitUsage, exitStatus(errUsage)) + assert.Equal(t, exitDiagnostics, exitStatus(errDiagnostics)) + assert.Equal(t, exitInvalidSpec, exitStatus(errInvalidSpec)) + assert.Equal(t, exitFailed, exitStatus(errUnrecognised)) +} diff --git a/cmd/genspec/output.go b/cmd/genspec/output.go new file mode 100644 index 00000000..febb02b6 --- /dev/null +++ b/cmd/genspec/output.go @@ -0,0 +1,77 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "encoding/json" + "fmt" + "io" + "os" + + "github.com/go-openapi/spec" + "go.yaml.in/yaml/v3" +) + +// documentMode is what a written specification is created with. +const documentMode os.FileMode = 0o644 + +// render marshals the document in the format asked for. +// +// The JSON rendering is produced either way: it is what YAML is derived from, and what -validate +// checks, so a caller reading the document and a caller validating it are looking at the same bytes. +func render(doc *spec.Swagger, format string, compact bool) (rendered, asJSON []byte, err error) { + asJSON, err = marshalJSON(doc, compact) + if err != nil { + return nil, nil, err + } + + if format != formatYAML { + return asJSON, asJSON, nil + } + + rendered, err = marshalYAML(asJSON) + if err != nil { + return nil, nil, err + } + + return rendered, asJSON, nil +} + +func marshalJSON(doc *spec.Swagger, compact bool) ([]byte, error) { + if compact { + return json.Marshal(doc) + } + + return json.MarshalIndent(doc, "", " ") +} + +// marshalYAML re-reads the JSON rendering and writes it back out as YAML. +// +// Through a plain any rather than the spec types, because that is the only thing that renders every +// vendor extension the same way the JSON does. The cost is key order: a Go map does not keep one, +// so a YAML document comes out alphabetical where the JSON came out in the order the spec types +// declare. The information is the same; the diff against a hand-written file is not. +func marshalYAML(asJSON []byte) ([]byte, error) { + var document any + if err := yaml.Unmarshal(asJSON, &document); err != nil { + return nil, fmt.Errorf("cannot re-read the document as YAML: %w", err) + } + + return yaml.Marshal(document) +} + +// write puts the rendered document where -output says. +func write(rendered []byte, output string, stdout io.Writer) error { + if output == "" || output == "-" { + _, err := fmt.Fprintf(stdout, "%s\n", rendered) + + return err + } + + if err := os.WriteFile(output, append(rendered, '\n'), documentMode); err != nil { + return fmt.Errorf("cannot write %s: %w", output, err) + } + + return nil +} diff --git a/cmd/genspec/validate.go b/cmd/genspec/validate.go new file mode 100644 index 00000000..621d06c5 --- /dev/null +++ b/cmd/genspec/validate.go @@ -0,0 +1,72 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "fmt" + "log/slog" + + "github.com/go-openapi/loads" + "github.com/go-openapi/strfmt" + "github.com/go-openapi/validate" + + "github.com/go-openapi/codescan" +) + +// rootPointer names what the empty JSON pointer addresses. +// +// An empty pointer is a location, not the absence of one: RFC 6901 spells the whole document that +// way, and a finding about something the document does not have at all - no info block, no paths - +// is reported there. Printed as prose because "" would render as nothing at all. +const rootPointer = "(the whole document)" + +// checkValid reports what is wrong with the document the scan produced. +// +// It runs on the rendered JSON rather than on the spec.Swagger it came from, so what is checked is +// exactly what was written - including whatever the round-trip through JSON did to it. +// +// Findings go through the same reporter as the scan's own diagnostics: to a reader they are the same +// kind of news about the same document, and giving them a second dialect would only mean two things +// to learn. +func checkValid(asJSON []byte, report *reporter) error { + document, err := loads.Analyzed(asJSON, "") + if err != nil { + return fmt.Errorf("cannot re-read the document to validate it: %w", err) + } + + // Everything is read off the FIRST result, which holds both lists. The second is a warnings-only + // view of the same run, and it carries them as its errors - so reading warnings from its Warnings + // field finds an empty slice, and a document with warnings reports as clean. + result, _ := validate.NewSpecValidator(document.Schema(), strfmt.Default).Validate(document) + + errored := result.LocatedErrors() + for _, located := range errored { + report.finding(codescan.SeverityError, located) + } + for _, located := range result.LocatedWarnings() { + report.finding(codescan.SeverityWarning, located) + } + + if len(errored) > 0 { + return fmt.Errorf("%w: %d finding(s)", errInvalidSpec, len(errored)) + } + + return nil +} + +// finding reports one validation result, located by the pointer the validator recorded. +// +// The pointer is carried straight through rather than recovered from the message: the validator +// records where it was as it walks, so it is the authority on it, and a sentence is a poor place to +// keep a machine-readable path. +func (r *reporter) finding(severity codescan.Severity, located validate.Located) { + r.record(severity) + + pointer := located.Pointer + if pointer == "" { + pointer = rootPointer + } + + r.log(severity, located.Err.Error(), slog.String("at", pointer)) +} diff --git a/go.work b/go.work index a555236e..958da577 100644 --- a/go.work +++ b/go.work @@ -14,6 +14,7 @@ go 1.25.0 // the oldstable CI job out. use ( . + ./cmd/genspec ./cmd/genspec-tui ./docs/examples ./fixtures From 6231aadcb21996b3cc4a03d06bab05ddd7ab953a Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Fri, 14 Aug 2026 15:26:36 +0200 Subject: [PATCH 3/8] doc: added relationship to go-swagger Signed-off-by: Frederic BIDON --- cmd/genspec/README.md | 14 ++++++++------ internal/cliopts/cliopts_test.go | 8 +++++--- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/cmd/genspec/README.md b/cmd/genspec/README.md index f9364cf5..8401af2a 100644 --- a/cmd/genspec/README.md +++ b/cmd/genspec/README.md @@ -21,10 +21,13 @@ This is the ordinary native command. Everything the library can be told is a flag, the document goes to standard output or to `-output`, and what the scan observed goes to standard error as colored diagnostics. +> **NOTE** `genspec` is a standalone CLI that works like `go-swagger` "swagger generate spec" command, +> but is released independently and will benefit fixes and enhancements at a faster pace than `go-swagger` releases. +> `go-swagger` has a larger scope and more dependencies. + ## Which one to reach for -Three commands run the same scan. The question is about the machine, not about -the specification: +Three commands run the same scan. The question is about the machine, not about the specification: | Command | Use it when | |---------|-------------| @@ -105,7 +108,6 @@ specific answer. - **what to emit** — `-scan-models`, `-prune-unused-models`, the alias and `allOf` knobs, `-skip-extensions`, the naming knobs, the doc-comment knobs -They are the library's own options under their own names: a flag is the -kebab-case of the field it writes, without exception. See the [package -documentation](https://pkg.go.dev/github.com/go-openapi/codescan#Options) for -what each one means. +They are the library's own options under their own names: a flag is the kebab-case of the field it writes, +without exception. +See the [package documentation](https://pkg.go.dev/github.com/go-openapi/codescan#Options) for what each one means. diff --git a/internal/cliopts/cliopts_test.go b/internal/cliopts/cliopts_test.go index e4e240f8..997a9156 100644 --- a/internal/cliopts/cliopts_test.go +++ b/internal/cliopts/cliopts_test.go @@ -15,10 +15,10 @@ import ( // notOnTheCommandLine are the value-typed options deliberately without a flag, with the reason. // -// Everything else must be reachable. An entry here is a decision, and reading it should be enough to +// Everything else must be reachable through flags. // judge whether it still holds. var notOnTheCommandLine = map[string]string{ //nolint:gochecknoglobals // table for the drift guard - "Packages": "positional arguments, so that `genspec ./api/...` reads like every other Go command", + "Packages": `positional arguments, so that "genspec ./api/..." reads well`, "DescWithRef": "deprecated in favour of EmitRefSiblings", "Debug": "deprecated no-op; the stderr logger was retired", } @@ -26,7 +26,9 @@ var notOnTheCommandLine = map[string]string{ //nolint:gochecknoglobals // table // coveredByLoader is the option the -loader flag discharges. // // It is a boolean the tables cannot carry, because the choice has three answers and the useful -// default is the third one. TestLoaderWritesTheToolchainFreeOption is what proves this excuse. +// default is the third one. +// +// NOTE: TestLoaderWritesTheToolchainFreeOption is what proves this excuse. const coveredByLoader = "ToolchainFreeLoader" // TestFlagsCoverEveryValueTypedOption is what stops a knob being unreachable from the command line. From b2095ac742e87cc6aa0c9c4cfb78690efb6956f4 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Fri, 14 Aug 2026 15:28:42 +0200 Subject: [PATCH 4/8] doc(genspec): carry the go-swagger relationship to its other readers The note lives in cmd/genspec/README.md; a reader arriving through pkg.go.dev, `go doc`, or the repository's front page meets none of it. Says the same thing in the package documentation and in the section of the top-level README that introduces the command. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- README.md | 4 ++++ cmd/genspec/doc.go | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/README.md b/README.md index 230f0fb5..446e6fd8 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,10 @@ Every option the library takes is a flag. Beyond those, it writes YAML as readil (`-output swagger.yaml` is enough), merges its discoveries into an existing document with `-input`, and checks what it produced with `-validate`. Its exit status says which of those went wrong. +It does the same job as go-swagger's `swagger generate spec`, which drives this same library, but is +released on its own — so fixes and enhancements reach it at this project's pace, and it carries only +the dependencies a spec generator needs. + See [cmd/genspec/README.md](cmd/genspec/README.md). ### Where there is no Go toolchain diff --git a/cmd/genspec/doc.go b/cmd/genspec/doc.go index d145fab6..bdc40d6f 100644 --- a/cmd/genspec/doc.go +++ b/cmd/genspec/doc.go @@ -23,6 +23,12 @@ // cross-references // genspec-tui the same scan, live, with the source and the document side by side // +// # Relationship to go-swagger +// +// genspec does the same job as go-swagger's `swagger generate spec`, which drives the same library. +// It is released on its own, so fixes and enhancements reach it at codescan's pace rather than +// go-swagger's; go-swagger has a wider scope, and the dependencies that come with it. +// // # Exit status // // A specification is written whenever one could be produced, so a non-zero status describes what was From fb4722ef294a8d7aa62e3ca97d485a76284c6648 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Fri, 14 Aug 2026 16:50:11 +0200 Subject: [PATCH 5/8] feat(cliconf): add the configuration file every command can read Presets a command line from a .codescan.yaml, found by searching upwards from where the command runs. Keys are grouped into sections, and inside a section a key is the flag it sets, spelled as on the command line, so the help text doubles as the file's reference: scan: workdir: ./api emit: scan-models: true Anything typed wins, decided by asking the flag set what it actually saw - so a flag typed with the value it already had wins too. Everything the file decides lands through flag.Set, the same path an argument takes, so a value is parsed and validated once and the file can mean nothing an argument could not. The values arrive as a plain map rather than being read here, which is what lets one contract serve commands with very different dependency budgets: cliconf.Parse needs only the yaml package the library already has, and cliconf.YAML satisfies koanf's parser interface structurally, so a command wanting koanf plugs it in and this package still imports nothing new. One file serves the whole family. A section a command does not know is skipped and reported rather than refused, so another command's settings may sit beside it; a key inside a section it does know must name one of its flags, which is what makes a typo an error rather than a setting that quietly never applied. Each flag in the shared table now declares the section it is addressed in, guarded the same way its name is: a flag reachable from no configuration key is a flag nobody can configure, and nothing else would have noticed. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- .claude/CLAUDE.md | 11 ++ internal/cliconf/cliconf.go | 187 +++++++++++++++++++ internal/cliconf/cliconf_test.go | 300 ++++++++++++++++++++++++++++++ internal/cliconf/discover.go | 86 +++++++++ internal/cliconf/discover_test.go | 141 ++++++++++++++ internal/cliconf/doc.go | 38 ++++ internal/cliconf/errors.go | 19 ++ internal/cliconf/parse.go | 101 ++++++++++ internal/cliconf/parse_test.go | 112 +++++++++++ internal/cliopts/cliopts.go | 95 +++++----- internal/cliopts/cliopts_test.go | 28 +++ internal/cliopts/sections.go | 47 +++++ 12 files changed, 1121 insertions(+), 44 deletions(-) create mode 100644 internal/cliconf/cliconf.go create mode 100644 internal/cliconf/cliconf_test.go create mode 100644 internal/cliconf/discover.go create mode 100644 internal/cliconf/discover_test.go create mode 100644 internal/cliconf/doc.go create mode 100644 internal/cliconf/errors.go create mode 100644 internal/cliconf/parse.go create mode 100644 internal/cliconf/parse_test.go create mode 100644 internal/cliopts/sections.go diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index e10ac526..bbe506b7 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -52,6 +52,17 @@ without exception. `-loader` (`go|own|auto`) is the tri-state that discharges `T Options that are not values — `FS`, `ExportData`, `InputSpec`, the callbacks — are the command's business, not the table's. +### `internal/cliconf/` — the configuration-file contract + +Where a `.codescan.yaml` is found (searching upwards), what may be in it, and how it loses to +anything typed. Values arrive as a plain flat map, so the source is the command's business: `cmd/genspec` +feeds it through koanf; a command that must stay dependency-free calls `cliconf.Parse` (YAML, via the +`go.yaml.in/yaml/v3` the root module already has). `cliconf.YAML` satisfies koanf's parser interface +*structurally*, so this package owes koanf no import — which is what keeps it usable from +`cmd/genspec-wasi` under wasm. Keys are `
.`: sections come from +`cliopts.ConfigSchema()` merged with the command's own, an unknown section is skipped (another +command's half of a shared file) and an unknown key inside a known section is an error. + ### `internal/packages/` — toolchain-free package loader (experimental) Owns **both** ways of resolving a package graph, behind one `Loader`: delegate to diff --git a/internal/cliconf/cliconf.go b/internal/cliconf/cliconf.go new file mode 100644 index 00000000..9133da1e --- /dev/null +++ b/internal/cliconf/cliconf.go @@ -0,0 +1,187 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliconf + +import ( + "flag" + "fmt" + "maps" + "slices" + "strconv" + "strings" +) + +// Schema says which section of a configuration file each flag is addressed in. +// +// It is what makes a key checkable: without it every key would have to be believed, and a +// misspelled one would read as a setting that quietly never applied. Commands build theirs by +// merging what the shared flag tables declare with what they add themselves. +type Schema map[string]string + +// Merge returns s with more added, leaving both alone. +// +// The command's own flags arrive this way, which is also where a collision would show up: a flag +// cannot be addressed in two sections, so the later one silently winning is not a thing to allow. +func (s Schema) Merge(more Schema) (Schema, error) { + merged := make(Schema, len(s)+len(more)) + maps.Copy(merged, s) + + for name, section := range more { + if existing, clash := merged[name]; clash { + return nil, fmt.Errorf("%w: flag %q is declared in sections %q and %q", + ErrBadConfig, name, existing, section) + } + merged[name] = section + } + + return merged, nil +} + +// Sections reports the sections the schema addresses, sorted. +func (s Schema) Sections() []string { + seen := make(map[string]bool, len(s)) + for _, section := range s { + seen[section] = true + } + + return slices.Sorted(maps.Keys(seen)) +} + +// Result reports what a configuration file did. +type Result struct { + // Set names the flags the file decided, in order. What is not here was either typed on the + // command line or left at its default. + Set []string + + // Ignored names the keys in sections this command does not know - another command's half of a + // shared file, or a section that was misspelled. Reported rather than dropped so that the second + // case is findable at all; a caller that mentions them under a -verbose gives that a way out. + Ignored []string +} + +// Apply writes a configuration file's values onto a flag set. +// +// Flags the caller typed are left alone. Everything else goes through [flag.FlagSet.Set], so the +// file is parsed by the same code as the command line and cannot mean anything an argument could +// not. +func Apply(fs *flag.FlagSet, values map[string]any, schema Schema) (Result, error) { + var result Result + + known := make(map[string]bool, len(schema)) + for _, section := range schema { + known[section] = true + } + + typed := passed(fs) + + // Sorted, so that what is reported and which key an error names do not depend on a map's whim. + for _, key := range slices.Sorted(maps.Keys(values)) { + section, name := Split(key) + + switch { + case section == "": + return result, fmt.Errorf("%w: %q is in no section; expected one of %s", + ErrUnknownKey, name, strings.Join(schema.Sections(), ", ")) + case !known[section]: + result.Ignored = append(result.Ignored, key) + + continue + } + + if err := check(name, section, schema, fs); err != nil { + return result, err + } + + if typed[name] { + // Typed on the command line, which outranks the file. + continue + } + + text, err := stringify(values[key]) + if err != nil { + return result, fmt.Errorf("%w: %s: %w", ErrBadValue, key, err) + } + if err := fs.Set(name, text); err != nil { + return result, fmt.Errorf("%w: %s: %w", ErrBadValue, key, err) + } + + result.Set = append(result.Set, name) + } + + return result, nil +} + +// check reports whether a key names a flag this command has, in the section it belongs to. +func check(name, section string, schema Schema, fs *flag.FlagSet) error { + want, declared := schema[name] + switch { + case !declared: + return fmt.Errorf("%w: %s%s%s", ErrUnknownKey, section, Delimiter, name) + case want != section: + return fmt.Errorf("%w: %s%s%s is addressed in section %q, not %q", + ErrUnknownKey, section, Delimiter, name, want, section) + } + + if fs.Lookup(name) == nil { + // The schema promised a flag the command never registered. A caller's mistake rather than the + // file's, so it says so in those terms. + return fmt.Errorf("%w: the schema declares %q, which is registered on no flag", ErrBadConfig, name) + } + + return nil +} + +// passed reports which flags appeared on the command line. +// +// Asked once, before anything is written: [flag.FlagSet.Set] records what it sets in the same place, +// so consulting it as we go would let the first value applied make the second look typed. +func passed(fs *flag.FlagSet) map[string]bool { + typed := make(map[string]bool) + fs.Visit(func(f *flag.Flag) { + typed[f.Name] = true + }) + + return typed +} + +// stringify renders a configuration value as the flag package would have received it. +// +// Everything is text in the end, because that is what a command line is. A list becomes the +// comma-separated form the list flags already take, so writing one as a YAML sequence and writing it +// as a string are the same thing. +func stringify(value any) (string, error) { + switch typed := value.(type) { + case nil: + // An explicitly empty value: `name-from-tags:` with nothing after it. That is a statement, and + // for the flags that tell an empty list from an absent one it is the interesting one. + return "", nil + case string: + return typed, nil + case bool: + return strconv.FormatBool(typed), nil + case int: + return strconv.Itoa(typed), nil + case int64: + return strconv.FormatInt(typed, 10), nil + case float64: + return strconv.FormatFloat(typed, 'g', -1, 64), nil + case []any: + return stringifyList(typed) + default: + return "", fmt.Errorf("%w: %T is not a value a flag can take", ErrBadValue, value) + } +} + +func stringifyList(values []any) (string, error) { + parts := make([]string, 0, len(values)) + for _, value := range values { + part, err := stringify(value) + if err != nil { + return "", err + } + parts = append(parts, part) + } + + return strings.Join(parts, ","), nil +} diff --git a/internal/cliconf/cliconf_test.go b/internal/cliconf/cliconf_test.go new file mode 100644 index 00000000..1cce791c --- /dev/null +++ b/internal/cliconf/cliconf_test.go @@ -0,0 +1,300 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliconf + +import ( + "flag" + "io" + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +// a flag set standing in for a command's: two sections, one of each kind of value. +type flags struct { + set *flag.FlagSet + workdir *string + models *bool + budget *float64 + tags *string + output *string +} + +func newFlags(t *testing.T, argv ...string) *flags { + t.Helper() + + fs := flag.NewFlagSet("test", flag.ContinueOnError) + fs.SetOutput(io.Discard) + + f := &flags{ + set: fs, + workdir: fs.String("workdir", ".", ""), + models: fs.Bool("scan-models", false, ""), + budget: fs.Float64("name-concat-budget", 0, ""), + tags: fs.String("exclude-tags", "", ""), + output: fs.String("output", "-", ""), + } + require.NoError(t, fs.Parse(argv)) + + return f +} + +func schema() Schema { + return Schema{ + "workdir": "scan", + "exclude-tags": "scan", + "scan-models": "emit", + "name-concat-budget": "emit", + "output": "write", + } +} + +func TestApplyWritesWhatTheFileSays(t *testing.T) { + t.Parallel() + + f := newFlags(t) + + result, err := Apply(f.set, map[string]any{ + "scan.workdir": "./api", + "scan.exclude-tags": []any{"internal", "debug"}, + "emit.scan-models": true, + "emit.name-concat-budget": 0.8, + }, schema()) + + require.NoError(t, err) + assert.Equal(t, "./api", *f.workdir) + assert.True(t, *f.models) + assert.InDelta(t, 0.8, *f.budget, 1e-9) + assert.Equal(t, "internal,debug", *f.tags, "a sequence is the comma-separated form a list flag takes") + assert.Equal(t, []string{"name-concat-budget", "scan-models", "exclude-tags", "workdir"}, result.Set, + "reported in the order the keys were read, which is the file's own sorted order") + assert.Empty(t, result.Ignored) +} + +// TestApplyLosesToTheCommandLine is the whole precedence rule, including the case that makes it +// subtle: a flag typed with the value it already had is still typed. +func TestApplyLosesToTheCommandLine(t *testing.T) { + t.Parallel() + + f := newFlags(t, "-workdir=/typed", "-scan-models=false") + + result, err := Apply(f.set, map[string]any{ + "scan.workdir": "./from-the-file", + "emit.scan-models": true, + }, schema()) + + require.NoError(t, err) + assert.Equal(t, "/typed", *f.workdir) + assert.False(t, *f.models, "-scan-models=false is a decision, not an absence") + assert.Empty(t, result.Set) +} + +// TestApplyDoesNotMistakeItsOwnWritesForTypedFlags is why what was typed is read once, up front: +// flag.Set records into the same place Visit reads, so asking as we go would make the first key +// applied look like a command-line argument to the second. +func TestApplyDoesNotMistakeItsOwnWritesForTypedFlags(t *testing.T) { + t.Parallel() + + f := newFlags(t) + + result, err := Apply(f.set, map[string]any{ + "scan.workdir": "./api", + "emit.scan-models": true, + "write.output": "spec.json", + }, schema()) + + require.NoError(t, err) + assert.Len(t, result.Set, 3, "every key applied, not just the first") + assert.Equal(t, "spec.json", *f.output) +} + +// TestApplySkipsSectionsThisCommandDoesNotKnow is what lets one file serve several commands. +func TestApplySkipsSectionsThisCommandDoesNotKnow(t *testing.T) { + t.Parallel() + + f := newFlags(t) + + result, err := Apply(f.set, map[string]any{ + "scan.workdir": "./api", + "tui.theme": "dark", + "tui.mouse": true, + }, schema()) + + require.NoError(t, err) + assert.Equal(t, "./api", *f.workdir) + assert.Equal(t, []string{"tui.mouse", "tui.theme"}, result.Ignored, + "reported rather than dropped, so a misspelled section is findable") +} + +func TestApplyRefusesAnUnknownKeyInAKnownSection(t *testing.T) { + t.Parallel() + + f := newFlags(t) + + _, err := Apply(f.set, map[string]any{"scan.workdirs": "./api"}, schema()) + + require.ErrorIs(t, err, ErrUnknownKey) + assert.Contains(t, err.Error(), "scan.workdirs") +} + +// TestApplySaysWhereAMisplacedKeyBelongs covers the mistake a sectioned file invites: the right +// key, in the wrong section. +func TestApplySaysWhereAMisplacedKeyBelongs(t *testing.T) { + t.Parallel() + + f := newFlags(t) + + _, err := Apply(f.set, map[string]any{"scan.scan-models": true}, schema()) + + require.ErrorIs(t, err, ErrUnknownKey) + assert.Contains(t, err.Error(), `is addressed in section "emit"`) +} + +func TestApplyRefusesAKeyOutsideAnySection(t *testing.T) { + t.Parallel() + + f := newFlags(t) + + _, err := Apply(f.set, map[string]any{"workdir": "./api"}, schema()) + + require.ErrorIs(t, err, ErrUnknownKey) + assert.Contains(t, err.Error(), "in no section") + assert.Contains(t, err.Error(), "emit, scan, write", "and says which sections there are") +} + +func TestApplyRefusesAValueTheFlagWillNotTake(t *testing.T) { + t.Parallel() + + f := newFlags(t) + + _, err := Apply(f.set, map[string]any{"emit.scan-models": "yes please"}, schema()) + + require.ErrorIs(t, err, ErrBadValue) + assert.Contains(t, err.Error(), "emit.scan-models") +} + +func TestApplyRefusesAValueOfNoUsableKind(t *testing.T) { + t.Parallel() + + f := newFlags(t) + + _, err := Apply(f.set, map[string]any{"scan.workdir": map[string]any{"nested": "deeper"}}, schema()) + + require.ErrorIs(t, err, ErrBadValue) +} + +// TestApplyTakesAnEmptyValueAsAStatement covers the flags that tell an empty list from an absent +// one: writing the key with nothing after it has to reach them as an explicit empty. +func TestApplyTakesAnEmptyValueAsAStatement(t *testing.T) { + t.Parallel() + + f := newFlags(t) + + result, err := Apply(f.set, map[string]any{"scan.exclude-tags": nil}, schema()) + + require.NoError(t, err) + assert.Empty(t, *f.tags) + assert.Equal(t, []string{"exclude-tags"}, result.Set, "the flag was set, not skipped") +} + +// TestApplyRefusesASchemaThatOutrunsTheFlagSet catches the caller's mistake rather than the file's. +func TestApplyRefusesASchemaThatOutrunsTheFlagSet(t *testing.T) { + t.Parallel() + + f := newFlags(t) + broken := schema() + broken["never-registered"] = "scan" + + _, err := Apply(f.set, map[string]any{"scan.never-registered": "x"}, broken) + + require.ErrorIs(t, err, ErrBadConfig) + assert.Contains(t, err.Error(), "registered on no flag") +} + +func TestApplyOnAnEmptyFile(t *testing.T) { + t.Parallel() + + f := newFlags(t) + + result, err := Apply(f.set, map[string]any{}, schema()) + + require.NoError(t, err) + assert.Empty(t, result.Set) + assert.Equal(t, ".", *f.workdir) +} + +func TestSchemaMerge(t *testing.T) { + t.Parallel() + + merged, err := schema().Merge(Schema{"color": "report"}) + require.NoError(t, err) + assert.Equal(t, "report", merged["color"]) + assert.Equal(t, "scan", merged["workdir"], "and the original survives") + assert.NotContains(t, schema(), "color", "which is left alone") +} + +func TestSchemaMergeRefusesAFlagInTwoSections(t *testing.T) { + t.Parallel() + + _, err := schema().Merge(Schema{"workdir": "write"}) + + require.ErrorIs(t, err, ErrBadConfig) + assert.Contains(t, err.Error(), "workdir") +} + +func TestSchemaSections(t *testing.T) { + t.Parallel() + + assert.Equal(t, []string{"emit", "scan", "write"}, schema().Sections()) +} + +func TestStringify(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + value any + want string + }{ + {"a string is itself", "yaml", "yaml"}, + {"a bool", true, "true"}, + {"an int", 7, "7"}, + {"a 64-bit int", int64(7), "7"}, + {"a float keeps no spurious decimals", 0.65, "0.65"}, + {"nothing is the empty string", nil, ""}, + {"a list is comma-separated", []any{"a", "b"}, "a,b"}, + {"a list of anything", []any{1, true}, "1,true"}, + {"an empty list", []any{}, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := stringify(tc.value) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestStringifyRefusesWhatAFlagCannotTake(t *testing.T) { + t.Parallel() + + _, err := stringify(struct{}{}) + require.Error(t, err) + + _, err = stringify([]any{struct{}{}}) + require.Error(t, err, "and inside a list too") +} + +func TestRegisterDeclaresTheFlag(t *testing.T) { + t.Parallel() + + fs := flag.NewFlagSet("test", flag.ContinueOnError) + fs.SetOutput(io.Discard) + config := Register(fs) + + require.NoError(t, fs.Parse([]string{"-config", "somewhere.yaml"})) + assert.Equal(t, "somewhere.yaml", *config) +} diff --git a/internal/cliconf/discover.go b/internal/cliconf/discover.go new file mode 100644 index 00000000..cad264e7 --- /dev/null +++ b/internal/cliconf/discover.go @@ -0,0 +1,86 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliconf + +import ( + "flag" + "fmt" + "os" + "path/filepath" +) + +// Flag is what the configuration file is named on the command line. +const Flag = "config" + +// Off is what -config is given to use no file at all, whatever is lying around. +// +// Spelled as a value rather than as a second flag, the way GOWORK spells it: a run that must be +// reproducible says so in one place, and there is no pair of flags that can disagree. +const Off = "off" + +// Names are the file names looked for, in order, when -config says nothing. +// +// One name for every command rather than one per command: the sections are what tell them apart, and +// a project configuring a scan has configured it for all of them. JSON is in the list because the +// parser reads it anyway, and a generated file is as likely to be JSON as YAML. +var Names = []string{ //nolint:gochecknoglobals // the search list, read once at startup + ".codescan.yaml", + ".codescan.yml", + ".codescan.json", +} + +const flagHelp = `configuration file to read before the flags ("off" for none; default: the nearest ` + + `.codescan.yaml, searching upwards)` + +// Register declares the -config flag and returns where its value lands. +func Register(fs *flag.FlagSet) *string { + return fs.String(Flag, "", flagHelp) +} + +// Discover reports which file to read, if any. +// +// A named file must exist: a caller who said -config meant that file, and silently scanning for +// another one - or for none - would answer a question they did not ask. Searching, on the other +// hand, is allowed to find nothing, which is the ordinary case. +// +// The search walks up from start, so running a command from anywhere inside a project finds the +// project's own file. It stops at the first hit rather than merging what it passes: a file that is +// half-overridden by one three directories up is not something anybody can read off the page. +func Discover(explicit, start string) (string, error) { + switch explicit { + case Off: + return "", nil + case "": + return search(start) + } + + if _, err := os.Stat(explicit); err != nil { + return "", fmt.Errorf("%w: -%s %q: %w", ErrBadConfig, Flag, explicit, err) + } + + return explicit, nil +} + +// search walks up from dir looking for a file named like a configuration. +func search(dir string) (string, error) { + current, err := filepath.Abs(dir) + if err != nil { + return "", fmt.Errorf("%w: cannot resolve %q: %w", ErrBadConfig, dir, err) + } + + for { + for _, name := range Names { + candidate := filepath.Join(current, name) + if info, err := os.Stat(candidate); err == nil && !info.IsDir() { + return candidate, nil + } + } + + parent := filepath.Dir(current) + if parent == current { + return "", nil + } + current = parent + } +} diff --git a/internal/cliconf/discover_test.go b/internal/cliconf/discover_test.go new file mode 100644 index 00000000..ac438fe8 --- /dev/null +++ b/internal/cliconf/discover_test.go @@ -0,0 +1,141 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliconf + +import ( + "os" + "path/filepath" + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +// write puts a configuration file at dir/name and reports its path. +func write(t *testing.T, dir, name string) string { + t.Helper() + + path := filepath.Join(dir, name) + require.NoError(t, os.WriteFile(path, []byte("scan:\n workdir: .\n"), 0o600)) + + return path +} + +func TestDiscoverFindsAFileWhereItStands(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + want := write(t, dir, Names[0]) + + got, err := Discover("", dir) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +// TestDiscoverWalksUp is what lets a command be run from anywhere inside a project. +func TestDiscoverWalksUp(t *testing.T) { + t.Parallel() + + root := t.TempDir() + want := write(t, root, Names[0]) + + deep := filepath.Join(root, "internal", "api", "handlers") + require.NoError(t, os.MkdirAll(deep, 0o750)) + + got, err := Discover("", deep) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +// TestDiscoverStopsAtTheNearest states that finding one is the end of it: a file further up does +// not half-override the one beside you. +func TestDiscoverStopsAtTheNearest(t *testing.T) { + t.Parallel() + + root := t.TempDir() + write(t, root, Names[0]) + + nearer := filepath.Join(root, "api") + require.NoError(t, os.MkdirAll(nearer, 0o750)) + want := write(t, nearer, Names[0]) + + got, err := Discover("", nearer) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +func TestDiscoverPrefersTheNamesInOrder(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + want := write(t, dir, Names[0]) + write(t, dir, Names[1]) + + got, err := Discover("", dir) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +// TestDiscoverFindingNothingIsNotAFailure: most projects have no configuration file, and that is +// not a thing to complain about. +func TestDiscoverFindingNothingIsNotAFailure(t *testing.T) { + t.Parallel() + + got, err := Discover("", t.TempDir()) + + require.NoError(t, err) + assert.Empty(t, got) +} + +func TestDiscoverIgnoresADirectoryNamedLikeAConfig(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(dir, Names[0]), 0o750)) + + got, err := Discover("", dir) + + require.NoError(t, err) + assert.Empty(t, got, "a directory is not a file, whatever it is called") +} + +func TestDiscoverTakesTheFileItIsGiven(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + want := write(t, dir, "elsewhere.yaml") + + got, err := Discover(want, t.TempDir()) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +// TestDiscoverRefusesAFileThatIsNotThere: a caller who named a file meant that file, and quietly +// searching for another one answers a question they did not ask. +func TestDiscoverRefusesAFileThatIsNotThere(t *testing.T) { + t.Parallel() + + _, err := Discover(filepath.Join(t.TempDir(), "absent.yaml"), t.TempDir()) + + require.ErrorIs(t, err, ErrBadConfig) + assert.Contains(t, err.Error(), "absent.yaml") +} + +// TestDiscoverOffFindsNothingEvenWhereThereIsSomething is the reproducible-run escape hatch. +func TestDiscoverOffFindsNothingEvenWhereThereIsSomething(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + write(t, dir, Names[0]) + + got, err := Discover(Off, dir) + + require.NoError(t, err) + assert.Empty(t, got) +} diff --git a/internal/cliconf/doc.go b/internal/cliconf/doc.go new file mode 100644 index 00000000..d97dbbcf --- /dev/null +++ b/internal/cliconf/doc.go @@ -0,0 +1,38 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +// Package cliconf supplements a command line with a configuration file. +// +// It owns the contract - where the file is, what may be in it, and how it loses to anything the +// caller typed - and nothing else. The values themselves arrive as a plain map, so a command is free +// to read them however it likes: through koanf, which brings environment variables and further +// formats with it, or through [Parse], which needs nothing this repository does not already have. +// That seam is why this package can be shared with a command that must cross-compile to +// WebAssembly with no dependency beyond the library. +// +// # What a file looks like +// +// Keys are grouped into sections, and within a section a key is the flag it sets, spelled exactly as +// on the command line: +// +// scan: +// workdir: ./api +// exclude-tags: [internal, debug] +// emit: +// scan-models: true +// name-from-tags: [form, json] +// +// Sections are how one file serves several commands: a section this command does not know is +// skipped rather than refused, and reported through [Result] so it can be mentioned rather than +// silently dropped. A key inside a section it does know must name one of its flags - that is what +// makes a typo an error rather than a setting that quietly never applied. +// +// # Precedence +// +// A flag the caller typed always wins, whatever the file says. That is decided by asking the flag +// set which flags were actually seen, so it holds for a flag set to its own default value too: +// -scan-models=false means false, even where the file says true. +// +// Everything else lands through [flag.FlagSet.Set], the same path the command line takes, so a value +// is parsed and validated exactly once and a file cannot express something an argument could not. +package cliconf diff --git a/internal/cliconf/errors.go b/internal/cliconf/errors.go new file mode 100644 index 00000000..c26dfa6a --- /dev/null +++ b/internal/cliconf/errors.go @@ -0,0 +1,19 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliconf + +import "errors" + +// What a configuration file can be wrong about. +// +// Sentinels rather than a formatted string at each site, so that a command can decide what a bad +// configuration file costs it - a usage status, usually - without matching on prose. +var ( + // ErrBadConfig is a file that cannot be read or is not a mapping of sections. + ErrBadConfig = errors.New("bad configuration file") + // ErrUnknownKey is a key naming no flag of the command reading it. + ErrUnknownKey = errors.New("unknown configuration key") + // ErrBadValue is a value the flag it addresses will not take. + ErrBadValue = errors.New("bad configuration value") +) diff --git a/internal/cliconf/parse.go b/internal/cliconf/parse.go new file mode 100644 index 00000000..d1bd89da --- /dev/null +++ b/internal/cliconf/parse.go @@ -0,0 +1,101 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliconf + +import ( + "fmt" + "strings" + + "go.yaml.in/yaml/v3" +) + +// Delimiter joins a section to the key inside it. +const Delimiter = "." + +// YAML reads a configuration file. +// +// The methods are shaped as koanf's parser interface, which it declares structurally, so a command +// using koanf can hand this straight to it - and this package still owes koanf no import. JSON needs +// no parser of its own: it is a subset of YAML, so a file written as JSON reads here unchanged. +type YAML struct{} + +// Unmarshal reads a configuration file into the nested map it describes. +func (YAML) Unmarshal(data []byte) (map[string]any, error) { + var nested map[string]any + if err := yaml.Unmarshal(data, &nested); err != nil { + return nil, fmt.Errorf("%w: %w", ErrBadConfig, err) + } + + if nested == nil { + // An empty file, or one holding nothing but comments. Not an error - it says nothing, which + // is a thing a configuration file is allowed to say. + return map[string]any{}, nil + } + + return nested, nil +} + +// Marshal writes what Unmarshal reads. Present to complete koanf's parser interface. +func (YAML) Marshal(values map[string]any) ([]byte, error) { + data, err := yaml.Marshal(values) + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrBadConfig, err) + } + + return data, nil +} + +// Parse reads a configuration file into the flat, section-qualified keys [Apply] takes. +// +// This is the whole of the dependency-free path: a command that does not want koanf calls this, and +// gets exactly what koanf's own All() would have handed it. +func Parse(data []byte) (map[string]any, error) { + nested, err := YAML{}.Unmarshal(data) + if err != nil { + return nil, err + } + + return Flatten(nested), nil +} + +// Flatten renders nested sections as section-qualified keys. +// +// Only maps are descended into. A list is a value - it is how a repeated flag is written - and +// flattening one would turn the entries of exclude-tags into keys named after their positions. +func Flatten(nested map[string]any) map[string]any { + flat := make(map[string]any, len(nested)) + flatten("", nested, flat) + + return flat +} + +func flatten(prefix string, nested, flat map[string]any) { + for key, value := range nested { + qualified := key + if prefix != "" { + qualified = prefix + Delimiter + key + } + + if section, ok := value.(map[string]any); ok { + flatten(qualified, section, flat) + + continue + } + + flat[qualified] = value + } +} + +// Split separates a key into the section it is in and the flag it names. +// +// A key with no section is reported as one with an empty section, which Apply refuses: sections are +// what let one file serve several commands, so a key outside them addresses nobody in particular. +func Split(key string) (section, name string) { + section, name, found := strings.Cut(key, Delimiter) + if !found { + return "", section + } + + return section, name +} diff --git a/internal/cliconf/parse_test.go b/internal/cliconf/parse_test.go new file mode 100644 index 00000000..3fbf6c14 --- /dev/null +++ b/internal/cliconf/parse_test.go @@ -0,0 +1,112 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliconf + +import ( + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +func TestParse(t *testing.T) { + t.Parallel() + + values, err := Parse([]byte(` +scan: + workdir: ./api + exclude-tags: [internal, debug] +emit: + scan-models: true + name-concat-budget: 0.8 +`)) + + require.NoError(t, err) + assert.Equal(t, map[string]any{ + "scan.workdir": "./api", + "scan.exclude-tags": []any{"internal", "debug"}, + "emit.scan-models": true, + "emit.name-concat-budget": 0.8, + }, values) +} + +// TestParseReadsJSON: JSON is a subset of YAML, so a file written that way needs no parser of its +// own - which is why .codescan.json is in the search list. +func TestParseReadsJSON(t *testing.T) { + t.Parallel() + + values, err := Parse([]byte(`{"scan": {"workdir": "./api"}, "emit": {"scan-models": true}}`)) + + require.NoError(t, err) + assert.Equal(t, map[string]any{"scan.workdir": "./api", "emit.scan-models": true}, values) +} + +func TestParseAnEmptyFileSaysNothing(t *testing.T) { + t.Parallel() + + for _, content := range []string{"", "\n", "# nothing but a comment\n"} { + values, err := Parse([]byte(content)) + + require.NoErrorf(t, err, "%q", content) + assert.Emptyf(t, values, "%q", content) + } +} + +func TestParseRefusesWhatIsNotAConfiguration(t *testing.T) { + t.Parallel() + + _, err := Parse([]byte("scan: [this is a list, not a section\n")) + + require.ErrorIs(t, err, ErrBadConfig) +} + +// TestFlattenLeavesListsAlone: a list is a value - it is how a repeated flag is written - and +// descending into one would turn the entries into keys named after their positions. +func TestFlattenLeavesListsAlone(t *testing.T) { + t.Parallel() + + flat := Flatten(map[string]any{ + "scan": map[string]any{"exclude-tags": []any{"a", "b"}}, + }) + + assert.Equal(t, map[string]any{"scan.exclude-tags": []any{"a", "b"}}, flat) +} + +func TestFlattenGoesAsDeepAsItIsGiven(t *testing.T) { + t.Parallel() + + flat := Flatten(map[string]any{"a": map[string]any{"b": map[string]any{"c": 1}}}) + + assert.Equal(t, map[string]any{"a.b.c": 1}, flat, + "a key deeper than a section is carried through, for Apply to refuse by name") +} + +func TestSplit(t *testing.T) { + t.Parallel() + + section, name := Split("scan.workdir") + assert.Equal(t, "scan", section) + assert.Equal(t, "workdir", name) + + section, name = Split("workdir") + assert.Empty(t, section, "a key in no section") + assert.Equal(t, "workdir", name) + + section, name = Split("a.b.c") + assert.Equal(t, "a", section) + assert.Equal(t, "b.c", name, "only the first segment is the section") +} + +// TestYAMLMarshalRoundTrips covers the half of koanf's parser interface this package does not use +// itself, but must supply. +func TestYAMLMarshalRoundTrips(t *testing.T) { + t.Parallel() + + data, err := YAML{}.Marshal(map[string]any{"scan": map[string]any{"workdir": "./api"}}) + require.NoError(t, err) + + values, err := Parse(data) + require.NoError(t, err) + assert.Equal(t, map[string]any{"scan.workdir": "./api"}, values) +} diff --git a/internal/cliopts/cliopts.go b/internal/cliopts/cliopts.go index b668e60a..ade5f2ed 100644 --- a/internal/cliopts/cliopts.go +++ b/internal/cliopts/cliopts.go @@ -23,10 +23,11 @@ type Options = scanner.Options // the compiler and leave the flag writing nowhere, and it is what lets the coverage guard ask which // field an entry actually touches rather than deriving a name from a name. type option[T any] struct { - name string - value T - help string - field func(*Options) *T + section string + name string + value T + help string + field func(*Options) *T } // boolOptions is every boolean knob, grouped as the help text lists them. @@ -36,87 +37,87 @@ type option[T any] struct { // package of annotated models should produce their definitions. var boolOptions = []option[bool]{ //nolint:gochecknoglobals // the flag table, read once at startup { - "scan-models", true, "also emit definitions for swagger:model types", + sectionEmit, "scan-models", true, "also emit definitions for swagger:model types", func(o *Options) *bool { return &o.ScanModels }, }, { - "prune-unused-models", false, "drop discovered models nothing references (needs -scan-models)", + sectionEmit, "prune-unused-models", false, "drop discovered models nothing references (needs -scan-models)", func(o *Options) *bool { return &o.PruneUnusedModels }, }, { - "exclude-deps", false, "scan only the packages matched by the patterns, never their dependencies", + sectionScan, "exclude-deps", false, "scan only the packages matched by the patterns, never their dependencies", func(o *Options) *bool { return &o.ExcludeDeps }, }, { - "ref-aliases", false, "aliases produce a $ref instead of being expanded", + sectionEmit, "ref-aliases", false, "aliases produce a $ref instead of being expanded", func(o *Options) *bool { return &o.RefAliases }, }, { - "transparent-aliases", false, "aliases dissolve entirely, never producing a definition", + sectionEmit, "transparent-aliases", false, "aliases dissolve entirely, never producing a definition", func(o *Options) *bool { return &o.TransparentAliases }, }, { - "emit-ref-siblings", false, "emit a $ref'd field's description and extensions beside the $ref", + sectionEmit, "emit-ref-siblings", false, "emit a $ref'd field's description and extensions beside the $ref", func(o *Options) *bool { return &o.EmitRefSiblings }, }, { - "skip-all-of-compounding", false, "never wrap a $ref in an allOf compound", + sectionEmit, "skip-all-of-compounding", false, "never wrap a $ref in an allOf compound", func(o *Options) *bool { return &o.SkipAllOfCompounding }, }, { - "default-all-of-for-embeds", false, "render a plain struct embed as allOf composition", + sectionEmit, "default-all-of-for-embeds", false, "render a plain struct embed as allOf composition", func(o *Options) *bool { return &o.DefaultAllOfForEmbeds }, }, { - "set-x-nullable-for-pointers", false, "mark pointer fields x-nullable", + sectionEmit, "set-x-nullable-for-pointers", false, "mark pointer fields x-nullable", func(o *Options) *bool { return &o.SetXNullableForPointers }, }, { - "skip-extensions", false, "omit the x-go-* vendor extensions", + sectionEmit, "skip-extensions", false, "omit the x-go-* vendor extensions", func(o *Options) *bool { return &o.SkipExtensions }, }, { - "emit-x-go-type", false, "stamp x-go-type on every emitted definition", + sectionEmit, "emit-x-go-type", false, "stamp x-go-type on every emitted definition", func(o *Options) *bool { return &o.EmitXGoType }, }, { - "single-line-comment-as-description", false, "a one-line doc comment is a description, never a title", + sectionEmit, "single-line-comment-as-description", false, "a one-line doc comment is a description, never a title", func(o *Options) *bool { return &o.SingleLineCommentAsDescription }, }, { - "after-decl-comments", false, "accept annotations inside or after a declaration", + sectionEmit, "after-decl-comments", false, "accept annotations inside or after a declaration", func(o *Options) *bool { return &o.AfterDeclComments }, }, { - "clean-go-doc", false, "rewrite godoc-only syntax carried into a title or description", + sectionEmit, "clean-go-doc", false, "rewrite godoc-only syntax carried into a title or description", func(o *Options) *bool { return &o.CleanGoDoc }, }, { - "skip-enum-descriptions", false, "keep the enum const-name mapping off the description", + sectionEmit, "skip-enum-descriptions", false, "keep the enum const-name mapping off the description", func(o *Options) *bool { return &o.SkipEnumDescriptions }, }, { - "skip-jsonify-interface-methods", false, "emit interface-method names verbatim", + sectionEmit, "skip-jsonify-interface-methods", false, "emit interface-method names verbatim", func(o *Options) *bool { return &o.SkipJSONifyInterfaceMethods }, }, { - "emit-hierarchical-names", false, "emit over-budget collision groups as nested definitions", + sectionEmit, "emit-hierarchical-names", false, "emit over-budget collision groups as nested definitions", func(o *Options) *bool { return &o.EmitHierarchicalNames }, }, { - "stub-stdlib", false, + sectionLoad, "stub-stdlib", false, "synthesize standard-library types instead of reading GOROOT: no Go installation and no module\n" + "cache needed, much smaller footprint, but not failsafe -- see the package documentation", func(o *Options) *bool { return &o.StubStdlib }, }, { - "skip-compiled-dependencies", false, + sectionLoad, "skip-compiled-dependencies", false, "read every dependency from source rather than taking its types from the compiler's export\n" + "data: the spec is the same either way, but this is much slower on a warm cache and much\n" + "faster on a cold one. Native builds only -- it needs -loader=go, which WebAssembly cannot run", @@ -130,34 +131,34 @@ var boolOptions = []option[bool]{ //nolint:gochecknoglobals // the flag table, r // these decides what gets built, and so what the specification says. var stringOptions = []option[string]{ //nolint:gochecknoglobals // the flag table, read once at startup { - "workdir", ".", "directory the scan runs from; patterns are relative to it", + sectionScan, "workdir", ".", "directory the scan runs from; patterns are relative to it", func(o *Options) *string { return &o.WorkDir }, }, { - "build-tags", "", "comma-separated go build tags to apply while loading", + sectionScan, "build-tags", "", "comma-separated go build tags to apply while loading", func(o *Options) *string { return &o.BuildTags }, }, { - "goos", "", "GOOS the scanned code is built for (default: this machine's)", + sectionGo, "goos", "", "GOOS the scanned code is built for (default: this machine's)", func(o *Options) *string { return &o.GOOS }, }, { - "goarch", "", "GOARCH the scanned code is built for (default: this machine's)", + sectionGo, "goarch", "", "GOARCH the scanned code is built for (default: this machine's)", func(o *Options) *string { return &o.GOARCH }, }, { - "goflags", "", + sectionGo, "goflags", "", `default go command flags, as GOFLAGS (e.g. "-tags=integration"); -build-tags wins`, func(o *Options) *string { return &o.GOFLAGS }, }, { - "gowork", "", + sectionGo, "gowork", "", `workspace selection, as GOWORK: "off" to disable, a path to a go.work, empty to search upwards`, func(o *Options) *string { return &o.GOWORK }, }, { - "goexperiment", "", + sectionGo, "goexperiment", "", `toolchain experiments to enable, as GOEXPERIMENT (e.g. "jsonv2")`, func(o *Options) *string { return &o.GOEXPERIMENT }, }, @@ -166,7 +167,7 @@ var stringOptions = []option[string]{ //nolint:gochecknoglobals // the flag tabl // floatOptions is every knob taking one number. var floatOptions = []option[float64]{ //nolint:gochecknoglobals // the flag table, read once at startup { - "name-concat-budget", 0, + sectionEmit, "name-concat-budget", 0, "readability cutoff for collision-renaming by concatenation (0 = codescan's default of 0.65)", func(o *Options) *float64 { return &o.NameConcatBudget }, }, @@ -178,6 +179,7 @@ var floatOptions = []option[float64]{ //nolint:gochecknoglobals // the flag tabl // collapse onto nil, which is what the scanner reads as "no filter" - so passing -include= is the // same as not passing it at all, and means what it looks like it means. type listOption struct { + section string name string help string field func(*Options) *[]string @@ -187,27 +189,32 @@ type listOption struct { // listOptions is every knob taking a list. var listOptions = []listOption{ //nolint:gochecknoglobals // the flag table, read once at startup { - name: "include", - help: "comma-separated patterns; only matching packages are scanned", - field: func(o *Options) *[]string { return &o.Include }, + name: "include", + section: sectionScan, + help: "comma-separated patterns; only matching packages are scanned", + field: func(o *Options) *[]string { return &o.Include }, }, { - name: "exclude", - help: "comma-separated patterns; matching packages are skipped", - field: func(o *Options) *[]string { return &o.Exclude }, + name: "exclude", + section: sectionScan, + help: "comma-separated patterns; matching packages are skipped", + field: func(o *Options) *[]string { return &o.Exclude }, }, { - name: "include-tags", - help: "comma-separated swagger tags; only matching operations are emitted", - field: func(o *Options) *[]string { return &o.IncludeTags }, + name: "include-tags", + section: sectionScan, + help: "comma-separated swagger tags; only matching operations are emitted", + field: func(o *Options) *[]string { return &o.IncludeTags }, }, { - name: "exclude-tags", - help: "comma-separated swagger tags; matching operations are skipped", - field: func(o *Options) *[]string { return &o.ExcludeTags }, + name: "exclude-tags", + section: sectionScan, + help: "comma-separated swagger tags; matching operations are skipped", + field: func(o *Options) *[]string { return &o.ExcludeTags }, }, { - name: "name-from-tags", + name: "name-from-tags", + section: sectionEmit, help: `ordered struct tags a field's name derives from (default "json"; pass empty to use the Go` + "\n" + "field name)", field: func(o *Options) *[]string { return &o.NameFromTags }, diff --git a/internal/cliopts/cliopts_test.go b/internal/cliopts/cliopts_test.go index 997a9156..27ef2b9c 100644 --- a/internal/cliopts/cliopts_test.go +++ b/internal/cliopts/cliopts_test.go @@ -89,6 +89,34 @@ func TestExcusedOptionsStillExist(t *testing.T) { } } +// TestEveryFlagIsAddressableInAConfigFile is the config-file half of the coverage guard. +// +// A flag with no section can be typed but not configured, and nothing else would notice: the file +// would simply have no way to name it, which reads from the outside exactly like the option not +// existing. +func TestEveryFlagIsAddressableInAConfigFile(t *testing.T) { + t.Parallel() + + fs := flag.NewFlagSet("test", flag.ContinueOnError) + fs.SetOutput(io.Discard) + Register(fs) + + schema := ConfigSchema() + sections := map[string]bool{sectionScan: true, sectionGo: true, sectionLoad: true, sectionEmit: true} + + fs.VisitAll(func(f *flag.Flag) { + section, declared := schema[f.Name] + assert.Truef(t, declared, + "flag -%s is addressed in no configuration section. Give its table entry a section.", f.Name) + assert.Truef(t, sections[section], + "flag -%s is addressed in section %q, which is not one of this package's", f.Name, section) + }) + + registered := 0 + fs.VisitAll(func(*flag.Flag) { registered++ }) + assert.Len(t, schema, registered, "the schema and the flag set describe different surfaces") +} + func TestDefaults(t *testing.T) { t.Parallel() diff --git a/internal/cliopts/sections.go b/internal/cliopts/sections.go new file mode 100644 index 00000000..51941ced --- /dev/null +++ b/internal/cliopts/sections.go @@ -0,0 +1,47 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package cliopts + +import "github.com/go-openapi/codescan/internal/cliconf" + +// The sections a configuration file addresses these flags in. +// +// They are the four questions a scan answers, in the order it answers them: which code, built how, +// read how, rendered how. The same grouping the command help and the READMEs use - a reader who has +// learnt one has learnt the others. +const ( + // sectionScan is which code is looked at. + sectionScan = "scan" + // sectionGo is what it is built as: the go environment that decides what compiles. + sectionGo = "go" + // sectionLoad is how the packages are read. + sectionLoad = "load" + // sectionEmit is what the specification ends up saying. + sectionEmit = "emit" +) + +// ConfigSchema reports the section each of these flags is addressed in. +// +// A command merges this with its own flags' sections to get the schema a configuration file is +// checked against. Built from the tables rather than written out again, so a flag cannot arrive with +// no way to configure it. +func ConfigSchema() cliconf.Schema { + schema := make(cliconf.Schema, len(boolOptions)+len(stringOptions)+len(floatOptions)+len(listOptions)+1) + + for _, opt := range boolOptions { + schema[opt.name] = opt.section + } + for _, opt := range stringOptions { + schema[opt.name] = opt.section + } + for _, opt := range floatOptions { + schema[opt.name] = opt.section + } + for _, opt := range listOptions { + schema[opt.name] = opt.section + } + schema[loaderFlag] = sectionLoad + + return schema +} From 6e4a9d396490f9e6867f8f01c7c2b8061cd8fa0d Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Fri, 14 Aug 2026 16:50:11 +0200 Subject: [PATCH 6/8] feat(genspec): read a configuration file before the flags Wires the command to the shared configuration contract, through koanf as its loader. genspec adds two sections of its own to the ones the library brings - document, for what is read and written, and diagnostics, for how loud it is - with the same guard behind them: a flag addressed in no section is one nobody can configure. The file is read after parsing and before anything reads a flag, so everything downstream sees one settled command line and needs to know nothing about where a value came from. Which file was read, and which of its keys belonged to another command, are reported under -verbose: reading a configuration file is not news, but a section nobody claimed is the only sign of a misspelt one. koanf loads it for what it will be asked for next - environment variables, a second format, several sources merged - rather than for reading one YAML file. It is given the bytes directly, since its file provider carries a filesystem watcher for a file this command reads once. Also drops the position from a diagnostic that has none: a route omitted by a tag rule was reported at "file= line=0 column=0", which is nowhere. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- cmd/genspec/README.md | 43 +++++++ cmd/genspec/configfile.go | 118 ++++++++++++++++++ cmd/genspec/configfile_test.go | 214 ++++++++++++++++++++++++++++++++ cmd/genspec/diagnostics.go | 42 ++++++- cmd/genspec/diagnostics_test.go | 17 +++ cmd/genspec/doc.go | 21 ++++ cmd/genspec/flags.go | 14 ++- cmd/genspec/go.mod | 5 + cmd/genspec/go.sum | 14 +++ cmd/genspec/main.go | 10 ++ cmd/genspec/main_test.go | 22 ++-- 11 files changed, 505 insertions(+), 15 deletions(-) create mode 100644 cmd/genspec/configfile.go create mode 100644 cmd/genspec/configfile_test.go diff --git a/cmd/genspec/README.md b/cmd/genspec/README.md index 8401af2a..4eff222f 100644 --- a/cmd/genspec/README.md +++ b/cmd/genspec/README.md @@ -39,6 +39,49 @@ They share their flag surface: `internal/cliopts` declares every knob the library takes, once, so `-name-from-tags` means the same thing whichever one you reach for. A guard there fails the build when an option lands with no flag. +## Configuration file + +Anything that can be a flag can be preset in a `.codescan.yaml`, found by +searching upwards from wherever you are — so a project configures itself once +and the command is run bare: + +```yaml +scan: + workdir: ./api + exclude-tags: [internal] + +emit: + scan-models: true + name-from-tags: [form, json] + +document: + format: yaml + output: swagger.yaml + +diagnostics: + validate: true + fail-on: warning +``` + +Keys are grouped into sections, and inside a section a key **is** the flag it +sets, spelled exactly as on the command line — so `genspec -h` doubles as the +reference for the file. The sections are the families below, plus `document` +and `diagnostics` for this command's own flags. + +**Anything typed wins.** That holds for a flag typed with the value it already +had: `-scan-models=false` means false even where the file says true. + +`-config ` reads a particular file, which must exist. `-config off` +ignores whatever is lying around, for a run that has to be reproducible. +`.codescan.yml` and `.codescan.json` are searched for too — JSON is a subset of +YAML, so it needs no parser of its own. + +One file serves the whole family: a section a command does not know is skipped +rather than refused, so `genspec-tui`'s settings can sit beside `genspec`'s. A +key inside a section it *does* know must name one of its flags — which is what +makes a typo an error rather than a setting that quietly never applied. Run +with `-verbose` to see which file was read and which keys it skipped. + ## Output `-output` names the file, or `-` for standard output (the default). `-format` diff --git a/cmd/genspec/configfile.go b/cmd/genspec/configfile.go new file mode 100644 index 00000000..07e9cff0 --- /dev/null +++ b/cmd/genspec/configfile.go @@ -0,0 +1,118 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "flag" + "fmt" + "os" + + "github.com/knadh/koanf/providers/rawbytes" + "github.com/knadh/koanf/v2" + + "github.com/go-openapi/codescan/internal/cliconf" + "github.com/go-openapi/codescan/internal/cliopts" +) + +// The sections this command's own flags are addressed in. +// +// The library's flags bring their own (scan, go, load, emit); these are what genspec adds on top: +// the document it reads and writes, and how loud it is about what it saw. +const ( + sectionDocument = "document" + sectionDiagnostics = "diagnostics" +) + +// commandSections says where each of this command's flags is addressed. +// +// A literal rather than something derived, because these flags are registered inline: there is no +// table to read a section off. TestEveryFlagIsAddressableInAConfigFile is what keeps it in step. +var commandSections = cliconf.Schema{ //nolint:gochecknoglobals // the schema, read once at startup + "input": sectionDocument, + "output": sectionDocument, + "format": sectionDocument, + "compact": sectionDocument, + + "color": sectionDiagnostics, + "quiet": sectionDiagnostics, + "verbose": sectionDiagnostics, + "validate": sectionDiagnostics, + "fail-on": sectionDiagnostics, +} + +// notConfigurable are the flags a file deliberately cannot set, with the reason. +var notConfigurable = map[string]string{ //nolint:gochecknoglobals // table for the drift guard + cliconf.Flag: "names the file itself, which cannot be read from inside it", + "version": "asks the command a question rather than configuring it", +} + +// configSchema is the whole of what a configuration file may address here. +func configSchema() (cliconf.Schema, error) { + schema, err := cliopts.ConfigSchema().Merge(commandSections) + if err != nil { + return nil, fmt.Errorf("%w: %w", errUsage, err) + } + + return schema, nil +} + +// configured reads a configuration file, if there is one, and presets the flags with it. +// +// Called after parsing and before anything reads a flag, so that everything downstream sees one +// settled command line and needs to know nothing about where a value came from. +func configured(fs *flag.FlagSet, named string) (cliconf.Result, string, error) { + var applied cliconf.Result + + // Searched from where the command was run, not from -workdir: the file is free to set -workdir, + // and a file found through the very value it sets would be reasoning in a circle. + here, err := os.Getwd() + if err != nil { + return applied, "", fmt.Errorf("cannot tell where this is running from: %w", err) + } + + path, err := cliconf.Discover(named, here) + if err != nil || path == "" { + return applied, "", err + } + + values, err := read(path) + if err != nil { + return applied, path, err + } + + schema, err := configSchema() + if err != nil { + return applied, path, err + } + + applied, err = cliconf.Apply(fs, values, schema) + if err != nil { + return applied, path, fmt.Errorf("%s: %w", path, err) + } + + return applied, path, nil +} + +// read loads a configuration file through koanf. +// +// koanf for what it will be asked for next - environment variables, a second format, several sources +// merged - rather than for reading one YAML file, which cliconf can do by itself. What it hands back +// is the same flat, section-qualified map [cliconf.Parse] produces, so the two are interchangeable +// and the commands that cannot take the dependency lose nothing. +// +// The bytes are read here rather than by koanf's file provider, which brings a filesystem watcher +// with it for a file this command reads once and never looks at again. +func read(path string) (map[string]any, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("%w: %w", cliconf.ErrBadConfig, err) + } + + loaded := koanf.New(cliconf.Delimiter) + if err := loaded.Load(rawbytes.Provider(data), cliconf.YAML{}); err != nil { + return nil, fmt.Errorf("%w: %s: %w", cliconf.ErrBadConfig, path, err) + } + + return loaded.All(), nil +} diff --git a/cmd/genspec/configfile_test.go b/cmd/genspec/configfile_test.go new file mode 100644 index 00000000..44f7fff3 --- /dev/null +++ b/cmd/genspec/configfile_test.go @@ -0,0 +1,214 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "encoding/json" + "flag" + "os" + "path/filepath" + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" + + "github.com/go-openapi/codescan/internal/cliconf" +) + +// configFile writes a configuration file in a fresh directory and reports its path. +func configFile(t *testing.T, content string) string { + t.Helper() + + path := filepath.Join(t.TempDir(), cliconf.Names[0]) + require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) + + return path +} + +// TestEveryFlagIsAddressableInAConfigFile is the config-file half of the coverage guard, for the +// flags this command adds to the shared ones. +// +// A flag with no section can be typed but not configured, and nothing would notice: from the outside +// that reads exactly like the option not existing. +func TestEveryFlagIsAddressableInAConfigFile(t *testing.T) { + t.Parallel() + + fs := flag.NewFlagSet("genspec", flag.ContinueOnError) + fs.SetOutput(nil) + registerFlags(fs) + + schema, err := configSchema() + require.NoError(t, err) + + fs.VisitAll(func(f *flag.Flag) { + if reason, excused := notConfigurable[f.Name]; excused { + assert.NotContainsf(t, schema, f.Name, + "-%s is excused from configuration (%s) but the schema addresses it anyway", f.Name, reason) + + return + } + + assert.Containsf(t, schema, f.Name, + "flag -%s is addressed in no configuration section. Add it to commandSections, or excuse "+ + "it in notConfigurable with a reason.", f.Name) + }) +} + +func TestConfigFileSetsFlags(t *testing.T) { + t.Parallel() + + path := configFile(t, ` +scan: + workdir: `+petstore(t)+` +emit: + scan-models: false +document: + compact: true +diagnostics: + quiet: true +`) + + stdout, _, err := exec(t, "-config", path, "./...") + require.NoError(t, err) + + assert.NotContains(t, stdout, "\n ", "document.compact came from the file") + assert.True(t, json.Valid([]byte(stdout))) +} + +// TestTypedFlagsBeatTheConfigFile is the precedence rule, end to end. +func TestTypedFlagsBeatTheConfigFile(t *testing.T) { + t.Parallel() + + path := configFile(t, ` +document: + compact: true +diagnostics: + quiet: true +`) + + stdout, _, err := exec(t, "-config", path, "-workdir", petstore(t), "-compact=false", "./...") + require.NoError(t, err) + + assert.Contains(t, stdout, "\n ", "-compact=false was typed, so the file does not get a say") +} + +func TestConfigOffIgnoresTheFile(t *testing.T) { + t.Parallel() + + path := configFile(t, "document:\n compact: true\n") + + stdout, _, err := exec(t, "-config=off", "-workdir", petstore(t), "-quiet", "./...") + require.NoError(t, err) + require.FileExists(t, path) + + assert.Contains(t, stdout, "\n ", "the file exists, and was not consulted") +} + +// TestConfigFileIsFoundByWalkingUp covers the path a user actually takes: a file in the project, +// and a command run from inside it. +// +// Not parallel: it is about the process's working directory, which is not a thing a test can hold +// an opinion on privately. +func TestConfigFileIsFoundByWalkingUp(t *testing.T) { + project := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(project, cliconf.Names[0]), + []byte("scan:\n workdir: "+petstore(t)+"\ndiagnostics:\n quiet: true\n"), 0o600)) + + deep := filepath.Join(project, "cmd", "api") + require.NoError(t, os.MkdirAll(deep, 0o750)) + t.Chdir(deep) + + stdout, _, err := exec(t, "./...") + + require.NoError(t, err) + assert.Contains(t, stdout, `"swagger": "2.0"`, "the scan ran where the file said") +} + +func TestConfigFileRefusesAnUnknownKey(t *testing.T) { + t.Parallel() + + path := configFile(t, "scan:\n workdirs: ./api\n") + + _, _, err := exec(t, "-config", path, "./...") + + require.ErrorIs(t, err, cliconf.ErrUnknownKey) + assert.Equal(t, exitUsage, exitStatus(err), "a file that cannot be obeyed is a usage error") + assert.Contains(t, err.Error(), "scan.workdirs") +} + +func TestConfigFileRefusesAValueTheFlagWillNotTake(t *testing.T) { + t.Parallel() + + path := configFile(t, "emit:\n scan-models: sometimes\n") + + _, _, err := exec(t, "-config", path, "./...") + + require.ErrorIs(t, err, cliconf.ErrBadValue) + assert.Equal(t, exitUsage, exitStatus(err)) +} + +func TestConfigFileMustExistWhenNamed(t *testing.T) { + t.Parallel() + + _, _, err := exec(t, "-config", filepath.Join(t.TempDir(), "absent.yaml"), "./...") + + require.ErrorIs(t, err, cliconf.ErrBadConfig) + assert.Equal(t, exitUsage, exitStatus(err)) +} + +// TestConfigFileReportsWhatItSkipped is how a section meant for another command - or a misspelled +// one - stays findable. +func TestConfigFileReportsWhatItSkipped(t *testing.T) { + t.Parallel() + + path := configFile(t, ` +scan: + workdir: `+petstore(t)+` +tui: + theme: dark +`) + + _, stderr, err := exec(t, "-config", path, "-verbose", "-color=never", "./...") + + require.NoError(t, err) + assert.Contains(t, stderr, "configuration read") + assert.Contains(t, stderr, "tui.theme") +} + +func TestConfigFileIsSilentWithoutVerbose(t *testing.T) { + t.Parallel() + + path := configFile(t, "scan:\n workdir: "+petstore(t)+"\n") + + _, stderr, err := exec(t, "-config", path, "-color=never", "./...") + + require.NoError(t, err) + assert.Empty(t, stderr, "reading a configuration file is not news") +} + +// TestConfigFileCannotNameAnotherConfigFile pins the one flag a file may not set. +func TestConfigFileCannotNameAnotherConfigFile(t *testing.T) { + t.Parallel() + + path := configFile(t, "scan:\n config: elsewhere.yaml\n") + + _, _, err := exec(t, "-config", path, "./...") + + require.ErrorIs(t, err, cliconf.ErrUnknownKey) +} + +func TestConfigSchemaCoversBothHalves(t *testing.T) { + t.Parallel() + + schema, err := configSchema() + require.NoError(t, err) + + assert.Equal(t, "scan", schema["workdir"], "the library's own flags") + assert.Equal(t, "emit", schema["scan-models"]) + assert.Equal(t, sectionDocument, schema["output"], "and this command's") + assert.Equal(t, sectionDiagnostics, schema["fail-on"]) + assert.Equal(t, + []string{sectionDiagnostics, sectionDocument, "emit", "go", "load", "scan"}, + schema.Sections()) +} diff --git a/cmd/genspec/diagnostics.go b/cmd/genspec/diagnostics.go index aac1ba11..29a6c4b2 100644 --- a/cmd/genspec/diagnostics.go +++ b/cmd/genspec/diagnostics.go @@ -10,10 +10,12 @@ import ( "log/slog" "os" "path/filepath" + "strings" "github.com/SladkyCitron/slogcolor" "github.com/go-openapi/codescan" + "github.com/go-openapi/codescan/internal/cliconf" ) // reporter is where everything the scan observed goes. @@ -75,14 +77,50 @@ func (r *reporter) onDiagnostic(diag codescan.Diagnostic) { return } - r.log(diag.Severity, diag.Message, - slog.String("code", string(diag.Code)), + r.log(diag.Severity, diag.Message, r.attrs(diag)...) +} + +// attrs is what is worth saying about a diagnostic besides its message. +// +// A diagnostic with no position - a whole route omitted by a tag rule, a definition pruned - is +// reported without one, rather than with the zero value dressed up as a location. "file= line=0" is +// not somewhere a reader can go. +func (r *reporter) attrs(diag codescan.Diagnostic) []any { + attrs := make([]any, 0, 4) //nolint:mnd // a code, and the three parts of a position + attrs = append(attrs, slog.String("code", string(diag.Code))) + if diag.Pos.Filename == "" { + return attrs + } + + return append(attrs, slog.String("file", r.relative(diag.Pos.Filename)), slog.Int("line", diag.Pos.Line), slog.Int("column", diag.Pos.Column), ) } +// configuration reports what a configuration file decided, under -verbose. +// +// Under -verbose rather than always, because on a good day this is the least interesting thing that +// happened. It is the only way to find out that a file was read at all, though - and, through the +// keys it skipped, that a section was misspelled rather than meant for another command. +func (r *reporter) configuration(path string, applied cliconf.Result) { + if path == "" || !r.hints { + return + } + + r.logger.Info("configuration read", + slog.String("file", r.relative(path)), + slog.Int("flags", len(applied.Set)), + ) + + if len(applied.Ignored) > 0 { + r.logger.Info("configuration keys skipped, in sections this command does not know", + slog.String("keys", strings.Join(applied.Ignored, ", ")), + ) + } +} + // record tallies one finding and decides whether it fails the command. // // Everything the command reports goes through here - what the scan observed and what -validate diff --git a/cmd/genspec/diagnostics_test.go b/cmd/genspec/diagnostics_test.go index a233d375..9197d1ce 100644 --- a/cmd/genspec/diagnostics_test.go +++ b/cmd/genspec/diagnostics_test.go @@ -64,6 +64,23 @@ func TestReporterMutesHintsButCountsThem(t *testing.T) { assert.Contains(t, out.String(), "hints=1") } +// TestReporterOmitsAPositionThereIsNoneOf covers the diagnostics about the document rather than +// about a place in it: a route dropped by a tag rule, a definition pruned. +func TestReporterOmitsAPositionThereIsNoneOf(t *testing.T) { + t.Parallel() + + report, out := reporterFor(t, "-color=never") + report.onDiagnostic(codescan.Diagnostic{ + Severity: codescan.SeverityWarning, + Code: "scan.ignored-by-tag", + Message: "a route went away", + }) + + assert.Contains(t, out.String(), "code=scan.ignored-by-tag") + assert.NotContains(t, out.String(), "line=0", `"file= line=0" is not somewhere a reader can go`) + assert.NotContains(t, out.String(), "file=") +} + func TestReporterVerboseShowsHints(t *testing.T) { t.Parallel() diff --git a/cmd/genspec/doc.go b/cmd/genspec/doc.go index bdc40d6f..dd2294b8 100644 --- a/cmd/genspec/doc.go +++ b/cmd/genspec/doc.go @@ -23,6 +23,27 @@ // cross-references // genspec-tui the same scan, live, with the source and the document side by side // +// # Configuration file +// +// Anything that can be a flag can be preset in a .codescan.yaml, found by searching upwards from +// wherever the command is run. Keys are grouped into sections, and inside a section a key is the +// flag it sets, spelled exactly as on the command line: +// +// scan: +// workdir: ./api +// emit: +// scan-models: true +// document: +// format: yaml +// +// Anything typed on the command line wins over the file, including a flag typed with the value it +// already had. -config names a particular file; -config off ignores whatever is lying around. +// +// One file serves the whole family: a section a command does not know is skipped rather than +// refused, so another command's settings may sit beside these. A key inside a section it does know +// must name one of its flags, which is what makes a typo an error rather than a setting that quietly +// never applied. +// // # Relationship to go-swagger // // genspec does the same job as go-swagger's `swagger generate spec`, which drives the same library. diff --git a/cmd/genspec/flags.go b/cmd/genspec/flags.go index 58353c0c..a288e78c 100644 --- a/cmd/genspec/flags.go +++ b/cmd/genspec/flags.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/go-openapi/codescan" + "github.com/go-openapi/codescan/internal/cliconf" "github.com/go-openapi/codescan/internal/cliopts" ) @@ -48,6 +49,10 @@ type config struct { // commands, so that a flag means the same thing whichever one you reach for. scan *cliopts.Values + // configFile names the file read before the flags, or "off". Its own field rather than one of + // the above: it decides where the others come from, so it cannot itself come from there. + configFile *string + input *string output *string format *string @@ -64,8 +69,9 @@ type config struct { func registerFlags(fs *flag.FlagSet) *config { return &config{ - set: fs, - scan: cliopts.Register(fs), + set: fs, + scan: cliopts.Register(fs), + configFile: cliconf.Register(fs), input: fs.String("input", "", "a specification to merge the scan's discoveries into, as JSON or YAML"), @@ -101,6 +107,10 @@ func usage(fs *flag.FlagSet, stderr io.Writer) func() { cliopts.DefaultPatterns) fmt.Fprintln(stderr, "The document goes to standard output unless -output names a file; what the scan") fmt.Fprintln(stderr, "observed goes to standard error.") + fmt.Fprintln(stderr, "\nFlags may be preset in a "+cliconf.Names[0]+", found by searching upwards from here.") + fmt.Fprintln(stderr, "Anything typed on the command line wins over it. Keys are grouped into sections and") + fmt.Fprintln(stderr, "named after the flags they set:") + fmt.Fprintln(stderr, "\n scan:\n workdir: ./api\n emit:\n scan-models: true") fmt.Fprintln(stderr, "\nFlags:") fs.PrintDefaults() } diff --git a/cmd/genspec/go.mod b/cmd/genspec/go.mod index 5f57131c..02e304de 100644 --- a/cmd/genspec/go.mod +++ b/cmd/genspec/go.mod @@ -13,6 +13,8 @@ require ( github.com/go-openapi/spec v0.22.9 github.com/go-openapi/strfmt v0.27.0 github.com/go-openapi/validate v0.26.3 + github.com/knadh/koanf/providers/rawbytes v1.0.1 + github.com/knadh/koanf/v2 v2.3.6 go.yaml.in/yaml/v3 v3.0.5 ) @@ -34,8 +36,11 @@ require ( github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/google/go-cmp v0.7.0 // indirect github.com/google/uuid v1.6.0 // indirect + github.com/knadh/koanf/maps v0.1.2 // indirect github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.24 // indirect + github.com/mitchellh/copystructure v1.2.0 // indirect + github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/oklog/ulid/v2 v2.1.2 // indirect golang.org/x/mod v0.38.0 // indirect golang.org/x/net v0.57.0 // indirect diff --git a/cmd/genspec/go.sum b/cmd/genspec/go.sum index 3c03e96a..f9b872cd 100644 --- a/cmd/genspec/go.sum +++ b/cmd/genspec/go.sum @@ -2,6 +2,8 @@ github.com/SladkyCitron/slogcolor v1.9.0 h1:fr4LeG+T6pZ1RYNNOP925jaBYrTdpA6BLzGy github.com/SladkyCitron/slogcolor v1.9.0/go.mod h1:ft8LEVIl4isUkebakhv+ngNXJjWBumnwhXfxTLApf3M= github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= +github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= +github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= github.com/go-openapi/analysis v0.26.0 h1:1xECln1iMMmQnTjgcknC1vi1hA4KISt6IHpSwnqcuwI= github.com/go-openapi/analysis v0.26.0/go.mod h1:40gERFi/2dyXA1FaqRRLxkv1IlC6X+GPDNd1xrYAjZE= github.com/go-openapi/codescan v0.36.3 h1:acI//gm/PfYaM6GI5k3+qiXGHZ2vbZOjSG3RidLgua0= @@ -50,11 +52,23 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/knadh/koanf/maps v0.1.2 h1:RBfmAW5CnZT+PJ1CVc1QSJKf4Xu9kxfQgYVQSu8hpbo= +github.com/knadh/koanf/maps v0.1.2/go.mod h1:npD/QZY3V6ghQDdcQzl1W4ICNVTkohC8E73eI2xW4yI= +github.com/knadh/koanf/providers/file v1.2.1 h1:bEWbtQwYrA+W2DtdBrQWyXqJaJSG3KrP3AESOJYp9wM= +github.com/knadh/koanf/providers/file v1.2.1/go.mod h1:bp1PM5f83Q+TOUu10J/0ApLBd9uIzg+n9UgthfY+nRA= +github.com/knadh/koanf/providers/rawbytes v1.0.1 h1:JCQoly+djX23Okr8kqtS19R7UXKleTAp62Vib2VrVYs= +github.com/knadh/koanf/providers/rawbytes v1.0.1/go.mod h1:KxwYJf1uezTKy6PBtfE+m725NGp4GPVA7XoNTJ/PtLo= +github.com/knadh/koanf/v2 v2.3.6 h1:JoQPSJmvS4aP0xNc8xMDr5tcrkSEInL23/Il7pITAKo= +github.com/knadh/koanf/v2 v2.3.6/go.mod h1:gRb40VRAbd4iJMYYD5IxZ6hfuopFcXBpc9bbQpZwo28= github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= github.com/mattn/go-isatty v0.0.24 h1:tGZZoVgT/KiqK1c8ocVLeDS8BSWMRd47J3Lbz7vsReI= github.com/mattn/go-isatty v0.0.24/go.mod h1:nMCL3Zebbrt45jsMDgnfIwz6ydEQApk5oEI3HqDio6A= +github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw= +github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s= +github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ= +github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= github.com/oklog/ulid/v2 v2.1.2 h1:IEclFb9JNvzYA6MW2SCxbLzcHTVsfqm3PrqGQJH5zec= github.com/oklog/ulid/v2 v2.1.2/go.mod h1:rcEKHmBBKfef9DhnvX7y1HZBYxjXb0cP5ExxNsTT1QQ= github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30 h1:BHT1/DKsYDGkUgQ2jmMaozVcdk+sVfz0+1ZJq4zkWgw= diff --git a/cmd/genspec/main.go b/cmd/genspec/main.go index 27716878..bc54dec2 100644 --- a/cmd/genspec/main.go +++ b/cmd/genspec/main.go @@ -52,6 +52,13 @@ func run(argv []string, stdout, stderr io.Writer) error { return nil } + // Before anything reads a flag, so that everything downstream sees one settled command line and + // needs to know nothing about where a value came from. + applied, configPath, err := configured(fs, *cfg.configFile) + if err != nil { + return fmt.Errorf("%w: %w", errUsage, err) + } + format, err := resolveFormat(*cfg.format, *cfg.output) if err != nil { return err @@ -61,6 +68,9 @@ func run(argv []string, stdout, stderr io.Writer) error { if err != nil { return err } + // Said now rather than when it was read: the file is free to decide how loud this command is, so + // there was nothing to say it through until here. + report.configuration(configPath, applied) opts, err := cfg.options(fs.Args(), report) if err != nil { diff --git a/cmd/genspec/main_test.go b/cmd/genspec/main_test.go index 470b51d0..e6de0a81 100644 --- a/cmd/genspec/main_test.go +++ b/cmd/genspec/main_test.go @@ -66,7 +66,7 @@ func scanned(t *testing.T, dir string, argv ...string) map[string]any { // Flags first, patterns last: everything after the first positional argument is a pattern, which // is the flag package's rule and not one this command is free to soften. - argv = append([]string{"-workdir", dir, "-quiet"}, argv...) + argv = append([]string{"-workdir", dir, "-quiet", "-config=off"}, argv...) stdout, _, err := exec(t, append(argv, "./...")...) require.NoError(t, err) @@ -92,7 +92,7 @@ func TestRunWritesToAFile(t *testing.T) { path := filepath.Join(t.TempDir(), "swagger.json") - stdout, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-output", path, "./...") + stdout, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-config=off", "-output", path, "./...") require.NoError(t, err) assert.Empty(t, stdout, "the document went to the file, so nothing goes to standard output") @@ -112,7 +112,7 @@ func TestRunInfersYAMLFromTheOutputName(t *testing.T) { path := filepath.Join(t.TempDir(), "swagger.yaml") - _, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-output", path, "./...") + _, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-config=off", "-output", path, "./...") require.NoError(t, err) written, err := os.ReadFile(path) @@ -125,7 +125,7 @@ func TestRunInfersYAMLFromTheOutputName(t *testing.T) { func TestRunCompactDropsTheIndentation(t *testing.T) { t.Parallel() - stdout, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-compact", "./...") + stdout, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-config=off", "-compact", "./...") require.NoError(t, err) assert.NotContains(t, stdout, "\n ", "compact means one line, not a differently indented one") @@ -152,7 +152,7 @@ func TestRunOverlaysTheInputSpecification(t *testing.T) { func TestRunRefusesADirectoryAsInput(t *testing.T) { t.Parallel() - _, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-input", t.TempDir(), "./...") + _, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-config=off", "-input", t.TempDir(), "./...") require.ErrorIs(t, err, errUsage) assert.Equal(t, exitUsage, exitStatus(err)) @@ -163,7 +163,7 @@ func TestRunRefusesADirectoryAsInput(t *testing.T) { func TestRunValidateFailsOnAnInvalidDocument(t *testing.T) { t.Parallel() - stdout, stderr, err := exec(t, "-workdir", unannotated(t), "-validate", "-color=never", "./...") + stdout, stderr, err := exec(t, "-workdir", unannotated(t), "-validate", "-config=off", "-color=never", "./...") require.ErrorIs(t, err, errInvalidSpec) assert.Equal(t, exitInvalidSpec, exitStatus(err)) @@ -174,7 +174,7 @@ func TestRunValidateFailsOnAnInvalidDocument(t *testing.T) { func TestRunFailsOnTheThresholdItWasGiven(t *testing.T) { t.Parallel() - _, _, err := exec(t, "-workdir", unannotated(t), "-fail-on=warning", "-color=never", "./...") + _, _, err := exec(t, "-workdir", unannotated(t), "-fail-on=warning", "-config=off", "-color=never", "./...") require.ErrorIs(t, err, errDiagnostics) assert.Equal(t, exitDiagnostics, exitStatus(err)) @@ -185,7 +185,7 @@ func TestRunFailsOnTheThresholdItWasGiven(t *testing.T) { func TestRunIsSilentByDefaultOnACleanScan(t *testing.T) { t.Parallel() - _, stderr, err := exec(t, "-workdir", petstore(t), "-color=never", "./...") + _, stderr, err := exec(t, "-workdir", petstore(t), "-color=never", "-config=off", "./...") require.NoError(t, err) assert.Empty(t, stderr) @@ -194,7 +194,7 @@ func TestRunIsSilentByDefaultOnACleanScan(t *testing.T) { func TestRunQuietSaysNothing(t *testing.T) { t.Parallel() - stdout, stderr, err := exec(t, "-workdir", unannotated(t), "-quiet", "-verbose", "./...") + stdout, stderr, err := exec(t, "-workdir", unannotated(t), "-quiet", "-config=off", "-verbose", "./...") require.NoError(t, err) assert.NotEmpty(t, stdout, "-quiet is about the diagnostics, not about the document") @@ -204,7 +204,7 @@ func TestRunQuietSaysNothing(t *testing.T) { func TestRunReportsPositionsRelativeToTheWorkingDirectory(t *testing.T) { t.Parallel() - _, stderr, err := exec(t, "-workdir", unannotated(t), "-color=never", "./...") + _, stderr, err := exec(t, "-workdir", unannotated(t), "-color=never", "-config=off", "./...") require.NoError(t, err) require.NotEmpty(t, stderr, "this fixture has something to say about itself") @@ -244,7 +244,7 @@ func TestRunRejectsAnUnknownFlag(t *testing.T) { func TestRunRejectsAnUnreadableWorkdir(t *testing.T) { t.Parallel() - _, _, err := exec(t, "-workdir", filepath.Join(t.TempDir(), "nowhere"), "-quiet", "./...") + _, _, err := exec(t, "-workdir", filepath.Join(t.TempDir(), "nowhere"), "-quiet", "-config=off", "./...") require.Error(t, err) assert.Equal(t, exitFailed, exitStatus(err)) From 2198496bbb59b4912e7adb093d22641a25526718 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Fri, 14 Aug 2026 17:26:07 +0200 Subject: [PATCH 7/8] feat(cliconf): add -c and -no-config to pin or refuse the file -c is the short form of -config, for the thing done often enough to be worth two keystrokes. -no-config reads none, whatever is lying around, which is what a run that has to be reproducible says. It replaces the "off" value -config used to take. A switch reads as what it is at a glance where a magic value has to be known, and there is now one way to say each thing rather than two that could disagree. The two spellings of the file are stored apart rather than sharing a variable, so -config and -c naming different files is refused instead of resolved by whichever the parser read last. Naming a file and asking for none is refused the same way: there is no reading of it the caller can have meant, and picking one would be inventing an intention. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- cmd/genspec/README.md | 5 +- cmd/genspec/configfile.go | 10 ++-- cmd/genspec/configfile_test.go | 29 +++++++++- cmd/genspec/doc.go | 2 +- cmd/genspec/flags.go | 4 +- cmd/genspec/main.go | 2 +- cmd/genspec/main_test.go | 22 ++++---- internal/cliconf/cliconf_test.go | 9 ++- internal/cliconf/discover.go | 94 +++++++++++++++++++++++-------- internal/cliconf/discover_test.go | 90 +++++++++++++++++++++++++---- 10 files changed, 207 insertions(+), 60 deletions(-) diff --git a/cmd/genspec/README.md b/cmd/genspec/README.md index 4eff222f..9ca32e5a 100644 --- a/cmd/genspec/README.md +++ b/cmd/genspec/README.md @@ -71,8 +71,9 @@ and `diagnostics` for this command's own flags. **Anything typed wins.** That holds for a flag typed with the value it already had: `-scan-models=false` means false even where the file says true. -`-config ` reads a particular file, which must exist. `-config off` -ignores whatever is lying around, for a run that has to be reproducible. +`-config ` — or `-c ` — reads a particular file, which must exist. +`--no-config` reads none, whatever is lying around, for a run that has to be +reproducible. Asking for both at once is an error rather than a coin toss. `.codescan.yml` and `.codescan.json` are searched for too — JSON is a subset of YAML, so it needs no parser of its own. diff --git a/cmd/genspec/configfile.go b/cmd/genspec/configfile.go index 07e9cff0..c7b1bdd7 100644 --- a/cmd/genspec/configfile.go +++ b/cmd/genspec/configfile.go @@ -43,8 +43,10 @@ var commandSections = cliconf.Schema{ //nolint:gochecknoglobals // the schema, r // notConfigurable are the flags a file deliberately cannot set, with the reason. var notConfigurable = map[string]string{ //nolint:gochecknoglobals // table for the drift guard - cliconf.Flag: "names the file itself, which cannot be read from inside it", - "version": "asks the command a question rather than configuring it", + cliconf.Flag: "names the file itself, which cannot be read from inside it", + cliconf.ShortFlag: "the short form of -" + cliconf.Flag, + cliconf.NoFlag: "decides whether there is a file at all, from outside it", + "version": "asks the command a question rather than configuring it", } // configSchema is the whole of what a configuration file may address here. @@ -61,7 +63,7 @@ func configSchema() (cliconf.Schema, error) { // // Called after parsing and before anything reads a flag, so that everything downstream sees one // settled command line and needs to know nothing about where a value came from. -func configured(fs *flag.FlagSet, named string) (cliconf.Result, string, error) { +func configured(fs *flag.FlagSet, which *cliconf.Flags) (cliconf.Result, string, error) { var applied cliconf.Result // Searched from where the command was run, not from -workdir: the file is free to set -workdir, @@ -71,7 +73,7 @@ func configured(fs *flag.FlagSet, named string) (cliconf.Result, string, error) return applied, "", fmt.Errorf("cannot tell where this is running from: %w", err) } - path, err := cliconf.Discover(named, here) + path, err := which.Discover(here) if err != nil || path == "" { return applied, "", err } diff --git a/cmd/genspec/configfile_test.go b/cmd/genspec/configfile_test.go index 44f7fff3..61af019c 100644 --- a/cmd/genspec/configfile_test.go +++ b/cmd/genspec/configfile_test.go @@ -93,18 +93,43 @@ diagnostics: assert.Contains(t, stdout, "\n ", "-compact=false was typed, so the file does not get a say") } -func TestConfigOffIgnoresTheFile(t *testing.T) { +func TestNoConfigIgnoresTheFile(t *testing.T) { t.Parallel() path := configFile(t, "document:\n compact: true\n") - stdout, _, err := exec(t, "-config=off", "-workdir", petstore(t), "-quiet", "./...") + stdout, _, err := exec(t, "-no-config", "-workdir", petstore(t), "-quiet", "./...") require.NoError(t, err) require.FileExists(t, path) assert.Contains(t, stdout, "\n ", "the file exists, and was not consulted") } +// TestShortConfigFlagPinsTheFile covers the spelling a caller reaches for often enough to want short. +func TestShortConfigFlagPinsTheFile(t *testing.T) { + t.Parallel() + + path := configFile(t, "scan:\n workdir: "+petstore(t)+"\ndocument:\n compact: true\n") + + stdout, _, err := exec(t, "-c", path, "-quiet", "./...") + + require.NoError(t, err) + assert.NotContains(t, stdout, "\n ", "the file -c named was read") +} + +// TestConfigAndNoConfigContradictEachOther: the pair of flags can be asked something with no +// answer, and saying so beats picking one. +func TestConfigAndNoConfigContradictEachOther(t *testing.T) { + t.Parallel() + + path := configFile(t, "scan:\n workdir: .\n") + + _, _, err := exec(t, "-c", path, "-no-config", "./...") + + require.ErrorIs(t, err, cliconf.ErrBadConfig) + assert.Equal(t, exitUsage, exitStatus(err)) +} + // TestConfigFileIsFoundByWalkingUp covers the path a user actually takes: a file in the project, // and a command run from inside it. // diff --git a/cmd/genspec/doc.go b/cmd/genspec/doc.go index dd2294b8..9f2830ce 100644 --- a/cmd/genspec/doc.go +++ b/cmd/genspec/doc.go @@ -37,7 +37,7 @@ // format: yaml // // Anything typed on the command line wins over the file, including a flag typed with the value it -// already had. -config names a particular file; -config off ignores whatever is lying around. +// already had. -config, or -c, names a particular file; -no-config reads none. // // One file serves the whole family: a section a command does not know is skipped rather than // refused, so another command's settings may sit beside these. A key inside a section it does know diff --git a/cmd/genspec/flags.go b/cmd/genspec/flags.go index a288e78c..aebaa8e3 100644 --- a/cmd/genspec/flags.go +++ b/cmd/genspec/flags.go @@ -49,9 +49,9 @@ type config struct { // commands, so that a flag means the same thing whichever one you reach for. scan *cliopts.Values - // configFile names the file read before the flags, or "off". Its own field rather than one of + // configFile is which file is read before the flags, if any. Its own field rather than one of // the above: it decides where the others come from, so it cannot itself come from there. - configFile *string + configFile *cliconf.Flags input *string output *string diff --git a/cmd/genspec/main.go b/cmd/genspec/main.go index bc54dec2..f914c789 100644 --- a/cmd/genspec/main.go +++ b/cmd/genspec/main.go @@ -54,7 +54,7 @@ func run(argv []string, stdout, stderr io.Writer) error { // Before anything reads a flag, so that everything downstream sees one settled command line and // needs to know nothing about where a value came from. - applied, configPath, err := configured(fs, *cfg.configFile) + applied, configPath, err := configured(fs, cfg.configFile) if err != nil { return fmt.Errorf("%w: %w", errUsage, err) } diff --git a/cmd/genspec/main_test.go b/cmd/genspec/main_test.go index e6de0a81..5da70ed8 100644 --- a/cmd/genspec/main_test.go +++ b/cmd/genspec/main_test.go @@ -66,7 +66,7 @@ func scanned(t *testing.T, dir string, argv ...string) map[string]any { // Flags first, patterns last: everything after the first positional argument is a pattern, which // is the flag package's rule and not one this command is free to soften. - argv = append([]string{"-workdir", dir, "-quiet", "-config=off"}, argv...) + argv = append([]string{"-workdir", dir, "-quiet", "-no-config"}, argv...) stdout, _, err := exec(t, append(argv, "./...")...) require.NoError(t, err) @@ -92,7 +92,7 @@ func TestRunWritesToAFile(t *testing.T) { path := filepath.Join(t.TempDir(), "swagger.json") - stdout, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-config=off", "-output", path, "./...") + stdout, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-no-config", "-output", path, "./...") require.NoError(t, err) assert.Empty(t, stdout, "the document went to the file, so nothing goes to standard output") @@ -112,7 +112,7 @@ func TestRunInfersYAMLFromTheOutputName(t *testing.T) { path := filepath.Join(t.TempDir(), "swagger.yaml") - _, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-config=off", "-output", path, "./...") + _, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-no-config", "-output", path, "./...") require.NoError(t, err) written, err := os.ReadFile(path) @@ -125,7 +125,7 @@ func TestRunInfersYAMLFromTheOutputName(t *testing.T) { func TestRunCompactDropsTheIndentation(t *testing.T) { t.Parallel() - stdout, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-config=off", "-compact", "./...") + stdout, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-no-config", "-compact", "./...") require.NoError(t, err) assert.NotContains(t, stdout, "\n ", "compact means one line, not a differently indented one") @@ -152,7 +152,7 @@ func TestRunOverlaysTheInputSpecification(t *testing.T) { func TestRunRefusesADirectoryAsInput(t *testing.T) { t.Parallel() - _, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-config=off", "-input", t.TempDir(), "./...") + _, _, err := exec(t, "-workdir", petstore(t), "-quiet", "-no-config", "-input", t.TempDir(), "./...") require.ErrorIs(t, err, errUsage) assert.Equal(t, exitUsage, exitStatus(err)) @@ -163,7 +163,7 @@ func TestRunRefusesADirectoryAsInput(t *testing.T) { func TestRunValidateFailsOnAnInvalidDocument(t *testing.T) { t.Parallel() - stdout, stderr, err := exec(t, "-workdir", unannotated(t), "-validate", "-config=off", "-color=never", "./...") + stdout, stderr, err := exec(t, "-workdir", unannotated(t), "-validate", "-no-config", "-color=never", "./...") require.ErrorIs(t, err, errInvalidSpec) assert.Equal(t, exitInvalidSpec, exitStatus(err)) @@ -174,7 +174,7 @@ func TestRunValidateFailsOnAnInvalidDocument(t *testing.T) { func TestRunFailsOnTheThresholdItWasGiven(t *testing.T) { t.Parallel() - _, _, err := exec(t, "-workdir", unannotated(t), "-fail-on=warning", "-config=off", "-color=never", "./...") + _, _, err := exec(t, "-workdir", unannotated(t), "-fail-on=warning", "-no-config", "-color=never", "./...") require.ErrorIs(t, err, errDiagnostics) assert.Equal(t, exitDiagnostics, exitStatus(err)) @@ -185,7 +185,7 @@ func TestRunFailsOnTheThresholdItWasGiven(t *testing.T) { func TestRunIsSilentByDefaultOnACleanScan(t *testing.T) { t.Parallel() - _, stderr, err := exec(t, "-workdir", petstore(t), "-color=never", "-config=off", "./...") + _, stderr, err := exec(t, "-workdir", petstore(t), "-color=never", "-no-config", "./...") require.NoError(t, err) assert.Empty(t, stderr) @@ -194,7 +194,7 @@ func TestRunIsSilentByDefaultOnACleanScan(t *testing.T) { func TestRunQuietSaysNothing(t *testing.T) { t.Parallel() - stdout, stderr, err := exec(t, "-workdir", unannotated(t), "-quiet", "-config=off", "-verbose", "./...") + stdout, stderr, err := exec(t, "-workdir", unannotated(t), "-quiet", "-no-config", "-verbose", "./...") require.NoError(t, err) assert.NotEmpty(t, stdout, "-quiet is about the diagnostics, not about the document") @@ -204,7 +204,7 @@ func TestRunQuietSaysNothing(t *testing.T) { func TestRunReportsPositionsRelativeToTheWorkingDirectory(t *testing.T) { t.Parallel() - _, stderr, err := exec(t, "-workdir", unannotated(t), "-color=never", "-config=off", "./...") + _, stderr, err := exec(t, "-workdir", unannotated(t), "-color=never", "-no-config", "./...") require.NoError(t, err) require.NotEmpty(t, stderr, "this fixture has something to say about itself") @@ -244,7 +244,7 @@ func TestRunRejectsAnUnknownFlag(t *testing.T) { func TestRunRejectsAnUnreadableWorkdir(t *testing.T) { t.Parallel() - _, _, err := exec(t, "-workdir", filepath.Join(t.TempDir(), "nowhere"), "-quiet", "-config=off", "./...") + _, _, err := exec(t, "-workdir", filepath.Join(t.TempDir(), "nowhere"), "-quiet", "-no-config", "./...") require.Error(t, err) assert.Equal(t, exitFailed, exitStatus(err)) diff --git a/internal/cliconf/cliconf_test.go b/internal/cliconf/cliconf_test.go index 1cce791c..fb390b01 100644 --- a/internal/cliconf/cliconf_test.go +++ b/internal/cliconf/cliconf_test.go @@ -288,13 +288,16 @@ func TestStringifyRefusesWhatAFlagCannotTake(t *testing.T) { require.Error(t, err, "and inside a list too") } -func TestRegisterDeclaresTheFlag(t *testing.T) { +func TestRegisterDeclaresTheFlags(t *testing.T) { t.Parallel() fs := flag.NewFlagSet("test", flag.ContinueOnError) fs.SetOutput(io.Discard) - config := Register(fs) + flags := Register(fs) require.NoError(t, fs.Parse([]string{"-config", "somewhere.yaml"})) - assert.Equal(t, "somewhere.yaml", *config) + + named, err := flags.named() + require.NoError(t, err) + assert.Equal(t, "somewhere.yaml", named) } diff --git a/internal/cliconf/discover.go b/internal/cliconf/discover.go index cad264e7..1fb39892 100644 --- a/internal/cliconf/discover.go +++ b/internal/cliconf/discover.go @@ -10,14 +10,20 @@ import ( "path/filepath" ) -// Flag is what the configuration file is named on the command line. -const Flag = "config" - -// Off is what -config is given to use no file at all, whatever is lying around. +// The flags every command registers for its configuration file. // -// Spelled as a value rather than as a second flag, the way GOWORK spells it: a run that must be -// reproducible says so in one place, and there is no pair of flags that can disagree. -const Off = "off" +// Three spellings of two questions: which file, and whether to read one at all. -c is the short form +// of -config, because naming a file is the thing done often enough to be worth two keystrokes; +// -no-config is the way to say none, and it is a switch rather than a magic value so that it reads +// as what it is at a glance. +const ( + // Flag names the file to read. + Flag = "config" + // ShortFlag is the short form of [Flag]. + ShortFlag = "c" + // NoFlag says to read no configuration file at all. + NoFlag = "no-config" +) // Names are the file names looked for, in order, when -config says nothing. // @@ -30,36 +36,78 @@ var Names = []string{ //nolint:gochecknoglobals // the search list, read once at ".codescan.json", } -const flagHelp = `configuration file to read before the flags ("off" for none; default: the nearest ` + - `.codescan.yaml, searching upwards)` +// Flags is where the configuration-file flags land. +// +// A type rather than a pair of pointers, because the two questions can be answered in ways that +// contradict each other, and something has to hold the answer to "then what". +type Flags struct { + // long and short hold the same answer, separately, so that giving both can be noticed rather + // than resolved by whichever the parser happened to read last. + long *string + short *string + + none *bool +} -// Register declares the -config flag and returns where its value lands. -func Register(fs *flag.FlagSet) *string { - return fs.String(Flag, "", flagHelp) +// Register declares the configuration-file flags and returns where their values land. +func Register(fs *flag.FlagSet) *Flags { + return &Flags{ + long: fs.String(Flag, "", + "configuration file to read before the flags (default: the nearest .codescan.yaml,\n"+ + "searching upwards)"), + short: fs.String(ShortFlag, "", "shorthand for -"+Flag), + none: fs.Bool(NoFlag, false, "read no configuration file, whatever is lying around"), + } } // Discover reports which file to read, if any. // -// A named file must exist: a caller who said -config meant that file, and silently scanning for -// another one - or for none - would answer a question they did not ask. Searching, on the other -// hand, is allowed to find nothing, which is the ordinary case. +// A named file must exist: a caller who named one meant that file, and silently searching for +// another - or for none - would answer a question they did not ask. Searching, on the other hand, is +// allowed to find nothing, which is the ordinary case. // // The search walks up from start, so running a command from anywhere inside a project finds the -// project's own file. It stops at the first hit rather than merging what it passes: a file that is +// project's own file. It stops at the first hit rather than merging what it passes: a file // half-overridden by one three directories up is not something anybody can read off the page. -func Discover(explicit, start string) (string, error) { - switch explicit { - case Off: +func (f *Flags) Discover(start string) (string, error) { + named, err := f.named() + if err != nil { + return "", err + } + + if *f.none { + if named != "" { + // Named by either spelling, so the message names both rather than guessing which was typed. + return "", fmt.Errorf("%w: -%s/-%s names %q, and -%s says to read none", + ErrBadConfig, Flag, ShortFlag, named, NoFlag) + } + return "", nil - case "": + } + + if named == "" { return search(start) } - if _, err := os.Stat(explicit); err != nil { - return "", fmt.Errorf("%w: -%s %q: %w", ErrBadConfig, Flag, explicit, err) + if _, err := os.Stat(named); err != nil { + return "", fmt.Errorf("%w: -%s %q: %w", ErrBadConfig, Flag, named, err) + } + + return named, nil +} + +// named reports the file the caller asked for, by either spelling. +func (f *Flags) named() (string, error) { + long, short := *f.long, *f.short + + if long != "" && short != "" && long != short { + return "", fmt.Errorf("%w: -%s says %q and -%s says %q", ErrBadConfig, Flag, long, ShortFlag, short) + } + if long != "" { + return long, nil } - return explicit, nil + return short, nil } // search walks up from dir looking for a file named like a configuration. diff --git a/internal/cliconf/discover_test.go b/internal/cliconf/discover_test.go index ac438fe8..d765eac9 100644 --- a/internal/cliconf/discover_test.go +++ b/internal/cliconf/discover_test.go @@ -4,6 +4,8 @@ package cliconf import ( + "flag" + "io" "os" "path/filepath" "testing" @@ -22,13 +24,25 @@ func write(t *testing.T, dir, name string) string { return path } +// discover parses argv into the configuration flags and asks them where the file is. +func discover(t *testing.T, start string, argv ...string) (string, error) { + t.Helper() + + fs := flag.NewFlagSet("test", flag.ContinueOnError) + fs.SetOutput(io.Discard) + flags := Register(fs) + require.NoError(t, fs.Parse(argv)) + + return flags.Discover(start) +} + func TestDiscoverFindsAFileWhereItStands(t *testing.T) { t.Parallel() dir := t.TempDir() want := write(t, dir, Names[0]) - got, err := Discover("", dir) + got, err := discover(t, dir) require.NoError(t, err) assert.Equal(t, want, got) @@ -44,7 +58,7 @@ func TestDiscoverWalksUp(t *testing.T) { deep := filepath.Join(root, "internal", "api", "handlers") require.NoError(t, os.MkdirAll(deep, 0o750)) - got, err := Discover("", deep) + got, err := discover(t, deep) require.NoError(t, err) assert.Equal(t, want, got) @@ -62,7 +76,7 @@ func TestDiscoverStopsAtTheNearest(t *testing.T) { require.NoError(t, os.MkdirAll(nearer, 0o750)) want := write(t, nearer, Names[0]) - got, err := Discover("", nearer) + got, err := discover(t, nearer) require.NoError(t, err) assert.Equal(t, want, got) @@ -75,7 +89,7 @@ func TestDiscoverPrefersTheNamesInOrder(t *testing.T) { want := write(t, dir, Names[0]) write(t, dir, Names[1]) - got, err := Discover("", dir) + got, err := discover(t, dir) require.NoError(t, err) assert.Equal(t, want, got) @@ -86,7 +100,7 @@ func TestDiscoverPrefersTheNamesInOrder(t *testing.T) { func TestDiscoverFindingNothingIsNotAFailure(t *testing.T) { t.Parallel() - got, err := Discover("", t.TempDir()) + got, err := discover(t, t.TempDir()) require.NoError(t, err) assert.Empty(t, got) @@ -98,7 +112,7 @@ func TestDiscoverIgnoresADirectoryNamedLikeAConfig(t *testing.T) { dir := t.TempDir() require.NoError(t, os.Mkdir(filepath.Join(dir, Names[0]), 0o750)) - got, err := Discover("", dir) + got, err := discover(t, dir) require.NoError(t, err) assert.Empty(t, got, "a directory is not a file, whatever it is called") @@ -110,7 +124,7 @@ func TestDiscoverTakesTheFileItIsGiven(t *testing.T) { dir := t.TempDir() want := write(t, dir, "elsewhere.yaml") - got, err := Discover(want, t.TempDir()) + got, err := discover(t, t.TempDir(), "-config", want) require.NoError(t, err) assert.Equal(t, want, got) @@ -121,21 +135,75 @@ func TestDiscoverTakesTheFileItIsGiven(t *testing.T) { func TestDiscoverRefusesAFileThatIsNotThere(t *testing.T) { t.Parallel() - _, err := Discover(filepath.Join(t.TempDir(), "absent.yaml"), t.TempDir()) + _, err := discover(t, t.TempDir(), "-config", filepath.Join(t.TempDir(), "absent.yaml")) require.ErrorIs(t, err, ErrBadConfig) assert.Contains(t, err.Error(), "absent.yaml") } -// TestDiscoverOffFindsNothingEvenWhereThereIsSomething is the reproducible-run escape hatch. -func TestDiscoverOffFindsNothingEvenWhereThereIsSomething(t *testing.T) { +// TestDiscoverNoConfigFindsNothingEvenWhereThereIsSomething is the reproducible-run escape hatch. +func TestDiscoverNoConfigFindsNothingEvenWhereThereIsSomething(t *testing.T) { t.Parallel() dir := t.TempDir() write(t, dir, Names[0]) - got, err := Discover(Off, dir) + got, err := discover(t, dir, "-no-config") require.NoError(t, err) assert.Empty(t, got) } + +func TestDiscoverTakesTheShortFlag(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + want := write(t, dir, "elsewhere.yaml") + + got, err := discover(t, t.TempDir(), "-c", want) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +// TestDiscoverAcceptsBothSpellingsOfTheSameFile: -c is -config, so saying both is repetition rather +// than a contradiction. +func TestDiscoverAcceptsBothSpellingsOfTheSameFile(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + want := write(t, dir, "elsewhere.yaml") + + got, err := discover(t, t.TempDir(), "-config", want, "-c", want) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +// TestDiscoverRefusesTwoDifferentFiles is why the two spellings are stored apart: sharing one +// variable would have let whichever the parser read last win in silence. +func TestDiscoverRefusesTwoDifferentFiles(t *testing.T) { + t.Parallel() + + _, err := discover(t, t.TempDir(), "-config", "one.yaml", "-c", "another.yaml") + + require.ErrorIs(t, err, ErrBadConfig) + assert.Contains(t, err.Error(), "one.yaml") + assert.Contains(t, err.Error(), "another.yaml") +} + +// TestDiscoverRefusesAFileAndNoFileAtOnce covers the contradiction the pair of flags makes possible. +// +// Refused rather than resolved: there is no reading of "use this file, and use no file" that the +// caller can have meant, and picking one would be inventing an intention. +func TestDiscoverRefusesAFileAndNoFileAtOnce(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := write(t, dir, Names[0]) + + _, err := discover(t, dir, "-c", path, "-no-config") + + require.ErrorIs(t, err, ErrBadConfig) + assert.Contains(t, err.Error(), NoFlag) +} From 13343a169c7e65b92ec5f07e170a4161e2936262 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Fri, 14 Aug 2026 19:08:27 +0200 Subject: [PATCH 8/8] doc: updated roadmap Signed-off-by: Frederic BIDON --- docs/doc-site/maintainers/ROADMAP.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/doc-site/maintainers/ROADMAP.md b/docs/doc-site/maintainers/ROADMAP.md index dcc17bb5..713593cc 100644 --- a/docs/doc-site/maintainers/ROADMAP.md +++ b/docs/doc-site/maintainers/ROADMAP.md @@ -34,12 +34,12 @@ timeline ✅ v0.36.0 (July 2026) : TUI : interactive spec building with TUI tool : polymorphic subtypes discovery - 🔶 v0.36.x (August 2026) : faster code scanner - : ✅ Optimized incremental type scanner - : ✅ toolchain-independent loader - : ✅ playground UI (wasi build) - : 🔶 more go-swagger backlog fixes & tunable knobs + ✅ v0.36.x (August 2026) : faster code scanner + : Optimized incremental type scanner + : toolchain-independent loader + : playground UI (wasi build) : dedicated CLI, independent from go-swagger + : more go-swagger backlog fixes & tunable knobs ⬜ v0.37.0 (September 2026) : decouple from `Spec` : go1.26+ : Internal document model