From d40638729c880e7f616e6eb162dd8b18801ceb99 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Mon, 22 Jun 2026 00:21:10 +0200 Subject: [PATCH] refactor: retire internal/logger; route scan observations through diagnostics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy debug logger (internal/logger.DebugLogf / UnsupportedTypeKind) and the dead slog logger on common.Builder predated the diagnostics system and were strictly less useful than it: unstructured, position-less, and written to stdout/stderr regardless of whether the caller wanted them. This removes that machinery and folds its genuinely-useful output into the single Options.OnDiagnostic surface: - pure control-flow trace and totally-normal drops (unexported field, swagger:ignore, anonymous field, untagged type) are deleted outright; - "I dropped your config-filtered package/route" becomes a Hint (scan.ignored-by-rules / scan.ignored-by-tag); - "I cannot model this Go type" becomes a Warning (validate.unsupported-go-type) — this closes a real gap, as those warnings previously escaped OnDiagnostic entirely; - four stray log.Printf("WARNING...") lines (DeclForType/PkgForType defaults, swagger:model/response malformed names, swagger:enum with no values) are likewise routed through diagnostics. Production code now makes zero writes to stdout/stderr, keeping codescan embeddable in a TUI or a WASI/WASM host. Options.Debug is retained as a documented deprecated no-op for API compatibility. The diagnostic surface needed by OnDiagnostic callers is re-exported at the root (Diagnostic / Severity / Code aliases + Severity* constants) so consumers need not import internal packages. Zero golden drift: diagnostics never alter the emitted spec. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Frederic BIDON --- .claude/CLAUDE.md | 10 +-- .github/copilot-instructions.md | 1 - api_test.go | 28 +------ diagnostics.go | 35 ++++++++ .../enhancements/unsupported-go-type/model.go | 21 +++++ internal/builders/common/README.md | 9 +- internal/builders/common/builder.go | 19 +---- internal/builders/parameters/parameters.go | 9 -- internal/builders/responses/responses.go | 12 --- internal/builders/schema/README.md | 4 +- internal/builders/schema/allof.go | 5 +- internal/builders/schema/embedded.go | 10 +-- internal/builders/schema/schema.go | 44 +++++++--- .../builders/schema/walker_classifiers.go | 8 +- internal/builders/spec/reduce.go | 20 +---- .../coverage_diagnostics_from_logger_test.go | 84 +++++++++++++++++++ internal/logger/debug.go | 32 ------- internal/parsers/grammar/diagnostic.go | 21 +++++ internal/scanner/index.go | 74 +++++++++------- internal/scanner/options.go | 12 ++- internal/scanner/scan_context.go | 28 +++---- internal/scanner/scan_context_test.go | 3 - 22 files changed, 285 insertions(+), 204 deletions(-) create mode 100644 diagnostics.go create mode 100644 fixtures/enhancements/unsupported-go-type/model.go create mode 100644 internal/integration/coverage_diagnostics_from_logger_test.go delete mode 100644 internal/logger/debug.go diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 9110bdb9..55edb53d 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -25,6 +25,7 @@ to builders without direct coupling. | File | Contents | |------|----------| | `api.go` | `Run(*Options) (*spec.Swagger, error)` entry point; re-exports `Options = scanner.Options` | +| `diagnostics.go` | Re-exports the diagnostic surface for `Options.OnDiagnostic` callers: `Diagnostic`/`Severity`/`Code` aliases + `Severity*` constants | | `doc.go` | Package godoc | | `errors.go` | `ErrCodeScan` sentinel error | @@ -82,10 +83,6 @@ Each sub-package owns one concern; `walker.go` carries the per-block grammar dis `SwaggerTypable`, `ValidationBuilder`, `OperationValidationBuilder`, `ValueParser`, `Objecter` — the glue that lets `parsers` write into any builder's target without importing concrete builders. -### `internal/logger/` — debug logging - -`debug.go` — gated on `Options.Debug`. - ### `internal/scantest/` — test utilities (do **not** import from production code) | File | Contents | @@ -127,7 +124,10 @@ malformed input, the petstore, aliased schemas, go123-specific forms, and cross- siblings), each with a diagnostic. For consumers (e.g. go-swagger) wanting bare refs. - `SetXNullableForPointers` — emit `x-nullable: true` on pointer fields. - `SkipExtensions` — suppress `x-go-*` vendor extensions. - - `Debug` — verbose logging via `internal/logger`. + - `OnDiagnostic` — callback sink for all scan-time observations (the only output + channel; codescan never writes to stdout/stderr). + - `Debug` — deprecated no-op (the legacy stderr debug logger was retired; wire + `OnDiagnostic` instead). ## Dependencies diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b0ed16fb..f2cec34d 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -42,7 +42,6 @@ Internal tree: | `internal/builders/validations` | Type-aware coercion (`CoerceEnum`, `ParseDefault`) + shape checks (`IsLegalForType`) | | `internal/builders/resolvers` | `SwaggerSchemaForType`, identity / assertion helpers, items-chain ifaces adapters | | `internal/ifaces` | `SwaggerTypable`, `ValidationBuilder`, `OperationValidationBuilder`, `ValueParser`, `Objecter` — decouples parsers from builders | -| `internal/logger` | Debug logging (gated on `Options.Debug`) | | `internal/scantest` | Test utilities: golden compare, fixture loading, mocks, classification helpers | | `internal/integration` | Black-box integration tests against `fixtures/integration/golden/*.json` | diff --git a/api_test.go b/api_test.go index ee5c8a20..1688e2ba 100644 --- a/api_test.go +++ b/api_test.go @@ -4,9 +4,6 @@ package codescan import ( - "flag" - "io" - "log" "os" "path/filepath" "testing" @@ -16,27 +13,10 @@ import ( // Public-API smoke suite. Fixture-heavy tests live in internal/integration. -var enableDebug bool //nolint:gochecknoglobals // test flag registered in init - -func init() { //nolint:gochecknoinits // registers test flags before TestMain - flag.BoolVar(&enableDebug, "enable-debug", false, "enable debug output in tests") -} - -func TestMain(m *testing.M) { - flag.Parse() - - if !enableDebug { - log.SetOutput(io.Discard) - } else { - log.SetFlags(log.LstdFlags | log.Lshortfile) - log.SetOutput(os.Stderr) - } - - os.Exit(m.Run()) -} - -func TestApplication_DebugLogging(t *testing.T) { - // Exercises the logger.DebugLogf code path with Debug: true. +func TestApplication_DeprecatedDebugOption(t *testing.T) { + // Options.Debug is a deprecated no-op (the legacy debug logger was + // retired in favour of diagnostics). Verify Run still accepts it without + // error and produces a spec. _, err := Run(&Options{ Packages: []string{"./goparsing/petstore/..."}, WorkDir: "fixtures", diff --git a/diagnostics.go b/diagnostics.go new file mode 100644 index 00000000..337f1000 --- /dev/null +++ b/diagnostics.go @@ -0,0 +1,35 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package codescan + +import "github.com/go-openapi/codescan/internal/parsers/grammar" + +// Diagnostic is one observation the scanner makes about the source it +// processes — a parse/validation issue, a dropped construct, or an +// informational note. Every scan-time observation is delivered to +// [Options.OnDiagnostic]; codescan never writes to stdout/stderr. +// +// Fields: Pos (a go/token.Position, zero when no single source location +// applies), Severity, Code (a stable machine-readable identifier), and a +// human-readable Message. The String method renders it in compiler-style +// one-line form. +type Diagnostic = grammar.Diagnostic + +// Severity classifies a [Diagnostic]'s seriousness. The scan never aborts on a +// Warning or Hint; the caller decides policy. Compare against [SeverityError], +// [SeverityWarning] and [SeverityHint]. +type Severity = grammar.Severity + +// Code is a stable, machine-readable identifier for a class of [Diagnostic] +// (e.g. "validate.unsupported-go-type", "scan.ignored-by-tag"). Codes are +// grouped by prefix: parse.* (lexer/parser), validate.* (semantic), scan.* +// (scan environment). Callers may switch on it to filter or route diagnostics. +type Code = grammar.Code + +// Severity levels, ordered from most to least serious. +const ( + SeverityError = grammar.SeverityError + SeverityWarning = grammar.SeverityWarning + SeverityHint = grammar.SeverityHint +) diff --git a/fixtures/enhancements/unsupported-go-type/model.go b/fixtures/enhancements/unsupported-go-type/model.go new file mode 100644 index 00000000..436cf704 --- /dev/null +++ b/fixtures/enhancements/unsupported-go-type/model.go @@ -0,0 +1,21 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +// Package unsupportedgotype carries a model with a field whose Go type +// cannot be represented in Swagger 2.0. The scanner drops the field and +// records a validate.unsupported-go-type warning (the diagnostic that +// replaced the legacy stderr "unsupported Go type" log line). +package unsupportedgotype + +// Widget is a model with one representable field and one that codescan +// cannot translate. +// +// swagger:model Widget +type Widget struct { + // Name is a normal field. + Name string `json:"name"` + + // Weird is a complex number — Swagger 2.0 has no such type, so the + // field is dropped with a validate.unsupported-go-type warning. + Weird complex128 `json:"weird"` +} diff --git a/internal/builders/common/README.md b/internal/builders/common/README.md index 9f00786d..7e8555c6 100644 --- a/internal/builders/common/README.md +++ b/internal/builders/common/README.md @@ -8,8 +8,8 @@ The source files keep godoc concise; complex invariants, design trade-offs, and (`schema`, `parameters`, `responses`, `routes`, `operations`, `spec`). It owns the scanner context, the active declaration, the -parsed-block memoisation cache, the diagnostic accumulator, the -post-decl queue, and the slog logger. +parsed-block memoisation cache, the diagnostic accumulator, and the +post-decl queue. --- @@ -152,11 +152,6 @@ shared with the parameters/responses field-signal scanners) and These are real maintenance items the package author noted; they remain open for a future pass. -- **logger configurability.** `New` instantiates `slog.Default()`. - An option to accept a user-supplied `*slog.Logger` (level, - coloured output, structured fields) would let callers opt into a - consistent logging surface across builders. Currently every - builder's `Warn`/`Debug` writes through the global default. - **`ireturn` on `ParseBlock`.** The `nolint:ireturn` directive on `ParseBlock` carries because `grammar.Block` is a polymorphic interface — that's the documented return type. The lint could diff --git a/internal/builders/common/builder.go b/internal/builders/common/builder.go index b92ef1b4..afd665c2 100644 --- a/internal/builders/common/builder.go +++ b/internal/builders/common/builder.go @@ -11,7 +11,6 @@ package common import ( "go/ast" "go/token" - "log/slog" "github.com/go-openapi/codescan/internal/ifaces" "github.com/go-openapi/codescan/internal/parsers/grammar" @@ -35,22 +34,16 @@ type Builder struct { postDeclSet map[*ast.Ident]struct{} // dedup index keyed by EntityDecl.Ident diagnostics []grammar.Diagnostic blockCache map[*ast.CommentGroup][]grammar.Block - logger *slog.Logger } // New builds a [Builder] bound to ctx and decl. // -// The blockCache is pre-allocated empty; logger defaults to [slog.Default]. -// -// See [§quirks-open] for the planned configurability. -// -// [§quirks-open]: https://github.com/go-openapi/codescan/blob/master/internal/common/README.md#quirks-open +// The blockCache is pre-allocated empty. func New(ctx *scanner.ScanCtx, decl *scanner.EntityDecl) *Builder { return &Builder{ Ctx: ctx, Decl: decl, blockCache: make(map[*ast.CommentGroup][]grammar.Block), - logger: slog.Default(), } } @@ -62,16 +55,6 @@ func (s *Builder) PostDeclarations() []*scanner.EntityDecl { return s.postDecls } -// Warn writes a warning to the Builder's slog logger. -func (s *Builder) Warn(msg string, args ...any) { - s.logger.Warn(msg, args...) -} - -// Debug writes a debug message to the Builder's slog logger. -func (s *Builder) Debug(msg string, args ...any) { - s.logger.Debug(msg, args...) -} - // Diagnostics returns every grammar.Diagnostic accumulated by this Builder during a Build pass. // // Source order is preserved; no deduplication is applied. diff --git a/internal/builders/parameters/parameters.go b/internal/builders/parameters/parameters.go index 783f98c7..17659c79 100644 --- a/internal/builders/parameters/parameters.go +++ b/internal/builders/parameters/parameters.go @@ -14,7 +14,6 @@ import ( "github.com/go-openapi/codescan/internal/builders/schema" "github.com/go-openapi/codescan/internal/builders/validations" "github.com/go-openapi/codescan/internal/ifaces" - "github.com/go-openapi/codescan/internal/logger" "github.com/go-openapi/codescan/internal/parsers/grammar" "github.com/go-openapi/codescan/internal/scanner" oaispec "github.com/go-openapi/spec" @@ -66,7 +65,6 @@ func (p *Builder) Build(operations map[string]*oaispec.Operation) error { operations[opid] = operation operation.ID = opid } - logger.DebugLogf(p.Ctx.Debug(), "building parameters for: %s", opid) p.currentOpID = opid // analyze struct body for fields etc @@ -91,7 +89,6 @@ func (p *Builder) buildFromType(otpe types.Type, op *oaispec.Operation, seen map case *types.Named: return p.buildNamedType(tpe, op, seen) case *types.Alias: - logger.DebugLogf(p.Ctx.Debug(), "alias(parameters.buildFromType): got alias %v to %v", tpe, tpe.Rhs()) return p.buildAlias(tpe, op, seen) default: return fmt.Errorf("unhandled type (%T): %s: %w", otpe, tpe.String(), ErrParameters) @@ -107,7 +104,6 @@ func (p *Builder) buildNamedType(tpe *types.Named, op *oaispec.Operation, seen m switch stpe := o.Type().Underlying().(type) { case *types.Struct: - logger.DebugLogf(p.Ctx.Debug(), "build from named type %s: %T", o.Name(), tpe) if decl, found := p.Ctx.DeclForType(o.Type()); found { return p.buildFromStruct(decl, stpe, op, seen) } @@ -140,8 +136,6 @@ func (p *Builder) buildAlias(tpe *types.Alias, op *oaispec.Operation, seen map[s } func (p *Builder) buildFromField(fld *types.Var, tpe types.Type, typable ifaces.SwaggerTypable, seen map[string]oaispec.Parameter) error { - logger.DebugLogf(p.Ctx.Debug(), "build from field %s: %T", fld.Name(), tpe) - switch ftpe := tpe.(type) { case *types.Basic: return resolvers.SwaggerSchemaForType(ftpe.Name(), typable) @@ -160,7 +154,6 @@ func (p *Builder) buildFromField(fld *types.Var, tpe types.Type, typable ifaces. case *types.Named: return p.buildNamedField(ftpe, typable) case *types.Alias: - logger.DebugLogf(p.Ctx.Debug(), "alias(parameters.buildFromField): got alias %v to %v", ftpe, ftpe.Rhs()) return p.buildFieldAlias(ftpe, typable, fld, seen) default: return fmt.Errorf("unknown type for %s: %T: %w", fld.String(), fld.Type(), ErrParameters) @@ -499,13 +492,11 @@ func (p *Builder) resolveParamType(signals fieldDocSignals, fld *types.Var, name // Returns the parameter name if the field was processed, or "" if it was skipped. func (p *Builder) processParamField(fld *types.Var, decl *scanner.EntityDecl, seen map[string]oaispec.Parameter) (string, error) { if !fld.Exported() { - logger.DebugLogf(p.Ctx.Debug(), "skipping field %s because it's not exported", fld.Name()) return "", nil } afld := resolvers.FindASTField(decl.File, fld.Pos()) if afld == nil { - logger.DebugLogf(p.Ctx.Debug(), "can't find source associated with %s", fld.String()) return "", nil } diff --git a/internal/builders/responses/responses.go b/internal/builders/responses/responses.go index 3bb0dbb1..02520306 100644 --- a/internal/builders/responses/responses.go +++ b/internal/builders/responses/responses.go @@ -14,7 +14,6 @@ import ( "github.com/go-openapi/codescan/internal/builders/resolvers" "github.com/go-openapi/codescan/internal/builders/schema" "github.com/go-openapi/codescan/internal/ifaces" - "github.com/go-openapi/codescan/internal/logger" "github.com/go-openapi/codescan/internal/parsers/grammar" "github.com/go-openapi/codescan/internal/scanner" oaispec "github.com/go-openapi/spec" @@ -68,7 +67,6 @@ func (r *Builder) Build(responses map[string]oaispec.Response) error { name, _ := r.Decl.ResponseNames() response := responses[name] - logger.DebugLogf(r.Ctx.Debug(), "building response: %s", name) // Cross-ref linkage: anchor this response's headers and in:body schema under // /responses/{name}. The response name is known here (no deferral, unlike a @@ -156,8 +154,6 @@ func (r *Builder) bodyPathFor(typable ifaces.SwaggerTypable) string { } func (r *Builder) buildFromField(fld *types.Var, tpe types.Type, typable ifaces.SwaggerTypable, seen map[string]bool) error { - logger.DebugLogf(r.Ctx.Debug(), "build from field %s: %T", fld.Name(), tpe) - switch ftpe := tpe.(type) { case *types.Basic: return resolvers.SwaggerSchemaForType(ftpe.Name(), typable) @@ -178,7 +174,6 @@ func (r *Builder) buildFromField(fld *types.Var, tpe types.Type, typable ifaces. case *types.Named: return r.buildNamedField(ftpe, typable) case *types.Alias: - logger.DebugLogf(r.Ctx.Debug(), "alias(responses.buildFromField): got alias %v to %v", ftpe, ftpe.Rhs()) return r.buildFieldAlias(ftpe, typable, fld, seen) default: return fmt.Errorf("unknown type for %s: %T: %w", fld.String(), fld.Type(), ErrResponses) @@ -255,7 +250,6 @@ func (r *Builder) buildFromType(otpe types.Type, resp *oaispec.Response, seen ma case *types.Named: return r.buildNamedType(tpe, resp, seen) case *types.Alias: - logger.DebugLogf(r.Ctx.Debug(), "alias(responses.buildFromType): got alias %v to %v", tpe, tpe.Rhs()) return r.buildAlias(tpe, resp, seen) default: return fmt.Errorf("anonymous types are currently not supported for responses: %w", ErrResponses) @@ -271,7 +265,6 @@ func (r *Builder) buildNamedType(tpe *types.Named, resp *oaispec.Response, seen switch stpe := o.Type().Underlying().(type) { case *types.Struct: - logger.DebugLogf(r.Ctx.Debug(), "build from type %s: %T", o.Name(), tpe) if decl, found := r.Ctx.DeclForType(o.Type()); found { return r.buildFromStruct(decl, stpe, resp, seen) } @@ -431,7 +424,6 @@ func (r *Builder) buildFromStruct(decl *scanner.EntityDecl, tpe *types.Struct, r } if fld.Anonymous() { - logger.DebugLogf(r.Ctx.Debug(), "skipping anonymous field") continue } @@ -500,20 +492,17 @@ func (r *Builder) buildBodyEmbed(fld *types.Var, resp *oaispec.Response, seen ma func (r *Builder) processResponseField(fld *types.Var, decl *scanner.EntityDecl, resp *oaispec.Response, seen map[string]bool) error { if !fld.Exported() { - logger.DebugLogf(r.Ctx.Debug(), "skipping field %s because it's not exported", fld.Name()) return nil } afld := resolvers.FindASTField(decl.File, fld.Pos()) if afld == nil { - logger.DebugLogf(r.Ctx.Debug(), "can't find source associated with %s", fld.String()) return nil } signals := scanFieldDocSignals(r.ParseBlocks(afld.Doc), afld.Doc) if signals.ignored { - logger.DebugLogf(r.Ctx.Debug(), "field %v is deliberately ignored", fld) return nil } @@ -593,7 +582,6 @@ func (r *Builder) processResponseField(fld *types.Var, decl *scanner.EntityDecl, resp.Schema = &oaispec.Schema{} resp.Schema.Typed("file", "") } else { - logger.DebugLogf(r.Ctx.Debug(), "build response %v (%v) (not a file)", fld, fld.Type()) var refAttempted bool if err := r.buildFromField(fld, fld.Type(), responseTypable{ in: in, diff --git a/internal/builders/schema/README.md b/internal/builders/schema/README.md index 067a1aed..6d8a2d1e 100644 --- a/internal/builders/schema/README.md +++ b/internal/builders/schema/README.md @@ -436,8 +436,8 @@ discriminator hint downstream go-swagger consumes. Strips pointers (recurses), routes `*types.Named` through `buildNamedAllOf`, routes `*types.Alias` through `buildAlias`. Any -other input is dropped silently with a `logger.UnsupportedTypeKind` -warning — parity with v1, which had no surface for non-Named / +other input is dropped with a `validate.unsupported-go-type` Warning +diagnostic (`warnUnsupportedGoType`) — v1 had no surface for non-Named / non-Alias allOf members. ### `buildNamedAllOf` — symmetric arm dispatch diff --git a/internal/builders/schema/allof.go b/internal/builders/schema/allof.go index 855c9ed5..58012e9d 100644 --- a/internal/builders/schema/allof.go +++ b/internal/builders/schema/allof.go @@ -9,7 +9,6 @@ import ( "go/types" "github.com/go-openapi/codescan/internal/builders/resolvers" - "github.com/go-openapi/codescan/internal/logger" "github.com/go-openapi/codescan/internal/scanner" oaispec "github.com/go-openapi/spec" ) @@ -156,7 +155,7 @@ func (s *Builder) buildAllOf(tpe types.Type, schema *oaispec.Schema) error { tgt := NewTypable(schema, 0, s.skipExtensions) return s.buildAlias(ftpe, tgt) default: - logger.UnsupportedTypeKind("buildAllOf", ftpe) + s.warnUnsupportedGoType("buildAllOf", ftpe) return nil } } @@ -197,7 +196,7 @@ func (s *Builder) buildNamedAllOf(ftpe *types.Named, schema *oaispec.Schema) err case *types.Interface: return s.buildFromInterface(decl, utpe, schema, make(map[string]propOwner)) default: - logger.UnsupportedTypeKind("buildNamedAllOf", utpe) + s.warnUnsupportedGoType("buildNamedAllOf", utpe) return nil } } diff --git a/internal/builders/schema/embedded.go b/internal/builders/schema/embedded.go index 2683b0ac..6302e2a2 100644 --- a/internal/builders/schema/embedded.go +++ b/internal/builders/schema/embedded.go @@ -6,10 +6,8 @@ package schema import ( "go/ast" "go/types" - "log/slog" "github.com/go-openapi/codescan/internal/builders/resolvers" - "github.com/go-openapi/codescan/internal/logger" "github.com/go-openapi/codescan/internal/scanner" oaispec "github.com/go-openapi/spec" ) @@ -60,7 +58,7 @@ func (s *Builder) buildEmbedded(tpe types.Type, schema *oaispec.Schema, nameByJS // these promotes nothing. return s.buildEmbedded(types.Unalias(ftpe), schema, nameByJSON) default: - logger.UnsupportedTypeKind("buildEmbedded", ftpe) + s.warnUnsupportedGoType("buildEmbedded", ftpe) return nil } } @@ -78,7 +76,7 @@ func (s *Builder) buildEmbedded(tpe types.Type, schema *oaispec.Schema, nameByJS // diagnostic. func (s *Builder) buildNamedEmbedded(tpe *types.Named, schema *oaispec.Schema, nameByJSON map[string]propOwner) error { if resolvers.UnsupportedBuiltin(tpe) { - s.Warn("skipped unsupported builtin", slog.Any("type", tpe)) + s.warnUnsupportedGoType("buildNamedEmbedded", tpe) return nil } @@ -113,7 +111,7 @@ func (s *Builder) buildNamedEmbedded(tpe *types.Named, schema *oaispec.Schema, n defer s.enterEmbed()() return s.buildFromInterface(decl, utpe, schema, nameByJSON) default: - logger.UnsupportedTypeKind("buildNamedEmbedded", utpe) + s.warnUnsupportedGoType("buildNamedEmbedded", utpe) return nil } } @@ -166,7 +164,7 @@ func (s *Builder) processEmbeddedType(fld types.Type, flist []*ast.Field, decl * schema.AddToAllOf(aliasedSchema) } default: - logger.UnsupportedTypeKind("buildNamedInterface.allOf", ftpe) + s.warnUnsupportedGoType("buildNamedInterface.allOf", ftpe) } return fieldHasAllOf, nil diff --git a/internal/builders/schema/schema.go b/internal/builders/schema/schema.go index 131fc8ea..5c317386 100644 --- a/internal/builders/schema/schema.go +++ b/internal/builders/schema/schema.go @@ -6,8 +6,8 @@ package schema import ( "fmt" "go/ast" + "go/token" "go/types" - "log/slog" "github.com/go-openapi/swag/mangling" @@ -122,6 +122,30 @@ func (s *Builder) SetDiscovered(discovered []*scanner.EntityDecl) { s.discovered = discovered } +// declPos returns the position of the declaration being built, used as a +// coarse fallback for diagnostics raised deep in the type dispatch where no +// finer AST node is in scope. Returns the zero Position when unavailable. +func (s *Builder) declPos() token.Position { + if s.Decl == nil || s.Decl.Ident == nil { + return token.Position{} + } + + return s.Ctx.PosOf(s.Decl.Ident.Pos()) +} + +// warnUnsupportedGoType records a [grammar.CodeUnsupportedGoType] Warning that +// tpe could not be translated to a Swagger 2.0 construct and was dropped from +// the spec. where names the dispatch site (function / switch arm) for triage. +// tpe is any (a types.Type, *types.TypeName, ifaces.Objecter, …); the message +// renders its dynamic type and value. +func (s *Builder) warnUnsupportedGoType(where string, tpe any) { + s.RecordDiagnostic(grammar.Warnf( + s.declPos(), + grammar.CodeUnsupportedGoType, + "%s: unsupported Go type %[2]T (%[2]v); skipping", where, tpe, + )) +} + // buildFromDecl emits the schema for a top-level type declaration // (named or alias). // @@ -190,7 +214,7 @@ func (s *Builder) buildFromDecl(schema *oaispec.Schema) error { } return s.buildDeclAlias(tpe, NewTypable(schema, 0, s.skipExtensions)) default: - s.Warn("unsupported Go type. Skipping", slog.Any("type", tpe)) + s.warnUnsupportedGoType("buildFromDecl", tpe) return nil } } @@ -292,7 +316,7 @@ func (s *Builder) buildDeclAlias(tpe *types.Alias, target ifaces.SwaggerTypable) case *types.Alias: ro := rtpe.Obj() if resolvers.UnsupportedBuiltin(rtpe) { - s.Warn("skipped unsupported builtin", slog.Any("type", rtpe)) + s.warnUnsupportedGoType("buildDeclAlias", rtpe) return nil } @@ -338,7 +362,7 @@ func (s *Builder) buildFromType(tpe types.Type, target ifaces.SwaggerTypable) er switch titpe := tpe.(type) { case *types.Basic: if resolvers.UnsupportedBuiltinType(titpe) { - s.Warn("skipped unsupported builtin", slog.Any("type", tpe)) + s.warnUnsupportedGoType("buildFromType", tpe) return nil } return resolvers.SwaggerSchemaForType(titpe.String(), target) @@ -361,7 +385,7 @@ func (s *Builder) buildFromType(tpe types.Type, target ifaces.SwaggerTypable) er default: // Unknown kind (TypeParam, Chan, Signature, Union, future additions). // Warn-and-skip — panicking would degrade UX on user code we can't inspect. - s.Warn("unsupported Go type. Skipping", slog.Any("type", tpe)) + s.warnUnsupportedGoType("buildFromType", tpe) return nil } } @@ -375,7 +399,7 @@ func (s *Builder) buildFromType(tpe types.Type, target ifaces.SwaggerTypable) er // See [§aliases](./README.md#aliases). func (s *Builder) buildAlias(tpe *types.Alias, target ifaces.SwaggerTypable) error { if resolvers.UnsupportedBuiltinType(tpe) { - s.Warn("skipped unsupported builtin", slog.Any("type", tpe)) + s.warnUnsupportedGoType("buildAlias", tpe) return nil } @@ -413,7 +437,7 @@ func (s *Builder) buildAlias(tpe *types.Alias, target ifaces.SwaggerTypable) err // See [§dispatch-table](./README.md#dispatch-table) and [§dissolve-named](./README.md#dissolve-named). func (s *Builder) buildNamedType(titpe *types.Named, target ifaces.SwaggerTypable) error { if resolvers.UnsupportedBuiltin(titpe) { - s.Warn("skipped unsupported builtin", slog.Any("type", titpe)) + s.warnUnsupportedGoType("buildNamedType", titpe) return nil } @@ -480,7 +504,7 @@ func (s *Builder) buildNamedType(titpe *types.Named, target ifaces.SwaggerTypabl case *types.Basic: if resolvers.UnsupportedBuiltinType(utitpe) { - s.Warn("skipped unsupported builtin", slog.Any("type", tio)) + s.warnUnsupportedGoType("buildNamedType", tio) return nil } if !refModel && s.classifierNamedBasic(cmt, pkg, utitpe, target, tio.Name()) { @@ -499,7 +523,7 @@ func (s *Builder) buildNamedType(titpe *types.Named, target ifaces.SwaggerTypabl return s.resolveRefOr(tio, target, nil) default: - s.Warn("unsupported Go type. Skipping", slog.Any("type", utitpe)) + s.warnUnsupportedGoType("buildNamedType", utitpe) return nil } } @@ -589,7 +613,7 @@ func (s *Builder) annotateSchema(schema *oaispec.Schema) func() { // Returns true if the caller should skip processing. func (s *Builder) guardDecl(tpe ifaces.Objecter) (skip bool) { if resolvers.UnsupportedBuiltin(tpe) { - s.Warn("skipped unsupported builtin", slog.Any("type", tpe)) + s.warnUnsupportedGoType("guardDecl", tpe) return true } resolvers.MustNotBeABuiltinType(tpe.Obj()) // invariant diff --git a/internal/builders/schema/walker_classifiers.go b/internal/builders/schema/walker_classifiers.go index fbfb0483..44039b93 100644 --- a/internal/builders/schema/walker_classifiers.go +++ b/internal/builders/schema/walker_classifiers.go @@ -7,14 +7,12 @@ import ( "go/ast" "go/token" "go/types" - "log" "reflect" "strconv" "strings" "github.com/go-openapi/codescan/internal/builders/resolvers" "github.com/go-openapi/codescan/internal/ifaces" - "github.com/go-openapi/codescan/internal/logger" "github.com/go-openapi/codescan/internal/parsers/grammar" "github.com/go-openapi/codescan/internal/scanner" "golang.org/x/tools/go/packages" @@ -177,11 +175,11 @@ func (s *Builder) classifierNamedBasic(cg *ast.CommentGroup, pkg *packages.Packa // the type-resolution engine can still decide what to do with // the underlying Go type (it may be a model, an alias, a // strfmt, …) rather than dropping the field entirely. - log.Printf("WARNING: swagger:enum %s: no matching const values found; dropping enum semantics", enumName) + s.RecordDiagnostic(grammar.Warnf(s.declPos(), grammar.CodeInvalidEnumOption, + "swagger:enum %s: no matching const values found; enum semantics dropped", enumName)) } - if defaultName, ok := s.findAnnotationArg(cg, grammar.AnnDefaultName); ok { - logger.DebugLogf(s.Ctx.Debug(), "default name: %s", defaultName) + if _, ok := s.findAnnotationArg(cg, grammar.AnnDefaultName); ok { return true } diff --git a/internal/builders/spec/reduce.go b/internal/builders/spec/reduce.go index ead6ea50..73599a10 100644 --- a/internal/builders/spec/reduce.go +++ b/internal/builders/spec/reduce.go @@ -10,7 +10,6 @@ import ( "strconv" "strings" - "github.com/go-openapi/codescan/internal/logger" "github.com/go-openapi/codescan/internal/parsers/grammar" oaispec "github.com/go-openapi/spec" "github.com/go-openapi/swag/mangling" @@ -121,7 +120,6 @@ func (s *Builder) computeNameReductions() map[string]string { taken[chosen[k]] = struct{}{} } s.diagnoseCollision(leaf, keys, chosen) - s.noteBudget(leaf, score) } return renames } @@ -354,23 +352,9 @@ func (s *Builder) diagnoseCollision(leaf string, keys []string, chosen map[strin leaf, len(keys), strings.Join(mappings, ", "))) } -// noteBudget records — under Debug only — when a collision group's best -// concat exceeds the configured readability budget. This is the seam for -// the hierarchical fallback (name-identity Stage 3 / K3): today it just -// logs; K3 will, when score > budget, replace the flat concat with a -// hierarchical name instead of merely noting it. -func (s *Builder) noteBudget(leaf string, score float64) { - budget := s.concatBudget() - if score > budget { - logger.DebugLogf(s.ctx.Debug(), - "reduce: concat names for %q score %.3f > budget %.3f; "+ - "enable EmitHierarchicalNames for the nested fallback", leaf, score, budget) - } -} - // rekeyDefinitions moves each definition from its old key to its new // name. A clash on the new name cannot occur for unique leaves; it is -// guarded defensively and logged under Debug rather than silently +// guarded defensively, skipping the rekey rather than silently // overwriting. func (s *Builder) rekeyDefinitions(renames map[string]string) { for old, nw := range renames { @@ -382,8 +366,6 @@ func (s *Builder) rekeyDefinitions(renames map[string]string) { continue } if _, clash := s.input.Definitions[nw]; clash { - logger.DebugLogf(s.ctx.Debug(), - "reduce: refusing to rekey %q -> %q: target already exists", old, nw) continue } s.input.Definitions[nw] = schema diff --git a/internal/integration/coverage_diagnostics_from_logger_test.go b/internal/integration/coverage_diagnostics_from_logger_test.go new file mode 100644 index 00000000..adbe6c92 --- /dev/null +++ b/internal/integration/coverage_diagnostics_from_logger_test.go @@ -0,0 +1,84 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package integration_test + +import ( + "testing" + + "github.com/go-openapi/codescan" + "github.com/go-openapi/codescan/internal/parsers/grammar" + "github.com/go-openapi/codescan/internal/scantest" + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +// TestDiagnostics_UnsupportedGoType locks the Warning that replaced the legacy +// stderr "unsupported Go type" log line. A struct field typed complex128 has no +// Swagger 2.0 representation: its schema is left untyped (the build skips it) +// and a validate.unsupported-go-type Warning is delivered through OnDiagnostic +// (never to stderr). +func TestDiagnostics_UnsupportedGoType(t *testing.T) { + var diags []grammar.Diagnostic + doc, err := codescan.Run(&codescan.Options{ + Packages: []string{"./enhancements/unsupported-go-type/..."}, + WorkDir: scantest.FixturesDir(), + ScanModels: true, + OnDiagnostic: func(d grammar.Diagnostic) { diags = append(diags, d) }, + }) + require.NoError(t, err) + require.NotNil(t, doc) + + // The representable field is typed; the unrepresentable one is left untyped + // (the builder skipped it after warning rather than guessing a type). + widget, ok := doc.Definitions["Widget"] + require.True(t, ok, "Widget must be discovered") + require.Contains(t, widget.Properties, "name") + assert.Equal(t, "string", widget.Properties["name"].Type[0]) + require.Contains(t, widget.Properties, "weird") + assert.Empty(t, widget.Properties["weird"].Type, "complex128 has no Swagger type") + + // A Warning diagnostic names the offending type. The position falls back to + // the declaration (no finer node is threaded into the type dispatch). + var found bool + for _, d := range diags { + if d.Code == grammar.CodeUnsupportedGoType { + assert.Equal(t, grammar.SeverityWarning, d.Severity) + assert.Contains(t, d.Message, "complex128") + found = true + } + } + assert.True(t, found, "a validate.unsupported-go-type Warning must be emitted") +} + +// TestDiagnostics_IgnoredByTag locks the Hint that replaced the legacy debug +// log line for tag-filtered routes. Excluding the "orders" tag drops the +// petstore's order routes and surfaces a scan.ignored-by-tag Hint per dropped +// route through OnDiagnostic. +func TestDiagnostics_IgnoredByTag(t *testing.T) { + var diags []grammar.Diagnostic + doc, err := codescan.Run(&codescan.Options{ + Packages: []string{"./goparsing/petstore/..."}, + WorkDir: scantest.FixturesDir(), + ExcludeTags: []string{"orders"}, + OnDiagnostic: func(d grammar.Diagnostic) { diags = append(diags, d) }, + }) + require.NoError(t, err) + require.NotNil(t, doc) + + // The excluded routes are absent from the spec. + for path := range doc.Paths.Paths { + assert.NotContains(t, path, "/orders", "order routes must be excluded") + } + + // Hints (not Warnings/Errors) were emitted for the dropped routes. + var hints int + for _, d := range diags { + if d.Code == grammar.CodeIgnoredByTag { + assert.Equal(t, grammar.SeverityHint, d.Severity) + assert.Contains(t, d.Message, "tag rules") + hints++ + } + } + assert.Positive(t, hints, "at least one scan.ignored-by-tag Hint must be emitted") +} diff --git a/internal/logger/debug.go b/internal/logger/debug.go deleted file mode 100644 index 3b2e50ef..00000000 --- a/internal/logger/debug.go +++ /dev/null @@ -1,32 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers -// SPDX-License-Identifier: Apache-2.0 - -package logger - -import ( - "fmt" - "go/types" - "log" -) - -// logCallerDepth is the caller depth for log.Output. -const logCallerDepth = 2 - -func DebugLogf(debug bool, format string, args ...any) { - if debug { - _ = log.Output(logCallerDepth, fmt.Sprintf(format, args...)) - } -} - -// UnsupportedTypeKind emits a uniform warning when a go/types kind -// cannot be translated to a Swagger 2.0 construct. -// -// The scanner runs on arbitrary user code in uncontrolled environments, -// so encountering an unsupported kind must not panic — we log and let -// the caller skip. `where` is the dispatcher site (typically the -// function name) so a future go/types evolution — e.g. a new kind we -// haven't modeled — surfaces one grep-able diagnostic instead of -// disappearing behind a silent default. -func UnsupportedTypeKind(where string, tpe types.Type) { - log.Printf("WARNING: %s: unsupported Go type kind %[2]T (%[2]v); skipping", where, tpe) -} diff --git a/internal/parsers/grammar/diagnostic.go b/internal/parsers/grammar/diagnostic.go index fc8e3dca..1206e746 100644 --- a/internal/parsers/grammar/diagnostic.go +++ b/internal/parsers/grammar/diagnostic.go @@ -98,6 +98,16 @@ const ( // `inline`, or the deprecated `swagger:alias` annotation. CodeDeprecated Code = "validate.deprecated" + // CodeUnsupportedGoType fires when a Go type, a `go/types` kind, or a + // builtin cannot be translated to a Swagger 2.0 construct and is + // therefore dropped from the spec. The scanner runs on arbitrary user + // code, so an unmodeled construct must not panic — it is skipped and + // surfaced as a Warning (real data loss, but the scan continues). The + // message names the construct (and the dispatch site) so a future + // go/types evolution surfaces one grep-able diagnostic instead of + // vanishing behind a silent default. + CodeUnsupportedGoType Code = "validate.unsupported-go-type" + // CodeDuplicateModelName fires when two distinct Go types in the // SAME package claim the same definition name (necessarily via a // `swagger:model ` override, since Go type names are unique @@ -148,6 +158,17 @@ const ( // go-swagger/go-swagger#2886. CodeInternalPanic Code = "scan.internal-panic" + // CodeIgnoredByRules fires when a package is skipped because it does not + // pass the caller's Include/Exclude package rules. Informational (Hint): + // the omission is the caller's own configuration, surfaced to aid + // "why is my package missing" triage. + CodeIgnoredByRules Code = "scan.ignored-by-rules" + + // CodeIgnoredByTag fires when a route or operation is skipped because its + // tags do not pass the caller's IncludeTags/ExcludeTags rules. + // Informational (Hint), like CodeIgnoredByRules. + CodeIgnoredByTag Code = "scan.ignored-by-tag" + // CodeDroppedRefSibling fires when SkipAllOfCompounding is set and a // $ref'd struct field carries sibling decoration (description, // validations, vendor extensions, externalDocs) that cannot ride a diff --git a/internal/scanner/index.go b/internal/scanner/index.go index 1f245298..01bc704f 100644 --- a/internal/scanner/index.go +++ b/internal/scanner/index.go @@ -6,12 +6,12 @@ package scanner import ( "fmt" "go/ast" + "go/token" "go/types" - "log" "regexp" - "github.com/go-openapi/codescan/internal/logger" "github.com/go-openapi/codescan/internal/parsers" + "github.com/go-openapi/codescan/internal/parsers/grammar" "golang.org/x/tools/go/packages" ) @@ -65,9 +65,14 @@ func WithTransparentAliases(enabled bool) TypeIndexOption { } } -func WithDebug(enabled bool) TypeIndexOption { +// WithOnDiagnostic wires the consumer's diagnostic sink so the index can +// surface scan-environment observations (e.g. a package or route omitted by +// the caller's own include/exclude rules) as informational Hints. The index +// is built before the ScanCtx exists, so it reports through the raw callback +// directly, exactly as detectDegradedLoad does. +func WithOnDiagnostic(cb func(grammar.Diagnostic)) TypeIndexOption { return func(a *TypeIndex) { - a.debug = enabled + a.onDiagnostic = cb } } @@ -88,7 +93,24 @@ type TypeIndex struct { setXNullableForPointers bool refAliases bool transparentAliases bool - debug bool + onDiagnostic func(grammar.Diagnostic) +} + +// emit delivers d to the consumer's diagnostic sink when one is wired. No-op +// otherwise. The index is built before the ScanCtx exists, so it reports +// through the raw callback (no dedup), exactly as detectNodes-level and +// detectDegradedLoad observations do. +func (a *TypeIndex) emit(d grammar.Diagnostic) { + if a.onDiagnostic == nil { + return + } + a.onDiagnostic(d) +} + +// emitHintf delivers an informational Hint with no source position (the index +// observes whole-package / whole-route omissions, not a single token). +func (a *TypeIndex) emitHintf(code grammar.Code, format string, args ...any) { + a.emit(grammar.Hintf(token.Position{}, code, format, args...)) } func NewTypeIndex(pkgs []*packages.Package, opts ...TypeIndexOption) (*TypeIndex, error) { @@ -126,7 +148,8 @@ func (a *TypeIndex) build(pkgs []*packages.Package) error { func (a *TypeIndex) processPackage(pkg *packages.Package) error { if !shouldAcceptPkg(pkg.PkgPath, a.includePkgs, a.excludePkgs) { - logger.DebugLogf(a.debug, "package %s is ignored due to rules", pkg.Name) + a.emitHintf(grammar.CodeIgnoredByRules, + "package %q is omitted by the include/exclude package rules", pkg.PkgPath) return nil } @@ -170,7 +193,8 @@ func (a *TypeIndex) collectOperationPathAnnotations(comments []*ast.CommentGroup } if !shouldAcceptTag(pp.Tags, a.includeTags, a.excludeTags) { - logger.DebugLogf(a.debug, "operation %s %s is ignored due to tag rules", pp.Method, pp.Path) + a.emitHintf(grammar.CodeIgnoredByTag, + "operation %s %s is omitted by the include/exclude tag rules", pp.Method, pp.Path) continue } dst = append(dst, pp) @@ -187,7 +211,8 @@ func (a *TypeIndex) collectRoutePathAnnotations(comments []*ast.CommentGroup, ds } if !shouldAcceptTag(pp.Tags, a.includeTags, a.excludeTags) { - logger.DebugLogf(a.debug, "operation %s %s is ignored due to tag rules", pp.Method, pp.Path) + a.emitHintf(grammar.CodeIgnoredByTag, + "route %s %s is omitted by the include/exclude tag rules", pp.Method, pp.Path) continue } dst = append(dst, pp) @@ -222,22 +247,17 @@ func (a *TypeIndex) processDecl(pkg *packages.Package, file *ast.File, n node, g for _, sp := range gd.Specs { switch ts := sp.(type) { case *ast.ValueSpec: - logger.DebugLogf(a.debug, "saw value spec: %v", ts.Names) return case *ast.ImportSpec: - logger.DebugLogf(a.debug, "saw import spec: %v", ts.Name) return case *ast.TypeSpec: def, ok := pkg.TypesInfo.Defs[ts.Name] if !ok { - logger.DebugLogf(a.debug, "couldn't find type info for %s", ts.Name) continue } nt, isNamed := def.Type().(*types.Named) at, isAliased := def.Type().(*types.Alias) if !isNamed && !isAliased { - logger.DebugLogf(a.debug, "%s is not a named or aliased type but a %T", ts.Name, def.Type()) - continue } @@ -263,12 +283,6 @@ func (a *TypeIndex) processDecl(pkg *packages.Package, file *ast.File, n node, g a.Parameters = append(a.Parameters, decl) case n&responseNode != 0 && decl.HasResponseAnnotation(): a.Responses = append(a.Responses, decl) - default: - logger.DebugLogf(a.debug, - "type %q skipped because it is not tagged as a model, a parameter or a response. %s", - decl.Obj().Name(), - "It may reenter the scope because it is a discovered dependency", - ) } } } @@ -330,7 +344,7 @@ func (a *TypeIndex) detectNodes(file *ast.File) (node, error) { n |= operationNode case "model": // annotation keyword matched from swagger comment. n |= modelNode - warnMalformedStructName(annotation, cline.Text) + a.warnMalformedStructName(annotation, cline.Text) if err := checkStructConflict(&seenStruct, annotation, cline.Text); err != nil { return 0, err } @@ -343,7 +357,7 @@ func (a *TypeIndex) detectNodes(file *ast.File) (node, error) { } case "response": n |= responseNode - warnMalformedStructName(annotation, cline.Text) + a.warnMalformedStructName(annotation, cline.Text) if err := checkStructConflict(&seenStruct, annotation, cline.Text); err != nil { return 0, err } @@ -360,25 +374,27 @@ func (a *TypeIndex) detectNodes(file *ast.File) (node, error) { return n, nil } -// warnMalformedStructName emits a warning when a single-name struct +// warnMalformedStructName emits a Warning diagnostic when a single-name struct // marker (swagger:model / swagger:response) on line carries a name that is // not a plain identifier — e.g. a package-qualified "utils.Error" // (go-swagger#874). Such names are JSON labels, not Go-qualified // identifiers; the strict override matcher rejects them and the marker is -// ignored. Warning rather than silently dropping it gives the author a -// clue. The type's package is resolved automatically, so a plain name +// ignored. The diagnostic gives the author a clue rather than silently +// dropping it. The type's package is resolved automatically, so a plain name // suffices regardless of which package the type lives in. -func warnMalformedStructName(annotation, line string) { +func (a *TypeIndex) warnMalformedStructName(annotation, line string) { switch annotation { case "model": if bad, ok := parsers.MalformedModelName(line); ok { - log.Printf("WARNING: swagger:model name %q is not a plain identifier "+ - "(definition names are JSON labels, not Go-qualified); annotation ignored", bad) + a.emit(grammar.Warnf(token.Position{}, grammar.CodeInvalidAnnotation, + "swagger:model name %q is not a plain identifier "+ + "(definition names are JSON labels, not Go-qualified); annotation ignored", bad)) } case "response": if bad, ok := parsers.MalformedResponseName(line); ok { - log.Printf("WARNING: swagger:response name %q is not a plain identifier "+ - "(response names are JSON labels, not Go-qualified); annotation ignored", bad) + a.emit(grammar.Warnf(token.Position{}, grammar.CodeInvalidAnnotation, + "swagger:response name %q is not a plain identifier "+ + "(response names are JSON labels, not Go-qualified); annotation ignored", bad)) } } } diff --git a/internal/scanner/options.go b/internal/scanner/options.go index a725b9a3..4cdb30b3 100644 --- a/internal/scanner/options.go +++ b/internal/scanner/options.go @@ -178,7 +178,17 @@ type Options struct { // See go-swagger/go-swagger#2626. SingleLineCommentAsDescription bool - Debug bool // enable verbose debug logging during scanning + // Debug is deprecated and has no effect. + // + // It formerly enabled verbose debug logging to stderr during scanning. + // That logger was retired: scan-time observations now flow exclusively + // through OnDiagnostic (which the caller routes to a logger of their + // choice), and codescan no longer writes to stdout/stderr — keeping it + // usable from a TUI or a WASI/WASM host. + // + // Deprecated: wire OnDiagnostic instead. This field is retained for API + // compatibility and is ignored. + Debug bool // OnDiagnostic, when non-nil, is invoked for every diagnostic the // builder layer records (lexer/parser warnings, semantic-validation diff --git a/internal/scanner/scan_context.go b/internal/scanner/scan_context.go index 87c5e546..94a24076 100644 --- a/internal/scanner/scan_context.go +++ b/internal/scanner/scan_context.go @@ -10,12 +10,10 @@ import ( "go/token" "go/types" "iter" - "log" "maps" "slices" "strings" - "github.com/go-openapi/codescan/internal/logger" "github.com/go-openapi/codescan/internal/parsers" "github.com/go-openapi/codescan/internal/parsers/grammar" "golang.org/x/tools/go/packages" @@ -41,9 +39,8 @@ const ( ) type ScanCtx struct { - pkgs []*packages.Package - app *TypeIndex - debug bool + pkgs []*packages.Package + app *TypeIndex opts *Options @@ -86,17 +83,16 @@ func NewScanCtx(opts *Options) (*ScanCtx, error) { WithXNullableForPointers(opts.SetXNullableForPointers), WithRefAliases(opts.RefAliases), WithTransparentAliases(opts.TransparentAliases), - WithDebug(opts.Debug), + WithOnDiagnostic(opts.OnDiagnostic), ) if err != nil { return nil, err } return &ScanCtx{ - pkgs: pkgs, - app: app, - debug: opts.Debug, - opts: opts, + pkgs: pkgs, + app: app, + opts: opts, }, nil } @@ -258,10 +254,6 @@ func (s *ScanCtx) PosOf(p token.Pos) token.Position { return fset.Position(p) } -func (s *ScanCtx) Debug() bool { - return s.debug -} - // diagKey identifies a diagnostic by its source location and content, for // suppressing exact duplicates over the lifetime of one scan. type diagKey struct { @@ -458,14 +450,12 @@ func (s *ScanCtx) FindDecl(pkgPath, name string) (*EntityDecl, bool) { def, ok := pkg.TypesInfo.Defs[ts.Name] if !ok { - logger.DebugLogf(s.debug, "couldn't find type info for %s", ts.Name) continue } nt, isNamed := def.Type().(*types.Named) at, isAliased := def.Type().(*types.Alias) if !isNamed && !isAliased { - logger.DebugLogf(s.debug, "%s is not a named or an aliased type but a %T", ts.Name, def.Type()) continue } @@ -602,7 +592,8 @@ func (s *ScanCtx) DeclForType(t types.Type) (*EntityDecl, bool) { case *types.Alias: return s.FindDecl(tpe.Obj().Pkg().Path(), tpe.Obj().Name()) default: - log.Printf("WARNING: unknown type to find the package for [%T]: %s", t, t.String()) + s.EmitDiagnostic(grammar.Warnf(token.Position{}, grammar.CodeUnsupportedGoType, + "unknown Go type %[1]T (%[1]v); cannot resolve its declaring source", t)) return nil, false } @@ -624,7 +615,8 @@ func (s *ScanCtx) PkgForType(t types.Type) (*packages.Package, bool) { v, ok := s.app.AllPackages[tpe.Obj().Pkg().Path()] return v, ok default: - log.Printf("WARNING: unknown type to find the package for [%T]: %s", t, t.String()) + s.EmitDiagnostic(grammar.Warnf(token.Position{}, grammar.CodeUnsupportedGoType, + "unknown Go type %[1]T (%[1]v); cannot resolve its declaring package", t)) return nil, false } } diff --git a/internal/scanner/scan_context_test.go b/internal/scanner/scan_context_test.go index fe58b59f..36e1d35e 100644 --- a/internal/scanner/scan_context_test.go +++ b/internal/scanner/scan_context_test.go @@ -39,7 +39,6 @@ func TestScanCtx_OptionAccessors(t *testing.T) { assert.False(t, sctx.SetXNullableForPointers()) assert.False(t, sctx.TransparentAliases()) assert.False(t, sctx.RefAliases()) - assert.False(t, sctx.Debug()) } func TestScanCtx_OptionAccessors_Enabled(t *testing.T) { @@ -51,7 +50,6 @@ func TestScanCtx_OptionAccessors_Enabled(t *testing.T) { SetXNullableForPointers: true, TransparentAliases: true, RefAliases: true, - Debug: true, }) require.NoError(t, err) @@ -60,7 +58,6 @@ func TestScanCtx_OptionAccessors_Enabled(t *testing.T) { assert.True(t, sctx.SetXNullableForPointers()) assert.True(t, sctx.TransparentAliases()) assert.True(t, sctx.RefAliases()) - assert.True(t, sctx.Debug()) } func TestScanCtx_Meta(t *testing.T) {