From 37581f3ade159f48fbb0a5ad4fc87ae6e7e3dc46 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 05:37:14 +0300 Subject: [PATCH 01/21] refactor(compilers/openapi): move document lowering into a package The root package lowered document-level metadata itself: meta.go's info, extensions, unknown keys, retained components and dialect, and servers.go's server lowering. That is lowering, not the compiler's public face, so it moves to internal/document, split by what each file lowers. Servers move with it rather than staying behind a callback the root would hand in. The document's list is the only one the IR holds, so the document is the lowering's one reader; a field for path-item and operation servers would move it to a package both walks reach, whichever package it leaves. docMeta and lowerMeta become document.Meta and document.Lower. Every other moved declaration is byte-identical, and the diagnostics keep their order, since Lower still calls the server lowering where lowerMeta did. The tests follow the code: unit tests into _internal_test.go files, the specs compiled end to end into the external test package, as auth's are. The root no longer imports the parser library, so its allowlist drops it. --- compilers/openapi/helpers_test.go | 4 +- .../openapi/internal/document/components.go | 69 +++++ .../document/components_test.go} | 7 +- .../openapi/internal/document/dialect.go | 28 ++ .../openapi/internal/document/dialect_test.go | 43 +++ compilers/openapi/internal/document/doc.go | 13 + .../openapi/internal/document/document.go | 47 +++ .../internal/document/document_test.go | 62 ++++ .../openapi/internal/document/helpers_test.go | 26 ++ compilers/openapi/internal/document/info.go | 38 +++ .../{ => internal/document}/servers.go | 2 +- .../document/servers_internal_test.go | 96 ++++++ .../{ => internal/document}/servers_test.go | 88 +----- compilers/openapi/internal/document/sites.go | 139 +++++++++ .../internal/document/sites_internal_test.go | 44 +++ .../openapi/internal/document/sites_test.go | 29 ++ compilers/openapi/meta.go | 290 ------------------ compilers/openapi/meta_test.go | 148 --------- compilers/openapi/openapi.go | 3 +- compilers/openapi/overlay_test.go | 4 +- docs/compilers/openapi.md | 12 +- internal/archtest/arch_test.go | 16 +- internal/archtest/recursion_test.go | 1 + 23 files changed, 666 insertions(+), 543 deletions(-) create mode 100644 compilers/openapi/internal/document/components.go rename compilers/openapi/{unreferenced_internal_test.go => internal/document/components_test.go} (97%) create mode 100644 compilers/openapi/internal/document/dialect.go create mode 100644 compilers/openapi/internal/document/dialect_test.go create mode 100644 compilers/openapi/internal/document/doc.go create mode 100644 compilers/openapi/internal/document/document.go create mode 100644 compilers/openapi/internal/document/document_test.go create mode 100644 compilers/openapi/internal/document/helpers_test.go create mode 100644 compilers/openapi/internal/document/info.go rename compilers/openapi/{ => internal/document}/servers.go (99%) create mode 100644 compilers/openapi/internal/document/servers_internal_test.go rename compilers/openapi/{ => internal/document}/servers_test.go (52%) create mode 100644 compilers/openapi/internal/document/sites.go create mode 100644 compilers/openapi/internal/document/sites_internal_test.go create mode 100644 compilers/openapi/internal/document/sites_test.go delete mode 100644 compilers/openapi/meta.go delete mode 100644 compilers/openapi/meta_test.go diff --git a/compilers/openapi/helpers_test.go b/compilers/openapi/helpers_test.go index 549a3139..9dfd7964 100644 --- a/compilers/openapi/helpers_test.go +++ b/compilers/openapi/helpers_test.go @@ -20,9 +20,9 @@ import ( // parseFull runs the whole public compiler pipeline over src. // -// It is one of the four copies openapitest cannot hold: a helper that drives the +// It is one of the copies openapitest cannot hold: a helper that drives the // compiler has to import it, and this package's own internal tests could then -// not import openapitest at all. The four are written identically so a reader +// not import openapitest at all. The copies are written identically so a reader // comparing them finds no difference to account for. func parseFull(t *testing.T, src string) (*ir.Document, []ir.Diagnostic) { t.Helper() diff --git a/compilers/openapi/internal/document/components.go b/compilers/openapi/internal/document/components.go new file mode 100644 index 00000000..528a010b --- /dev/null +++ b/compilers/openapi/internal/document/components.go @@ -0,0 +1,69 @@ +package document + +import ( + "github.com/dexpace/morphic/compilers/openapi/internal/annotation" + "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/compilers/openapi/internal/ids" + "github.com/dexpace/morphic/compilers/openapi/internal/lowering" + "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" + "github.com/dexpace/morphic/ir" +) + +// retainedSections are the components sections with no registry. Schemas and +// security schemes are lowered whole and unconditionally; an entry of one of +// these reaches the IR only where a lowering follows a reference to it. +// components/mediaTypes is kept whole instead until a reference into it is +// followed (GitHub #856). +func retainedSections() []string { + return []string{"responses", "parameters", "examples", "requestBodies", "headers", "links", "callbacks", "pathItems"} +} + +// retainUnreferencedComponents keeps every entry of a retainedSections section +// that no lowering resolved a reference to verbatim on the document, under +// openapi:components/
/ with ReasonNoIRHome and no diagnostic +// (GitHub #616). It must run after every other lowering: only they know what +// they followed (lowering.Ctx.Reached), and a $ref string they never follow, +// in a links map, an x- extension or an example's value, carries nothing of +// its target into the IR. A section is read as the parser reads it, through +// `<<` merge keys, so a merged-in entry is kept under its own name. +func retainUnreferencedComponents(c lowering.Ctx) (ir.Unmodeled, []ir.Diagnostic) { + view := nodeview.New() + components := view.ChildByToken(nodeview.DocumentRoot(c.Doc.GetRootNode()), "components") + var out ir.Unmodeled + var diags []ir.Diagnostic + for _, section := range retainedSections() { + for _, entry := range view.MappingPairs(view.ChildByToken(nodeview.Deref(components), section)) { + pointer := ids.Ptr("components", section, entry.Key) + if c.Reached(pointer) { + continue + } + _, keptDiags := annotation.PreserveNodeInto(&out, "openapi:"+ids.Scope("components", section, entry.Key), + entry.Val, ir.ReasonNoIRHome, c.ProvenanceAt(pointer)) + diags = append(diags, keptDiags...) + } + } + if view.Exhausted() { + diags = append(diags, c.DiagAt(ir.SeverityWarning, diag.DegradedConstruct, ids.Ptr("components"), + "components merges through `<<` deeper than %d levels; entries past that are not kept", nodeview.MergeDepthLimit)) + } + return out, diags +} + +// keepComponentMediaTypes keeps the components/mediaTypes map OpenAPI 3.2 adds +// verbatim on the document, read as the parser reads it, at the entry and +// pointer the components census would use, so the census leaves it alone. The +// parser models no such section and this compiler does not resolve a reference +// into it yet (GitHub #856), so the map has no IR home. It is kept at every +// version, as load reports it below 3.2 (diag.LaterVersionConstruct). +func keepComponentMediaTypes(c lowering.Ctx, p *ir.Unmodeled) []ir.Diagnostic { + view := nodeview.New() + components := view.ChildByToken(nodeview.DocumentRoot(c.Doc.GetRootNode()), "components") + at := ids.Ptr("components", "mediaTypes") + kept, diags := annotation.PreserveNodeInto(p, annotation.EntryKey("components", "mediaTypes"), + view.ChildByToken(nodeview.Deref(components), "mediaTypes"), ir.ReasonNoIRHome, c.ProvenanceAt(at)) + if !kept { + return diags + } + return append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, at, + "components/mediaTypes is not resolved by this compiler yet (GitHub #856); kept verbatim under Unmodeled")) +} diff --git a/compilers/openapi/unreferenced_internal_test.go b/compilers/openapi/internal/document/components_test.go similarity index 97% rename from compilers/openapi/unreferenced_internal_test.go rename to compilers/openapi/internal/document/components_test.go index 12cb1a77..d25269ac 100644 --- a/compilers/openapi/unreferenced_internal_test.go +++ b/compilers/openapi/internal/document/components_test.go @@ -1,4 +1,4 @@ -package openapi +package document_test import ( "os" @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" "github.com/dexpace/morphic/compilers" + "github.com/dexpace/morphic/compilers/openapi" "github.com/dexpace/morphic/compilers/openapi/internal/diag" "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" @@ -233,8 +234,8 @@ components: P: {name: p, in: query, schema: {type: string}} ` require.NoError(t, os.WriteFile(path, []byte(src), 0o600)) - doc, diags, err := New().Compile(t.Context(), []compilers.Source{{Path: path, Data: []byte(src)}}, - compilers.Options{Reader: reader.Disk{}, FormatOptions: Options{AllowExternalRefs: true}}) + doc, diags, err := openapi.New().Compile(t.Context(), []compilers.Source{{Path: path, Data: []byte(src)}}, + compilers.Options{Reader: reader.Disk{}, FormatOptions: openapi.Options{AllowExternalRefs: true}}) require.NoError(t, err) openapitest.RequireNoErrorDiags(t, diags) require.Len(t, openapitest.FindOp(t, doc, "a").Params, 1, "the reference resolves and the parameter lowers") diff --git a/compilers/openapi/internal/document/dialect.go b/compilers/openapi/internal/document/dialect.go new file mode 100644 index 00000000..f55788e2 --- /dev/null +++ b/compilers/openapi/internal/document/dialect.go @@ -0,0 +1,28 @@ +package document + +import ( + "github.com/dexpace/morphic/compilers/openapi/internal/annotation" + "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/compilers/openapi/internal/ids" + "github.com/dexpace/morphic/compilers/openapi/internal/lowering" + "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" + "github.com/dexpace/morphic/ir" +) + +// keepJSONSchemaDialect keeps the document's jsonSchemaDialect verbatim on the +// document, with an info. It is the default `$schema` of the document's Schema +// Objects, and the IR models no dialect, so it is kept as a schema's own +// `$schema` is (annotation.DialectKeywords): ReasonOutOfScope, and every schema +// read in the OpenAPI dialect whatever it names. +func keepJSONSchemaDialect(c lowering.Ctx, p *ir.Unmodeled) []ir.Diagnostic { + at := ids.Ptr("jsonSchemaDialect") + node := nodeview.New().ChildByToken(nodeview.DocumentRoot(c.Doc.GetRootNode()), "jsonSchemaDialect") + kept, diags := annotation.PreserveNodeInto(p, annotation.EntryKey("", "jsonSchemaDialect"), node, + ir.ReasonOutOfScope, c.ProvenanceAt(at)) + if !kept { + return diags + } + return append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, at, + "jsonSchemaDialect names the JSON Schema dialect of the document's schemas; the IR models no "+ + "dialect, so it is kept verbatim under Unmodeled and every schema is read in the OpenAPI dialect")) +} diff --git a/compilers/openapi/internal/document/dialect_test.go b/compilers/openapi/internal/document/dialect_test.go new file mode 100644 index 00000000..11737850 --- /dev/null +++ b/compilers/openapi/internal/document/dialect_test.go @@ -0,0 +1,43 @@ +package document_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" + "github.com/dexpace/morphic/ir" +) + +// TestMeta_JSONSchemaDialectIsKept pins the document's jsonSchemaDialect, which +// nothing read: it is kept verbatim on the document with an info, as a schema's +// own `$schema` is, written directly or merged in through `<<`, and in a 3.0 +// document, which it postdates, load also reports it. Declaring none keeps +// nothing. +func TestMeta_JSONSchemaDialectIsKept(t *testing.T) { + t.Parallel() + const dialect = "https://json-schema.org/draft/2020-12/schema" + for _, tc := range []struct { + name, version, line string + }{ + {"3.1", "3.1.0", "jsonSchemaDialect: " + dialect}, + {"3.1 merged in", "3.1.0", "<<: {jsonSchemaDialect: '" + dialect + "'}"}, + {"3.0", "3.0.3", "jsonSchemaDialect: " + dialect}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, "openapi: "+tc.version+"\n"+tc.line+"\ninfo: {title: T, version: \"1\"}\npaths: {}\n") + kept, ok := doc.Unmodeled["openapi:jsonSchemaDialect"] + require.True(t, ok, "the dialect is kept on the document") + assert.Equal(t, ir.ReasonOutOfScope, kept.Reason) + assert.JSONEq(t, `"`+dialect+`"`, string(kept.Value)) + openapitest.AssertInfoDiagAt(t, diags, "/jsonSchemaDialect") + assert.Equal(t, tc.version == "3.0.3", + openapitest.HasDiagCodeAt(diags, diag.LaterVersionConstruct, "/jsonSchemaDialect")) + }) + } + doc, _ := parseFull(t, "openapi: 3.1.0\ninfo: {title: T, version: \"1\"}\npaths: {}\n") + assert.NotContains(t, doc.Unmodeled, "openapi:jsonSchemaDialect") +} diff --git a/compilers/openapi/internal/document/doc.go b/compilers/openapi/internal/document/doc.go new file mode 100644 index 00000000..ae93698d --- /dev/null +++ b/compilers/openapi/internal/document/doc.go @@ -0,0 +1,13 @@ +// Package document lowers what a document says about itself rather than about +// its types or operations: its info, root externalDocs and servers, and, kept +// verbatim, what the objects around them write that lowers to no node of its +// own (docs/compilers/openapi.md#document-metadata, +// docs/compilers/openapi.md#unreferenced-components). +// +// It runs after every other lowering, since only they know which components +// they reached, and returns a Meta for the compiler to assign. +// +// Servers are lowered here while the IR holds only the document's own. A field +// for a path item's or an operation's (GitHub #285) would move their lowering +// to a package both walks reach. +package document diff --git a/compilers/openapi/internal/document/document.go b/compilers/openapi/internal/document/document.go new file mode 100644 index 00000000..707ca9f6 --- /dev/null +++ b/compilers/openapi/internal/document/document.go @@ -0,0 +1,47 @@ +package document + +import ( + "github.com/dexpace/morphic/compilers/openapi/internal/annotation" + "github.com/dexpace/morphic/compilers/openapi/internal/lowering" + "github.com/dexpace/morphic/ir" +) + +// Meta is the document-level metadata: what ir.Document carries that belongs +// to neither the type graph nor the service graph. +// +// It exists so the lowering can return what it read instead of writing into the +// document as it goes. The caller owns the document and does the assigning, +// which is what lets every function below be checked against a literal context +// and nothing else. +type Meta struct { + Name string + Version string + TermsOfService string + Docs ir.Docs + Contact *ir.Contact + License *ir.License + Servers []ir.Server + Unmodeled ir.Unmodeled +} + +// Lower lowers the document-level metadata that is not part of the type or +// service graph: info, servers, and the extensions of every object around them +// that lowers to no node of its own (ir-design §10, §12). +func Lower(c lowering.Ctx) (Meta, []ir.Diagnostic) { + m := lowerInfo(c) + + ext, diags := documentExtensions(c) + m.Unmodeled = ext + + retained, retainDiags := retainUnreferencedComponents(c) + m.Unmodeled = annotation.MergeUnmodeled(m.Unmodeled, retained) + diags = append(diags, retainDiags...) + + servers, serverDiags := lowerServers(c) + m.Servers = servers + diags = append(diags, serverDiags...) + + diags = append(diags, keepComponentMediaTypes(c, &m.Unmodeled)...) + diags = append(diags, keepJSONSchemaDialect(c, &m.Unmodeled)...) + return m, append(diags, documentUnknownKeys(c, &m.Unmodeled)...) +} diff --git a/compilers/openapi/internal/document/document_test.go b/compilers/openapi/internal/document/document_test.go new file mode 100644 index 00000000..979f6b87 --- /dev/null +++ b/compilers/openapi/internal/document/document_test.go @@ -0,0 +1,62 @@ +package document_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/ir" +) + +const metaSpec = `openapi: 3.2.0 +info: + title: Meta + version: "2" + summary: A summary + description: A description + contact: {name: Team, url: 'https://team', email: t@example.com} + license: {name: Apache-2.0, identifier: Apache-2.0} +externalDocs: {url: 'https://docs', description: docs} +x-top: {a: 1} +servers: + - url: https://{region}.example.com + name: primary + description: Primary + variables: + region: {default: us, enum: [us, eu], description: Region} +paths: + /p: + get: {operationId: p, responses: {"200": {description: ok}}} +` + +func TestMeta_FullDocumentMetadata(t *testing.T) { + t.Parallel() + doc, _ := parseFull(t, metaSpec) + assert.Equal(t, "Meta", doc.Name) + assert.Equal(t, "2", doc.Version) + require.NotNil(t, doc.Contact) + assert.Equal(t, "t@example.com", doc.Contact.Email) + require.NotNil(t, doc.License) + assert.Equal(t, "Apache-2.0", doc.License.Identifier) + assert.NotEmpty(t, doc.Docs.ExternalDocs, "root externalDocs folded into docs") + assert.NotEmpty(t, doc.Unmodeled, "top-level x-* extension") + require.Len(t, doc.Servers, 1) + assert.Equal(t, ir.Naming{Source: "primary", Canonical: "primary"}, doc.Servers[0].Name, + "a declared 3.2 name is the server's name; the URL hint is for a server with none") + require.Len(t, doc.Servers[0].Variables, 1) + assert.Equal(t, []string{"us", "eu"}, doc.Servers[0].Variables[0].Enum) +} + +func TestMeta_NoInfoNoServers(t *testing.T) { + t.Parallel() + // With no info block the title is empty; with no servers the library injects + // a default "/" server that is lowered. + spec := "openapi: 3.1.0\npaths: {}\n" + doc, _ := parseFull(t, spec) + assert.Empty(t, doc.Name) + require.Len(t, doc.Servers, 1) + assert.Equal(t, "/", doc.Servers[0].URLTemplate) + assert.Equal(t, ir.Naming{Hint: "server"}, doc.Servers[0].Name, + "the injected server is still a server an emitter has to name (GitHub #258)") +} diff --git a/compilers/openapi/internal/document/helpers_test.go b/compilers/openapi/internal/document/helpers_test.go new file mode 100644 index 00000000..55079147 --- /dev/null +++ b/compilers/openapi/internal/document/helpers_test.go @@ -0,0 +1,26 @@ +package document_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/compilers" + "github.com/dexpace/morphic/compilers/openapi" + "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" + "github.com/dexpace/morphic/ir" +) + +// parseFull runs the whole public compiler pipeline over src. It reaches back +// through openapi deliberately: which components Lower keeps depends on what +// every other lowering reached, and only a whole compile runs them all. That +// reach is also why openapitest cannot hold it — see that package's doc +// comment. +func parseFull(t *testing.T, src string) (*ir.Document, []ir.Diagnostic) { + t.Helper() + doc, diags, err := openapi.New().Compile(t.Context(), + []compilers.Source{openapitest.SourceOf(src)}, compilers.Options{}) + require.NoError(t, err) + require.NotNil(t, doc) + return doc, diags +} diff --git a/compilers/openapi/internal/document/info.go b/compilers/openapi/internal/document/info.go new file mode 100644 index 00000000..f601d3d8 --- /dev/null +++ b/compilers/openapi/internal/document/info.go @@ -0,0 +1,38 @@ +package document + +import ( + soa "github.com/speakeasy-api/openapi/openapi" + + "github.com/dexpace/morphic/compilers/openapi/internal/lowering" + "github.com/dexpace/morphic/ir" +) + +// lowerInfo maps info onto the document identity, docs, contact, and license. +// GetInfo always returns a non-nil Info (it addresses an embedded struct value), +// so no nil guard is needed. +func lowerInfo(c lowering.Ctx) Meta { + info := c.Doc.GetInfo() + m := Meta{ + Name: info.GetTitle(), + Version: info.GetVersion(), + TermsOfService: info.GetTermsOfService(), + Docs: infoDocs(c, info), + } + if ct := info.GetContact(); ct != nil { + m.Contact = &ir.Contact{Name: ct.GetName(), URL: ct.GetURL(), Email: ct.GetEmail()} + } + if lic := info.GetLicense(); lic != nil { + m.License = &ir.License{Name: lic.GetName(), Identifier: lic.GetIdentifier(), URL: lic.GetURL()} + } + return m +} + +// infoDocs builds the document docs from info summary and description, folding +// in the root externalDocs link when present. +func infoDocs(c lowering.Ctx, info *soa.Info) ir.Docs { + d := ir.Docs{Summary: info.GetSummary(), Description: info.GetDescription()} + if ed := c.Doc.GetExternalDocs(); ed != nil { + d.ExternalDocs = append(d.ExternalDocs, ir.Link{URL: ed.GetURL(), Description: ed.GetDescription()}) + } + return d +} diff --git a/compilers/openapi/servers.go b/compilers/openapi/internal/document/servers.go similarity index 99% rename from compilers/openapi/servers.go rename to compilers/openapi/internal/document/servers.go index 783a43ad..077ab1b5 100644 --- a/compilers/openapi/servers.go +++ b/compilers/openapi/internal/document/servers.go @@ -1,4 +1,4 @@ -package openapi +package document import ( "encoding/json/jsontext" diff --git a/compilers/openapi/internal/document/servers_internal_test.go b/compilers/openapi/internal/document/servers_internal_test.go new file mode 100644 index 00000000..21e102dd --- /dev/null +++ b/compilers/openapi/internal/document/servers_internal_test.go @@ -0,0 +1,96 @@ +package document + +import ( + "testing" + + soa "github.com/speakeasy-api/openapi/openapi" + "github.com/speakeasy-api/openapi/sequencedmap" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/compilers/openapi/internal/lowering" + "github.com/dexpace/morphic/ir" +) + +// TestServerName_DerivedFromURLWhenUnnamed pins how a server is named when the +// source declares no name for it, which before OpenAPI 3.2 it has no way to do. +// Every such server used to reach the IR with all three name channels empty, +// leaving an emitter that renders one client per server nothing to name them by +// (GitHub #258). +func TestServerName_DerivedFromURLWhenUnnamed(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + url string + want ir.Naming + }{ + {"absolute url with a template variable", "https://{env}.example.com/v1", + ir.Naming{Hint: "https_env_example_com_v_1"}}, + {"relative url", "/v2", ir.Naming{Hint: "v_2"}}, + {"root url has no word in it", "/", ir.Naming{Hint: "server"}}, + {"no url at all", "", ir.Naming{Hint: "server"}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, serverName(&soa.Server{URL: tc.url})) + }) + } +} + +// TestServerName_DistinguishesServersDifferingOnlyInPath is why the hint is the +// whole URL template and not the host inside it: one host serving two API +// versions is an ordinary OpenAPI document, and a host-derived hint would name +// both servers the same. +func TestServerName_DistinguishesServersDifferingOnlyInPath(t *testing.T) { + t.Parallel() + v1 := serverName(&soa.Server{URL: "https://api.example.com/v1"}) + v2 := serverName(&soa.Server{URL: "https://api.example.com/v2"}) + assert.NotEqual(t, v1.Hint, v2.Hint, "two distinct servers get two distinct hints") +} + +// TestServerName_CollidesOnPunctuationAlone bounds that claim, which must not be +// read as "distinct servers get distinct hints": canonicalizing drops every +// non-word character, so two URLs differing only in punctuation reduce to one +// word sequence. Uniquifying it is an emitter's job, and pinning it keeps that a +// known bound rather than a later discovery. +func TestServerName_CollidesOnPunctuationAlone(t *testing.T) { + t.Parallel() + dotted := serverName(&soa.Server{URL: "https://api.example.com/v1"}) + dashed := serverName(&soa.Server{URL: "https://api.example.com/v-1"}) + assert.Equal(t, dotted.Hint, dashed.Hint, + "neutral words carry no punctuation, so these two collide") +} + +func TestLowerServers_NilEntrySkipped(t *testing.T) { + t.Parallel() + doc := &soa.OpenAPI{Servers: []*soa.Server{nil, {URL: "https://x.example.com"}}} + got, diags := lowerServers(lowering.Ctx{Doc: doc}) + + assert.Empty(t, diags) + require.Len(t, got, 1, "nil server entry skipped, valid one lowered") + assert.Equal(t, "https://x.example.com", got[0].URLTemplate) +} + +func TestServerVariables_NilEntrySkipped(t *testing.T) { + t.Parallel() + vars := sequencedmap.New( + sequencedmap.NewElem("skip", (*soa.ServerVariable)(nil)), + sequencedmap.NewElem("keep", &soa.ServerVariable{}), + ) + srv, diags := lowerServer(lowering.Ctx{}, &soa.Server{URL: "https://x", Variables: vars}, "/servers/0") + assert.Empty(t, diags) + require.Len(t, srv.Variables, 1, "nil variable entry skipped") + assert.Equal(t, "keep", srv.Variables[0].Name) +} + +// TestLowerServers_EveryEntrySkippedIsNil pins the same guard on the servers +// side: when no entry survives, the field stays unset rather than becoming an +// empty list the source never declared. +func TestLowerServers_EveryEntrySkippedIsNil(t *testing.T) { + t.Parallel() + doc := &soa.OpenAPI{Servers: []*soa.Server{nil, nil}} + + got, diags := lowerServers(lowering.Ctx{Doc: doc}) + assert.Nil(t, got) + assert.Empty(t, diags) +} diff --git a/compilers/openapi/servers_test.go b/compilers/openapi/internal/document/servers_test.go similarity index 52% rename from compilers/openapi/servers_test.go rename to compilers/openapi/internal/document/servers_test.go index 3f178c83..99e9ad6c 100644 --- a/compilers/openapi/servers_test.go +++ b/compilers/openapi/internal/document/servers_test.go @@ -1,102 +1,16 @@ -package openapi +package document_test import ( "testing" - soa "github.com/speakeasy-api/openapi/openapi" - "github.com/speakeasy-api/openapi/sequencedmap" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/dexpace/morphic/compilers/openapi/internal/diag" - "github.com/dexpace/morphic/compilers/openapi/internal/lowering" "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" "github.com/dexpace/morphic/ir" ) -// TestServerName_DerivedFromURLWhenUnnamed pins how a server is named when the -// source declares no name for it, which before OpenAPI 3.2 it has no way to do. -// Every such server used to reach the IR with all three name channels empty, -// leaving an emitter that renders one client per server nothing to name them by -// (GitHub #258). -func TestServerName_DerivedFromURLWhenUnnamed(t *testing.T) { - t.Parallel() - for _, tc := range []struct { - name string - url string - want ir.Naming - }{ - {"absolute url with a template variable", "https://{env}.example.com/v1", - ir.Naming{Hint: "https_env_example_com_v_1"}}, - {"relative url", "/v2", ir.Naming{Hint: "v_2"}}, - {"root url has no word in it", "/", ir.Naming{Hint: "server"}}, - {"no url at all", "", ir.Naming{Hint: "server"}}, - } { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - assert.Equal(t, tc.want, serverName(&soa.Server{URL: tc.url})) - }) - } -} - -// TestServerName_DistinguishesServersDifferingOnlyInPath is why the hint is the -// whole URL template and not the host inside it: one host serving two API -// versions is an ordinary OpenAPI document, and a host-derived hint would name -// both servers the same. -func TestServerName_DistinguishesServersDifferingOnlyInPath(t *testing.T) { - t.Parallel() - v1 := serverName(&soa.Server{URL: "https://api.example.com/v1"}) - v2 := serverName(&soa.Server{URL: "https://api.example.com/v2"}) - assert.NotEqual(t, v1.Hint, v2.Hint, "two distinct servers get two distinct hints") -} - -// TestServerName_CollidesOnPunctuationAlone bounds that claim, which must not be -// read as "distinct servers get distinct hints": canonicalizing drops every -// non-word character, so two URLs differing only in punctuation reduce to one -// word sequence. Uniquifying it is an emitter's job, and pinning it keeps that a -// known bound rather than a later discovery. -func TestServerName_CollidesOnPunctuationAlone(t *testing.T) { - t.Parallel() - dotted := serverName(&soa.Server{URL: "https://api.example.com/v1"}) - dashed := serverName(&soa.Server{URL: "https://api.example.com/v-1"}) - assert.Equal(t, dotted.Hint, dashed.Hint, - "neutral words carry no punctuation, so these two collide") -} - -func TestLowerServers_NilEntrySkipped(t *testing.T) { - t.Parallel() - doc := &soa.OpenAPI{Servers: []*soa.Server{nil, {URL: "https://x.example.com"}}} - got, diags := lowerServers(lowering.Ctx{Doc: doc}) - - assert.Empty(t, diags) - require.Len(t, got, 1, "nil server entry skipped, valid one lowered") - assert.Equal(t, "https://x.example.com", got[0].URLTemplate) -} - -func TestServerVariables_NilEntrySkipped(t *testing.T) { - t.Parallel() - vars := sequencedmap.New( - sequencedmap.NewElem("skip", (*soa.ServerVariable)(nil)), - sequencedmap.NewElem("keep", &soa.ServerVariable{}), - ) - srv, diags := lowerServer(lowering.Ctx{}, &soa.Server{URL: "https://x", Variables: vars}, "/servers/0") - assert.Empty(t, diags) - require.Len(t, srv.Variables, 1, "nil variable entry skipped") - assert.Equal(t, "keep", srv.Variables[0].Name) -} - -// TestLowerServers_EveryEntrySkippedIsNil pins the same guard on the servers -// side: when no entry survives, the field stays unset rather than becoming an -// empty list the source never declared. -func TestLowerServers_EveryEntrySkippedIsNil(t *testing.T) { - t.Parallel() - doc := &soa.OpenAPI{Servers: []*soa.Server{nil, nil}} - - got, diags := lowerServers(lowering.Ctx{Doc: doc}) - assert.Nil(t, got) - assert.Empty(t, diags) -} - // TestDuplicateServerName_BothServersLower is GitHub #631's repro compiled end // to end: the repeat is reported, a warning at its own name naming the first, // and nothing is dropped or merged, each server lowering with the name it diff --git a/compilers/openapi/internal/document/sites.go b/compilers/openapi/internal/document/sites.go new file mode 100644 index 00000000..e32969ba --- /dev/null +++ b/compilers/openapi/internal/document/sites.go @@ -0,0 +1,139 @@ +package document + +import ( + "encoding/json/jsontext" + "strconv" + + "github.com/dexpace/morphic/compilers/openapi/internal/annotation" + "github.com/dexpace/morphic/compilers/openapi/internal/ids" + "github.com/dexpace/morphic/compilers/openapi/internal/lowering" + "github.com/dexpace/morphic/ir" +) + +// documentExtensions collects the x-* of every object that lowers to no IR node +// of its own: the document root, the info block and the contact and license +// inside it, the root externalDocs, the components object, and each declared +// tag with its own externalDocs. ir.Document is the nearest node with an +// Unmodeled map for all of them, so each object's entries are keyed under the +// source path it was written at — see annotation.ExtensionsUnder for why one +// unscoped key for all of them would not do. +func documentExtensions(c lowering.Ctx) (ir.Unmodeled, []ir.Diagnostic) { + return annotation.ExtensionsAt(c.ProvenanceAt, append(rootExtensions(c), tagExtensions(c)...)...) +} + +// rootExtensions returns the extension sites a document has exactly one of. The +// root's own take no scope, since ir.Document stands for the OpenAPI Object +// itself; the rest are keyed by the path from it down to the object that wrote +// them. +func rootExtensions(c lowering.Ctx) []annotation.ExtensionSite { + info := c.Doc.GetInfo() + infoPtr := ids.Ptr("info") + return []annotation.ExtensionSite{ + {Scope: "", Owner: "", Ext: c.Doc.GetExtensions()}, + {Scope: "info", Owner: infoPtr, Ext: info.GetExtensions()}, + {Scope: "info/contact", Owner: infoPtr + ids.Ptr("contact"), Ext: info.GetContact().GetExtensions()}, + {Scope: "info/license", Owner: infoPtr + ids.Ptr("license"), Ext: info.GetLicense().GetExtensions()}, + {Scope: "externalDocs", Owner: ids.Ptr("externalDocs"), Ext: c.Doc.GetExternalDocs().GetExtensions()}, + {Scope: "components", Owner: ids.Ptr("components"), Ext: c.Doc.GetComponents().GetExtensions()}, + } +} + +// tagExtensions returns the extension sites each declared tag contributes: its +// own, and its externalDocs object's. ir.TagDef holds no Unmodeled map and +// neither does ir.Link, so both ride on the document. +// +// Keyed by the tag's index rather than its name, which is the pointer the tag +// is written at. A name would read better and is what TagDefs are found by, but +// OpenAPI's requirement that tag names be unique is the document's to keep, not +// this compiler's to rely on: two tags spelled alike would silently leave one +// entry. +func tagExtensions(c lowering.Ctx) []annotation.ExtensionSite { + tags := c.Doc.GetTags() + out := make([]annotation.ExtensionSite, 0, 2*len(tags)) + for i, t := range tags { + if t == nil { + continue + } + scope := "tags/" + strconv.Itoa(i) + ptr := ids.Ptr("tags", strconv.Itoa(i)) + out = append(out, + annotation.ExtensionSite{Scope: scope, Owner: ptr, Ext: t.GetExtensions()}, + annotation.ExtensionSite{Scope: scope + "/externalDocs", Owner: ptr + ids.Ptr("externalDocs"), + Ext: t.GetExternalDocs().GetExtensions()}) + } + return out +} + +// documentUnknownKeys collects the keys the OpenAPI model names no field for +// from every object around the document metadata that lowers to no node of its +// own: the root, info with its contact and license, the root externalDocs, the +// components object, and each tag with its own externalDocs. +// +// ir.Document is the nearest node with an Unmodeled map for all of them, so each +// object's keys are scoped by the source path they were written at. One unscoped +// "openapi:status" would be a single key for every object, and the entry that +// survived would be whichever site ran last. +func documentUnknownKeys(c lowering.Ctx, p *ir.Unmodeled) []ir.Diagnostic { + sites := append(rootUnknownSites(c), tagUnknownSites(c)...) + diags := make([]ir.Diagnostic, 0, len(sites)) + for _, site := range sites { + diags = append(diags, + annotation.UnknownKeysUnder(p, site.model, c.ProvenanceAt, site.owner, site.scope)...) + } + return diags +} + +// unknownSite is one object's census: what it keys under on the carrier holding +// it, the object's own source pointer, and the parsed object itself. +type unknownSite struct { + scope string + owner jsontext.Pointer + model any +} + +// rootUnknownSites returns the census sites a document has exactly one of. The +// root's keys take no scope, since ir.Document stands for the OpenAPI Object +// itself; the rest are keyed by the path from it down to the object that wrote +// them. +// +// The components object is one of them, unlike the maps beneath it: `paths`, +// `responses` and a callback each embed a sequenced map, so every key they hold +// is an entry of it (docs/parser-library.md#embedded-map-keys), while the +// Components Object is a fixed-field struct and a key it does not define is as +// undeclared as one on any other object here. +func rootUnknownSites(c lowering.Ctx) []unknownSite { + info := c.Doc.GetInfo() + infoPtr := ids.Ptr("info") + return []unknownSite{ + {"", "", c.Doc}, + {"info", infoPtr, info}, + {"info/contact", infoPtr + ids.Ptr("contact"), info.GetContact()}, + {"info/license", infoPtr + ids.Ptr("license"), info.GetLicense()}, + {"externalDocs", ids.Ptr("externalDocs"), c.Doc.GetExternalDocs()}, + {"components", ids.Ptr("components"), c.Doc.GetComponents()}, + } +} + +// tagUnknownSites returns the census sites each declared tag contributes: its +// own, and its externalDocs object's. Neither ir.TagDef nor ir.Link holds an +// Unmodeled map, so both ride on the document. +// +// Scoped by index rather than name, which is the pointer a tag is written at. A +// name would read better, but OpenAPI's requirement that tag names be unique is +// the document's to keep and not this compiler's to rely on: two tags spelled +// alike would silently leave one entry. +func tagUnknownSites(c lowering.Ctx) []unknownSite { + tags := c.Doc.GetTags() + out := make([]unknownSite, 0, 2*len(tags)) + for i, t := range tags { + if t == nil { + continue + } + index := strconv.Itoa(i) + ptr := ids.Ptr("tags", index) + out = append(out, + unknownSite{"tags/" + index, ptr, t}, + unknownSite{"tags/" + index + "/externalDocs", ptr + ids.Ptr("externalDocs"), t.GetExternalDocs()}) + } + return out +} diff --git a/compilers/openapi/internal/document/sites_internal_test.go b/compilers/openapi/internal/document/sites_internal_test.go new file mode 100644 index 00000000..74b22327 --- /dev/null +++ b/compilers/openapi/internal/document/sites_internal_test.go @@ -0,0 +1,44 @@ +package document + +import ( + "encoding/json/jsontext" + "testing" + + soa "github.com/speakeasy-api/openapi/openapi" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/compilers/openapi/internal/lowering" +) + +// TestTagExtensions_NilEntrySkipped pins the same guard lowerTagDefs keeps on +// the other side of the tag list: a nil entry contributes no extension site, so +// nothing dereferences it and no site is keyed at an index holding no tag. +func TestTagExtensions_NilEntrySkipped(t *testing.T) { + t.Parallel() + doc := &soa.OpenAPI{Tags: []*soa.Tag{nil, {Name: "kept"}}} + + got := tagExtensions(lowering.Ctx{Doc: doc}) + + require.Len(t, got, 2, "one surviving tag contributes its own site and its externalDocs one") + assert.Equal(t, "tags/1", got[0].Scope, "the site is keyed at the tag's own index, not its position") + assert.Equal(t, jsontext.Pointer("/tags/1"), got[0].Owner) +} + +// TestTagUnknownSites_NilEntrySkipped is TestTagExtensions_NilEntrySkipped's +// twin on the census side: the two walks of the tag list keep the same guard, so +// neither can be the one that dereferences a nil entry or keys a site at an +// index holding no tag. +func TestTagUnknownSites_NilEntrySkipped(t *testing.T) { + t.Parallel() + doc := &soa.OpenAPI{Tags: []*soa.Tag{nil, {Name: "kept"}}} + + got := tagUnknownSites(lowering.Ctx{Doc: doc}) + + require.Len(t, got, 2, + "the nil tag contributes nothing, and the surviving one contributes its own site and its externalDocs") + assert.Equal(t, "tags/1", got[0].scope, "the site is keyed at the tag's own index, not its position") + assert.Equal(t, jsontext.Pointer("/tags/1"), got[0].owner) + assert.Equal(t, "tags/1/externalDocs", got[1].scope, "and its externalDocs is scoped under that same index") + assert.Equal(t, jsontext.Pointer("/tags/1/externalDocs"), got[1].owner) +} diff --git a/compilers/openapi/internal/document/sites_test.go b/compilers/openapi/internal/document/sites_test.go new file mode 100644 index 00000000..387ef62f --- /dev/null +++ b/compilers/openapi/internal/document/sites_test.go @@ -0,0 +1,29 @@ +package document_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" + "github.com/dexpace/morphic/ir" +) + +// TestMeta_UnserializableExtensionStillWarns pins the document-level twin of +// TestAuth_UnserializableExtensionStillWarns: when every top-level x-* extension +// fails to serialize, Lower ends up with an empty Unmodeled map, so the +// warning must come from the extension reader rather than from a branch guarded +// on what was kept — the shape that used to drop it silently. +func TestMeta_UnserializableExtensionStillWarns(t *testing.T) { + t.Parallel() + spec := `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: {} +x-bad: {1: intkey} +` + doc, diags := parseFull(t, spec) + assert.Positive(t, openapitest.CountDiagsAt(diags, diag.DegradedConstruct, ir.SeverityWarning), + "an entirely unserializable top-level extension still warns even though Unmodeled ends up empty") + assert.Empty(t, doc.Unmodeled, "the unserializable extension is dropped, not stored") +} diff --git a/compilers/openapi/meta.go b/compilers/openapi/meta.go deleted file mode 100644 index 4dceff75..00000000 --- a/compilers/openapi/meta.go +++ /dev/null @@ -1,290 +0,0 @@ -package openapi - -import ( - "encoding/json/jsontext" - "strconv" - - soa "github.com/speakeasy-api/openapi/openapi" - - "github.com/dexpace/morphic/compilers/openapi/internal/annotation" - "github.com/dexpace/morphic/compilers/openapi/internal/diag" - "github.com/dexpace/morphic/compilers/openapi/internal/ids" - "github.com/dexpace/morphic/compilers/openapi/internal/lowering" - "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" - "github.com/dexpace/morphic/ir" -) - -// docMeta is the document-level metadata: what ir.Document carries that belongs -// to neither the type graph nor the service graph. -// -// It exists so the lowering can return what it read instead of writing into the -// document as it goes. The caller owns the document and does the assigning, -// which is what lets every function below be checked against a literal context -// and nothing else. -type docMeta struct { - Name string - Version string - TermsOfService string - Docs ir.Docs - Contact *ir.Contact - License *ir.License - Servers []ir.Server - Unmodeled ir.Unmodeled -} - -// retainedSections are the components sections with no registry. Schemas and -// security schemes are lowered whole and unconditionally; an entry of one of -// these reaches the IR only where a lowering follows a reference to it. -// components/mediaTypes is kept whole instead until a reference into it is -// followed (GitHub #856). -func retainedSections() []string { - return []string{"responses", "parameters", "examples", "requestBodies", "headers", "links", "callbacks", "pathItems"} -} - -// retainUnreferencedComponents keeps every entry of a retainedSections section -// that no lowering resolved a reference to verbatim on the document, under -// openapi:components/
/ with ReasonNoIRHome and no diagnostic -// (GitHub #616). It must run after every other lowering: only they know what -// they followed (lowering.Ctx.Reached), and a $ref string they never follow, -// in a links map, an x- extension or an example's value, carries nothing of -// its target into the IR. A section is read as the parser reads it, through -// `<<` merge keys, so a merged-in entry is kept under its own name. -func retainUnreferencedComponents(c lowering.Ctx) (ir.Unmodeled, []ir.Diagnostic) { - view := nodeview.New() - components := view.ChildByToken(nodeview.DocumentRoot(c.Doc.GetRootNode()), "components") - var out ir.Unmodeled - var diags []ir.Diagnostic - for _, section := range retainedSections() { - for _, entry := range view.MappingPairs(view.ChildByToken(nodeview.Deref(components), section)) { - pointer := ids.Ptr("components", section, entry.Key) - if c.Reached(pointer) { - continue - } - _, keptDiags := annotation.PreserveNodeInto(&out, "openapi:"+ids.Scope("components", section, entry.Key), - entry.Val, ir.ReasonNoIRHome, c.ProvenanceAt(pointer)) - diags = append(diags, keptDiags...) - } - } - if view.Exhausted() { - diags = append(diags, c.DiagAt(ir.SeverityWarning, diag.DegradedConstruct, ids.Ptr("components"), - "components merges through `<<` deeper than %d levels; entries past that are not kept", nodeview.MergeDepthLimit)) - } - return out, diags -} - -// lowerMeta lowers the document-level metadata that is not part of the type or -// service graph: info, servers, and the extensions of every object around them -// that lowers to no node of its own (ir-design §10, §12). -func lowerMeta(c lowering.Ctx) (docMeta, []ir.Diagnostic) { - m := lowerInfo(c) - - ext, diags := documentExtensions(c) - m.Unmodeled = ext - - retained, retainDiags := retainUnreferencedComponents(c) - m.Unmodeled = annotation.MergeUnmodeled(m.Unmodeled, retained) - diags = append(diags, retainDiags...) - - servers, serverDiags := lowerServers(c) - m.Servers = servers - diags = append(diags, serverDiags...) - - diags = append(diags, keepComponentMediaTypes(c, &m.Unmodeled)...) - diags = append(diags, keepJSONSchemaDialect(c, &m.Unmodeled)...) - return m, append(diags, documentUnknownKeys(c, &m.Unmodeled)...) -} - -// keepJSONSchemaDialect keeps the document's jsonSchemaDialect verbatim on the -// document, with an info. It is the default `$schema` of the document's Schema -// Objects, and the IR models no dialect, so it is kept as a schema's own -// `$schema` is (annotation.DialectKeywords): ReasonOutOfScope, and every schema -// read in the OpenAPI dialect whatever it names. -func keepJSONSchemaDialect(c lowering.Ctx, p *ir.Unmodeled) []ir.Diagnostic { - at := ids.Ptr("jsonSchemaDialect") - node := nodeview.New().ChildByToken(nodeview.DocumentRoot(c.Doc.GetRootNode()), "jsonSchemaDialect") - kept, diags := annotation.PreserveNodeInto(p, annotation.EntryKey("", "jsonSchemaDialect"), node, - ir.ReasonOutOfScope, c.ProvenanceAt(at)) - if !kept { - return diags - } - return append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, at, - "jsonSchemaDialect names the JSON Schema dialect of the document's schemas; the IR models no "+ - "dialect, so it is kept verbatim under Unmodeled and every schema is read in the OpenAPI dialect")) -} - -// keepComponentMediaTypes keeps the components/mediaTypes map OpenAPI 3.2 adds -// verbatim on the document, read as the parser reads it, at the entry and -// pointer the components census would use, so the census leaves it alone. The -// parser models no such section and this compiler does not resolve a reference -// into it yet (GitHub #856), so the map has no IR home. It is kept at every -// version, as load reports it below 3.2 (diag.LaterVersionConstruct). -func keepComponentMediaTypes(c lowering.Ctx, p *ir.Unmodeled) []ir.Diagnostic { - view := nodeview.New() - components := view.ChildByToken(nodeview.DocumentRoot(c.Doc.GetRootNode()), "components") - at := ids.Ptr("components", "mediaTypes") - kept, diags := annotation.PreserveNodeInto(p, annotation.EntryKey("components", "mediaTypes"), - view.ChildByToken(nodeview.Deref(components), "mediaTypes"), ir.ReasonNoIRHome, c.ProvenanceAt(at)) - if !kept { - return diags - } - return append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, at, - "components/mediaTypes is not resolved by this compiler yet (GitHub #856); kept verbatim under Unmodeled")) -} - -// documentUnknownKeys collects the keys the OpenAPI model names no field for -// from every object around the document metadata that lowers to no node of its -// own: the root, info with its contact and license, the root externalDocs, the -// components object, and each tag with its own externalDocs. -// -// ir.Document is the nearest node with an Unmodeled map for all of them, so each -// object's keys are scoped by the source path they were written at. One unscoped -// "openapi:status" would be a single key for every object, and the entry that -// survived would be whichever site ran last. -func documentUnknownKeys(c lowering.Ctx, p *ir.Unmodeled) []ir.Diagnostic { - sites := append(rootUnknownSites(c), tagUnknownSites(c)...) - diags := make([]ir.Diagnostic, 0, len(sites)) - for _, site := range sites { - diags = append(diags, - annotation.UnknownKeysUnder(p, site.model, c.ProvenanceAt, site.owner, site.scope)...) - } - return diags -} - -// unknownSite is one object's census: what it keys under on the carrier holding -// it, the object's own source pointer, and the parsed object itself. -type unknownSite struct { - scope string - owner jsontext.Pointer - model any -} - -// rootUnknownSites returns the census sites a document has exactly one of. The -// root's keys take no scope, since ir.Document stands for the OpenAPI Object -// itself; the rest are keyed by the path from it down to the object that wrote -// them. -// -// The components object is one of them, unlike the maps beneath it: `paths`, -// `responses` and a callback each embed a sequenced map, so every key they hold -// is an entry of it (docs/parser-library.md#embedded-map-keys), while the -// Components Object is a fixed-field struct and a key it does not define is as -// undeclared as one on any other object here. -func rootUnknownSites(c lowering.Ctx) []unknownSite { - info := c.Doc.GetInfo() - infoPtr := ids.Ptr("info") - return []unknownSite{ - {"", "", c.Doc}, - {"info", infoPtr, info}, - {"info/contact", infoPtr + ids.Ptr("contact"), info.GetContact()}, - {"info/license", infoPtr + ids.Ptr("license"), info.GetLicense()}, - {"externalDocs", ids.Ptr("externalDocs"), c.Doc.GetExternalDocs()}, - {"components", ids.Ptr("components"), c.Doc.GetComponents()}, - } -} - -// tagUnknownSites returns the census sites each declared tag contributes: its -// own, and its externalDocs object's. Neither ir.TagDef nor ir.Link holds an -// Unmodeled map, so both ride on the document. -// -// Scoped by index rather than name, which is the pointer a tag is written at. A -// name would read better, but OpenAPI's requirement that tag names be unique is -// the document's to keep and not this compiler's to rely on: two tags spelled -// alike would silently leave one entry. -func tagUnknownSites(c lowering.Ctx) []unknownSite { - tags := c.Doc.GetTags() - out := make([]unknownSite, 0, 2*len(tags)) - for i, t := range tags { - if t == nil { - continue - } - index := strconv.Itoa(i) - ptr := ids.Ptr("tags", index) - out = append(out, - unknownSite{"tags/" + index, ptr, t}, - unknownSite{"tags/" + index + "/externalDocs", ptr + ids.Ptr("externalDocs"), t.GetExternalDocs()}) - } - return out -} - -// documentExtensions collects the x-* of every object that lowers to no IR node -// of its own: the document root, the info block and the contact and license -// inside it, the root externalDocs, the components object, and each declared -// tag with its own externalDocs. ir.Document is the nearest node with an -// Unmodeled map for all of them, so each object's entries are keyed under the -// source path it was written at — see annotation.ExtensionsUnder for why one -// unscoped key for all of them would not do. -func documentExtensions(c lowering.Ctx) (ir.Unmodeled, []ir.Diagnostic) { - return annotation.ExtensionsAt(c.ProvenanceAt, append(rootExtensions(c), tagExtensions(c)...)...) -} - -// rootExtensions returns the extension sites a document has exactly one of. The -// root's own take no scope, since ir.Document stands for the OpenAPI Object -// itself; the rest are keyed by the path from it down to the object that wrote -// them. -func rootExtensions(c lowering.Ctx) []annotation.ExtensionSite { - info := c.Doc.GetInfo() - infoPtr := ids.Ptr("info") - return []annotation.ExtensionSite{ - {Scope: "", Owner: "", Ext: c.Doc.GetExtensions()}, - {Scope: "info", Owner: infoPtr, Ext: info.GetExtensions()}, - {Scope: "info/contact", Owner: infoPtr + ids.Ptr("contact"), Ext: info.GetContact().GetExtensions()}, - {Scope: "info/license", Owner: infoPtr + ids.Ptr("license"), Ext: info.GetLicense().GetExtensions()}, - {Scope: "externalDocs", Owner: ids.Ptr("externalDocs"), Ext: c.Doc.GetExternalDocs().GetExtensions()}, - {Scope: "components", Owner: ids.Ptr("components"), Ext: c.Doc.GetComponents().GetExtensions()}, - } -} - -// tagExtensions returns the extension sites each declared tag contributes: its -// own, and its externalDocs object's. ir.TagDef holds no Unmodeled map and -// neither does ir.Link, so both ride on the document. -// -// Keyed by the tag's index rather than its name, which is the pointer the tag -// is written at. A name would read better and is what TagDefs are found by, but -// OpenAPI's requirement that tag names be unique is the document's to keep, not -// this compiler's to rely on: two tags spelled alike would silently leave one -// entry. -func tagExtensions(c lowering.Ctx) []annotation.ExtensionSite { - tags := c.Doc.GetTags() - out := make([]annotation.ExtensionSite, 0, 2*len(tags)) - for i, t := range tags { - if t == nil { - continue - } - scope := "tags/" + strconv.Itoa(i) - ptr := ids.Ptr("tags", strconv.Itoa(i)) - out = append(out, - annotation.ExtensionSite{Scope: scope, Owner: ptr, Ext: t.GetExtensions()}, - annotation.ExtensionSite{Scope: scope + "/externalDocs", Owner: ptr + ids.Ptr("externalDocs"), - Ext: t.GetExternalDocs().GetExtensions()}) - } - return out -} - -// lowerInfo maps info onto the document identity, docs, contact, and license. -// GetInfo always returns a non-nil Info (it addresses an embedded struct value), -// so no nil guard is needed. -func lowerInfo(c lowering.Ctx) docMeta { - info := c.Doc.GetInfo() - m := docMeta{ - Name: info.GetTitle(), - Version: info.GetVersion(), - TermsOfService: info.GetTermsOfService(), - Docs: infoDocs(c, info), - } - if ct := info.GetContact(); ct != nil { - m.Contact = &ir.Contact{Name: ct.GetName(), URL: ct.GetURL(), Email: ct.GetEmail()} - } - if lic := info.GetLicense(); lic != nil { - m.License = &ir.License{Name: lic.GetName(), Identifier: lic.GetIdentifier(), URL: lic.GetURL()} - } - return m -} - -// infoDocs builds the document docs from info summary and description, folding -// in the root externalDocs link when present. -func infoDocs(c lowering.Ctx, info *soa.Info) ir.Docs { - d := ir.Docs{Summary: info.GetSummary(), Description: info.GetDescription()} - if ed := c.Doc.GetExternalDocs(); ed != nil { - d.ExternalDocs = append(d.ExternalDocs, ir.Link{URL: ed.GetURL(), Description: ed.GetDescription()}) - } - return d -} diff --git a/compilers/openapi/meta_test.go b/compilers/openapi/meta_test.go deleted file mode 100644 index 809ab01e..00000000 --- a/compilers/openapi/meta_test.go +++ /dev/null @@ -1,148 +0,0 @@ -package openapi - -import ( - "encoding/json/jsontext" - "testing" - - soa "github.com/speakeasy-api/openapi/openapi" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/dexpace/morphic/compilers/openapi/internal/diag" - "github.com/dexpace/morphic/compilers/openapi/internal/lowering" - "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" - "github.com/dexpace/morphic/ir" -) - -const metaSpec = `openapi: 3.2.0 -info: - title: Meta - version: "2" - summary: A summary - description: A description - contact: {name: Team, url: 'https://team', email: t@example.com} - license: {name: Apache-2.0, identifier: Apache-2.0} -externalDocs: {url: 'https://docs', description: docs} -x-top: {a: 1} -servers: - - url: https://{region}.example.com - name: primary - description: Primary - variables: - region: {default: us, enum: [us, eu], description: Region} -paths: - /p: - get: {operationId: p, responses: {"200": {description: ok}}} -` - -// TestMeta_UnserializableExtensionStillWarns pins the document-level twin of -// TestAuth_UnserializableExtensionStillWarns: when every top-level x-* extension -// fails to serialize, lowerMeta ends up with an empty Unmodeled map, so the -// warning must come from the extension reader rather than from a branch guarded -// on what was kept — the shape that used to drop it silently. -func TestMeta_UnserializableExtensionStillWarns(t *testing.T) { - t.Parallel() - spec := `openapi: 3.1.0 -info: {title: T, version: "1"} -paths: {} -x-bad: {1: intkey} -` - doc, diags := parseFull(t, spec) - assert.Positive(t, openapitest.CountDiagsAt(diags, diag.DegradedConstruct, ir.SeverityWarning), - "an entirely unserializable top-level extension still warns even though Unmodeled ends up empty") - assert.Empty(t, doc.Unmodeled, "the unserializable extension is dropped, not stored") -} - -func TestMeta_FullDocumentMetadata(t *testing.T) { - t.Parallel() - doc, _ := parseFull(t, metaSpec) - assert.Equal(t, "Meta", doc.Name) - assert.Equal(t, "2", doc.Version) - require.NotNil(t, doc.Contact) - assert.Equal(t, "t@example.com", doc.Contact.Email) - require.NotNil(t, doc.License) - assert.Equal(t, "Apache-2.0", doc.License.Identifier) - assert.NotEmpty(t, doc.Docs.ExternalDocs, "root externalDocs folded into docs") - assert.NotEmpty(t, doc.Unmodeled, "top-level x-* extension") - require.Len(t, doc.Servers, 1) - assert.Equal(t, ir.Naming{Source: "primary", Canonical: "primary"}, doc.Servers[0].Name, - "a declared 3.2 name is the server's name; the URL hint is for a server with none") - require.Len(t, doc.Servers[0].Variables, 1) - assert.Equal(t, []string{"us", "eu"}, doc.Servers[0].Variables[0].Enum) -} - -// TestTagExtensions_NilEntrySkipped pins the same guard lowerTagDefs keeps on -// the other side of the tag list: a nil entry contributes no extension site, so -// nothing dereferences it and no site is keyed at an index holding no tag. -func TestTagExtensions_NilEntrySkipped(t *testing.T) { - t.Parallel() - doc := &soa.OpenAPI{Tags: []*soa.Tag{nil, {Name: "kept"}}} - - got := tagExtensions(lowering.Ctx{Doc: doc}) - - require.Len(t, got, 2, "one surviving tag contributes its own site and its externalDocs one") - assert.Equal(t, "tags/1", got[0].Scope, "the site is keyed at the tag's own index, not its position") - assert.Equal(t, jsontext.Pointer("/tags/1"), got[0].Owner) -} - -// TestTagUnknownSites_NilEntrySkipped is TestTagExtensions_NilEntrySkipped's -// twin on the census side: the two walks of the tag list keep the same guard, so -// neither can be the one that dereferences a nil entry or keys a site at an -// index holding no tag. -func TestTagUnknownSites_NilEntrySkipped(t *testing.T) { - t.Parallel() - doc := &soa.OpenAPI{Tags: []*soa.Tag{nil, {Name: "kept"}}} - - got := tagUnknownSites(lowering.Ctx{Doc: doc}) - - require.Len(t, got, 2, - "the nil tag contributes nothing, and the surviving one contributes its own site and its externalDocs") - assert.Equal(t, "tags/1", got[0].scope, "the site is keyed at the tag's own index, not its position") - assert.Equal(t, jsontext.Pointer("/tags/1"), got[0].owner) - assert.Equal(t, "tags/1/externalDocs", got[1].scope, "and its externalDocs is scoped under that same index") - assert.Equal(t, jsontext.Pointer("/tags/1/externalDocs"), got[1].owner) -} - -func TestMeta_NoInfoNoServers(t *testing.T) { - t.Parallel() - // With no info block the title is empty; with no servers the library injects - // a default "/" server that is lowered. - spec := "openapi: 3.1.0\npaths: {}\n" - doc, _ := parseFull(t, spec) - assert.Empty(t, doc.Name) - require.Len(t, doc.Servers, 1) - assert.Equal(t, "/", doc.Servers[0].URLTemplate) - assert.Equal(t, ir.Naming{Hint: "server"}, doc.Servers[0].Name, - "the injected server is still a server an emitter has to name (GitHub #258)") -} - -// TestMeta_JSONSchemaDialectIsKept pins the document's jsonSchemaDialect, which -// nothing read: it is kept verbatim on the document with an info, as a schema's -// own `$schema` is, written directly or merged in through `<<`, and in a 3.0 -// document, which it postdates, load also reports it. Declaring none keeps -// nothing. -func TestMeta_JSONSchemaDialectIsKept(t *testing.T) { - t.Parallel() - const dialect = "https://json-schema.org/draft/2020-12/schema" - for _, tc := range []struct { - name, version, line string - }{ - {"3.1", "3.1.0", "jsonSchemaDialect: " + dialect}, - {"3.1 merged in", "3.1.0", "<<: {jsonSchemaDialect: '" + dialect + "'}"}, - {"3.0", "3.0.3", "jsonSchemaDialect: " + dialect}, - } { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - doc, diags := parseFull(t, "openapi: "+tc.version+"\n"+tc.line+"\ninfo: {title: T, version: \"1\"}\npaths: {}\n") - kept, ok := doc.Unmodeled["openapi:jsonSchemaDialect"] - require.True(t, ok, "the dialect is kept on the document") - assert.Equal(t, ir.ReasonOutOfScope, kept.Reason) - assert.JSONEq(t, `"`+dialect+`"`, string(kept.Value)) - openapitest.AssertInfoDiagAt(t, diags, "/jsonSchemaDialect") - assert.Equal(t, tc.version == "3.0.3", - openapitest.HasDiagCodeAt(diags, diag.LaterVersionConstruct, "/jsonSchemaDialect")) - }) - } - doc, _ := parseFull(t, "openapi: 3.1.0\ninfo: {title: T, version: \"1\"}\npaths: {}\n") - assert.NotContains(t, doc.Unmodeled, "openapi:jsonSchemaDialect") -} diff --git a/compilers/openapi/openapi.go b/compilers/openapi/openapi.go index 6193bb76..878ec42e 100644 --- a/compilers/openapi/openapi.go +++ b/compilers/openapi/openapi.go @@ -12,6 +12,7 @@ import ( "github.com/dexpace/morphic/compilers/openapi/internal/annotation" "github.com/dexpace/morphic/compilers/openapi/internal/auth" "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/compilers/openapi/internal/document" "github.com/dexpace/morphic/compilers/openapi/internal/ids" "github.com/dexpace/morphic/compilers/openapi/internal/load" "github.com/dexpace/morphic/compilers/openapi/internal/lowering" @@ -201,7 +202,7 @@ func run(ctx context.Context, c lowering.Ctx, ts *compile.Types) (*ir.Document, } out.Types = reg - m, metaDiags := lowerMeta(c) + m, metaDiags := document.Lower(c) out.Name, out.Version = m.Name, m.Version out.TermsOfService, out.Docs = m.TermsOfService, m.Docs out.Contact, out.License = m.Contact, m.License diff --git a/compilers/openapi/overlay_test.go b/compilers/openapi/overlay_test.go index 0211d725..44c8ebfd 100644 --- a/compilers/openapi/overlay_test.go +++ b/compilers/openapi/overlay_test.go @@ -926,8 +926,8 @@ func TestCompile_OverlayIsCreditedWithTheKeysItAdds(t *testing.T) { {"OAuth flows unknown key (schemes.go applySchemeAnnotations)", "openapi:flows/flowsBogus", 1}, {"OAuth flow extension (flows.go applyFlowAnnotations)", "openapi:x-flow", 1}, {"OAuth flow unknown key (flows.go applyFlowAnnotations)", "openapi:flowBogus", 1}, - {"document info extension (meta.go documentExtensions)", "openapi:info/x-info", 1}, - {"document info unknown key (meta.go documentUnknownKeys)", "openapi:info/infoBogus", 1}, + {"document info extension (sites.go documentExtensions)", "openapi:info/x-info", 1}, + {"document info unknown key (sites.go documentUnknownKeys)", "openapi:info/infoBogus", 1}, {"server extension (servers.go lowerServer)", "openapi:x-srv", 1}, {"server unknown key (servers.go lowerServer)", "openapi:srvBogus", 1}, {"server variable extension (servers.go serverVariables)", "openapi:x-var", 1}, diff --git a/docs/compilers/openapi.md b/docs/compilers/openapi.md index b0b8f5a0..b5a263c2 100644 --- a/docs/compilers/openapi.md +++ b/docs/compilers/openapi.md @@ -499,7 +499,7 @@ of its own is reported as an `openapi/cycle-scan-failed` warning, and the compil `info.title` ([`naming`](#naming)). - Cases: `docs-summary-desc` -- Tests: `openapi.TestMeta_FullDocumentMetadata`, `openapi.TestCompile_EndToEnd` +- Tests: `document.TestMeta_FullDocumentMetadata`, `openapi.TestCompile_EndToEnd` ### `self-uri` @@ -606,9 +606,9 @@ that decision lives (§12), which is what keeps a promotion pass from mistaking - Cases: `unreferenced-components`, `component-media-types-32` - Codes: `openapi/degraded-construct` -- Tests: `openapi.TestUnreferencedComponents_ReferenceNothingFollowsKeepsItsTarget`, - `openapi.TestUnreferencedComponents_ReadAsTheParserReadsThem`, - `openapi.TestUnreferencedComponents_SelfReferenceByFileNameReaches` +- Tests: `document.TestUnreferencedComponents_ReferenceNothingFollowsKeepsItsTarget`, + `document.TestUnreferencedComponents_ReadAsTheParserReadsThem`, + `document.TestUnreferencedComponents_SelfReferenceByFileNameReaches` - Status: partly built: no reference into `components/mediaTypes` is followed, so the section is kept whole however much of it a document uses (#856), and a section entry whose key is no scalar is dropped without a diagnostic (#947). @@ -1890,7 +1890,7 @@ schema declaring it. - Cases: `dialect-keywords` - Codes: `openapi/degraded-construct` -- Tests: `schema.TestDialectKeywords_KeptOutOfScope`, `openapi.TestMeta_JSONSchemaDialectIsKept` +- Tests: `schema.TestDialectKeywords_KeptOutOfScope`, `document.TestMeta_JSONSchemaDialectIsKept` ### `vocabulary-coverage` @@ -2566,7 +2566,7 @@ others in the lists [`path-item-servers`](#path-item-servers) keeps, and reporte - Cases: `servers-variables` - Codes: `openapi/duplicate-server-name` -- Tests: `openapi.TestDuplicateServerName_BothServersLower`, +- Tests: `document.TestDuplicateServerName_BothServersLower`, `load.TestServerRepeats_ReportsEveryListTheSourceWrites`, `load.TestServerRepeats_ReportsOnlyANameOneListRepeatsExactly` - Status: partly built: the default server the parser injects for a document writing none is lowered diff --git a/internal/archtest/arch_test.go b/internal/archtest/arch_test.go index 301a3ea2..02ef4bc2 100644 --- a/internal/archtest/arch_test.go +++ b/internal/archtest/arch_test.go @@ -43,8 +43,7 @@ var rules = map[string][]string{ // and ID grammars. It reaches ir alone, so no compiler's vocabulary rides in. "compilers/compile": {module + "/ir"}, "compilers/openapi": {module + "/ir", module + "/compilers", module + "/compilers/compile", - module + "/compilers/openapi/internal" + subtreeSuffix, - "github.com/speakeasy-api/openapi" + subtreeSuffix, "gopkg.in/yaml.v3"}, + module + "/compilers/openapi/internal" + subtreeSuffix, "gopkg.in/yaml.v3"}, // The diagnostic vocabulary sits at the bottom of the compiler's own import // graph: every package that reports anything reaches it, so it must reach // nothing but ir. Its own entry says that, rather than letting it inherit the @@ -265,6 +264,17 @@ var rules = map[string][]string{ module + "/compilers/openapi/internal/resolve", module + "/compilers/openapi/internal/schema", "github.com/speakeasy-api/openapi" + subtreeSuffix, "gopkg.in/yaml.v3"}, + // What a document says about itself: its info, its servers, and what the + // objects around them write that lowers to no node of its own. It runs after + // both walks and reaches neither: the compiler hands it the context and + // assigns what it returns onto the Document. + "compilers/openapi/internal/document": {module + "/ir", module + "/compilers/compile", + module + "/compilers/openapi/internal/annotation", + module + "/compilers/openapi/internal/diag", + module + "/compilers/openapi/internal/ids", + module + "/compilers/openapi/internal/lowering", + module + "/compilers/openapi/internal/nodeview", + "github.com/speakeasy-api/openapi/openapi"}, // The canaries that pin docs/parser-library.md. Its one production file is // the package doc: the tests call the library directly, and test files are // outside these rules. @@ -430,7 +440,7 @@ func TestImportGraph_ContractEntryAdmitsNoSiblingCompiler(t *testing.T) { allows := func(ip string) bool { return admit(ip, allowed, make([]bool, len(allowed))) } assert.True(t, allows(module+"/compilers"), "the contract package") assert.True(t, allows(module+"/compilers/compile"), "the shared framework package") - assert.True(t, allows("github.com/speakeasy-api/openapi/jsonschema/oas3"), + assert.True(t, allows(module+"/compilers/openapi/internal/schema"), "a subtree entry still admits the packages under it") assert.False(t, allows(module+"/compilers/graphql"), "a sibling compiler must not ride in on the contract entry") diff --git a/internal/archtest/recursion_test.go b/internal/archtest/recursion_test.go index 684e913d..48fb1400 100644 --- a/internal/archtest/recursion_test.go +++ b/internal/archtest/recursion_test.go @@ -97,6 +97,7 @@ var schemaRecursion = []string{ var loweringPackages = []string{ "compilers/openapi", "compilers/openapi/internal/auth", + "compilers/openapi/internal/document", "compilers/openapi/internal/operation", "compilers/openapi/internal/schema", } From 81b6f9b8f77006670431708765ac13e4178fc1b6 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 05:37:14 +0300 Subject: [PATCH 02/21] refactor(compilers/openapi): move detection's reading into a package Most of detect.go was not the Detect method but how detection reads a source: the parse and the probe of its root mapping, and the byte scan that decides whether a source that will not parse is this compiler's to report. That moves to internal/detect behind one entry, Recognize, which takes the source index, the bytes and the byte budget and answers as Detect does. Detect is left turning the caller's options into that budget. One entry, rather than exporting the probe's parts for Detect to switch on: which decline to report needs what only the probe knows, its cut-short merge read and its scan of unreadable bytes, so the switch belongs with it. Recognize takes no path, which makes Detect's "the path is not consulted" true by construction, and the test tables lose the path column they carried for nothing. The tests follow the code, renamed TestRecognize_* where they called Detect. The root keeps those of its own: the byte budget read from the caller's options and detection's agreement with the compile, now in its external test package. Two new tests pin from inside the package what only the root's tests reached: the byte budget, and that every decline names the source index it is handed. rootMergeChain, wanted by both packages, moves to openapitest. The root no longer imports yaml.v3, so its allowlist drops it. --- compilers/openapi/detect.go | 459 +--------- compilers/openapi/detect_budget_test.go | 65 -- compilers/openapi/detect_test.go | 787 ++---------------- compilers/openapi/internal/detect/declares.go | 220 +++++ .../openapi/internal/detect/declares_test.go | 88 ++ compilers/openapi/internal/detect/doc.go | 10 + .../openapi/internal/detect/helpers_test.go | 59 ++ compilers/openapi/internal/detect/probe.go | 172 ++++ .../openapi/internal/detect/probe_test.go | 256 ++++++ .../detect/reading_test.go} | 110 +-- .../openapi/internal/detect/recognize.go | 58 ++ .../openapi/internal/detect/recognize_test.go | 403 +++++++++ compilers/openapi/internal/detect/version.go | 34 + .../openapi/internal/detect/version_test.go | 14 + .../openapi/internal/openapitest/spec.go | 15 + .../openapi/internal/openapitest/spec_test.go | 15 + compilers/openapi/openapi_test.go | 19 + docs/compilers/openapi.md | 8 +- internal/archtest/arch_test.go | 10 +- 19 files changed, 1483 insertions(+), 1319 deletions(-) delete mode 100644 compilers/openapi/detect_budget_test.go create mode 100644 compilers/openapi/internal/detect/declares.go create mode 100644 compilers/openapi/internal/detect/declares_test.go create mode 100644 compilers/openapi/internal/detect/doc.go create mode 100644 compilers/openapi/internal/detect/helpers_test.go create mode 100644 compilers/openapi/internal/detect/probe.go create mode 100644 compilers/openapi/internal/detect/probe_test.go rename compilers/openapi/{detect_reading_test.go => internal/detect/reading_test.go} (80%) create mode 100644 compilers/openapi/internal/detect/recognize.go create mode 100644 compilers/openapi/internal/detect/recognize_test.go create mode 100644 compilers/openapi/internal/detect/version.go create mode 100644 compilers/openapi/internal/detect/version_test.go diff --git a/compilers/openapi/detect.go b/compilers/openapi/detect.go index 78d7e363..82f2adff 100644 --- a/compilers/openapi/detect.go +++ b/compilers/openapi/detect.go @@ -1,35 +1,11 @@ package openapi import ( - "bytes" - "errors" - "fmt" - "strings" - - yaml "gopkg.in/yaml.v3" - "github.com/dexpace/morphic/compilers" - "github.com/dexpace/morphic/compilers/openapi/internal/diag" - "github.com/dexpace/morphic/compilers/openapi/internal/load" - "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" + "github.com/dexpace/morphic/compilers/openapi/internal/detect" "github.com/dexpace/morphic/ir" ) -// sniffProbe holds the two discriminating top-level keys. Which one is present -// is the whole of the format question: an OpenAPI 3.x document declares -// `openapi`, a Swagger 2.0 document declares `swagger`. -// -// It carries no struct tags: nothing decodes into it. The parse over a tree -// names the two keys itself, in fieldFor. -type sniffProbe struct { - OpenAPI string - Swagger string - // Cut reports a root mapping that merges through `<<` past - // nodeview.MergeDepthLimit without settling the answer, so a key the read - // did not reach may outrank what it found. - Cut bool -} - // Detect implements compilers.Compiler. It reports the dialect src declares by // its version's major.minor prefix, naming swagger@2.0 too, which it does not // serve, so the caller can say unsupported rather than unreadable. The path is @@ -40,35 +16,12 @@ type sniffProbe struct { // past it is declined unread with Compile's own refusal. // // Unreadable bytes are declined silently unless they declare a version under a -// discriminating key (declaresProbeKey); then the parser's complaint is -// reported, since that source is this compiler's own and broken. +// discriminating key; then the parser's complaint is reported, since that +// source is this compiler's own and broken. func (*Compiler) Detect(src compilers.Source, opts compilers.Options) (compilers.Recognition, []ir.Diagnostic, bool) { // Source 0 is src: the one source detection is handed is the whole of the // table a declined diagnostic can index (compilers.Compiler.Detect). - atSource := ir.Provenance{Source: rootSrcIndex} - if d, over := load.OverByteBudget(atSource, src.Data, detectionBudget(opts)); over { - return compilers.Recognition{}, []ir.Diagnostic{d}, false - } - - probe, parsed, err := sniff(src.Data) - switch { - case probe.Cut: - // Declining in silence would let the bound pass for an answer. - return compilers.Recognition{}, []ir.Diagnostic{diag.Newf( - ir.SeverityError, diag.BudgetExceeded, atSource, - "source cannot be recognized: its root mapping merges through `<<` deeper than %d levels, "+ - "past which a version it declares is not read", nodeview.MergeDepthLimit)}, false - case probe.OpenAPI != "": - return recognized("openapi", probe.OpenAPI, parsed), nil, true - case probe.Swagger != "": - return recognized("swagger", probe.Swagger, parsed), nil, true - case err != nil && declaresProbeKey(src.Data): - return compilers.Recognition{}, []ir.Diagnostic{diag.Newf( - ir.SeverityError, diag.UndecodableSource, atSource, - "source declares an OpenAPI or Swagger version and cannot be read: %s", diag.OneLine(err))}, false - default: - return compilers.Recognition{}, nil, false - } + return detect.Recognize(rootSrcIndex, src.Data, detectionBudget(opts)) } // detectionBudget is the byte budget detection is held to: the one opts would @@ -82,407 +35,3 @@ func detectionBudget(opts compilers.Options) int { } return bounded(o.Limits.MaxSourceBytes) } - -// recognized builds the recognition for a source that declared version under -// name, carrying the parse that read it. -// -// parsed is never nil here. A probe names a key only when decodeYAML read it -// off a parsed root, so a recognition always has a parse to carry — which -// matters, since a nil *load.Parsed put into the interface would make one that -// is not nil, and a consumer's type assertion would accept it and read through. -func recognized(name, version string, parsed *load.Parsed) compilers.Recognition { - return compilers.Recognition{ - Format: compilers.SourceFormat{Name: name, Version: majorMinor(version)}, - Parsed: parsed, - } -} - -// declaresProbeKey reports whether data declares a version under a -// discriminating key. Detect asks only once the probe cannot read the source, -// where "not YAML" alone fits a Protobuf or Smithy source too, and only within -// its byte budget. -// -// Neither reading may widen to "the name occurs before a colon": other formats -// nest such a key and must not get this compiler's parse error. The version -// makes the claim (GitHub #497), so a spec broken on its version line, -// `openapi: [3.1.0`, is unrecognized rather than unreadable, and a README -// quoting a spec in a code block is claimed like one. -func declaresProbeKey(data []byte) bool { - data = bytes.TrimPrefix(data, []byte(bomUTF8)) - return declaresBlockKey(data, "openapi") || declaresBlockKey(data, "swagger") || - declaresFlowKey(data) -} - -// declaresBlockKey reports whether data writes key bare at the start of a line -// with a version beside it. Column 0 is where block style puts a top-level key; -// nested keys and block scalar content are indented. -// -// Only the bare spelling is read: a quoted key is flow style's, scoped by -// declaresFlowKey. A block document that quotes its top-level key is declined -// in silence, the safe direction to be wrong, since admitting quoted keys at -// column 0 would take in what JSON writes at every depth. The line is parsed -// alone, since the document around it did not parse. -func declaresBlockKey(data []byte, key string) bool { - prefix := []byte(key + ":") - read := 0 - for i := 0; i < len(data) && read < maxVersionLines; { - line, next := nextLine(data, i) - if bytes.HasPrefix(line, prefix) { - if lineDeclaresVersion(line, key) { - return true - } - read++ - } - i = next - } - return false -} - -// maxVersionLines bounds how many lines declaresBlockKey parses. A document -// declares its version once and a stream a few times over, so the bound is room -// to spare for anything written; without it, a crafted file of nothing but such -// lines, broken at the end, cost nearly three times the parse that failed on -// it, one small parse per line. -const maxVersionLines = 8 - -// lineDeclaresVersion reports whether line, parsed on its own, is a mapping of -// key to a scalar that reads as a version. An alias there names a node on -// another line, which the line alone cannot resolve, so it declares nothing. -func lineDeclaresVersion(line []byte, key string) bool { - var doc yaml.Node - if yaml.Unmarshal(line, &doc) != nil { - return false - } - root := documentRoot(&doc) - if root == nil || root.Kind != yaml.MappingNode || len(root.Content) != 2 || root.Content[0].Value != key { - return false - } - version, err := probeVersion(root.Content[1]) - return err == nil && isVersion(version) -} - -// declaresFlowKey reports whether data opens a flow mapping, the shape JSON -// writes, that names one of the discriminating keys among its own entries. -// -// Nesting depth is what makes the answer top-level: a plain search for -// `"openapi":` matches wherever it sits, and other formats nest one. A source -// that opens no mapping, a JSON array say, declares nothing. -// -// It is a lexer, not a parser, because it exists for bytes that will not parse: -// there is no tree to ask. -func declaresFlowKey(data []byte) bool { - i := skipNodeProperties(data, skipSpaceAndComments(data, 0)) - if i == len(data) || data[i] != '{' { - return false - } - - for depth := 0; i < len(data); { - switch data[i] { - case '"': - name, next := flowString(data, i) - if depth == 1 && isProbeName(name) && versionAfterColon(data, next) { - return true - } - i = next - case '{', '[': - depth++ - i++ - case '}', ']': - depth-- - i++ - default: - i++ - } - } - return false -} - -// flowString returns the bytes between the quotes of the string data[i] opens, -// and the index just past its closing quote. An unterminated string runs to the -// end of data: there is nothing past it left to read. -func flowString(data []byte, i int) ([]byte, int) { - for j := i + 1; j < len(data); j++ { - switch data[j] { - case '\\': - j++ - case '"': - return data[i+1 : j], j + 1 - } - } - return nil, len(data) -} - -// isProbeName reports whether name is one of the discriminating keys. -func isProbeName(name []byte) bool { - return string(name) == "openapi" || string(name) == "swagger" -} - -// skipSpace returns the index of the first byte at or after i that is not -// whitespace, or len(data) if there is none. -func skipSpace(data []byte, i int) int { - for i < len(data) && (data[i] == ' ' || data[i] == '\t' || data[i] == '\r' || data[i] == '\n') { - i++ - } - return i -} - -// skipSpaceAndComments returns the index of the first byte at or after i that -// begins content: past whitespace, and past any line a `#` opens. A flow -// document may follow a comment line, and yaml.v3 reads it there. -func skipSpaceAndComments(data []byte, i int) int { - for i = skipSpace(data, i); i < len(data) && data[i] == '#'; i = skipSpace(data, i) { - _, i = nextLine(data, i) - } - return i -} - -// maxNodeProperties is how many node properties a node may carry: one anchor -// and one tag, in either order. It bounds skipNodeProperties. -const maxNodeProperties = 2 - -// skipNodeProperties returns the index just past any node properties at i — an -// `&anchor` or a `!tag`, each a word ended by whitespace or a flow indicator, -// neither of which a name may contain — and the whitespace after them. YAML -// writes them in front of the node they annotate, so a document's root flow -// mapping may sit behind one or two of them: `&x {`, `!!map &x {`. A guard -// testing the first byte for the opener would see the property instead, and -// decline a broken document that is this compiler's own. -func skipNodeProperties(data []byte, i int) int { - for range maxNodeProperties { - if i == len(data) || (data[i] != '&' && data[i] != '!') { - return i - } - for i < len(data) && !endsWord(data[i]) { - i++ - } - i = skipSpace(data, i) - } - return i -} - -// nextLine returns the line beginning at i, without its terminator, and the -// index of the line after it. -func nextLine(data []byte, i int) (line []byte, next int) { - line = data[i:] - if j := bytes.IndexByte(line, '\n'); j >= 0 { - return line[:j], i + j + 1 - } - return line, len(data) -} - -// endsWord reports whether b ends a bare word in flow context: whitespace, or a -// flow indicator, none of which an anchor, a tag or a bare version contains. -func endsWord(b byte) bool { - switch b { - case ',', '}', ']', '{', '[', ' ', '\t', '\r', '\n': - return true - default: - return false - } -} - -// versionAfterColon reports whether a colon follows i, making the name before it -// a key, and a version follows the colon: a quoted string or a bare word that -// reads as one. A collection there opens with a flow indicator, so it reads as -// an empty word and declares nothing. -func versionAfterColon(data []byte, i int) bool { - i = skipSpace(data, i) - if i == len(data) || data[i] != ':' { - return false - } - i = skipSpace(data, i+1) - if i < len(data) && data[i] == '"' { - value, _ := flowString(data, i) - return isVersion(string(value)) - } - end := i - for end < len(data) && !endsWord(data[end]) { - end++ - } - return isVersion(string(data[i:end])) -} - -// sniff reads the discriminating keys out of data. For anything it cannot read -// it returns the zero probe and the parser's error; whether that error is -// worth reporting is Detect's question. -// -// The document is decoded whole and exactly, the only way to tell one that -// declares nothing from one that will not parse. The parse is the one the -// compile lowers, not a second reading beside it that would have to be kept in -// agreement with it (GitHub #486). The caller's byte budget, which Detect -// checks first, bounds it. -func sniff(data []byte) (sniffProbe, *load.Parsed, error) { - probe, parsed, err := decodeYAML(data) - return declaredVersions(probe), parsed, err -} - -// declaredVersions drops any value that does not read as a version. A key alone -// does not declare a format: another format's document may write the word — at -// column 0 in Markdown prose, or as a field of its own — and what separates that -// from a declaration is the version beside it. Claiming it instead reports this -// compiler's complaint over a file that was never its own. -func declaredVersions(probe sniffProbe) sniffProbe { - if !isVersion(probe.OpenAPI) { - probe.OpenAPI = "" - } - if !isVersion(probe.Swagger) { - probe.Swagger = "" - } - return probe -} - -// isVersion reports whether value reads as a version rather than prose: one -// word, beginning with a digit. That admits every shape majorMinor is written -// for, and suffixed ones such as "3.1.0-rc1" on purpose: a document writing one -// is this compiler's own and wrong, and the precise complaint is load's to -// make, where declining here would give the engine's generic "unrecognized -// format". Prose has a space in it, and that is what the guard keeps out. -func isVersion(value string) bool { - if value == "" || value[0] < '0' || value[0] > '9' { - return false - } - return !strings.ContainsAny(value, " \t") -} - -// bomUTF8 is the UTF-8 byte-order mark. YAML admits one at the start of a -// stream and JSON is its subset, so a spec written by an editor that emits one -// is a spec like any other: yaml.v3 reads straight through it and so does the -// loader. declaresProbeKey reads bytes rather than a parse and has to skip it -// itself, or a spec with one would be declined in silence when broken where a -// spec without one is reported. -const bomUTF8 = "\xef\xbb\xbf" - -// decodeYAML reads the probe keys from a complete YAML (or JSON, its subset) -// document. -// -// The document is parsed and its root mapping read, never decoded into -// sniffProbe: yaml.v3 compares every pair of a mapping's keys before reading -// any, so a mapping repeating one key n times raises n(n-1)/2 errors and then -// abandons the mapping, leaving the probe empty as well as expensive. Reading -// the keys off the tree is linear and answers the same for repeated keys, -// which the compile's own parse reports once each, sited. -func decodeYAML(data []byte) (sniffProbe, *load.Parsed, error) { - parsed, err := load.Decode(data) - if err != nil { - return sniffProbe{}, nil, err - } - - root := documentRoot(parsed.Root()) - switch { - case root == nil: - // A stream that carried no document declares no key, which is a decline - // and not a failure: empty bytes are no more this compiler's than - // anybody else's. - return sniffProbe{}, nil, nil - case root.Kind != yaml.MappingNode: - return sniffProbe{}, nil, fmt.Errorf("document root is %s, not a mapping", root.ShortTag()) - default: - probe, err := probeFromMapping(root) - return probe, parsed, err - } -} - -// documentRoot returns the content node of the document load handed back, or -// nil for a stream that carried none. That document is the first in the stream -// that holds content — the one the compile lowers, read by the same function, -// so a source routed here on what that document declares is the source lowered -// here (GitHub #481). The documents after it are reported as dropped by load -// (GitHub #387); detection routes the source and says nothing about its shape. -func documentRoot(doc *yaml.Node) *yaml.Node { - if doc.Kind != yaml.DocumentNode || len(doc.Content) != 1 { - return nil - } - return doc.Content[0] -} - -// probeFromMapping reads the probe keys off a root mapping through nodeview, -// the one reading of a mapping the compile's parser gives: through aliases, its -// `<<` merges expanded as the library expands them, and a key written twice at -// its last value (docs/parser-library.md#duplicate-key). A second reading -// would let detection name a version the compile does not read, which -// compilers.Detection.Compile checks. -// -// Only a key's effective value is read, so a value it overrides cannot fail -// the probe. Only the depth bound hides a key (View.Exhausted): a merge cycle, -// which the compile refuses, closes having read every mapping on it. -func probeFromMapping(root *yaml.Node) (sniffProbe, error) { - view := nodeview.New() - var probe sniffProbe - for _, pair := range view.MappingPairs(root) { - field := probe.fieldFor(pair.Key) - if field == nil { - continue - } - version, err := probeVersion(pair.Val) - if err != nil { - return sniffProbe{}, err - } - *field = version - } - probe.Cut = view.Exhausted() && !settledByOwnPairs(root, probe) - return probe, nil -} - -// settledByOwnPairs reports whether probe, read off root, decides detection -// although root's merges were cut short. A mapping's own pair outranks every -// merge, so a key root writes itself holds the value read: `openapi` settles it -// when that value is a version, and with `swagger` written too whatever it is. -// A key root leaves to its merges may lie past the bound. -func settledByOwnPairs(root *yaml.Node, probe sniffProbe) bool { - own := map[string]bool{} - for i := 0; i+1 < len(root.Content); i += 2 { - if key, ok := nodeview.KeyOf(root.Content[i]); ok { - own[key] = true - } - } - return own["openapi"] && (isVersion(probe.OpenAPI) || own["swagger"]) -} - -// probeVersion returns the version string a probe key's value declares, and an -// error for a value that is no scalar, or none at all. -// -// The scalar's text is taken as written rather than decoded, because the two -// disagree only for tags no version carries — a version key is not !!binary — -// and because decoding is what must not happen here: a mapping handed back to -// the decoder is the quadratic path decodeYAML exists to avoid, and a probe -// key's own value is the last place one could still be handed to it. -func probeVersion(value *yaml.Node) (string, error) { - if value == nil { - return "", errors.New("version key has no value") - } - if value.Kind != yaml.ScalarNode { - return "", fmt.Errorf("version key is %s, not a scalar", value.ShortTag()) - } - return value.Value, nil -} - -// fieldFor returns the probe field that key names, or nil for a key that names -// neither. -func (p *sniffProbe) fieldFor(key string) *string { - switch key { - case "openapi": - return &p.OpenAPI - case "swagger": - return &p.Swagger - default: - return nil - } -} - -// majorMinor returns the "major.minor" prefix of a dotted version string, -// e.g. "3.1.0" → "3.1". Strings with fewer than two dots — a bare major -// version, or a version already in major.minor form — are returned unchanged. -func majorMinor(version string) string { - firstDot := -1 - for i := range len(version) { - if version[i] != '.' { - continue - } - if firstDot < 0 { - firstDot = i - continue - } - return version[:i] - } - return version -} diff --git a/compilers/openapi/detect_budget_test.go b/compilers/openapi/detect_budget_test.go deleted file mode 100644 index 23f356dc..00000000 --- a/compilers/openapi/detect_budget_test.go +++ /dev/null @@ -1,65 +0,0 @@ -package openapi - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/dexpace/morphic/compilers" - "github.com/dexpace/morphic/compilers/openapi/internal/diag" -) - -const budgetedSpec = "openapi: 3.1.0\ninfo: {title: T, version: \"1\"}\npaths: {}\n" - -func withBytes(limit int) compilers.Options { - return compilers.Options{FormatOptions: Options{Limits: Limits{MaxSourceBytes: limit}}} -} - -// TestDetect_DeclinesASourcePastTheCallersByteBudget pins that recognition is -// held to the budget the compile is. A source past it is declined unread and -// the decline says why, in the words Compile uses for the same source, since -// "nothing recognized it" would send the caller to the document rather than to -// the budget they set. -func TestDetect_DeclinesASourcePastTheCallersByteBudget(t *testing.T) { - t.Parallel() - src := compilers.Source{Path: "api.yaml", Data: []byte(budgetedSpec)} - limit := len(budgetedSpec) - 1 - - rec, diags, ok := New().Detect(src, withBytes(limit)) - - assert.False(t, ok) - assert.Equal(t, compilers.Recognition{}, rec) - require.Len(t, diags, 1) - assert.Equal(t, diag.BudgetExceeded, diags[0].Code) - assert.Equal(t, 0, diags[0].Provenance.Source, "detection names the one source it was handed") - - _, compiled, err := New().Compile(t.Context(), []compilers.Source{src}, withBytes(limit)) - require.NoError(t, err) - require.Len(t, compiled, 1) - assert.Equal(t, compiled[0].Message, diags[0].Message, "detection and the compile refuse the source alike") -} - -// TestDetect_ReadsASourceWithinTheCallersByteBudget pins the other side, -// including a caller who turned the budget off. That case is asserted on a -// small source: the byte budget is the only bound on what detection reads, so -// off means unbounded, but a source past the 64 MiB default costs 15 s and -// 1.3 GB under -race, which is what the gate's only test step runs with. It -// was measured by hand instead when the byte scan was deleted (GitHub #486): -// with the budget off, a source past 64 MiB is recognized. -func TestDetect_ReadsASourceWithinTheCallersByteBudget(t *testing.T) { - t.Parallel() - src := compilers.Source{Path: "api.yaml", Data: []byte(budgetedSpec)} - want := compilers.SourceFormat{Name: "openapi", Version: "3.1"} - - for _, opts := range []compilers.Options{ - withBytes(len(budgetedSpec)), - withBytes(-1), - {}, - {FormatOptions: "another compiler's options"}, - } { - rec, diags, ok := New().Detect(src, opts) - require.True(t, ok, "%+v: %+v", opts, diags) - assert.Equal(t, want, rec.Format, "%+v", opts) - } -} diff --git a/compilers/openapi/detect_test.go b/compilers/openapi/detect_test.go index 10bbd842..d81079fe 100644 --- a/compilers/openapi/detect_test.go +++ b/compilers/openapi/detect_test.go @@ -1,739 +1,102 @@ -package openapi +package openapi_test import ( - "fmt" - "strings" "testing" - soa "github.com/speakeasy-api/openapi/openapi" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - yaml "gopkg.in/yaml.v3" "github.com/dexpace/morphic/compilers" + "github.com/dexpace/morphic/compilers/openapi" "github.com/dexpace/morphic/compilers/openapi/internal/diag" - "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" - "github.com/dexpace/morphic/compilers/openapi/internal/ynode" - "github.com/dexpace/morphic/ir" + "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" ) -func TestDetect_Formats(t *testing.T) { - t.Parallel() - cases := []struct { - name, path, src string - want compilers.SourceFormat - wantOK bool - // wantCode is the codes of the diagnostics a decline carries. Empty for - // every source this compiler recognizes, and for every source that is - // another format's business. - wantCode []string - }{ - {"openapi 3.1 yaml", "api.yaml", "openapi: 3.1.0\ninfo: {}\n", - compilers.SourceFormat{Name: "openapi", Version: "3.1"}, true, nil}, - {"openapi 3.0 json", "api.json", `{"openapi": "3.0.3"}`, - compilers.SourceFormat{Name: "openapi", Version: "3.0"}, true, nil}, - {"openapi 3.2 yaml", "api.yaml", "openapi: 3.2.0\ninfo: {}\n", - compilers.SourceFormat{Name: "openapi", Version: "3.2"}, true, nil}, - // A version already in major.minor form (single dot) exercises - // majorMinor's unchanged-passthrough return. - {"openapi major.minor only", "api.yaml", "openapi: \"3.1\"\n", - compilers.SourceFormat{Name: "openapi", Version: "3.1"}, true, nil}, - // A bare-major version (no dot) also reaches the passthrough return. - {"openapi bare major", "api.yaml", "openapi: \"4\"\n", - compilers.SourceFormat{Name: "openapi", Version: "4"}, true, nil}, - // Recognized, and deliberately not a format Formats reports: naming it - // lets the caller say "unsupported" rather than "unreadable". - {"swagger", "api.yaml", "swagger: \"2.0\"\n", - compilers.SourceFormat{Name: "swagger", Version: "2.0"}, true, nil}, - - // Everything below is another format's input, and none of it is an error - // here: three of the five planned compilers take bytes that are not YAML. - {"protobuf", "svc.proto", "syntax = \"proto3\";\nservice S { rpc Get (Q) returns (A); }\n", - compilers.SourceFormat{}, false, nil}, - {"typespec", "main.tsp", "import \"@typespec/http\";\nmodel Pet { name: string; }\n", - compilers.SourceFormat{}, false, nil}, - {"graphql", "s.graphql", "type Query {\n pet(id: ID!): Pet\n}\n", - compilers.SourceFormat{}, false, nil}, - // Valid YAML that declares neither key: whether a source parses is not - // the question detection answers. - {"graphql that parses as yaml", "s.graphql", "type Query { a: String }\n", - compilers.SourceFormat{}, false, nil}, - {"yaml that is no spec", "junk.yaml", "hello: world\n", - compilers.SourceFormat{}, false, nil}, - // Declares a version and will not parse: this compiler's own source, - // broken, which nothing else is in a position to say. - {"unparseable yaml", "api.yaml", "openapi: 3.1.0\ninfo: [unterminated\n", - compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, - // Broken on its own version line, so it declares no version to read, and - // is declined like any file that does not say what it is. That is the - // price of not claiming prose (GitHub #497). - {"unparseable yaml, broken on the version line", "api.yaml", "openapi: [unterminated\n", - compilers.SourceFormat{}, false, nil}, - // The two files #497 is about: the word written where a spec declares its - // version, with no version beside it. A README with prose after the key - // and a framework's configuration section under it are other formats' - // files, and neither is reported as a spec that failed to read. - {"a readme that does not parse", "README.md", "# Notes\n\nfiller text\nopenapi: is a format\n", - compilers.SourceFormat{}, false, nil}, - {"a config section that does not parse", "app.yaml", "openapi:\n enabled: true\nserver: [\n", - compilers.SourceFormat{}, false, nil}, - {"unparseable json", "api.json", `{"openapi": "3.1.0", "info": {`, - compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, - // A key that is not UTF-8 refuses the whole document, which is what lets - // ids.Ptr assume every token it escapes is UTF-8. - {"undecodable yaml key", "api.yaml", "openapi: 3.1.0\ninfo: {}\n\xff: x\n", - compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, - {"undecodable json key", "api.json", "{\"openapi\": \"3.1.0\", \"\xff\": 1}", - compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, - // Broken, and never this compiler's: the key it names is a value, not a - // key, so the parse error describes a parser that was wrong to be asked. - {"unparseable, key only mentioned", "svc.proto", "syntax = \"openapi\";\n{[", - compilers.SourceFormat{}, false, nil}, - // A large document is parsed as a small one is, so one that declares the - // key and will not parse is reported at any size. Detection used to scan - // past 64 KiB instead, and a scan cannot tell a broken spec from another - // format's file naming the word, so the same broken spec drew a complaint - // or silence depending on its length (GitHub #486). - {"unparseable, large", "api.yaml", - padTo("openapi: 3.1.0\ninfo: [unterminated\n", "filler: x\n"), - compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, - // The key written behind a prefix that does not parse. The scan read past - // the prefix to the version and named the format, leaving the compile to - // find the document broken; the parse finds it broken here, and the - // version declared at column 0 makes it this compiler's to say so. - {"key behind an unparseable prefix, large", "api.yaml", - padTo("bad: [unterminated\n", "filler: x\n") + "openapi: 3.1.0\n", - compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, - // yaml.v3 refuses a document that follows an explicit end marker without - // a `---` of its own, whatever the document's size. - {"key after an end-marked empty document", "api.yaml", - "---\n...\nopenapi: 3.1.0\n", - compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, - {"key after an end-marked empty document, large", "api.yaml", - "---\n...\n" + padTo("openapi: 3.1.0\n", "filler: x\n"), - compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, - {"key behind an unterminated quoted scalar, large", "api.yaml", - padTo("bad: \"unterminated\n", "filler: x\n") + "openapi: 3.1.0\n", - compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, - // The same in flow style: the key is one of the root mapping's own - // entries, which is what makes a broken document this compiler's. - {"key behind an unparseable flow prefix, large", "spec3.json", - `{"pad":"` + flowPad() + `","bad" 1,"openapi":"3.1.0"}`, - compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, - // Another format's document, large, naming the word as a key and broken - // besides. It opens no mapping of its own, so the key is not its - // declaration of itself and this compiler has nothing to say: reporting a - // parse error here would claim bytes that were never its own. - {"a broken document of another format names the key", "asyncapi.json", - `[{"openapi":"3.1.0"},"` + flowPad() + `"`, - compilers.SourceFormat{}, false, nil}, - // The same, one level down inside a mapping that does open the document. - // A nested key names a field, not the format of the file holding it. - {"a broken document of another format nests the key", "other.json", - `{"pad":"` + flowPad() + `","deep":{"openapi":"3.1.0"},"bad" 1}`, - compilers.SourceFormat{}, false, nil}, - {"empty", "empty.yaml", "", compilers.SourceFormat{}, false, nil}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - got, diags, ok := New().Detect(compilers.Source{Path: tc.path, Data: []byte(tc.src)}, compilers.Options{}) - assert.Equal(t, tc.wantOK, ok) - assert.Equal(t, tc.want, got.Format) - assert.Equal(t, tc.wantCode, codesOf(diags), - "a decline says something only when the source is recognizably this compiler's") - }) - } -} - -// TestDetect_KeyOrderDoesNotDecideTheFormat pins a published spec whose -// `components` object runs long and whose `openapi` key sits behind it. A JSON -// object's keys are unordered, so the same document written with its version -// key first and with it last is one document, and detection has to answer the -// same for both. A prefix-only sniff once declined the version-last spellings. -func TestDetect_KeyOrderDoesNotDecideTheFormat(t *testing.T) { - t.Parallel() - flow, block := bigComponents() - cases := []struct{ name, path, src string }{ - {"flow json, version first", "spec3.json", `{"openapi":"3.0.3",` + flow + `}`}, - {"flow json, version last", "spec3.json", `{` + flow + `,"openapi":"3.0.3"}`}, - {"block yaml, version first", "api.yaml", "openapi: 3.0.3\n" + block}, - {"block yaml, version last", "api.yaml", block + "openapi: 3.0.3\n"}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - require.Greater(t, len(tc.src), scanCutover, "the case must be large to test it") - got, diags, ok := New().Detect(compilers.Source{Path: tc.path, Data: []byte(tc.src)}, compilers.Options{}) - assert.True(t, ok, "a valid document must not be declined over where it declares its version") - assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: "3.0"}, got.Format) - assert.Nil(t, codesOf(diags), "a document this compiler recognizes carries no complaint") - }) - } -} - -// bigComponents returns a `components` entry whose value alone runs past -// scanCutover, in flow and in block style. It stands in for the schema catalogue a -// published spec leads with; what matters is only that it is one entry too large -// to read past. -func bigComponents() (flow, block string) { - var f, b strings.Builder - f.WriteString(`"components":{"schemas":{`) - b.WriteString("components:\n schemas:\n") - for i := 0; f.Len() <= scanCutover || b.Len() <= scanCutover; i++ { - if i > 0 { - f.WriteByte(',') - } - fmt.Fprintf(&f, `"S%d":{"type":"object","description":"a schema"}`, i) - fmt.Fprintf(&b, " S%d:\n type: object\n description: a schema\n", i) - } - f.WriteString(`}}`) - return f.String(), b.String() -} - -// scanCutover is the size past which detection used to scan bytes rather than -// parse, and still the size where the change of reading shows: a case larger -// than this pins that a large document is read as a small one is. -const scanCutover = 64 << 10 - -// flowPad returns a run of bytes long enough that a flow entry holding it puts -// everything after it past scanCutover. -func flowPad() string { return strings.Repeat("p", scanCutover) } - -// padTo returns src grown past scanCutover by appending filler. -func padTo(src, filler string) string { - var b strings.Builder - b.WriteString(src) - for b.Len() <= scanCutover { - b.WriteString(filler) - } - return b.String() -} - -// TestSniff_ReadsALargeDocumentWhole pins the reading a large document gets: -// the parse a small one gets, which finds a key wherever it sits and in either -// style, because where a writer put a key in a mapping says nothing about what -// the document is. Each case says whether the document parsed, so a case that -// expects no declaration cannot pass on bytes that never parsed at all. -func TestSniff_ReadsALargeDocumentWhole(t *testing.T) { - t.Parallel() - const filler = "# a line of padding that says nothing about the format\n" - pad := strings.Repeat("p", scanCutover) - cases := []struct { - name, src string - want sniffProbe - unreadable bool - }{ - {"block yaml declaring first", - padTo("openapi: 3.1.0\n", filler), sniffProbe{OpenAPI: "3.1.0"}, false}, - {"block yaml declaring last", - padTo("", filler) + "openapi: 3.1.0\n", sniffProbe{OpenAPI: "3.1.0"}, false}, - {"flow json declaring first", - `{"openapi":"3.1.0","x":"` + pad + `"}`, sniffProbe{OpenAPI: "3.1.0"}, false}, - {"flow json declaring last", - `{"x":"` + pad + `","openapi":"3.1.0"}`, sniffProbe{OpenAPI: "3.1.0"}, false}, - {"flow json swagger first", - `{"swagger":"2.0","x":"` + pad + `"}`, sniffProbe{Swagger: "2.0"}, false}, - {"flow json swagger last", - `{"x":"` + pad + `","swagger":"2.0"}`, sniffProbe{Swagger: "2.0"}, false}, - // Neither YAML nor JSON: the parse fails and declares nothing. - {"protobuf, large", - padTo("syntax = \"proto3\";\n", "message M { string a = 1; }\n"), sniffProbe{}, true}, - // The word is there and is not a key: a value is not a declaration. - {"the word as a value, large", - `{"x":"` + pad + `","note":"openapi"}`, sniffProbe{}, false}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - require.Greater(t, len(tc.src), scanCutover, "the case must be large to test it") - probe, parsed, err := sniff([]byte(tc.src)) - assert.Equal(t, tc.want, probe) - if tc.unreadable { - assert.Error(t, err) - return - } - require.NoError(t, err) - assert.NotNil(t, parsed, "a document that parsed carries its parse") - }) - } -} - -// TestDeclaresProbeKey_ScopesTheNameToItsOwnDocument pins the guard that -// decides whose bytes these are. A name followed by a colon is a key wherever -// it sits, so the guard says whose: block style answers with column 0, flow -// style with the root mapping's own depth. A key there declares the format only -// with a version beside it (GitHub #497). The cases answering no are documents -// naming the word where none declares its format, or declaring it with prose, a -// collection or nothing beside it; saying yes would report this compiler's -// parse error over a file that was never its own. -func TestDeclaresProbeKey_ScopesTheNameToItsOwnDocument(t *testing.T) { - t.Parallel() - cases := []struct { - name, src string - want bool - }{ - {"flow mapping declares it", `{"openapi":"3.1.0"}`, true}, - {"flow mapping declares swagger", `{"swagger":"2.0"}`, true}, - {"space around the mapping and the colon", " \n\t{\"openapi\" : \"3.1.0\"}", true}, - {"an escape hides no key from the scan", `{"a\"b":1,"openapi":"3.1.0"}`, true}, - {"block style at column 0", "openapi: 3.1.0\n", true}, - {"block style past a byte-order mark", "\xef\xbb\xbfopenapi: 3.1.0\ninfo: [\n", true}, - // The version beside the key is read as YAML reads it, on its own line. - {"block style, a quoted version", "openapi: \"3.1.0\"\ninfo: [\n", true}, - {"block style, a version and a comment", "openapi: 3.1.0 # v\ninfo: [\n", true}, - {"block style, CRLF", "openapi: 3.1.0\r\ninfo: [\r\n", true}, - {"block style, swagger", "swagger: \"2.0\"\ninfo: [\n", true}, - {"flow style, a bare version", `{"openapi":3.1,`, true}, - // And a key with no version beside it declares nothing, in either style. - {"block style, prose beside the key", "openapi: is a format\n[\n", false}, - {"block style, a mapping under the key", "openapi:\n enabled: true\n[\n", false}, - {"block style, a collection that does not close", "openapi: [unterminated\n", false}, - {"block style, no space after the colon", "openapi:3.1.0\n[\n", false}, - {"block style, a longer key the name begins", "openapi:x: 3.1.0\n[\n", false}, - {"block style, the version on the next line", "openapi:\n 3.1.0\n[\n", false}, - {"flow style, a collection for the version", `{"openapi":{"a":"3.1.0"},`, false}, - {"flow style, prose for the version", `{"openapi":"is a format",`, false}, - {"flow style, nothing after the colon", `{"openapi":`, false}, - {"flow style past a byte-order mark", "\xef\xbb\xbf{\"openapi\":\"3.1.0\",", true}, - // What may stand in front of a root flow mapping: comment lines, and up - // to two node properties — an anchor and a tag, in either order. - {"flow style behind a comment line", "# c\n # d\n{\"openapi\":\"3.1.0\",", true}, - {"flow style behind an anchor", "&x {\"openapi\":\"3.1.0\",", true}, - {"flow style behind a tag and an anchor", "!!map &x {\"openapi\":\"3.1.0\",", true}, - {"flow style behind a property ended by a flow indicator", "&x{\"openapi\":\"3.1.0\",", true}, - {"a comment with nothing after it", "# openapi: 3.1.0", false}, - {"a property with nothing after it", "&x", false}, - {"three properties are more than a node carries", "&a &b &c {\"openapi\":\"3.1.0\",", false}, - - {"nested one level down", `{"a":{"openapi":"3.1.0"}}`, false}, - {"nested inside a sequence", `{"a":[{"openapi":"3.1.0"}]}`, false}, - {"a document that opens a sequence", `[{"openapi":"3.1.0"}]`, false}, - {"block style indented under another key", "a:\n openapi: 3.1.0\n", false}, - {"the name is a value", `{"note":"openapi"}`, false}, - {"the name in a comment", "x: 1\n# openapi is a format\n", false}, - {"the name has no colon after it", `{"openapi",1}`, false}, - {"the name ends the bytes", `{"openapi"`, false}, - {"a string runs off the end", `{"a":"unterminated`, false}, - {"nothing but whitespace", " \n\t ", false}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - assert.Equal(t, tc.want, declaresProbeKey([]byte(tc.src))) - }) - } -} - -func TestMajorMinor(t *testing.T) { - t.Parallel() - assert.Equal(t, "3.1", majorMinor("3.1.0")) - assert.Equal(t, "3.1", majorMinor("3.1")) - assert.Equal(t, "4", majorMinor("4")) -} - -// codesOf reduces diagnostics to their codes, which is the whole of what the -// detection tests assert about them: the message carries a parser's wording and -// pinning it would test yaml.v3 rather than this package. -func codesOf(diags []ir.Diagnostic) []string { - if len(diags) == 0 { - return nil - } - codes := make([]string, 0, len(diags)) - for _, d := range diags { - codes = append(codes, d.Code) - } - return codes -} - -// dupRepeats is how many times the fixtures below repeat a root key. It is not a -// threshold: the wrong answer reproduces at two repeats, and this many only -// makes the fixture recognizably a document rather than a corner. It is -// deliberately far below the 6,553 of the report — yaml.v3 raises one error per -// pair of matching keys, so that count produced 21,467,628 of them and a 1.2 GB -// message, and a fixture that large turns a revert into an out-of-memory kill -// instead of a failing assertion. What guards the cost is -// TestSniff_CostIsNotQuadraticInRepeatedKeys, which measures growth rather than -// paying for it. -const dupRepeats = 512 - -// TestDetect_RepeatedKeysDoNotDecideTheFormat pins the fix for the blow-up. A -// document that repeats a top-level key has a duplicate key, which the parser -// this compiler goes on to use reports once per repeat, sited. It is neither -// another format's nor unreadable, yet decoding the root mapping to read two -// keys said both, because yaml.v3 abandons a mapping that repeats any key. -// -// Both orders are pinned: a fixture that declares the version first cannot see -// a regression that loses it to the repeats that follow. -func TestDetect_RepeatedKeysDoNotDecideTheFormat(t *testing.T) { - t.Parallel() - repeats := strings.Repeat("x: y\n", dupRepeats) - cases := []struct{ name, src string }{ - {"version first", "openapi: 3.0.3\ninfo: {title: t, version: v}\n" + repeats}, - {"version last", "info: {title: t, version: v}\n" + repeats + "openapi: 3.0.3\n"}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - got, diags, ok := New().Detect(compilers.Source{Path: "api.yaml", Data: []byte(tc.src)}, compilers.Options{}) - assert.True(t, ok, "a document this compiler can lower must not be declined over a repeated key") - assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: "3.0"}, got.Format) - assert.Nil(t, codesOf(diags), "the repeats are the parser's to report, sited, not detection's") - }) - } -} - -// TestDetect_ARepeatedVersionKeyAgreesWithTheParser holds detection to the -// answer the lowering will reach. load reads the version off the parsed document -// and records it on ir.SourceInfo, so each case asks that parser which spelling -// of a repeated key it keeps; detection naming another would give one document -// two dialects, one routing it and one describing it. -// -// The two orders are the test: a single order passes whichever spelling is -// taken. -func TestDetect_ARepeatedVersionKeyAgreesWithTheParser(t *testing.T) { - t.Parallel() - cases := []struct{ first, second, want string }{ - {"3.1.0", "3.0.3", "3.0"}, - {"3.0.3", "3.1.0", "3.1"}, - } - for _, tc := range cases { - t.Run(tc.first+" then "+tc.second, func(t *testing.T) { - t.Parallel() - src := "openapi: " + tc.first + "\nopenapi: " + tc.second + "\ninfo: {}\n" - parsed, _, err := soa.Unmarshal(t.Context(), strings.NewReader(src)) - require.NoError(t, err) - require.NotNil(t, parsed) - assert.Equal(t, tc.want, majorMinor(parsed.OpenAPI), "the spelling the parser keeps") - - got, _, ok := New().Detect(compilers.Source{Path: "api.yaml", Data: []byte(src)}, compilers.Options{}) - require.True(t, ok) - assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: majorMinor(parsed.OpenAPI)}, got.Format, - "detection names the version the parser records") - }) - } -} - -// TestDetect_ReadsAVersionKeyThroughAMergeKey holds the merge cases the decoder -// this replaced handled for free. A root that merges another mapping declares -// what that mapping declares, and dropping it would put a new instance of "a -// field supplied through a merge key reaches the IR in no form" into the one -// place that decides whether the document is read at all. -func TestDetect_ReadsAVersionKeyThroughAMergeKey(t *testing.T) { - t.Parallel() - cases := []struct { - name, src, want string - }{ - {"alias", "base: &b\n openapi: 3.1.0\n<<: *b\ninfo: {}\n", "3.1"}, - {"mapping written out", "<<: {openapi: 3.1.0}\ninfo: {}\n", "3.1"}, - {"sequence of aliases", "one: &o\n unrelated: x\ntwo: &t\n openapi: 3.1.0\n<<: [*o, *t]\n", "3.1"}, - {"earlier merge wins", "one: &o\n openapi: 3.0.3\ntwo: &t\n openapi: 3.1.0\n<<: [*o, *t]\n", "3.0"}, - {"a written key beats a merged one", "base: &b\n openapi: 3.0.3\n<<: *b\nopenapi: 3.1.0\n", "3.1"}, - {"a quoted << merges nothing", "base: &b\n openapi: 3.1.0\n\"<<\": *b\ninfo: {}\n", ""}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - got, _, ok := New().Detect(compilers.Source{Path: "api.yaml", Data: []byte(tc.src)}, compilers.Options{}) - if tc.want == "" { - assert.False(t, ok, "a key spelled << as a plain string merges nothing") - return - } - require.True(t, ok) - assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: tc.want}, got.Format) - }) - } -} +const budgetedSpec = "openapi: 3.1.0\ninfo: {title: T, version: \"1\"}\npaths: {}\n" -// rootMergeChain writes a document whose root merges a chain of levels -// anchored mappings, the innermost holding leaf, and writes own after the merge. -func rootMergeChain(levels int, leaf, own string) []byte { - var b strings.Builder - fmt.Fprintf(&b, "l0: &a0 {%s}\n", leaf) - for i := 1; i < levels; i++ { - fmt.Fprintf(&b, "l%d: &a%d {<<: *a%d}\n", i, i, i-1) - } - fmt.Fprintf(&b, "<<: *a%d\n%s", levels-1, own) - return []byte(b.String()) +func withBytes(limit int) compilers.Options { + return compilers.Options{FormatOptions: openapi.Options{Limits: openapi.Limits{MaxSourceBytes: limit}}} } -// TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt pins where the bound on -// the root's merges lies, nodeview's, and what a read it cuts short answers. A -// key past the bound may outrank what the read found, so the answer is -// unknown, and Detect says the bound stopped it, unless the root's own pairs, -// which outrank every merge, settle it. -func TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt(t *testing.T) { +// TestDetect_DeclinesASourcePastTheCallersByteBudget pins that recognition is +// held to the budget the compile is. A source past it is declined unread and +// the decline says why, in the words Compile uses for the same source, since +// "nothing recognized it" would send the caller to the document rather than to +// the budget they set. +func TestDetect_DeclinesASourcePastTheCallersByteBudget(t *testing.T) { t.Parallel() - const limit = nodeview.MergeDepthLimit - cases := map[string]struct { - src []byte - version string - cut bool - }{ - "a chain the view expands": {rootMergeChain(limit, "openapi: 3.0.3", ""), "3.0.3", false}, - "a chain past it": {rootMergeChain(limit+1, "openapi: 3.0.3", ""), "", true}, - "a version the root writes itself": { - rootMergeChain(limit+1, "openapi: 3.0.3", "openapi: 3.1.0\n"), "3.1.0", false}, - "a merged version a deeper one may outrank": { - []byte(string(rootMergeChain(limit+1, "openapi: 3.1.0", "")) + "s: &s {openapi: 3.0.3}\n"), "", true}, - "a root writing no version and a swagger one": { - rootMergeChain(limit+1, "openapi: 3.0.3", "openapi: prose words\nswagger: '2.0'\n"), "", false}, - "a root writing no version and leaving swagger to its merges": { - rootMergeChain(limit+1, "swagger: '2.0'", "openapi: prose words\n"), "", true}, - } - for name, tc := range cases { - t.Run(name, func(t *testing.T) { - t.Parallel() - probe, _, err := sniff(tc.src) - require.NoError(t, err, "a read cut short is no parse failure") - assert.Equal(t, tc.cut, probe.Cut) - if tc.cut { - return - } - assert.Equal(t, tc.version, probe.OpenAPI) - }) - } -} + src := compilers.Source{Path: "api.yaml", Data: []byte(budgetedSpec)} + limit := len(budgetedSpec) - 1 -// TestDetect_AReadCutShortIsReported pins that the bound never passes for a -// decline: the source may be this compiler's own, so detection says why it -// could not tell. -func TestDetect_AReadCutShortIsReported(t *testing.T) { - t.Parallel() - _, diags, ok := New().Detect(compilers.Source{ - Path: "api.yaml", Data: rootMergeChain(nodeview.MergeDepthLimit+1, "openapi: 3.1.0", ""), - }, compilers.Options{}) + rec, diags, ok := openapi.New().Detect(src, withBytes(limit)) assert.False(t, ok) + assert.Equal(t, compilers.Recognition{}, rec) require.Len(t, diags, 1) assert.Equal(t, diag.BudgetExceeded, diags[0].Code) - assert.Equal(t, ir.SeverityError, diags[0].Severity, "it is the whole reason the source went unclaimed") - assert.Contains(t, diags[0].Message, fmt.Sprint(nodeview.MergeDepthLimit)) -} - -// TestDetect_AMergeCycleHidesNoKey pins the other way a root's merges can stop -// short of a complete expansion: a cycle, which the compile's alias pre-scan -// refuses and detection meets first. It reads every mapping on it, so it cuts -// no read short, and a root that declares nothing is declined in silence as -// any other file that is not a spec is, while one that declares a version is -// claimed for the compile to refuse. -func TestDetect_AMergeCycleHidesNoKey(t *testing.T) { - t.Parallel() - cases := map[string]struct { - src string - version string - }{ - "a root merging itself": {"&r\n<<: *r\nfoo: 1\n", ""}, - "a root merging itself beside its version": {"&r\n<<: *r\nopenapi: 3.1.0\n", "3.1"}, - "a root merging a mapping that merges it back": {"&r\nx: &x {<<: *r, openapi: 3.0.3}\n<<: *x\n", "3.0"}, - } - for name, tc := range cases { + assert.Equal(t, 0, diags[0].Provenance.Source, "detection names the one source it was handed") + + _, compiled, err := openapi.New().Compile(t.Context(), []compilers.Source{src}, withBytes(limit)) + require.NoError(t, err) + require.Len(t, compiled, 1) + assert.Equal(t, compiled[0].Message, diags[0].Message, "detection and the compile refuse the source alike") +} + +// TestDetect_ReadsASourceWithinTheCallersByteBudget pins the other side, +// including a caller who turned the budget off. That case is asserted on a +// small source: the byte budget is the only bound on what detection reads, so +// off means unbounded, but a source past the 64 MiB default costs 15 s and +// 1.3 GB under -race, which is what the gate's only test step runs with. It +// was measured by hand instead when the byte scan was deleted (GitHub #486): +// with the budget off, a source past 64 MiB is recognized. +func TestDetect_ReadsASourceWithinTheCallersByteBudget(t *testing.T) { + t.Parallel() + src := compilers.Source{Path: "api.yaml", Data: []byte(budgetedSpec)} + want := compilers.SourceFormat{Name: "openapi", Version: "3.1"} + + for _, opts := range []compilers.Options{ + withBytes(len(budgetedSpec)), + withBytes(-1), + {}, + {FormatOptions: "another compiler's options"}, + } { + rec, diags, ok := openapi.New().Detect(src, opts) + require.True(t, ok, "%+v: %+v", opts, diags) + assert.Equal(t, want, rec.Format, "%+v", opts) + } +} + +// TestDetect_ReadsTheVersionTheCompileReads holds detection to the compile on +// the roots where a reading of its own disagreed with the parser's: each is +// routed as the version its compile lowers, the agreement +// compilers.Detection.Compile checks. A key repeated over an unreadable value, +// an alias in either place, a key tagged `!!merge` that is not `<<`, an alias +// key outranking a merge, and a chain deeper than the old walk followed. +func TestDetect_ReadsTheVersionTheCompileReads(t *testing.T) { + t.Parallel() + const tail = "info: {title: t, version: '1'}\npaths: {}\n" + cases := map[string]string{ + "a version over an unreadable value": "openapi: [x]\nopenapi: 3.1.0\n", + "an alias as the key": "k: &k openapi\n*k : 3.1.0\n", + "an alias as the value": "v: &v 3.1.0\nopenapi: *v\n", + "a key tagged !!merge that is no <<": "!!merge openapi: 3.1.0\n", + "an alias key outranking a merge": "b: &b {openapi: 3.0.3}\nk: &k openapi\n<<: *b\n*k : 3.1.0\n", + "a merge chain ten mappings deep": string(openapitest.RootMergeChain(10, "openapi: 3.1.0", "")), + "a merge written before its own key": "b: &b {openapi: 3.0.3}\n<<: *b\nopenapi: 3.1.0\n", + "the first of two merges that agree": "a: &a {openapi: 3.1.0}\nb: &b {openapi: 3.0.3}\n<<: [*a, *b]\n", + } + for name, root := range cases { t.Run(name, func(t *testing.T) { t.Parallel() - probe, _, err := sniff([]byte(tc.src)) - require.NoError(t, err) - assert.False(t, probe.Cut, "a cycle reads every mapping on it") + src := compilers.Source{Path: "api.yaml", Data: []byte(root + tail)} + rec, diags, ok := openapi.New().Detect(src, compilers.Options{}) + require.True(t, ok, "the compile lowers it, so detection claims it: %v", diags) - rec, diags, ok := New().Detect(compilers.Source{Path: "api.yaml", Data: []byte(tc.src)}, compilers.Options{}) - assert.Equal(t, tc.version != "", ok) - assert.Equal(t, tc.version, rec.Format.Version) - assert.Empty(t, diags, "a cycle is no budget, and a file declaring nothing earns no complaint") - }) - } -} - -// TestDecodeYAML_RefusesARootThatIsNoMapping pins the shape complaint. A source -// with no root mapping has no top-level keys, and saying so is what lets Detect -// report bytes that declare a key it serves and will not read. -func TestDecodeYAML_RefusesARootThatIsNoMapping(t *testing.T) { - t.Parallel() - cases := []struct{ name, src, wantErr string }{ - {"sequence", "- openapi: 3.1.0\n", "!!seq"}, - {"scalar", "just a string\n", "!!str"}, - {"empty", "", ""}, - {"comment only", "# openapi: 3.1.0\n", ""}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - probe, _, err := decodeYAML([]byte(tc.src)) - assert.Empty(t, probe.OpenAPI) - if tc.wantErr == "" { - assert.NoError(t, err, "bytes that carry no document decline rather than fail") - return - } - require.Error(t, err) - assert.Contains(t, err.Error(), tc.wantErr, "the complaint names the shape that was read") - }) - } -} - -// TestSniff_CostIsNotQuadraticInRepeatedKeys guards the half of the defect an -// answer cannot see. Reading two keys off a parsed tree is linear; decoding the -// root mapping to read them is quadratic in how often a key repeats, since -// yaml.v3 compares every pair of keys before reading any. -// -// Allocation count is the probe because it is deterministic where wall time is -// not. Doubling the repeats doubles the parse, so the bound sits between that -// and a quadratic term: at this size the linear reading grows by 1.97 and the -// quadratic one by 4.64. -func TestSniff_CostIsNotQuadraticInRepeatedKeys(t *testing.T) { - head := "openapi: 3.0.3\ninfo: {}\n" - small := []byte(head + strings.Repeat("x: y\n", dupRepeats)) - large := []byte(head + strings.Repeat("x: y\n", dupRepeats*2)) - - smallAllocs := testing.AllocsPerRun(2, func() { _, _, _ = sniff(small) }) - largeAllocs := testing.AllocsPerRun(2, func() { _, _, _ = sniff(large) }) - - require.Positive(t, smallAllocs, "a measurement of nothing bounds nothing") - assert.Less(t, largeAllocs, smallAllocs*3, - "twice the repeats must cost about twice, not about four times") -} - -// TestDecodeYAML_RefusesAVersionKeyThatIsNoScalar pins the complaint for a -// version key whose value is a mapping or a sequence, written directly and -// reached through a `<<`. Such bytes name a key this compiler serves and do not -// say what dialect, so the probe cannot read them. -// -// Whether Detect reports that or declines in silence is declaresProbeKey's -// answer and not this one's, and it is asserted separately below: a mapping -// under the key is another tool's configuration as often as a broken spec, and -// declares no version either way, so Detect declines all of them. -func TestDecodeYAML_RefusesAVersionKeyThatIsNoScalar(t *testing.T) { - t.Parallel() - cases := []struct{ name, src, wantErr string }{ - {"mapping", "openapi: {a: b}\ninfo: {}\n", "!!map"}, - {"sequence", "openapi: [3.1.0]\ninfo: {}\n", "!!seq"}, - {"merged mapping", "base: &b\n openapi: {a: b}\n<<: *b\n", "!!map"}, - {"merged through a sequence", "base: &b\n openapi: {a: b}\n<<: [*b]\n", "!!map"}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - probe, _, err := decodeYAML([]byte(tc.src)) - require.Error(t, err) - assert.Contains(t, err.Error(), tc.wantErr, "the complaint names the shape that was read") - assert.Empty(t, probe.OpenAPI) - }) - } -} - -// TestDetect_AVersionKeyOfTheWrongShapeIsNoDeclaration pins that a mapping -// where a version goes declares no version, wherever it sits. Such a document -// parses, so it is the tree and not the guard that has the key — but a mapping -// under `openapi` is what another tool's configuration writes, and a spec -// written that way has no version to route it by either. Detection used to -// report the column-0 spelling as this compiler's own and unreadable; it -// declines all three alike now (GitHub #497). -func TestDetect_AVersionKeyOfTheWrongShapeIsNoDeclaration(t *testing.T) { - t.Parallel() - cases := []struct { - name, src string - wantCode []string - }{ - {"declared at column 0", "openapi: {a: b}\ninfo: {}\n", nil}, - {"a framework's configuration section", "openapi:\n path: /docs\n enabled: true\n", nil}, - {"reached through a merge", "base: &b\n openapi: {a: b}\n<<: *b\n", nil}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - _, diags, ok := New().Detect(compilers.Source{Path: "api.yaml", Data: []byte(tc.src)}, compilers.Options{}) - assert.False(t, ok) - assert.Equal(t, tc.wantCode, codesOf(diags)) - }) - } -} - -// TestDecodeYAML_PassesOverWhatNamesNoVersion holds the walk to reading only -// what it came for. A mapping may key an entry with a sequence, and a `<<` may -// be written with a value that merges nothing; both are the source's business -// and neither stops the two keys beside them from being read. -func TestDecodeYAML_PassesOverWhatNamesNoVersion(t *testing.T) { - t.Parallel() - cases := []struct{ name, src string }{ - {"a key that is a sequence", "? [a, b]\n: v\nopenapi: 3.1.0\n"}, - {"a merge of a scalar", "<<: not-a-mapping\nopenapi: 3.1.0\n"}, - {"a merge of a sequence of scalars", "<<: [x, y]\nopenapi: 3.1.0\n"}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - probe, _, err := decodeYAML([]byte(tc.src)) + doc, compiled, err := openapi.New().Compile(t.Context(), []compilers.Source{src}, compilers.Options{}) require.NoError(t, err) - assert.Equal(t, "3.1.0", probe.OpenAPI) + require.NotNil(t, doc, "%v", compiled) + assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: "3.1"}, rec.Format) + assert.Equal(t, rec.Format.String(), doc.Sources[0].Format, "detection and the compile read one version") }) } } - -// TestProbeVersion_RefusesAnAliasThatResolvedToNothing covers the guard on a -// value nodeview could not dereference. A parser resolves every alias it -// accepts, so the node is built here rather than parsed: the guard exists -// because reading the value is what the next line does, and a nil there is a -// panic in detection, which runs before the compiler has decided the bytes are -// even its own. -func TestProbeVersion_RefusesAnAliasThatResolvedToNothing(t *testing.T) { - t.Parallel() - root := ynode.Map(ynode.Scalar("openapi"), &yaml.Node{Kind: yaml.AliasNode}) - _, err := probeFromMapping(root) - require.Error(t, err) - assert.Contains(t, err.Error(), "no value") -} - -// breadthMergeDoc writes a document whose root merges one anchor k times, where -// that anchor's mapping merges another k times. It is the shape that made the -// probe's walk quadratic: the inner mapping was entered once per merge key -// naming it, so the work was the product of the two rather than their sum. The -// version sits at the innermost anchor, so a walk that stops early is caught by -// the assertion rather than looking cheap. -func breadthMergeDoc(k int) []byte { - var b strings.Builder - b.WriteString("l0: &a0\n openapi: 3.1.0\n") - b.WriteString("l1: &a1\n") - for range k { - b.WriteString(" <<: *a0\n") - } - for range k { - b.WriteString("<<: *a1\n") - } - return []byte(b.String()) -} - -// TestProbeFromMapping_CostIsLinearInMergeBreadth pins the bound GitHub #487 -// was about: the merge bound limits how deep a chain is followed, not how wide, -// and a walk entering an anchored mapping once per merge key naming it costs -// the product of the two. nodeview's memo is what keeps it a sum. It counts -// allocations, not time, so load cannot move it, and runs serially because -// AllocsPerRun measures the whole process. -func TestProbeFromMapping_CostIsLinearInMergeBreadth(t *testing.T) { - for _, k := range []int{2000, 4000} { - var root yaml.Node - require.NoError(t, yaml.Unmarshal(breadthMergeDoc(k), &root)) - content := documentRoot(&root) - - var probe sniffProbe - var err error - allocs := testing.AllocsPerRun(1, func() { - probe, err = probeFromMapping(content) - }) - require.NoError(t, err) - require.Equal(t, "3.1.0", probe.OpenAPI, - "the version sits at the innermost anchor, so a walk that stops early fails here rather than looking cheap") - assert.Less(t, allocs, float64(k), - "a walk of %d merge keys made %.0f allocations, not fewer than the keys", k, allocs) - } -} - -// TestDeclaresBlockKey_ReadsABoundedNumberOfLines pins maxVersionLines. The -// guard parses each line that begins with the key, so a version behind as many -// lines without one as the bound allows is still found, and one behind a line -// more is not: the bound is what stops a file of nothing but such lines costing -// a parse per line. -func TestDeclaresBlockKey_ReadsABoundedNumberOfLines(t *testing.T) { - t.Parallel() - behind := func(n int) []byte { - return []byte(strings.Repeat("openapi: is prose\n", n) + "openapi: 3.1.0\n[\n") - } - assert.True(t, declaresBlockKey(behind(maxVersionLines-1), "openapi"), "the last line the bound reads") - assert.False(t, declaresBlockKey(behind(maxVersionLines), "openapi"), "one line past the bound") -} diff --git a/compilers/openapi/internal/detect/declares.go b/compilers/openapi/internal/detect/declares.go new file mode 100644 index 00000000..60c34faa --- /dev/null +++ b/compilers/openapi/internal/detect/declares.go @@ -0,0 +1,220 @@ +package detect + +import ( + "bytes" + + yaml "gopkg.in/yaml.v3" +) + +// declaresProbeKey reports whether data declares a version under a +// discriminating key. Detect asks only once the probe cannot read the source, +// where "not YAML" alone fits a Protobuf or Smithy source too, and only within +// its byte budget. +// +// Neither reading may widen to "the name occurs before a colon": other formats +// nest such a key and must not get this compiler's parse error. The version +// makes the claim (GitHub #497), so a spec broken on its version line, +// `openapi: [3.1.0`, is unrecognized rather than unreadable, and a README +// quoting a spec in a code block is claimed like one. +func declaresProbeKey(data []byte) bool { + data = bytes.TrimPrefix(data, []byte(bomUTF8)) + return declaresBlockKey(data, "openapi") || declaresBlockKey(data, "swagger") || + declaresFlowKey(data) +} + +// bomUTF8 is the UTF-8 byte-order mark. YAML admits one at the start of a +// stream and JSON is its subset, so a spec written by an editor that emits one +// is a spec like any other: yaml.v3 reads straight through it and so does the +// loader. declaresProbeKey reads bytes rather than a parse and has to skip it +// itself, or a spec with one would be declined in silence when broken where a +// spec without one is reported. +const bomUTF8 = "\xef\xbb\xbf" + +// declaresBlockKey reports whether data writes key bare at the start of a line +// with a version beside it. Column 0 is where block style puts a top-level key; +// nested keys and block scalar content are indented. +// +// Only the bare spelling is read: a quoted key is flow style's, scoped by +// declaresFlowKey. A block document that quotes its top-level key is declined +// in silence, the safe direction to be wrong, since admitting quoted keys at +// column 0 would take in what JSON writes at every depth. The line is parsed +// alone, since the document around it did not parse. +func declaresBlockKey(data []byte, key string) bool { + prefix := []byte(key + ":") + read := 0 + for i := 0; i < len(data) && read < maxVersionLines; { + line, next := nextLine(data, i) + if bytes.HasPrefix(line, prefix) { + if lineDeclaresVersion(line, key) { + return true + } + read++ + } + i = next + } + return false +} + +// maxVersionLines bounds how many lines declaresBlockKey parses. A document +// declares its version once and a stream a few times over, so the bound is room +// to spare for anything written; without it, a crafted file of nothing but such +// lines, broken at the end, cost nearly three times the parse that failed on +// it, one small parse per line. +const maxVersionLines = 8 + +// lineDeclaresVersion reports whether line, parsed on its own, is a mapping of +// key to a scalar that reads as a version. An alias there names a node on +// another line, which the line alone cannot resolve, so it declares nothing. +func lineDeclaresVersion(line []byte, key string) bool { + var doc yaml.Node + if yaml.Unmarshal(line, &doc) != nil { + return false + } + root := documentRoot(&doc) + if root == nil || root.Kind != yaml.MappingNode || len(root.Content) != 2 || root.Content[0].Value != key { + return false + } + version, err := probeVersion(root.Content[1]) + return err == nil && isVersion(version) +} + +// declaresFlowKey reports whether data opens a flow mapping, the shape JSON +// writes, that names one of the discriminating keys among its own entries. +// +// Nesting depth is what makes the answer top-level: a plain search for +// `"openapi":` matches wherever it sits, and other formats nest one. A source +// that opens no mapping, a JSON array say, declares nothing. +// +// It is a lexer, not a parser, because it exists for bytes that will not parse: +// there is no tree to ask. +func declaresFlowKey(data []byte) bool { + i := skipNodeProperties(data, skipSpaceAndComments(data, 0)) + if i == len(data) || data[i] != '{' { + return false + } + + for depth := 0; i < len(data); { + switch data[i] { + case '"': + name, next := flowString(data, i) + if depth == 1 && isProbeName(name) && versionAfterColon(data, next) { + return true + } + i = next + case '{', '[': + depth++ + i++ + case '}', ']': + depth-- + i++ + default: + i++ + } + } + return false +} + +// flowString returns the bytes between the quotes of the string data[i] opens, +// and the index just past its closing quote. An unterminated string runs to the +// end of data: there is nothing past it left to read. +func flowString(data []byte, i int) ([]byte, int) { + for j := i + 1; j < len(data); j++ { + switch data[j] { + case '\\': + j++ + case '"': + return data[i+1 : j], j + 1 + } + } + return nil, len(data) +} + +// isProbeName reports whether name is one of the discriminating keys. +func isProbeName(name []byte) bool { + return string(name) == "openapi" || string(name) == "swagger" +} + +// versionAfterColon reports whether a colon follows i, making the name before it +// a key, and a version follows the colon: a quoted string or a bare word that +// reads as one. A collection there opens with a flow indicator, so it reads as +// an empty word and declares nothing. +func versionAfterColon(data []byte, i int) bool { + i = skipSpace(data, i) + if i == len(data) || data[i] != ':' { + return false + } + i = skipSpace(data, i+1) + if i < len(data) && data[i] == '"' { + value, _ := flowString(data, i) + return isVersion(string(value)) + } + end := i + for end < len(data) && !endsWord(data[end]) { + end++ + } + return isVersion(string(data[i:end])) +} + +// skipSpace returns the index of the first byte at or after i that is not +// whitespace, or len(data) if there is none. +func skipSpace(data []byte, i int) int { + for i < len(data) && (data[i] == ' ' || data[i] == '\t' || data[i] == '\r' || data[i] == '\n') { + i++ + } + return i +} + +// skipSpaceAndComments returns the index of the first byte at or after i that +// begins content: past whitespace, and past any line a `#` opens. A flow +// document may follow a comment line, and yaml.v3 reads it there. +func skipSpaceAndComments(data []byte, i int) int { + for i = skipSpace(data, i); i < len(data) && data[i] == '#'; i = skipSpace(data, i) { + _, i = nextLine(data, i) + } + return i +} + +// maxNodeProperties is how many node properties a node may carry: one anchor +// and one tag, in either order. It bounds skipNodeProperties. +const maxNodeProperties = 2 + +// skipNodeProperties returns the index just past any node properties at i — an +// `&anchor` or a `!tag`, each a word ended by whitespace or a flow indicator, +// neither of which a name may contain — and the whitespace after them. YAML +// writes them in front of the node they annotate, so a document's root flow +// mapping may sit behind one or two of them: `&x {`, `!!map &x {`. A guard +// testing the first byte for the opener would see the property instead, and +// decline a broken document that is this compiler's own. +func skipNodeProperties(data []byte, i int) int { + for range maxNodeProperties { + if i == len(data) || (data[i] != '&' && data[i] != '!') { + return i + } + for i < len(data) && !endsWord(data[i]) { + i++ + } + i = skipSpace(data, i) + } + return i +} + +// nextLine returns the line beginning at i, without its terminator, and the +// index of the line after it. +func nextLine(data []byte, i int) (line []byte, next int) { + line = data[i:] + if j := bytes.IndexByte(line, '\n'); j >= 0 { + return line[:j], i + j + 1 + } + return line, len(data) +} + +// endsWord reports whether b ends a bare word in flow context: whitespace, or a +// flow indicator, none of which an anchor, a tag or a bare version contains. +func endsWord(b byte) bool { + switch b { + case ',', '}', ']', '{', '[', ' ', '\t', '\r', '\n': + return true + default: + return false + } +} diff --git a/compilers/openapi/internal/detect/declares_test.go b/compilers/openapi/internal/detect/declares_test.go new file mode 100644 index 00000000..f3045629 --- /dev/null +++ b/compilers/openapi/internal/detect/declares_test.go @@ -0,0 +1,88 @@ +package detect + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestDeclaresProbeKey_ScopesTheNameToItsOwnDocument pins the guard that +// decides whose bytes these are. A name followed by a colon is a key wherever +// it sits, so the guard says whose: block style answers with column 0, flow +// style with the root mapping's own depth. A key there declares the format only +// with a version beside it (GitHub #497). The cases answering no are documents +// naming the word where none declares its format, or declaring it with prose, a +// collection or nothing beside it; saying yes would report this compiler's +// parse error over a file that was never its own. +func TestDeclaresProbeKey_ScopesTheNameToItsOwnDocument(t *testing.T) { + t.Parallel() + cases := []struct { + name, src string + want bool + }{ + {"flow mapping declares it", `{"openapi":"3.1.0"}`, true}, + {"flow mapping declares swagger", `{"swagger":"2.0"}`, true}, + {"space around the mapping and the colon", " \n\t{\"openapi\" : \"3.1.0\"}", true}, + {"an escape hides no key from the scan", `{"a\"b":1,"openapi":"3.1.0"}`, true}, + {"block style at column 0", "openapi: 3.1.0\n", true}, + {"block style past a byte-order mark", "\xef\xbb\xbfopenapi: 3.1.0\ninfo: [\n", true}, + // The version beside the key is read as YAML reads it, on its own line. + {"block style, a quoted version", "openapi: \"3.1.0\"\ninfo: [\n", true}, + {"block style, a version and a comment", "openapi: 3.1.0 # v\ninfo: [\n", true}, + {"block style, CRLF", "openapi: 3.1.0\r\ninfo: [\r\n", true}, + {"block style, swagger", "swagger: \"2.0\"\ninfo: [\n", true}, + {"flow style, a bare version", `{"openapi":3.1,`, true}, + // And a key with no version beside it declares nothing, in either style. + {"block style, prose beside the key", "openapi: is a format\n[\n", false}, + {"block style, a mapping under the key", "openapi:\n enabled: true\n[\n", false}, + {"block style, a collection that does not close", "openapi: [unterminated\n", false}, + {"block style, no space after the colon", "openapi:3.1.0\n[\n", false}, + {"block style, a longer key the name begins", "openapi:x: 3.1.0\n[\n", false}, + {"block style, the version on the next line", "openapi:\n 3.1.0\n[\n", false}, + {"flow style, a collection for the version", `{"openapi":{"a":"3.1.0"},`, false}, + {"flow style, prose for the version", `{"openapi":"is a format",`, false}, + {"flow style, nothing after the colon", `{"openapi":`, false}, + {"flow style past a byte-order mark", "\xef\xbb\xbf{\"openapi\":\"3.1.0\",", true}, + // What may stand in front of a root flow mapping: comment lines, and up + // to two node properties — an anchor and a tag, in either order. + {"flow style behind a comment line", "# c\n # d\n{\"openapi\":\"3.1.0\",", true}, + {"flow style behind an anchor", "&x {\"openapi\":\"3.1.0\",", true}, + {"flow style behind a tag and an anchor", "!!map &x {\"openapi\":\"3.1.0\",", true}, + {"flow style behind a property ended by a flow indicator", "&x{\"openapi\":\"3.1.0\",", true}, + {"a comment with nothing after it", "# openapi: 3.1.0", false}, + {"a property with nothing after it", "&x", false}, + {"three properties are more than a node carries", "&a &b &c {\"openapi\":\"3.1.0\",", false}, + + {"nested one level down", `{"a":{"openapi":"3.1.0"}}`, false}, + {"nested inside a sequence", `{"a":[{"openapi":"3.1.0"}]}`, false}, + {"a document that opens a sequence", `[{"openapi":"3.1.0"}]`, false}, + {"block style indented under another key", "a:\n openapi: 3.1.0\n", false}, + {"the name is a value", `{"note":"openapi"}`, false}, + {"the name in a comment", "x: 1\n# openapi is a format\n", false}, + {"the name has no colon after it", `{"openapi",1}`, false}, + {"the name ends the bytes", `{"openapi"`, false}, + {"a string runs off the end", `{"a":"unterminated`, false}, + {"nothing but whitespace", " \n\t ", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, declaresProbeKey([]byte(tc.src))) + }) + } +} + +// TestDeclaresBlockKey_ReadsABoundedNumberOfLines pins maxVersionLines. The +// guard parses each line that begins with the key, so a version behind as many +// lines without one as the bound allows is still found, and one behind a line +// more is not: the bound is what stops a file of nothing but such lines costing +// a parse per line. +func TestDeclaresBlockKey_ReadsABoundedNumberOfLines(t *testing.T) { + t.Parallel() + behind := func(n int) []byte { + return []byte(strings.Repeat("openapi: is prose\n", n) + "openapi: 3.1.0\n[\n") + } + assert.True(t, declaresBlockKey(behind(maxVersionLines-1), "openapi"), "the last line the bound reads") + assert.False(t, declaresBlockKey(behind(maxVersionLines), "openapi"), "one line past the bound") +} diff --git a/compilers/openapi/internal/detect/doc.go b/compilers/openapi/internal/detect/doc.go new file mode 100644 index 00000000..40b80d7d --- /dev/null +++ b/compilers/openapi/internal/detect/doc.go @@ -0,0 +1,10 @@ +// Package detect recognizes an OpenAPI or Swagger document from its bytes: the +// discriminating key it writes and the version beside it, read off the parse +// the compile goes on to lower. +// +// The root mapping is read through nodeview, as the parser library reads it, so +// the version that routes a source is the version its compile reads +// (docs/compilers/openapi.md#yaml-decoding). Only bytes that will not parse are +// read another way: a scan for a version written where a document declares +// one, which decides whether the failure is this compiler's to report. +package detect diff --git a/compilers/openapi/internal/detect/helpers_test.go b/compilers/openapi/internal/detect/helpers_test.go new file mode 100644 index 00000000..739441ba --- /dev/null +++ b/compilers/openapi/internal/detect/helpers_test.go @@ -0,0 +1,59 @@ +package detect + +import ( + "strings" + + "github.com/dexpace/morphic/compilers" + "github.com/dexpace/morphic/ir" +) + +// recognize is Recognize as Detect calls it on the one source it is handed, +// source 0, with no byte budget: no case here comes near the default, and +// TestRecognize_HoldsTheParseToItsByteBudget sets one of its own. +func recognize(src string) (compilers.Recognition, []ir.Diagnostic, bool) { + return Recognize(0, []byte(src), 0) +} + +// codesOf reduces diagnostics to their codes, which is the whole of what the +// detection tests assert about them: the message carries a parser's wording and +// pinning it would test yaml.v3 rather than this package. +func codesOf(diags []ir.Diagnostic) []string { + if len(diags) == 0 { + return nil + } + codes := make([]string, 0, len(diags)) + for _, d := range diags { + codes = append(codes, d.Code) + } + return codes +} + +// scanCutover is the size past which detection used to scan bytes rather than +// parse, and still the size where the change of reading shows: a case larger +// than this pins that a large document is read as a small one is. +const scanCutover = 64 << 10 + +// flowPad returns a run of bytes long enough that a flow entry holding it puts +// everything after it past scanCutover. +func flowPad() string { return strings.Repeat("p", scanCutover) } + +// padTo returns src grown past scanCutover by appending filler. +func padTo(src, filler string) string { + var b strings.Builder + b.WriteString(src) + for b.Len() <= scanCutover { + b.WriteString(filler) + } + return b.String() +} + +// dupRepeats is how many times the fixtures below repeat a root key. It is not a +// threshold: the wrong answer reproduces at two repeats, and this many only +// makes the fixture recognizably a document rather than a corner. It is +// deliberately far below the 6,553 of the report — yaml.v3 raises one error per +// pair of matching keys, so that count produced 21,467,628 of them and a 1.2 GB +// message, and a fixture that large turns a revert into an out-of-memory kill +// instead of a failing assertion. What guards the cost is +// TestSniff_CostIsNotQuadraticInRepeatedKeys, which measures growth rather than +// paying for it. +const dupRepeats = 512 diff --git a/compilers/openapi/internal/detect/probe.go b/compilers/openapi/internal/detect/probe.go new file mode 100644 index 00000000..f3763734 --- /dev/null +++ b/compilers/openapi/internal/detect/probe.go @@ -0,0 +1,172 @@ +package detect + +import ( + "errors" + "fmt" + + yaml "gopkg.in/yaml.v3" + + "github.com/dexpace/morphic/compilers/openapi/internal/load" + "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" +) + +// sniffProbe holds the two discriminating top-level keys. Which one is present +// is the whole of the format question: an OpenAPI 3.x document declares +// `openapi`, a Swagger 2.0 document declares `swagger`. +// +// It carries no struct tags: nothing decodes into it. The parse over a tree +// names the two keys itself, in fieldFor. +type sniffProbe struct { + OpenAPI string + Swagger string + // Cut reports a root mapping that merges through `<<` past + // nodeview.MergeDepthLimit without settling the answer, so a key the read + // did not reach may outrank what it found. + Cut bool +} + +// sniff reads the discriminating keys out of data. For anything it cannot read +// it returns the zero probe and the parser's error; whether that error is +// worth reporting is Detect's question. +// +// The document is decoded whole and exactly, the only way to tell one that +// declares nothing from one that will not parse. The parse is the one the +// compile lowers, not a second reading beside it that would have to be kept in +// agreement with it (GitHub #486). The caller's byte budget, which Detect +// checks first, bounds it. +func sniff(data []byte) (sniffProbe, *load.Parsed, error) { + probe, parsed, err := decodeYAML(data) + return declaredVersions(probe), parsed, err +} + +// declaredVersions drops any value that does not read as a version. A key alone +// does not declare a format: another format's document may write the word — at +// column 0 in Markdown prose, or as a field of its own — and what separates that +// from a declaration is the version beside it. Claiming it instead reports this +// compiler's complaint over a file that was never its own. +func declaredVersions(probe sniffProbe) sniffProbe { + if !isVersion(probe.OpenAPI) { + probe.OpenAPI = "" + } + if !isVersion(probe.Swagger) { + probe.Swagger = "" + } + return probe +} + +// decodeYAML reads the probe keys from a complete YAML (or JSON, its subset) +// document. +// +// The document is parsed and its root mapping read, never decoded into +// sniffProbe: yaml.v3 compares every pair of a mapping's keys before reading +// any, so a mapping repeating one key n times raises n(n-1)/2 errors and then +// abandons the mapping, leaving the probe empty as well as expensive. Reading +// the keys off the tree is linear and answers the same for repeated keys, +// which the compile's own parse reports once each, sited. +func decodeYAML(data []byte) (sniffProbe, *load.Parsed, error) { + parsed, err := load.Decode(data) + if err != nil { + return sniffProbe{}, nil, err + } + + root := documentRoot(parsed.Root()) + switch { + case root == nil: + // A stream that carried no document declares no key, which is a decline + // and not a failure: empty bytes are no more this compiler's than + // anybody else's. + return sniffProbe{}, nil, nil + case root.Kind != yaml.MappingNode: + return sniffProbe{}, nil, fmt.Errorf("document root is %s, not a mapping", root.ShortTag()) + default: + probe, err := probeFromMapping(root) + return probe, parsed, err + } +} + +// documentRoot returns the content node of the document load handed back, or +// nil for a stream that carried none. That document is the first in the stream +// that holds content — the one the compile lowers, read by the same function, +// so a source routed here on what that document declares is the source lowered +// here (GitHub #481). The documents after it are reported as dropped by load +// (GitHub #387); detection routes the source and says nothing about its shape. +func documentRoot(doc *yaml.Node) *yaml.Node { + if doc.Kind != yaml.DocumentNode || len(doc.Content) != 1 { + return nil + } + return doc.Content[0] +} + +// probeFromMapping reads the probe keys off a root mapping through nodeview, +// the one reading of a mapping the compile's parser gives: through aliases, its +// `<<` merges expanded as the library expands them, and a key written twice at +// its last value (docs/parser-library.md#duplicate-key). A second reading +// would let detection name a version the compile does not read, which +// compilers.Detection.Compile checks. +// +// Only a key's effective value is read, so a value it overrides cannot fail +// the probe. Only the depth bound hides a key (View.Exhausted): a merge cycle, +// which the compile refuses, closes having read every mapping on it. +func probeFromMapping(root *yaml.Node) (sniffProbe, error) { + view := nodeview.New() + var probe sniffProbe + for _, pair := range view.MappingPairs(root) { + field := probe.fieldFor(pair.Key) + if field == nil { + continue + } + version, err := probeVersion(pair.Val) + if err != nil { + return sniffProbe{}, err + } + *field = version + } + probe.Cut = view.Exhausted() && !settledByOwnPairs(root, probe) + return probe, nil +} + +// settledByOwnPairs reports whether probe, read off root, decides detection +// although root's merges were cut short. A mapping's own pair outranks every +// merge, so a key root writes itself holds the value read: `openapi` settles it +// when that value is a version, and with `swagger` written too whatever it is. +// A key root leaves to its merges may lie past the bound. +func settledByOwnPairs(root *yaml.Node, probe sniffProbe) bool { + own := map[string]bool{} + for i := 0; i+1 < len(root.Content); i += 2 { + if key, ok := nodeview.KeyOf(root.Content[i]); ok { + own[key] = true + } + } + return own["openapi"] && (isVersion(probe.OpenAPI) || own["swagger"]) +} + +// probeVersion returns the version string a probe key's value declares, and an +// error for a value that is no scalar, or none at all. +// +// The scalar's text is taken as written rather than decoded, because the two +// disagree only for tags no version carries — a version key is not !!binary — +// and because decoding is what must not happen here: a mapping handed back to +// the decoder is the quadratic path decodeYAML exists to avoid, and a probe +// key's own value is the last place one could still be handed to it. +func probeVersion(value *yaml.Node) (string, error) { + if value == nil { + return "", errors.New("version key has no value") + } + if value.Kind != yaml.ScalarNode { + return "", fmt.Errorf("version key is %s, not a scalar", value.ShortTag()) + } + return value.Value, nil +} + +// fieldFor returns the probe field that key names, or nil for a key that names +// neither. +func (p *sniffProbe) fieldFor(key string) *string { + switch key { + case "openapi": + return &p.OpenAPI + case "swagger": + return &p.Swagger + default: + return nil + } +} diff --git a/compilers/openapi/internal/detect/probe_test.go b/compilers/openapi/internal/detect/probe_test.go new file mode 100644 index 00000000..f39808fe --- /dev/null +++ b/compilers/openapi/internal/detect/probe_test.go @@ -0,0 +1,256 @@ +package detect + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + yaml "gopkg.in/yaml.v3" + + "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" + "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" + "github.com/dexpace/morphic/compilers/openapi/internal/ynode" +) + +// TestSniff_ReadsALargeDocumentWhole pins the reading a large document gets: +// the parse a small one gets, which finds a key wherever it sits and in either +// style, because where a writer put a key in a mapping says nothing about what +// the document is. Each case says whether the document parsed, so a case that +// expects no declaration cannot pass on bytes that never parsed at all. +func TestSniff_ReadsALargeDocumentWhole(t *testing.T) { + t.Parallel() + const filler = "# a line of padding that says nothing about the format\n" + pad := strings.Repeat("p", scanCutover) + cases := []struct { + name, src string + want sniffProbe + unreadable bool + }{ + {"block yaml declaring first", + padTo("openapi: 3.1.0\n", filler), sniffProbe{OpenAPI: "3.1.0"}, false}, + {"block yaml declaring last", + padTo("", filler) + "openapi: 3.1.0\n", sniffProbe{OpenAPI: "3.1.0"}, false}, + {"flow json declaring first", + `{"openapi":"3.1.0","x":"` + pad + `"}`, sniffProbe{OpenAPI: "3.1.0"}, false}, + {"flow json declaring last", + `{"x":"` + pad + `","openapi":"3.1.0"}`, sniffProbe{OpenAPI: "3.1.0"}, false}, + {"flow json swagger first", + `{"swagger":"2.0","x":"` + pad + `"}`, sniffProbe{Swagger: "2.0"}, false}, + {"flow json swagger last", + `{"x":"` + pad + `","swagger":"2.0"}`, sniffProbe{Swagger: "2.0"}, false}, + // Neither YAML nor JSON: the parse fails and declares nothing. + {"protobuf, large", + padTo("syntax = \"proto3\";\n", "message M { string a = 1; }\n"), sniffProbe{}, true}, + // The word is there and is not a key: a value is not a declaration. + {"the word as a value, large", + `{"x":"` + pad + `","note":"openapi"}`, sniffProbe{}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + require.Greater(t, len(tc.src), scanCutover, "the case must be large to test it") + probe, parsed, err := sniff([]byte(tc.src)) + assert.Equal(t, tc.want, probe) + if tc.unreadable { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.NotNil(t, parsed, "a document that parsed carries its parse") + }) + } +} + +// TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt pins where the bound on +// the root's merges lies, nodeview's, and what a read it cuts short answers. A +// key past the bound may outrank what the read found, so the answer is +// unknown, and Detect says the bound stopped it, unless the root's own pairs, +// which outrank every merge, settle it. +func TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt(t *testing.T) { + t.Parallel() + const limit = nodeview.MergeDepthLimit + cases := map[string]struct { + src []byte + version string + cut bool + }{ + "a chain the view expands": {openapitest.RootMergeChain(limit, "openapi: 3.0.3", ""), "3.0.3", false}, + "a chain past it": {openapitest.RootMergeChain(limit+1, "openapi: 3.0.3", ""), "", true}, + "a version the root writes itself": { + openapitest.RootMergeChain(limit+1, "openapi: 3.0.3", "openapi: 3.1.0\n"), "3.1.0", false}, + "a merged version a deeper one may outrank": { + []byte(string(openapitest.RootMergeChain(limit+1, "openapi: 3.1.0", "")) + "s: &s {openapi: 3.0.3}\n"), "", true}, + "a root writing no version and a swagger one": { + openapitest.RootMergeChain(limit+1, "openapi: 3.0.3", "openapi: prose words\nswagger: '2.0'\n"), "", false}, + "a root writing no version and leaving swagger to its merges": { + openapitest.RootMergeChain(limit+1, "swagger: '2.0'", "openapi: prose words\n"), "", true}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + probe, _, err := sniff(tc.src) + require.NoError(t, err, "a read cut short is no parse failure") + assert.Equal(t, tc.cut, probe.Cut) + if tc.cut { + return + } + assert.Equal(t, tc.version, probe.OpenAPI) + }) + } +} + +// TestDecodeYAML_RefusesARootThatIsNoMapping pins the shape complaint. A source +// with no root mapping has no top-level keys, and saying so is what lets Detect +// report bytes that declare a key it serves and will not read. +func TestDecodeYAML_RefusesARootThatIsNoMapping(t *testing.T) { + t.Parallel() + cases := []struct{ name, src, wantErr string }{ + {"sequence", "- openapi: 3.1.0\n", "!!seq"}, + {"scalar", "just a string\n", "!!str"}, + {"empty", "", ""}, + {"comment only", "# openapi: 3.1.0\n", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + probe, _, err := decodeYAML([]byte(tc.src)) + assert.Empty(t, probe.OpenAPI) + if tc.wantErr == "" { + assert.NoError(t, err, "bytes that carry no document decline rather than fail") + return + } + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr, "the complaint names the shape that was read") + }) + } +} + +// TestSniff_CostIsNotQuadraticInRepeatedKeys guards the half of the defect an +// answer cannot see. Reading two keys off a parsed tree is linear; decoding the +// root mapping to read them is quadratic in how often a key repeats, since +// yaml.v3 compares every pair of keys before reading any. +// +// Allocation count is the probe because it is deterministic where wall time is +// not. Doubling the repeats doubles the parse, so the bound sits between that +// and a quadratic term: at this size the linear reading grows by 1.97 and the +// quadratic one by 4.64. +func TestSniff_CostIsNotQuadraticInRepeatedKeys(t *testing.T) { + head := "openapi: 3.0.3\ninfo: {}\n" + small := []byte(head + strings.Repeat("x: y\n", dupRepeats)) + large := []byte(head + strings.Repeat("x: y\n", dupRepeats*2)) + + smallAllocs := testing.AllocsPerRun(2, func() { _, _, _ = sniff(small) }) + largeAllocs := testing.AllocsPerRun(2, func() { _, _, _ = sniff(large) }) + + require.Positive(t, smallAllocs, "a measurement of nothing bounds nothing") + assert.Less(t, largeAllocs, smallAllocs*3, + "twice the repeats must cost about twice, not about four times") +} + +// TestDecodeYAML_RefusesAVersionKeyThatIsNoScalar pins the complaint for a +// version key whose value is a mapping or a sequence, written directly and +// reached through a `<<`. Such bytes name a key this compiler serves and do not +// say what dialect, so the probe cannot read them. +// +// Whether Detect reports that or declines in silence is declaresProbeKey's +// answer and not this one's, and it is asserted separately below: a mapping +// under the key is another tool's configuration as often as a broken spec, and +// declares no version either way, so Detect declines all of them. +func TestDecodeYAML_RefusesAVersionKeyThatIsNoScalar(t *testing.T) { + t.Parallel() + cases := []struct{ name, src, wantErr string }{ + {"mapping", "openapi: {a: b}\ninfo: {}\n", "!!map"}, + {"sequence", "openapi: [3.1.0]\ninfo: {}\n", "!!seq"}, + {"merged mapping", "base: &b\n openapi: {a: b}\n<<: *b\n", "!!map"}, + {"merged through a sequence", "base: &b\n openapi: {a: b}\n<<: [*b]\n", "!!map"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + probe, _, err := decodeYAML([]byte(tc.src)) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr, "the complaint names the shape that was read") + assert.Empty(t, probe.OpenAPI) + }) + } +} + +// TestDecodeYAML_PassesOverWhatNamesNoVersion holds the walk to reading only +// what it came for. A mapping may key an entry with a sequence, and a `<<` may +// be written with a value that merges nothing; both are the source's business +// and neither stops the two keys beside them from being read. +func TestDecodeYAML_PassesOverWhatNamesNoVersion(t *testing.T) { + t.Parallel() + cases := []struct{ name, src string }{ + {"a key that is a sequence", "? [a, b]\n: v\nopenapi: 3.1.0\n"}, + {"a merge of a scalar", "<<: not-a-mapping\nopenapi: 3.1.0\n"}, + {"a merge of a sequence of scalars", "<<: [x, y]\nopenapi: 3.1.0\n"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + probe, _, err := decodeYAML([]byte(tc.src)) + require.NoError(t, err) + assert.Equal(t, "3.1.0", probe.OpenAPI) + }) + } +} + +// TestProbeVersion_RefusesAnAliasThatResolvedToNothing covers the guard on a +// value nodeview could not dereference. A parser resolves every alias it +// accepts, so the node is built here rather than parsed: the guard exists +// because reading the value is what the next line does, and a nil there is a +// panic in detection, which runs before the compiler has decided the bytes are +// even its own. +func TestProbeVersion_RefusesAnAliasThatResolvedToNothing(t *testing.T) { + t.Parallel() + root := ynode.Map(ynode.Scalar("openapi"), &yaml.Node{Kind: yaml.AliasNode}) + _, err := probeFromMapping(root) + require.Error(t, err) + assert.Contains(t, err.Error(), "no value") +} + +// breadthMergeDoc writes a document whose root merges one anchor k times, where +// that anchor's mapping merges another k times. It is the shape that made the +// probe's walk quadratic: the inner mapping was entered once per merge key +// naming it, so the work was the product of the two rather than their sum. The +// version sits at the innermost anchor, so a walk that stops early is caught by +// the assertion rather than looking cheap. +func breadthMergeDoc(k int) []byte { + var b strings.Builder + b.WriteString("l0: &a0\n openapi: 3.1.0\n") + b.WriteString("l1: &a1\n") + for range k { + b.WriteString(" <<: *a0\n") + } + for range k { + b.WriteString("<<: *a1\n") + } + return []byte(b.String()) +} + +// TestProbeFromMapping_CostIsLinearInMergeBreadth pins the bound GitHub #487 +// was about: the merge bound limits how deep a chain is followed, not how wide, +// and a walk entering an anchored mapping once per merge key naming it costs +// the product of the two. nodeview's memo is what keeps it a sum. It counts +// allocations, not time, so load cannot move it, and runs serially because +// AllocsPerRun measures the whole process. +func TestProbeFromMapping_CostIsLinearInMergeBreadth(t *testing.T) { + for _, k := range []int{2000, 4000} { + var root yaml.Node + require.NoError(t, yaml.Unmarshal(breadthMergeDoc(k), &root)) + content := documentRoot(&root) + + var probe sniffProbe + var err error + allocs := testing.AllocsPerRun(1, func() { + probe, err = probeFromMapping(content) + }) + require.NoError(t, err) + require.Equal(t, "3.1.0", probe.OpenAPI, + "the version sits at the innermost anchor, so a walk that stops early fails here rather than looking cheap") + assert.Less(t, allocs, float64(k), + "a walk of %d merge keys made %.0f allocations, not fewer than the keys", k, allocs) + } +} diff --git a/compilers/openapi/detect_reading_test.go b/compilers/openapi/internal/detect/reading_test.go similarity index 80% rename from compilers/openapi/detect_reading_test.go rename to compilers/openapi/internal/detect/reading_test.go index 62cc6d84..c7cee84e 100644 --- a/compilers/openapi/detect_reading_test.go +++ b/compilers/openapi/internal/detect/reading_test.go @@ -1,8 +1,6 @@ -package openapi +package detect import ( - "context" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -212,7 +210,7 @@ func declarations() []declarationRow { } } -func TestDetect_ReadsWhatADocumentDeclares(t *testing.T) { +func TestRecognize_ReadsWhatADocumentDeclares(t *testing.T) { t.Parallel() for _, tc := range declarations() { t.Run(tc.name, func(t *testing.T) { @@ -225,7 +223,7 @@ func TestDetect_ReadsWhatADocumentDeclares(t *testing.T) { require.Error(t, err, "the row reads as a document that does not parse") } - rec, diags, ok := New().Detect(compilers.Source{Path: "api.yaml", Data: []byte(tc.src)}, compilers.Options{}) + rec, diags, ok := recognize(tc.src) assert.Equal(t, tc.want != "", ok, "a declared version is what recognizes the source") assert.Equal(t, majorMinor(tc.want), rec.Format.Version) var wantCodes []string @@ -237,11 +235,11 @@ func TestDetect_ReadsWhatADocumentDeclares(t *testing.T) { } } -// TestDetect_BothKeysNameOneFormat pins a document that declares both keys. It +// TestRecognize_BothKeysNameOneFormat pins a document that declares both keys. It // names one format, OpenAPI, whichever key comes first and at any size: a // reader that stopped at the first key it met read the same bytes as Swagger // once they grew past the point where it took over. -func TestDetect_BothKeysNameOneFormat(t *testing.T) { +func TestRecognize_BothKeysNameOneFormat(t *testing.T) { t.Parallel() want := compilers.SourceFormat{Name: "openapi", Version: "3.0"} for name, src := range map[string]string{ @@ -250,18 +248,18 @@ func TestDetect_BothKeysNameOneFormat(t *testing.T) { } { t.Run(name, func(t *testing.T) { t.Parallel() - got, _, ok := New().Detect(compilers.Source{Path: "spec.json", Data: []byte(src)}, compilers.Options{}) + got, _, ok := recognize(src) assert.True(t, ok) assert.Equal(t, want, got.Format) }) } } -// TestDetect_AValueThatIsNoVersionIsNoDeclaration pins the other half of whose +// TestRecognize_AValueThatIsNoVersionIsNoDeclaration pins the other half of whose // bytes these are. A key alone does not declare a format: prose sitting beside // the word is what a document of another format writes, and claiming it reports // this compiler's complaint over a file that was never its own. -func TestDetect_AValueThatIsNoVersionIsNoDeclaration(t *testing.T) { +func TestRecognize_AValueThatIsNoVersionIsNoDeclaration(t *testing.T) { t.Parallel() cases := []struct{ name, src string }{ {"prose beside the key", "openapi: is a format\n"}, @@ -273,7 +271,7 @@ func TestDetect_AValueThatIsNoVersionIsNoDeclaration(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got, diags, ok := New().Detect(compilers.Source{Path: "README.md", Data: []byte(tc.src)}, compilers.Options{}) + got, diags, ok := recognize(tc.src) assert.False(t, ok, "prose beside the word is not a declaration of this format") assert.Equal(t, compilers.SourceFormat{}, got.Format) assert.Nil(t, codesOf(diags), "another format's file earns no complaint from this one") @@ -281,13 +279,13 @@ func TestDetect_AValueThatIsNoVersionIsNoDeclaration(t *testing.T) { } } -// TestDetect_AVersionThatIsNotServedIsStillADeclaration pins the other edge of +// TestRecognize_AVersionThatIsNotServedIsStillADeclaration pins the other edge of // the prose guard. A prerelease or otherwise unserved version is still one // word beside the key, which is what a declaration looks like and prose does // not; declining it would hand a document whose first line is `openapi:` to // the engine's generic "unrecognized format", where claiming it lets load and // the validator say exactly which version was wrong. -func TestDetect_AVersionThatIsNotServedIsStillADeclaration(t *testing.T) { +func TestRecognize_AVersionThatIsNotServedIsStillADeclaration(t *testing.T) { t.Parallel() cases := []struct{ name, src, want string }{ {"a prerelease suffix", "openapi: 3.1.0-rc1\n", "3.1"}, @@ -298,7 +296,7 @@ func TestDetect_AVersionThatIsNotServedIsStillADeclaration(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got, diags, ok := New().Detect(compilers.Source{Path: "api.yaml", Data: []byte(tc.src)}, compilers.Options{}) + got, diags, ok := recognize(tc.src) assert.True(t, ok, "one word beside the key declares the format, whether or not it is served") assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: tc.want}, got.Format) assert.Nil(t, codesOf(diags), "which version is wrong is load's to say") @@ -306,13 +304,13 @@ func TestDetect_AVersionThatIsNotServedIsStillADeclaration(t *testing.T) { } } -// TestDetect_ABrokenDocumentIsReportedAtAnySize pins what reading one way +// TestRecognize_ABrokenDocumentIsReportedAtAnySize pins what reading one way // bought. A broken spec that declares a version at column 0 is this // compiler's own, and saying so needs the whole document read: the byte scan that stood // in for the parse past 64 KiB could not tell such a spec from another format's // file naming the word, so the same document drew a complaint when small and // silence when large. -func TestDetect_ABrokenDocumentIsReportedAtAnySize(t *testing.T) { +func TestRecognize_ABrokenDocumentIsReportedAtAnySize(t *testing.T) { t.Parallel() for name, src := range map[string]string{ "small": "openapi: 3.1.0\ninfo: [unterminated\n", @@ -320,7 +318,7 @@ func TestDetect_ABrokenDocumentIsReportedAtAnySize(t *testing.T) { } { t.Run(name, func(t *testing.T) { t.Parallel() - got, diags, ok := New().Detect(compilers.Source{Path: "api.yaml", Data: []byte(src)}, compilers.Options{}) + got, diags, ok := recognize(src) assert.False(t, ok) assert.Equal(t, compilers.SourceFormat{}, got.Format) assert.Equal(t, []string{diag.UndecodableSource}, codesOf(diags)) @@ -328,28 +326,11 @@ func TestDetect_ABrokenDocumentIsReportedAtAnySize(t *testing.T) { } } -// TestCompile_AnUnreadableSourceIsADiagnostic pins where the complaint about a -// broken document comes from when nothing detected it: a caller compiling -// directly hands Compile bytes no parse has read. It has to stay a diagnostic: -// engine.Run turns a compiler's Go error into its own, and the CLI maps that to -// exit 2 — the code it uses for being invoked wrong — so a spec it read would -// be reported as a misuse of itself. -func TestCompile_AnUnreadableSourceIsADiagnostic(t *testing.T) { - t.Parallel() - src := "bad: [unterminated\n" + strings.Repeat("filler: x\n", 8000) + "openapi: 3.1.0\n" - doc, diags, err := New().Compile(context.Background(), - []compilers.Source{{Path: "api.yaml", Data: []byte(src)}}, compilers.Options{}) - - require.NoError(t, err, "a document that will not parse is a finding, not a failure of the compiler") - assert.Nil(t, doc) - assert.Equal(t, []string{diag.UndecodableSource}, codesOf(diags)) -} - -// TestDetect_AnEmptyFirstDocumentIsNotTheDocument pins GitHub #481 at the +// TestRecognize_AnEmptyFirstDocumentIsNotTheDocument pins GitHub #481 at the // public face: a stream that opens with a document holding nothing is routed on // the document behind it, where it used to be declined as undecodable — with a // whole spec two lines further down. -func TestDetect_AnEmptyFirstDocumentIsNotTheDocument(t *testing.T) { +func TestRecognize_AnEmptyFirstDocumentIsNotTheDocument(t *testing.T) { t.Parallel() want := compilers.SourceFormat{Name: "openapi", Version: "3.1"} for name, lead := range map[string]string{ @@ -360,7 +341,7 @@ func TestDetect_AnEmptyFirstDocumentIsNotTheDocument(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() src := lead + "openapi: 3.1.0\ninfo: {title: T, version: \"1\"}\npaths: {}\n" - got, diags, ok := New().Detect(compilers.Source{Path: "spec.yaml", Data: []byte(src)}, compilers.Options{}) + got, diags, ok := recognize(src) assert.True(t, ok, "the document behind the empty one is what the source declares") assert.Equal(t, want, got.Format) assert.Nil(t, codesOf(diags)) @@ -368,11 +349,11 @@ func TestDetect_AnEmptyFirstDocumentIsNotTheDocument(t *testing.T) { } } -// TestDetect_AByteOrderMarkIsNotAFormat pins a mark before the first byte, +// TestRecognize_AByteOrderMarkIsNotAFormat pins a mark before the first byte, // which the parse reads straight through and the key guard, reading bytes, has // to skip itself: a broken spec written by an editor that emits one is as much // this compiler's to report as the same spec without it. -func TestDetect_AByteOrderMarkIsNotAFormat(t *testing.T) { +func TestRecognize_AByteOrderMarkIsNotAFormat(t *testing.T) { t.Parallel() const bom = "\xef\xbb\xbf" want := compilers.SourceFormat{Name: "openapi", Version: "3.1"} @@ -382,7 +363,7 @@ func TestDetect_AByteOrderMarkIsNotAFormat(t *testing.T) { } { t.Run(name, func(t *testing.T) { t.Parallel() - got, diags, ok := New().Detect(compilers.Source{Path: "spec.yaml", Data: []byte(bom + body)}, compilers.Options{}) + got, diags, ok := recognize(bom + body) assert.True(t, ok, "a byte-order mark is not part of what a document declares") assert.Equal(t, want, got.Format) assert.Nil(t, codesOf(diags)) @@ -394,18 +375,18 @@ func TestDetect_AByteOrderMarkIsNotAFormat(t *testing.T) { } { t.Run(name, func(t *testing.T) { t.Parallel() - _, diags, ok := New().Detect(compilers.Source{Path: "spec.yaml", Data: []byte(bom + body)}, compilers.Options{}) + _, diags, ok := recognize(bom + body) assert.False(t, ok) assert.Equal(t, []string{diag.UndecodableSource}, codesOf(diags), "the mark hides no key") }) } } -// TestDetect_CarriesTheParseItRead pins what a recognition hands over at any +// TestRecognize_CarriesTheParseItRead pins what a recognition hands over at any // size: the parse that read the keys, which is what the compile lowers. Past // 64 KiB detection used to scan and carry nothing, so a large document was // parsed twice — once here and again in the compile. -func TestDetect_CarriesTheParseItRead(t *testing.T) { +func TestRecognize_CarriesTheParseItRead(t *testing.T) { t.Parallel() const spec = "openapi: 3.1.0\ninfo: {title: T, version: \"1\"}\npaths: {}\n" for name, src := range map[string]string{ @@ -414,7 +395,7 @@ func TestDetect_CarriesTheParseItRead(t *testing.T) { } { t.Run(name, func(t *testing.T) { t.Parallel() - rec, _, ok := New().Detect(compilers.Source{Path: "spec.yaml", Data: []byte(src)}, compilers.Options{}) + rec, _, ok := recognize(src) require.True(t, ok) _, isParse := rec.Parsed.(*load.Parsed) assert.True(t, isParse, "the parse that read the keys is the one the compile lowers") @@ -422,48 +403,13 @@ func TestDetect_CarriesTheParseItRead(t *testing.T) { } } -// TestDetect_ReadsTheVersionTheCompileReads holds detection to the compile on -// the roots where a reading of its own disagreed with the parser's: each is -// routed as the version its compile lowers, the agreement -// compilers.Detection.Compile checks. A key repeated over an unreadable value, -// an alias in either place, a key tagged `!!merge` that is not `<<`, an alias -// key outranking a merge, and a chain deeper than the old walk followed. -func TestDetect_ReadsTheVersionTheCompileReads(t *testing.T) { - t.Parallel() - const tail = "info: {title: t, version: '1'}\npaths: {}\n" - cases := map[string]string{ - "a version over an unreadable value": "openapi: [x]\nopenapi: 3.1.0\n", - "an alias as the key": "k: &k openapi\n*k : 3.1.0\n", - "an alias as the value": "v: &v 3.1.0\nopenapi: *v\n", - "a key tagged !!merge that is no <<": "!!merge openapi: 3.1.0\n", - "an alias key outranking a merge": "b: &b {openapi: 3.0.3}\nk: &k openapi\n<<: *b\n*k : 3.1.0\n", - "a merge chain ten mappings deep": string(rootMergeChain(10, "openapi: 3.1.0", "")), - "a merge written before its own key": "b: &b {openapi: 3.0.3}\n<<: *b\nopenapi: 3.1.0\n", - "the first of two merges that agree": "a: &a {openapi: 3.1.0}\nb: &b {openapi: 3.0.3}\n<<: [*a, *b]\n", - } - for name, root := range cases { - t.Run(name, func(t *testing.T) { - t.Parallel() - src := compilers.Source{Path: "api.yaml", Data: []byte(root + tail)} - rec, diags, ok := New().Detect(src, compilers.Options{}) - require.True(t, ok, "the compile lowers it, so detection claims it: %v", diags) - - doc, compiled, err := New().Compile(t.Context(), []compilers.Source{src}, compilers.Options{}) - require.NoError(t, err) - require.NotNil(t, doc, "%v", compiled) - assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: "3.1"}, rec.Format) - assert.Equal(t, rec.Format.String(), doc.Sources[0].Format, "detection and the compile read one version") - }) - } -} - -// TestDetect_AKeyTaggedMergeThatIsNoScalarMergesNothing pins the other half +// TestRecognize_AKeyTaggedMergeThatIsNoScalarMergesNothing pins the other half // of the merge-key predicate: a mapping key tagged `!!merge` merges nothing, // as the parser reads it, so the version under it is no declaration. -func TestDetect_AKeyTaggedMergeThatIsNoScalarMergesNothing(t *testing.T) { +func TestRecognize_AKeyTaggedMergeThatIsNoScalarMergesNothing(t *testing.T) { t.Parallel() src := "? !!merge [z]\n: {openapi: 3.1.0}\ninfo: {title: t, version: '1'}\npaths: {}\n" - _, diags, ok := New().Detect(compilers.Source{Path: "api.yaml", Data: []byte(src)}, compilers.Options{}) + _, diags, ok := recognize(src) assert.False(t, ok) assert.Empty(t, diags, "a source that declares nothing is declined in silence") } diff --git a/compilers/openapi/internal/detect/recognize.go b/compilers/openapi/internal/detect/recognize.go new file mode 100644 index 00000000..2c371f7b --- /dev/null +++ b/compilers/openapi/internal/detect/recognize.go @@ -0,0 +1,58 @@ +package detect + +import ( + "github.com/dexpace/morphic/compilers" + "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/compilers/openapi/internal/load" + "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" + "github.com/dexpace/morphic/ir" +) + +// Recognize reads the dialect data declares, data being the source at srcIndex: +// the discriminating key it writes and its version's major.minor prefix, with +// the parse that read them. maxBytes bounds that parse, zero being none: data +// past it is declined unread, with the refusal Load gives it. +// +// A decline says why only when the source is this compiler's own: its root +// merges past the depth the read expands, or it will not parse and declares a +// version where a document declares one (declaresProbeKey). +func Recognize(srcIndex int, data []byte, maxBytes int) (compilers.Recognition, []ir.Diagnostic, bool) { + atSource := ir.Provenance{Source: srcIndex} + if d, over := load.OverByteBudget(atSource, data, maxBytes); over { + return compilers.Recognition{}, []ir.Diagnostic{d}, false + } + + probe, parsed, err := sniff(data) + switch { + case probe.Cut: + // Declining in silence would let the bound pass for an answer. + return compilers.Recognition{}, []ir.Diagnostic{diag.Newf( + ir.SeverityError, diag.BudgetExceeded, atSource, + "source cannot be recognized: its root mapping merges through `<<` deeper than %d levels, "+ + "past which a version it declares is not read", nodeview.MergeDepthLimit)}, false + case probe.OpenAPI != "": + return recognized("openapi", probe.OpenAPI, parsed), nil, true + case probe.Swagger != "": + return recognized("swagger", probe.Swagger, parsed), nil, true + case err != nil && declaresProbeKey(data): + return compilers.Recognition{}, []ir.Diagnostic{diag.Newf( + ir.SeverityError, diag.UndecodableSource, atSource, + "source declares an OpenAPI or Swagger version and cannot be read: %s", diag.OneLine(err))}, false + default: + return compilers.Recognition{}, nil, false + } +} + +// recognized builds the recognition for a source that declared version under +// name, carrying the parse that read it. +// +// parsed is never nil here. A probe names a key only when decodeYAML read it +// off a parsed root, so a recognition always has a parse to carry — which +// matters, since a nil *load.Parsed put into the interface would make one that +// is not nil, and a consumer's type assertion would accept it and read through. +func recognized(name, version string, parsed *load.Parsed) compilers.Recognition { + return compilers.Recognition{ + Format: compilers.SourceFormat{Name: name, Version: majorMinor(version)}, + Parsed: parsed, + } +} diff --git a/compilers/openapi/internal/detect/recognize_test.go b/compilers/openapi/internal/detect/recognize_test.go new file mode 100644 index 00000000..488779d6 --- /dev/null +++ b/compilers/openapi/internal/detect/recognize_test.go @@ -0,0 +1,403 @@ +package detect + +import ( + "fmt" + "strings" + "testing" + + soa "github.com/speakeasy-api/openapi/openapi" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/compilers" + "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/compilers/openapi/internal/load" + "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" + "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" + "github.com/dexpace/morphic/ir" +) + +// TestRecognize_HoldsTheParseToItsByteBudget pins the bound on what recognition +// reads: the parse is of the whole source, so a source past maxBytes is +// declined unread, with the refusal Load gives the same source. One at the +// budget is read, and so is any source under no budget at all. +func TestRecognize_HoldsTheParseToItsByteBudget(t *testing.T) { + t.Parallel() + const src = "openapi: 3.1.0\ninfo: {title: T, version: \"1\"}\npaths: {}\n" + refusal, over := load.OverByteBudget(ir.Provenance{Source: 0}, []byte(src), len(src)-1) + require.True(t, over, "the refusal the case expects") + + rec, diags, ok := Recognize(0, []byte(src), len(src)-1) + assert.False(t, ok) + assert.Equal(t, compilers.Recognition{}, rec, "a source declined unread carries no parse") + assert.Equal(t, []ir.Diagnostic{refusal}, diags, "the refusal Load gives the same source") + + for _, budget := range []int{len(src), 0} { + rec, diags, ok := Recognize(0, []byte(src), budget) + require.True(t, ok, "budget %d: %v", budget, diags) + assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: "3.1"}, rec.Format, "budget %d", budget) + } +} + +// TestRecognize_ADeclineNamesTheSourceItWasHanded holds every decline that +// says why to the index its caller passed. Detect passes 0, so a recognition +// that named source 0 whatever it was handed would pass every other test here. +func TestRecognize_ADeclineNamesTheSourceItWasHanded(t *testing.T) { + t.Parallel() + const srcIndex = 3 + cases := map[string]struct { + src []byte + maxBytes int + code string + }{ + "past the byte budget": {[]byte("openapi: 3.1.0\n"), 1, diag.BudgetExceeded}, + "merged past the depth the read expands": { + openapitest.RootMergeChain(nodeview.MergeDepthLimit+1, "openapi: 3.1.0", ""), 0, diag.BudgetExceeded}, + "declaring a version and unreadable": {[]byte("openapi: 3.1.0\ninfo: [unterminated\n"), 0, diag.UndecodableSource}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + _, diags, ok := Recognize(srcIndex, tc.src, tc.maxBytes) + assert.False(t, ok) + require.Len(t, diags, 1) + assert.Equal(t, tc.code, diags[0].Code) + assert.Equal(t, srcIndex, diags[0].Provenance.Source) + }) + } +} + +func TestRecognize_Formats(t *testing.T) { + t.Parallel() + cases := []struct { + name, src string + want compilers.SourceFormat + wantOK bool + // wantCode is the codes of the diagnostics a decline carries. Empty for + // every source this compiler recognizes, and for every source that is + // another format's business. + wantCode []string + }{ + {"openapi 3.1 yaml", "openapi: 3.1.0\ninfo: {}\n", + compilers.SourceFormat{Name: "openapi", Version: "3.1"}, true, nil}, + {"openapi 3.0 json", `{"openapi": "3.0.3"}`, + compilers.SourceFormat{Name: "openapi", Version: "3.0"}, true, nil}, + {"openapi 3.2 yaml", "openapi: 3.2.0\ninfo: {}\n", + compilers.SourceFormat{Name: "openapi", Version: "3.2"}, true, nil}, + // A version already in major.minor form (single dot) exercises + // majorMinor's unchanged-passthrough return. + {"openapi major.minor only", "openapi: \"3.1\"\n", + compilers.SourceFormat{Name: "openapi", Version: "3.1"}, true, nil}, + // A bare-major version (no dot) also reaches the passthrough return. + {"openapi bare major", "openapi: \"4\"\n", + compilers.SourceFormat{Name: "openapi", Version: "4"}, true, nil}, + // Recognized, and deliberately not a format Formats reports: naming it + // lets the caller say "unsupported" rather than "unreadable". + {"swagger", "swagger: \"2.0\"\n", + compilers.SourceFormat{Name: "swagger", Version: "2.0"}, true, nil}, + + // Everything below is another format's input, and none of it is an error + // here: three of the five planned compilers take bytes that are not YAML. + {"protobuf", "syntax = \"proto3\";\nservice S { rpc Get (Q) returns (A); }\n", + compilers.SourceFormat{}, false, nil}, + {"typespec", "import \"@typespec/http\";\nmodel Pet { name: string; }\n", + compilers.SourceFormat{}, false, nil}, + {"graphql", "type Query {\n pet(id: ID!): Pet\n}\n", + compilers.SourceFormat{}, false, nil}, + // Valid YAML that declares neither key: whether a source parses is not + // the question detection answers. + {"graphql that parses as yaml", "type Query { a: String }\n", + compilers.SourceFormat{}, false, nil}, + {"yaml that is no spec", "hello: world\n", + compilers.SourceFormat{}, false, nil}, + // Declares a version and will not parse: this compiler's own source, + // broken, which nothing else is in a position to say. + {"unparseable yaml", "openapi: 3.1.0\ninfo: [unterminated\n", + compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, + // Broken on its own version line, so it declares no version to read, and + // is declined like any file that does not say what it is. That is the + // price of not claiming prose (GitHub #497). + {"unparseable yaml, broken on the version line", "openapi: [unterminated\n", + compilers.SourceFormat{}, false, nil}, + // The two files #497 is about: the word written where a spec declares its + // version, with no version beside it. A README with prose after the key + // and a framework's configuration section under it are other formats' + // files, and neither is reported as a spec that failed to read. + {"a readme that does not parse", "# Notes\n\nfiller text\nopenapi: is a format\n", + compilers.SourceFormat{}, false, nil}, + {"a config section that does not parse", "openapi:\n enabled: true\nserver: [\n", + compilers.SourceFormat{}, false, nil}, + {"unparseable json", `{"openapi": "3.1.0", "info": {`, + compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, + // A key that is not UTF-8 refuses the whole document, which is what lets + // ids.Ptr assume every token it escapes is UTF-8. + {"undecodable yaml key", "openapi: 3.1.0\ninfo: {}\n\xff: x\n", + compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, + {"undecodable json key", "{\"openapi\": \"3.1.0\", \"\xff\": 1}", + compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, + // Broken, and never this compiler's: the key it names is a value, not a + // key, so the parse error describes a parser that was wrong to be asked. + {"unparseable, key only mentioned", "syntax = \"openapi\";\n{[", + compilers.SourceFormat{}, false, nil}, + // A large document is parsed as a small one is, so one that declares the + // key and will not parse is reported at any size. Detection used to scan + // past 64 KiB instead, and a scan cannot tell a broken spec from another + // format's file naming the word, so the same broken spec drew a complaint + // or silence depending on its length (GitHub #486). + {"unparseable, large", + padTo("openapi: 3.1.0\ninfo: [unterminated\n", "filler: x\n"), + compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, + // The key written behind a prefix that does not parse. The scan read past + // the prefix to the version and named the format, leaving the compile to + // find the document broken; the parse finds it broken here, and the + // version declared at column 0 makes it this compiler's to say so. + {"key behind an unparseable prefix, large", + padTo("bad: [unterminated\n", "filler: x\n") + "openapi: 3.1.0\n", + compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, + // yaml.v3 refuses a document that follows an explicit end marker without + // a `---` of its own, whatever the document's size. + {"key after an end-marked empty document", + "---\n...\nopenapi: 3.1.0\n", + compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, + {"key after an end-marked empty document, large", + "---\n...\n" + padTo("openapi: 3.1.0\n", "filler: x\n"), + compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, + {"key behind an unterminated quoted scalar, large", + padTo("bad: \"unterminated\n", "filler: x\n") + "openapi: 3.1.0\n", + compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, + // The same in flow style: the key is one of the root mapping's own + // entries, which is what makes a broken document this compiler's. + {"key behind an unparseable flow prefix, large", + `{"pad":"` + flowPad() + `","bad" 1,"openapi":"3.1.0"}`, + compilers.SourceFormat{}, false, []string{diag.UndecodableSource}}, + // Another format's document, large, naming the word as a key and broken + // besides. It opens no mapping of its own, so the key is not its + // declaration of itself and this compiler has nothing to say: reporting a + // parse error here would claim bytes that were never its own. + {"a broken document of another format names the key", + `[{"openapi":"3.1.0"},"` + flowPad() + `"`, + compilers.SourceFormat{}, false, nil}, + // The same, one level down inside a mapping that does open the document. + // A nested key names a field, not the format of the file holding it. + {"a broken document of another format nests the key", + `{"pad":"` + flowPad() + `","deep":{"openapi":"3.1.0"},"bad" 1}`, + compilers.SourceFormat{}, false, nil}, + {"empty", "", compilers.SourceFormat{}, false, nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, diags, ok := recognize(tc.src) + assert.Equal(t, tc.wantOK, ok) + assert.Equal(t, tc.want, got.Format) + assert.Equal(t, tc.wantCode, codesOf(diags), + "a decline says something only when the source is recognizably this compiler's") + }) + } +} + +// TestRecognize_KeyOrderDoesNotDecideTheFormat pins a published spec whose +// `components` object runs long and whose `openapi` key sits behind it. A JSON +// object's keys are unordered, so the same document written with its version +// key first and with it last is one document, and detection has to answer the +// same for both. A prefix-only sniff once declined the version-last spellings. +func TestRecognize_KeyOrderDoesNotDecideTheFormat(t *testing.T) { + t.Parallel() + flow, block := bigComponents() + cases := []struct{ name, src string }{ + {"flow json, version first", `{"openapi":"3.0.3",` + flow + `}`}, + {"flow json, version last", `{` + flow + `,"openapi":"3.0.3"}`}, + {"block yaml, version first", "openapi: 3.0.3\n" + block}, + {"block yaml, version last", block + "openapi: 3.0.3\n"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + require.Greater(t, len(tc.src), scanCutover, "the case must be large to test it") + got, diags, ok := recognize(tc.src) + assert.True(t, ok, "a valid document must not be declined over where it declares its version") + assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: "3.0"}, got.Format) + assert.Nil(t, codesOf(diags), "a document this compiler recognizes carries no complaint") + }) + } +} + +// bigComponents returns a `components` entry whose value alone runs past +// scanCutover, in flow and in block style. It stands in for the schema catalogue a +// published spec leads with; what matters is only that it is one entry too large +// to read past. +func bigComponents() (flow, block string) { + var f, b strings.Builder + f.WriteString(`"components":{"schemas":{`) + b.WriteString("components:\n schemas:\n") + for i := 0; f.Len() <= scanCutover || b.Len() <= scanCutover; i++ { + if i > 0 { + f.WriteByte(',') + } + fmt.Fprintf(&f, `"S%d":{"type":"object","description":"a schema"}`, i) + fmt.Fprintf(&b, " S%d:\n type: object\n description: a schema\n", i) + } + f.WriteString(`}}`) + return f.String(), b.String() +} + +// TestRecognize_RepeatedKeysDoNotDecideTheFormat pins the fix for the blow-up. A +// document that repeats a top-level key has a duplicate key, which the parser +// this compiler goes on to use reports once per repeat, sited. It is neither +// another format's nor unreadable, yet decoding the root mapping to read two +// keys said both, because yaml.v3 abandons a mapping that repeats any key. +// +// Both orders are pinned: a fixture that declares the version first cannot see +// a regression that loses it to the repeats that follow. +func TestRecognize_RepeatedKeysDoNotDecideTheFormat(t *testing.T) { + t.Parallel() + repeats := strings.Repeat("x: y\n", dupRepeats) + cases := []struct{ name, src string }{ + {"version first", "openapi: 3.0.3\ninfo: {title: t, version: v}\n" + repeats}, + {"version last", "info: {title: t, version: v}\n" + repeats + "openapi: 3.0.3\n"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, diags, ok := recognize(tc.src) + assert.True(t, ok, "a document this compiler can lower must not be declined over a repeated key") + assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: "3.0"}, got.Format) + assert.Nil(t, codesOf(diags), "the repeats are the parser's to report, sited, not detection's") + }) + } +} + +// TestRecognize_ARepeatedVersionKeyAgreesWithTheParser holds detection to the +// answer the lowering will reach. load reads the version off the parsed document +// and records it on ir.SourceInfo, so each case asks that parser which spelling +// of a repeated key it keeps; detection naming another would give one document +// two dialects, one routing it and one describing it. +// +// The two orders are the test: a single order passes whichever spelling is +// taken. +func TestRecognize_ARepeatedVersionKeyAgreesWithTheParser(t *testing.T) { + t.Parallel() + cases := []struct{ first, second, want string }{ + {"3.1.0", "3.0.3", "3.0"}, + {"3.0.3", "3.1.0", "3.1"}, + } + for _, tc := range cases { + t.Run(tc.first+" then "+tc.second, func(t *testing.T) { + t.Parallel() + src := "openapi: " + tc.first + "\nopenapi: " + tc.second + "\ninfo: {}\n" + parsed, _, err := soa.Unmarshal(t.Context(), strings.NewReader(src)) + require.NoError(t, err) + require.NotNil(t, parsed) + assert.Equal(t, tc.want, majorMinor(parsed.OpenAPI), "the spelling the parser keeps") + + got, _, ok := recognize(src) + require.True(t, ok) + assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: majorMinor(parsed.OpenAPI)}, got.Format, + "detection names the version the parser records") + }) + } +} + +// TestRecognize_ReadsAVersionKeyThroughAMergeKey holds the merge cases the decoder +// this replaced handled for free. A root that merges another mapping declares +// what that mapping declares, and dropping it would put a new instance of "a +// field supplied through a merge key reaches the IR in no form" into the one +// place that decides whether the document is read at all. +func TestRecognize_ReadsAVersionKeyThroughAMergeKey(t *testing.T) { + t.Parallel() + cases := []struct { + name, src, want string + }{ + {"alias", "base: &b\n openapi: 3.1.0\n<<: *b\ninfo: {}\n", "3.1"}, + {"mapping written out", "<<: {openapi: 3.1.0}\ninfo: {}\n", "3.1"}, + {"sequence of aliases", "one: &o\n unrelated: x\ntwo: &t\n openapi: 3.1.0\n<<: [*o, *t]\n", "3.1"}, + {"earlier merge wins", "one: &o\n openapi: 3.0.3\ntwo: &t\n openapi: 3.1.0\n<<: [*o, *t]\n", "3.0"}, + {"a written key beats a merged one", "base: &b\n openapi: 3.0.3\n<<: *b\nopenapi: 3.1.0\n", "3.1"}, + {"a quoted << merges nothing", "base: &b\n openapi: 3.1.0\n\"<<\": *b\ninfo: {}\n", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, _, ok := recognize(tc.src) + if tc.want == "" { + assert.False(t, ok, "a key spelled << as a plain string merges nothing") + return + } + require.True(t, ok) + assert.Equal(t, compilers.SourceFormat{Name: "openapi", Version: tc.want}, got.Format) + }) + } +} + +// TestRecognize_AReadCutShortIsReported pins that the bound never passes for a +// decline: the source may be this compiler's own, so detection says why it +// could not tell. +func TestRecognize_AReadCutShortIsReported(t *testing.T) { + t.Parallel() + src := openapitest.RootMergeChain(nodeview.MergeDepthLimit+1, "openapi: 3.1.0", "") + _, diags, ok := recognize(string(src)) + + assert.False(t, ok) + require.Len(t, diags, 1) + assert.Equal(t, diag.BudgetExceeded, diags[0].Code) + assert.Equal(t, ir.SeverityError, diags[0].Severity, "it is the whole reason the source went unclaimed") + assert.Contains(t, diags[0].Message, fmt.Sprint(nodeview.MergeDepthLimit)) +} + +// TestRecognize_AMergeCycleHidesNoKey pins the other way a root's merges can stop +// short of a complete expansion: a cycle, which the compile's alias pre-scan +// refuses and detection meets first. It reads every mapping on it, so it cuts +// no read short, and a root that declares nothing is declined in silence as +// any other file that is not a spec is, while one that declares a version is +// claimed for the compile to refuse. +func TestRecognize_AMergeCycleHidesNoKey(t *testing.T) { + t.Parallel() + cases := map[string]struct { + src string + version string + }{ + "a root merging itself": {"&r\n<<: *r\nfoo: 1\n", ""}, + "a root merging itself beside its version": {"&r\n<<: *r\nopenapi: 3.1.0\n", "3.1"}, + "a root merging a mapping that merges it back": {"&r\nx: &x {<<: *r, openapi: 3.0.3}\n<<: *x\n", "3.0"}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + probe, _, err := sniff([]byte(tc.src)) + require.NoError(t, err) + assert.False(t, probe.Cut, "a cycle reads every mapping on it") + + rec, diags, ok := recognize(tc.src) + assert.Equal(t, tc.version != "", ok) + assert.Equal(t, tc.version, rec.Format.Version) + assert.Empty(t, diags, "a cycle is no budget, and a file declaring nothing earns no complaint") + }) + } +} + +// TestRecognize_AVersionKeyOfTheWrongShapeIsNoDeclaration pins that a mapping +// where a version goes declares no version, wherever it sits. Such a document +// parses, so it is the tree and not the guard that has the key — but a mapping +// under `openapi` is what another tool's configuration writes, and a spec +// written that way has no version to route it by either. Detection used to +// report the column-0 spelling as this compiler's own and unreadable; it +// declines all three alike now (GitHub #497). +func TestRecognize_AVersionKeyOfTheWrongShapeIsNoDeclaration(t *testing.T) { + t.Parallel() + cases := []struct { + name, src string + wantCode []string + }{ + {"declared at column 0", "openapi: {a: b}\ninfo: {}\n", nil}, + {"a framework's configuration section", "openapi:\n path: /docs\n enabled: true\n", nil}, + {"reached through a merge", "base: &b\n openapi: {a: b}\n<<: *b\n", nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, diags, ok := recognize(tc.src) + assert.False(t, ok) + assert.Equal(t, tc.wantCode, codesOf(diags)) + }) + } +} diff --git a/compilers/openapi/internal/detect/version.go b/compilers/openapi/internal/detect/version.go new file mode 100644 index 00000000..3e9c90ba --- /dev/null +++ b/compilers/openapi/internal/detect/version.go @@ -0,0 +1,34 @@ +package detect + +import "strings" + +// isVersion reports whether value reads as a version rather than prose: one +// word, beginning with a digit. That admits every shape majorMinor is written +// for, and suffixed ones such as "3.1.0-rc1" on purpose: a document writing one +// is this compiler's own and wrong, and the precise complaint is load's to +// make, where declining here would give the engine's generic "unrecognized +// format". Prose has a space in it, and that is what the guard keeps out. +func isVersion(value string) bool { + if value == "" || value[0] < '0' || value[0] > '9' { + return false + } + return !strings.ContainsAny(value, " \t") +} + +// majorMinor returns the "major.minor" prefix of a dotted version string, +// e.g. "3.1.0" → "3.1". Strings with fewer than two dots — a bare major +// version, or a version already in major.minor form — are returned unchanged. +func majorMinor(version string) string { + firstDot := -1 + for i := range len(version) { + if version[i] != '.' { + continue + } + if firstDot < 0 { + firstDot = i + continue + } + return version[:i] + } + return version +} diff --git a/compilers/openapi/internal/detect/version_test.go b/compilers/openapi/internal/detect/version_test.go new file mode 100644 index 00000000..8873597b --- /dev/null +++ b/compilers/openapi/internal/detect/version_test.go @@ -0,0 +1,14 @@ +package detect + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMajorMinor(t *testing.T) { + t.Parallel() + assert.Equal(t, "3.1", majorMinor("3.1.0")) + assert.Equal(t, "3.1", majorMinor("3.1")) + assert.Equal(t, "4", majorMinor("4")) +} diff --git a/compilers/openapi/internal/openapitest/spec.go b/compilers/openapi/internal/openapitest/spec.go index b983d744..dbb29933 100644 --- a/compilers/openapi/internal/openapitest/spec.go +++ b/compilers/openapi/internal/openapitest/spec.go @@ -1,6 +1,9 @@ package openapitest import ( + "fmt" + "strings" + oas3 "github.com/speakeasy-api/openapi/jsonschema/oas3" soa "github.com/speakeasy-api/openapi/openapi" "github.com/speakeasy-api/openapi/sequencedmap" @@ -129,3 +132,15 @@ func ScalarNode(tag, val string) *yaml.Node { const InlineProbeBody = `{type: string, title: SUM, description: DOC, ` + `externalDocs: {url: 'https://e.example', description: ED}, deprecated: true, ` + `example: abc, x-vendor: V, xml: {name: X}, not: {const: N}, maxLength: 3}` + +// RootMergeChain writes a document whose root merges a chain of levels +// anchored mappings, the innermost holding leaf, and writes own after the merge. +func RootMergeChain(levels int, leaf, own string) []byte { + var b strings.Builder + fmt.Fprintf(&b, "l0: &a0 {%s}\n", leaf) + for i := 1; i < levels; i++ { + fmt.Fprintf(&b, "l%d: &a%d {<<: *a%d}\n", i, i, i-1) + } + fmt.Fprintf(&b, "<<: *a%d\n%s", levels-1, own) + return []byte(b.String()) +} diff --git a/compilers/openapi/internal/openapitest/spec_test.go b/compilers/openapi/internal/openapitest/spec_test.go index eebf40ed..94b3d89c 100644 --- a/compilers/openapi/internal/openapitest/spec_test.go +++ b/compilers/openapi/internal/openapitest/spec_test.go @@ -127,3 +127,18 @@ func TestInlineProbeBody_WritesEveryKeywordTheProbeAssertsOn(t *testing.T) { assert.Contains(t, openapitest.InlineProbeBody, want) } } + +// TestRootMergeChain_MergesEachLevelIntoTheNext pins the shape the merge-depth +// cases count on: levels anchored mappings, each merging the one before it, the +// innermost holding leaf, and the root merging the outermost before writing +// own. A chain one level off would put each case on the wrong side of the bound +// it tests. +func TestRootMergeChain_MergesEachLevelIntoTheNext(t *testing.T) { + t.Parallel() + assert.Equal(t, "l0: &a0 {openapi: 3.1.0}\nl1: &a1 {<<: *a0}\nl2: &a2 {<<: *a1}\n<<: *a2\nown: x\n", + string(openapitest.RootMergeChain(3, "openapi: 3.1.0", "own: x\n"))) + + var doc map[string]any + require.NoError(t, yaml.Unmarshal(openapitest.RootMergeChain(3, "openapi: 3.1.0", ""), &doc)) + assert.Equal(t, "3.1.0", doc["openapi"], "the leaf reaches the root through every level") +} diff --git a/compilers/openapi/openapi_test.go b/compilers/openapi/openapi_test.go index 99e12475..8ff78a92 100644 --- a/compilers/openapi/openapi_test.go +++ b/compilers/openapi/openapi_test.go @@ -5,6 +5,7 @@ import ( "encoding/json/v2" "os" "slices" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -170,3 +171,21 @@ func TestCompile_RejectsMultipleSources(t *testing.T) { }, compilers.Options{}) require.Error(t, err) } + +// TestCompile_AnUnreadableSourceIsADiagnostic pins where the complaint about a +// broken document comes from when nothing detected it: a caller compiling +// directly hands Compile bytes no parse has read. It has to stay a diagnostic: +// engine.Run turns a compiler's Go error into its own, and the CLI maps that to +// exit 2 — the code it uses for being invoked wrong — so a spec it read would +// be reported as a misuse of itself. +func TestCompile_AnUnreadableSourceIsADiagnostic(t *testing.T) { + t.Parallel() + src := "bad: [unterminated\n" + strings.Repeat("filler: x\n", 8000) + "openapi: 3.1.0\n" + doc, diags, err := openapi.New().Compile(context.Background(), + []compilers.Source{{Path: "api.yaml", Data: []byte(src)}}, compilers.Options{}) + + require.NoError(t, err, "a document that will not parse is a finding, not a failure of the compiler") + assert.Nil(t, doc) + require.Len(t, diags, 1) + assert.Equal(t, diag.UndecodableSource, diags[0].Code) +} diff --git a/docs/compilers/openapi.md b/docs/compilers/openapi.md index b5a263c2..64deac46 100644 --- a/docs/compilers/openapi.md +++ b/docs/compilers/openapi.md @@ -48,7 +48,7 @@ Document. - Tests: `openapi.TestCompile_UnsupportedVersion`, `load.TestSupportedMinor_AcceptsOnlyTheThreeMinors`, `oas.TestCheck_TheOpenAPIFieldIsAVersionNumber`, - `openapi.TestDetect_AVersionThatIsNotServedIsStillADeclaration` + `detect.TestRecognize_AVersionThatIsNotServedIsStillADeclaration` ### `yaml-decoding` @@ -70,9 +70,9 @@ to is [`values`](#values)'. - Codes: `openapi/undecodable-source`, `openapi/stream-documents-dropped`, `openapi/tagged-mapping`, `openapi/budget-exceeded` -- Tests: `openapi.TestDetect_ABrokenDocumentIsReportedAtAnySize`, - `openapi.TestDetect_ReadsTheVersionTheCompileReads`, `openapi.TestDetect_AReadCutShortIsReported`, - `openapi.TestDetect_AMergeCycleHidesNoKey`, +- Tests: `detect.TestRecognize_ABrokenDocumentIsReportedAtAnySize`, + `openapi.TestDetect_ReadsTheVersionTheCompileReads`, + `detect.TestRecognize_AReadCutShortIsReported`, `detect.TestRecognize_AMergeCycleHidesNoKey`, `load.TestLoad_AStreamLowersItsFirstDocumentAndReportsTheRest`, `load.TestLoad_ATaggedMappingIsRefusedBeforeParsing` - Status: partly built: a JSON source holding a surrogate-pair escape is refused as undecodable diff --git a/internal/archtest/arch_test.go b/internal/archtest/arch_test.go index 02ef4bc2..e6d73ea9 100644 --- a/internal/archtest/arch_test.go +++ b/internal/archtest/arch_test.go @@ -43,7 +43,7 @@ var rules = map[string][]string{ // and ID grammars. It reaches ir alone, so no compiler's vocabulary rides in. "compilers/compile": {module + "/ir"}, "compilers/openapi": {module + "/ir", module + "/compilers", module + "/compilers/compile", - module + "/compilers/openapi/internal" + subtreeSuffix, "gopkg.in/yaml.v3"}, + module + "/compilers/openapi/internal" + subtreeSuffix}, // The diagnostic vocabulary sits at the bottom of the compiler's own import // graph: every package that reports anything reaches it, so it must reach // nothing but ir. Its own entry says that, rather than letting it inherit the @@ -170,6 +170,14 @@ var rules = map[string][]string{ "github.com/speakeasy-api/openapi/openapi", "github.com/speakeasy-api/openapi/references", "github.com/speakeasy-api/openapi/yml", "gopkg.in/yaml.v3"}, + // Recognition of a source from its bytes, before the compile. It reaches load + // for the parse the compile goes on to lower and nodeview to read the root + // mapping as the parser does, and nothing that lowers: it runs before + // anything has decided the bytes are this compiler's. + "compilers/openapi/internal/detect": {module + "/ir", module + "/compilers", + module + "/compilers/openapi/internal/diag", + module + "/compilers/openapi/internal/load", + module + "/compilers/openapi/internal/nodeview", "gopkg.in/yaml.v3"}, // What a $ref names: the pointer it addresses, the schema declared there and // the type already interned there. It reaches bind for where a schema $ref // lands, annotation to ask whether a referenced position declares a body at From 857c1d77f56b2a46fcca94e90fadca9274dd920f Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 05:37:14 +0300 Subject: [PATCH 03/21] docs(compilers/openapi): describe the moved code where it now lives The moves carried comments that described their old home. Detection's comments named Detect where Recognize now does the asking; two test comments pointed "below" at tests in another file; one called a test of Recognize a test of the public face; Meta's comment spoke of "every function below" in a file that now holds one. Each names what it means now. Lower's comment also states its precondition, that it runs after every other lowering, which only retainUnreferencedComponents' comment said. The root package comment no longer lists recognition and document metadata among what the package holds, and docs/architecture.md places version sniffing in internal/detect and adds both packages to the layout. --- compilers/openapi/doc.go | 15 ++++++------ compilers/openapi/internal/detect/declares.go | 6 ++--- .../openapi/internal/detect/helpers_test.go | 6 ++--- compilers/openapi/internal/detect/probe.go | 4 ++-- .../openapi/internal/detect/probe_test.go | 23 ++++++++++--------- .../openapi/internal/detect/reading_test.go | 18 +++++++-------- .../openapi/internal/document/document.go | 11 +++++---- docs/architecture.md | 10 ++++---- 8 files changed, 50 insertions(+), 43 deletions(-) diff --git a/compilers/openapi/doc.go b/compilers/openapi/doc.go index a6bfdbac..86b24af3 100644 --- a/compilers/openapi/doc.go +++ b/compilers/openapi/doc.go @@ -2,12 +2,13 @@ // implements compilers.Compiler. // // This package is the compiler's public face and the assembly behind it: the -// Compiler, its Options and the text vocabulary DecodeOptions reads, recognition of an OpenAPI -// document from its bytes, the document metadata, and the run that calls the -// lowerings in order and builds a Document from what they return. +// Compiler, its Options and the text vocabulary DecodeOptions reads, and the +// run that calls the lowerings in order and builds a Document from what they +// return. // -// Parsing is delegated to github.com/speakeasy-api/openapi. The lowering -// itself (identity, hoisting, normalization, and lossless preservation -// of constructs the IR does not model structurally) lives in the packages under -// internal/, each of which states its own place in the order. +// Parsing is delegated to github.com/speakeasy-api/openapi. Recognizing a +// source from its bytes, loading it and the lowering itself (identity, +// hoisting, normalization, and lossless preservation of constructs the IR does +// not model structurally) live in the packages under internal/, each of which +// states its own place in the order. package openapi diff --git a/compilers/openapi/internal/detect/declares.go b/compilers/openapi/internal/detect/declares.go index 60c34faa..4f5e1cdd 100644 --- a/compilers/openapi/internal/detect/declares.go +++ b/compilers/openapi/internal/detect/declares.go @@ -7,9 +7,9 @@ import ( ) // declaresProbeKey reports whether data declares a version under a -// discriminating key. Detect asks only once the probe cannot read the source, -// where "not YAML" alone fits a Protobuf or Smithy source too, and only within -// its byte budget. +// discriminating key. Recognize asks only once the probe cannot read the +// source, where "not YAML" alone fits a Protobuf or Smithy source too, and only +// within its byte budget. // // Neither reading may widen to "the name occurs before a colon": other formats // nest such a key and must not get this compiler's parse error. The version diff --git a/compilers/openapi/internal/detect/helpers_test.go b/compilers/openapi/internal/detect/helpers_test.go index 739441ba..0ace385c 100644 --- a/compilers/openapi/internal/detect/helpers_test.go +++ b/compilers/openapi/internal/detect/helpers_test.go @@ -47,9 +47,9 @@ func padTo(src, filler string) string { return b.String() } -// dupRepeats is how many times the fixtures below repeat a root key. It is not a -// threshold: the wrong answer reproduces at two repeats, and this many only -// makes the fixture recognizably a document rather than a corner. It is +// dupRepeats is how many times the repeated-key fixtures repeat a root key. It +// is not a threshold: the wrong answer reproduces at two repeats, and this many +// only makes the fixture recognizably a document rather than a corner. It is // deliberately far below the 6,553 of the report — yaml.v3 raises one error per // pair of matching keys, so that count produced 21,467,628 of them and a 1.2 GB // message, and a fixture that large turns a revert into an out-of-memory kill diff --git a/compilers/openapi/internal/detect/probe.go b/compilers/openapi/internal/detect/probe.go index f3763734..ebe65826 100644 --- a/compilers/openapi/internal/detect/probe.go +++ b/compilers/openapi/internal/detect/probe.go @@ -27,12 +27,12 @@ type sniffProbe struct { // sniff reads the discriminating keys out of data. For anything it cannot read // it returns the zero probe and the parser's error; whether that error is -// worth reporting is Detect's question. +// worth reporting is Recognize's question. // // The document is decoded whole and exactly, the only way to tell one that // declares nothing from one that will not parse. The parse is the one the // compile lowers, not a second reading beside it that would have to be kept in -// agreement with it (GitHub #486). The caller's byte budget, which Detect +// agreement with it (GitHub #486). The caller's byte budget, which Recognize // checks first, bounds it. func sniff(data []byte) (sniffProbe, *load.Parsed, error) { probe, parsed, err := decodeYAML(data) diff --git a/compilers/openapi/internal/detect/probe_test.go b/compilers/openapi/internal/detect/probe_test.go index f39808fe..b0e69d02 100644 --- a/compilers/openapi/internal/detect/probe_test.go +++ b/compilers/openapi/internal/detect/probe_test.go @@ -62,11 +62,11 @@ func TestSniff_ReadsALargeDocumentWhole(t *testing.T) { } } -// TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt pins where the bound on -// the root's merges lies, nodeview's, and what a read it cuts short answers. A -// key past the bound may outrank what the read found, so the answer is -// unknown, and Detect says the bound stopped it, unless the root's own pairs, -// which outrank every merge, settle it. +// TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt pins where the bound on the +// root's merges lies, nodeview's, and what a read it cuts short answers. A key +// past the bound may outrank what the read found, so the answer is unknown, and +// Recognize says the bound stopped it, unless the root's own pairs, which +// outrank every merge, settle it. func TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt(t *testing.T) { t.Parallel() const limit = nodeview.MergeDepthLimit @@ -101,8 +101,8 @@ func TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt(t *testing.T) { } // TestDecodeYAML_RefusesARootThatIsNoMapping pins the shape complaint. A source -// with no root mapping has no top-level keys, and saying so is what lets Detect -// report bytes that declare a key it serves and will not read. +// with no root mapping has no top-level keys, and saying so is what lets +// Recognize report bytes that declare a key it serves and will not read. func TestDecodeYAML_RefusesARootThatIsNoMapping(t *testing.T) { t.Parallel() cases := []struct{ name, src, wantErr string }{ @@ -153,10 +153,11 @@ func TestSniff_CostIsNotQuadraticInRepeatedKeys(t *testing.T) { // reached through a `<<`. Such bytes name a key this compiler serves and do not // say what dialect, so the probe cannot read them. // -// Whether Detect reports that or declines in silence is declaresProbeKey's -// answer and not this one's, and it is asserted separately below: a mapping -// under the key is another tool's configuration as often as a broken spec, and -// declares no version either way, so Detect declines all of them. +// Whether Recognize reports that or declines in silence is declaresProbeKey's +// answer and not this one's, and +// TestRecognize_AVersionKeyOfTheWrongShapeIsNoDeclaration asserts it: a +// mapping under the key is another tool's configuration as often as a broken +// spec, and declares no version either way, so Recognize declines all of them. func TestDecodeYAML_RefusesAVersionKeyThatIsNoScalar(t *testing.T) { t.Parallel() cases := []struct{ name, src, wantErr string }{ diff --git a/compilers/openapi/internal/detect/reading_test.go b/compilers/openapi/internal/detect/reading_test.go index c7cee84e..16e07248 100644 --- a/compilers/openapi/internal/detect/reading_test.go +++ b/compilers/openapi/internal/detect/reading_test.go @@ -12,7 +12,7 @@ import ( ) // reading is how detection reads a document: it parses, or it does not and -// Detect says so, or it does not and Detect is silent. +// Recognize says so, or it does not and Recognize is silent. type reading int const ( @@ -20,11 +20,11 @@ const ( parses reading = iota // unreadable is a document the probe cannot read — it does not parse, or // its version key is no scalar — and that declares no version where a - // document declares one, so Detect declines it in silence: its bytes may + // document declares one, so Recognize declines it in silence: its bytes may // well be another format's. unreadable // reported is a document the probe cannot read that declares a version - // where a document declares one, so Detect reports it as this compiler's + // where a document declares one, so Recognize reports it as this compiler's // own and broken. reported ) @@ -44,8 +44,8 @@ type declarationRow struct { // wrong, and a correct parser is no reason to stop asking. // // Each row says how it reads, not only what it declares. A row declaring nothing -// would otherwise pass on a document that never parsed, and Detect's answer to -// one, a complaint or silence, is part of what the row pins. +// would otherwise pass on a document that never parsed, and Recognize's answer +// to one, a complaint or silence, is part of what the row pins. func declarations() []declarationRow { const v = "3.1.0" return []declarationRow{ @@ -326,10 +326,10 @@ func TestRecognize_ABrokenDocumentIsReportedAtAnySize(t *testing.T) { } } -// TestRecognize_AnEmptyFirstDocumentIsNotTheDocument pins GitHub #481 at the -// public face: a stream that opens with a document holding nothing is routed on -// the document behind it, where it used to be declined as undecodable — with a -// whole spec two lines further down. +// TestRecognize_AnEmptyFirstDocumentIsNotTheDocument pins GitHub #481 where a +// source is recognized: a stream that opens with a document holding nothing is +// routed on the document behind it, where it used to be declined as undecodable +// — with a whole spec two lines further down. func TestRecognize_AnEmptyFirstDocumentIsNotTheDocument(t *testing.T) { t.Parallel() want := compilers.SourceFormat{Name: "openapi", Version: "3.1"} diff --git a/compilers/openapi/internal/document/document.go b/compilers/openapi/internal/document/document.go index 707ca9f6..fee2d9f6 100644 --- a/compilers/openapi/internal/document/document.go +++ b/compilers/openapi/internal/document/document.go @@ -11,8 +11,8 @@ import ( // // It exists so the lowering can return what it read instead of writing into the // document as it goes. The caller owns the document and does the assigning, -// which is what lets every function below be checked against a literal context -// and nothing else. +// which is what lets every lowering in this package be checked against a +// literal context and nothing else. type Meta struct { Name string Version string @@ -25,8 +25,11 @@ type Meta struct { } // Lower lowers the document-level metadata that is not part of the type or -// service graph: info, servers, and the extensions of every object around them -// that lowers to no node of its own (ir-design §10, §12). +// service graph: info, servers, and what the document writes that lowers to no +// node of its own, kept verbatim (ir-design §10, §12). +// +// It must run after every other lowering: which components it keeps depends on +// what they reached (lowering.Ctx.Reached). func Lower(c lowering.Ctx) (Meta, []ir.Diagnostic) { m := lowerInfo(c) diff --git a/docs/architecture.md b/docs/architecture.md index 548bffe8..acc447a3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -151,7 +151,7 @@ it, the schema stamp, the namespaces, a service and a well-formed source table a reports, a `compile/registry-refused` error for each entry the registry refused among them. **What stays in the OpenAPI compiler.** Three of its pieces read as if any format could use them: -the YAML view (`internal/nodeview`), version sniffing (`detect.go`) and document identity +the YAML view (`internal/nodeview`), version sniffing (`internal/detect`) and document identity (`resolve.SameDocument`, `resolve.IsURL`). None moves into `compilers/compile`, because what each owes is agreement with the OpenAPI parser library, not with YAML or RFC 3986. The view reads a mapping as the library's unmarshaller does, its `<<` merges and repeated keys included, which @@ -383,8 +383,8 @@ morphic/ │ ├── compile/ # What every compiler shares: type registry + coordinates, │ │ # diagnostics, and the naming/identifier grammars. │ ├── openapi/ # OpenAPI 3.x → IR (milestone 1). The public face: the -│ │ │ # Compiler, its Options, document metadata, and the run that -│ │ │ # calls the four lowerings below in order. +│ │ │ # Compiler, its Options, and the run that calls the +│ │ │ # lowerings below in order. │ │ └── internal/ # Its own packages, each below that face: │ │ ├── diag/ # diagnostic codes and the constructor │ │ ├── ids/ # pointer construction; pointer → TypeID @@ -395,12 +395,14 @@ morphic/ │ │ ├── annotation/ # what a schema says about itself, not its shape │ │ ├── oas/ # the OpenAPI Specification's rules by minor, and their check │ │ ├── load/ # parse, validate, resolve +│ │ ├── detect/ # a source recognized from its bytes, on load's parse │ │ ├── resolve/ # what a $ref names; reference-or-inline entries │ │ ├── merge/ # allOf property reconciliation │ │ ├── lowering/ # the immutable context every lowering reads │ │ ├── schema/ # the schema walk, composition, preservation │ │ ├── auth/ # security schemes and requirements -│ │ └── operation/ # path items, webhooks, callbacks, params, content +│ │ ├── operation/ # path items, webhooks, callbacks, params, content +│ │ └── document/ # info, servers, and what the document keeps verbatim │ ├── typespec/ smithy/ graphql/ asyncapi/ protobuf/ otp/ (future) ├── pass/ # Layer 1 — IR → IR passes (validate, dedup, filter, slice, overlay). ├── emitters/ # Layer 2 — emitter contract, plan layer, registry (future). From 688af0762fee42342b8b588c1afe97d0e53c8b0d Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 05:37:14 +0300 Subject: [PATCH 04/21] refactor(compilers/openapi): read the document's sites from one list The x-* and the undeclared keys of the objects around the document metadata were each collected over a list of their own: rootExtensions and tagExtensions for one, rootUnknownSites and tagUnknownSites for the other. The two lists named the same objects at the same scopes and pointers, gave the same reasons twice, and needed a pair of twin tests to keep their nil guards alike. One list, sites, now serves both readers. Each site holds the parsed object, and the extensions are read off it, so a site cannot name one object for its keys and another for its x-*. The order is unchanged, and so is the order of the diagnostics each reader reports. --- compilers/openapi/internal/document/sites.go | 126 ++++++------------ .../internal/document/sites_internal_test.go | 54 ++++---- internal/archtest/arch_test.go | 1 + 3 files changed, 67 insertions(+), 114 deletions(-) diff --git a/compilers/openapi/internal/document/sites.go b/compilers/openapi/internal/document/sites.go index e32969ba..18b4bd97 100644 --- a/compilers/openapi/internal/document/sites.go +++ b/compilers/openapi/internal/document/sites.go @@ -4,107 +4,74 @@ import ( "encoding/json/jsontext" "strconv" + "github.com/speakeasy-api/openapi/extensions" + "github.com/dexpace/morphic/compilers/openapi/internal/annotation" "github.com/dexpace/morphic/compilers/openapi/internal/ids" "github.com/dexpace/morphic/compilers/openapi/internal/lowering" "github.com/dexpace/morphic/ir" ) -// documentExtensions collects the x-* of every object that lowers to no IR node -// of its own: the document root, the info block and the contact and license -// inside it, the root externalDocs, the components object, and each declared -// tag with its own externalDocs. ir.Document is the nearest node with an -// Unmodeled map for all of them, so each object's entries are keyed under the -// source path it was written at — see annotation.ExtensionsUnder for why one -// unscoped key for all of them would not do. +// documentExtensions collects the x-* of every site. ir.Document is the nearest +// node with an Unmodeled map for all of them, so each object's entries are keyed +// under the source path it was written at — see annotation.ExtensionsUnder for +// why one unscoped key for all of them would not do. func documentExtensions(c lowering.Ctx) (ir.Unmodeled, []ir.Diagnostic) { - return annotation.ExtensionsAt(c.ProvenanceAt, append(rootExtensions(c), tagExtensions(c)...)...) -} - -// rootExtensions returns the extension sites a document has exactly one of. The -// root's own take no scope, since ir.Document stands for the OpenAPI Object -// itself; the rest are keyed by the path from it down to the object that wrote -// them. -func rootExtensions(c lowering.Ctx) []annotation.ExtensionSite { - info := c.Doc.GetInfo() - infoPtr := ids.Ptr("info") - return []annotation.ExtensionSite{ - {Scope: "", Owner: "", Ext: c.Doc.GetExtensions()}, - {Scope: "info", Owner: infoPtr, Ext: info.GetExtensions()}, - {Scope: "info/contact", Owner: infoPtr + ids.Ptr("contact"), Ext: info.GetContact().GetExtensions()}, - {Scope: "info/license", Owner: infoPtr + ids.Ptr("license"), Ext: info.GetLicense().GetExtensions()}, - {Scope: "externalDocs", Owner: ids.Ptr("externalDocs"), Ext: c.Doc.GetExternalDocs().GetExtensions()}, - {Scope: "components", Owner: ids.Ptr("components"), Ext: c.Doc.GetComponents().GetExtensions()}, + all := sites(c) + ext := make([]annotation.ExtensionSite, 0, len(all)) + for _, s := range all { + ext = append(ext, annotation.ExtensionSite{Scope: s.scope, Owner: s.owner, Ext: s.model.GetExtensions()}) } -} - -// tagExtensions returns the extension sites each declared tag contributes: its -// own, and its externalDocs object's. ir.TagDef holds no Unmodeled map and -// neither does ir.Link, so both ride on the document. -// -// Keyed by the tag's index rather than its name, which is the pointer the tag -// is written at. A name would read better and is what TagDefs are found by, but -// OpenAPI's requirement that tag names be unique is the document's to keep, not -// this compiler's to rely on: two tags spelled alike would silently leave one -// entry. -func tagExtensions(c lowering.Ctx) []annotation.ExtensionSite { - tags := c.Doc.GetTags() - out := make([]annotation.ExtensionSite, 0, 2*len(tags)) - for i, t := range tags { - if t == nil { - continue - } - scope := "tags/" + strconv.Itoa(i) - ptr := ids.Ptr("tags", strconv.Itoa(i)) - out = append(out, - annotation.ExtensionSite{Scope: scope, Owner: ptr, Ext: t.GetExtensions()}, - annotation.ExtensionSite{Scope: scope + "/externalDocs", Owner: ptr + ids.Ptr("externalDocs"), - Ext: t.GetExternalDocs().GetExtensions()}) - } - return out + return annotation.ExtensionsAt(c.ProvenanceAt, ext...) } // documentUnknownKeys collects the keys the OpenAPI model names no field for -// from every object around the document metadata that lowers to no node of its -// own: the root, info with its contact and license, the root externalDocs, the -// components object, and each tag with its own externalDocs. +// from every site. // // ir.Document is the nearest node with an Unmodeled map for all of them, so each // object's keys are scoped by the source path they were written at. One unscoped // "openapi:status" would be a single key for every object, and the entry that // survived would be whichever site ran last. func documentUnknownKeys(c lowering.Ctx, p *ir.Unmodeled) []ir.Diagnostic { - sites := append(rootUnknownSites(c), tagUnknownSites(c)...) - diags := make([]ir.Diagnostic, 0, len(sites)) - for _, site := range sites { - diags = append(diags, - annotation.UnknownKeysUnder(p, site.model, c.ProvenanceAt, site.owner, site.scope)...) + all := sites(c) + diags := make([]ir.Diagnostic, 0, len(all)) + for _, s := range all { + diags = append(diags, annotation.UnknownKeysUnder(p, s.model, c.ProvenanceAt, s.owner, s.scope)...) } return diags } -// unknownSite is one object's census: what it keys under on the carrier holding -// it, the object's own source pointer, and the parsed object itself. -type unknownSite struct { +// site is one object around the document metadata that lowers to no node of +// its own, so its x-* and its undeclared keys ride on ir.Document, keyed under +// scope: the path from the root down to the object, empty for the root itself, +// which ir.Document stands for. A tag's path holds its index rather than its +// name, which is the pointer it is written at: OpenAPI's requirement that tag +// names be unique is the document's to keep, not this compiler's to rely on, +// and two tags spelled alike would silently leave one entry. +type site struct { scope string owner jsontext.Pointer - model any + model extensible } -// rootUnknownSites returns the census sites a document has exactly one of. The -// root's keys take no scope, since ir.Document stands for the OpenAPI Object -// itself; the rest are keyed by the path from it down to the object that wrote -// them. +// extensible is a parsed object that carries x-* extensions. +type extensible interface { + GetExtensions() *extensions.Extensions +} + +// sites returns every site: the root, info with its contact and license, the +// root externalDocs, the components object, and each declared tag with its own +// externalDocs, since neither ir.TagDef nor ir.Link holds an Unmodeled map. // -// The components object is one of them, unlike the maps beneath it: `paths`, +// The components object is a site, unlike the maps beneath it: `paths`, // `responses` and a callback each embed a sequenced map, so every key they hold // is an entry of it (docs/parser-library.md#embedded-map-keys), while the // Components Object is a fixed-field struct and a key it does not define is as -// undeclared as one on any other object here. -func rootUnknownSites(c lowering.Ctx) []unknownSite { +// undeclared as one on any other site. +func sites(c lowering.Ctx) []site { info := c.Doc.GetInfo() infoPtr := ids.Ptr("info") - return []unknownSite{ + out := []site{ {"", "", c.Doc}, {"info", infoPtr, info}, {"info/contact", infoPtr + ids.Ptr("contact"), info.GetContact()}, @@ -112,28 +79,15 @@ func rootUnknownSites(c lowering.Ctx) []unknownSite { {"externalDocs", ids.Ptr("externalDocs"), c.Doc.GetExternalDocs()}, {"components", ids.Ptr("components"), c.Doc.GetComponents()}, } -} - -// tagUnknownSites returns the census sites each declared tag contributes: its -// own, and its externalDocs object's. Neither ir.TagDef nor ir.Link holds an -// Unmodeled map, so both ride on the document. -// -// Scoped by index rather than name, which is the pointer a tag is written at. A -// name would read better, but OpenAPI's requirement that tag names be unique is -// the document's to keep and not this compiler's to rely on: two tags spelled -// alike would silently leave one entry. -func tagUnknownSites(c lowering.Ctx) []unknownSite { - tags := c.Doc.GetTags() - out := make([]unknownSite, 0, 2*len(tags)) - for i, t := range tags { + for i, t := range c.Doc.GetTags() { if t == nil { continue } index := strconv.Itoa(i) ptr := ids.Ptr("tags", index) out = append(out, - unknownSite{"tags/" + index, ptr, t}, - unknownSite{"tags/" + index + "/externalDocs", ptr + ids.Ptr("externalDocs"), t.GetExternalDocs()}) + site{"tags/" + index, ptr, t}, + site{"tags/" + index + "/externalDocs", ptr + ids.Ptr("externalDocs"), t.GetExternalDocs()}) } return out } diff --git a/compilers/openapi/internal/document/sites_internal_test.go b/compilers/openapi/internal/document/sites_internal_test.go index 74b22327..adc9dda5 100644 --- a/compilers/openapi/internal/document/sites_internal_test.go +++ b/compilers/openapi/internal/document/sites_internal_test.go @@ -6,39 +6,37 @@ import ( soa "github.com/speakeasy-api/openapi/openapi" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/dexpace/morphic/compilers/openapi/internal/lowering" ) -// TestTagExtensions_NilEntrySkipped pins the same guard lowerTagDefs keeps on -// the other side of the tag list: a nil entry contributes no extension site, so -// nothing dereferences it and no site is keyed at an index holding no tag. -func TestTagExtensions_NilEntrySkipped(t *testing.T) { +// TestSites_ListEveryObjectAroundTheMetadata pins the one list the extension +// and undeclared-key censuses share: each object, keyed by its path from the +// root, at its own pointer. A nil tag contributes no site, the guard +// lowerTagDefs keeps on the other side of the tag list, so nothing +// dereferences it and no site is keyed at an index holding no tag. +func TestSites_ListEveryObjectAroundTheMetadata(t *testing.T) { t.Parallel() doc := &soa.OpenAPI{Tags: []*soa.Tag{nil, {Name: "kept"}}} - got := tagExtensions(lowering.Ctx{Doc: doc}) - - require.Len(t, got, 2, "one surviving tag contributes its own site and its externalDocs one") - assert.Equal(t, "tags/1", got[0].Scope, "the site is keyed at the tag's own index, not its position") - assert.Equal(t, jsontext.Pointer("/tags/1"), got[0].Owner) -} - -// TestTagUnknownSites_NilEntrySkipped is TestTagExtensions_NilEntrySkipped's -// twin on the census side: the two walks of the tag list keep the same guard, so -// neither can be the one that dereferences a nil entry or keys a site at an -// index holding no tag. -func TestTagUnknownSites_NilEntrySkipped(t *testing.T) { - t.Parallel() - doc := &soa.OpenAPI{Tags: []*soa.Tag{nil, {Name: "kept"}}} - - got := tagUnknownSites(lowering.Ctx{Doc: doc}) - - require.Len(t, got, 2, - "the nil tag contributes nothing, and the surviving one contributes its own site and its externalDocs") - assert.Equal(t, "tags/1", got[0].scope, "the site is keyed at the tag's own index, not its position") - assert.Equal(t, jsontext.Pointer("/tags/1"), got[0].owner) - assert.Equal(t, "tags/1/externalDocs", got[1].scope, "and its externalDocs is scoped under that same index") - assert.Equal(t, jsontext.Pointer("/tags/1/externalDocs"), got[1].owner) + type keyed struct { + scope string + owner jsontext.Pointer + } + all := sites(lowering.Ctx{Doc: doc}) + got := make([]keyed, 0, len(all)) + for _, s := range all { + got = append(got, keyed{s.scope, s.owner}) + } + + assert.Equal(t, []keyed{ + {"", ""}, + {"info", "/info"}, + {"info/contact", "/info/contact"}, + {"info/license", "/info/license"}, + {"externalDocs", "/externalDocs"}, + {"components", "/components"}, + {"tags/1", "/tags/1"}, + {"tags/1/externalDocs", "/tags/1/externalDocs"}, + }, got, "the surviving tag is keyed at its own index, not its position among the survivors") } diff --git a/internal/archtest/arch_test.go b/internal/archtest/arch_test.go index e6d73ea9..f342abcf 100644 --- a/internal/archtest/arch_test.go +++ b/internal/archtest/arch_test.go @@ -282,6 +282,7 @@ var rules = map[string][]string{ module + "/compilers/openapi/internal/ids", module + "/compilers/openapi/internal/lowering", module + "/compilers/openapi/internal/nodeview", + "github.com/speakeasy-api/openapi/extensions", "github.com/speakeasy-api/openapi/openapi"}, // The canaries that pin docs/parser-library.md. Its one production file is // the package doc: the tests call the library directly, and test files are From 4d5f47ff538ea2b7d37310661531e2d0e3eeb888 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 05:48:35 +0300 Subject: [PATCH 05/21] docs(micro-compiler-design): record where meta and auth landed Section 5.1's target layout put meta and auth under operation/. Neither landed there: auth founded a package because both halves of the compiler reach it, and document metadata and detection stayed in the root until each became a package of its own. The section's as-landed note now says so, as the document records the other places where its layout and the tree parted. --- docs/micro-compiler-design.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/micro-compiler-design.md b/docs/micro-compiler-design.md index e4cdfd5c..ad2fa777 100644 --- a/docs/micro-compiler-design.md +++ b/docs/micro-compiler-design.md @@ -446,6 +446,13 @@ go list -f '{{$p := .ImportPath}}{{range .Imports}}{{$p}} -> {{.}} {{end}}' ./compilers/openapi/... | grep -E '> .*internal/(nodeview|load)$' ``` +*As landed:* `operation/` took neither `auth` nor `meta`. `auth` founded a package of its own, +because both halves of the compiler reach it and neither may reach the other. Document-level +metadata, its servers included, stayed in the root, as did detection's reading of a source, until +#1091 gave each a package: `document`, which is no part of lowering an operation and runs after both +walks, reaching neither, and `detect`, which runs before anything has decided a source is this +compiler's. The root now holds the face this layout names, with `Detect` a call into `detect`. + `diag` sits at the bottom because its constructor is the single one that populates severity, code and provenance, and eight files call it. The OpenAPI diagnostic codes are format-specific strings, so they stay in the compiler rather than moving to `compilers/compile`; whether the constructor itself later From 6de49fc527163d95d6e887b5ff3e2549dfc4fe06 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 06:00:29 +0300 Subject: [PATCH 06/21] docs(compilers/openapi): keep Detect's and document's comments whole document's package comment said a field for path-item and operation servers would move their lowering to a package both walks reach. That is one of two choices: the operation walk could reach internal/document instead, and the change that adds the field decides which. The comment now names both. Detect's comment said only that unreadable bytes declaring a version are reported. A root merging deeper than detection reads is declined with an error as well, as Recognize's comment already says; Detect's says so too. --- compilers/openapi/detect.go | 12 ++++++------ compilers/openapi/internal/document/doc.go | 18 +++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/compilers/openapi/detect.go b/compilers/openapi/detect.go index 82f2adff..3ec23668 100644 --- a/compilers/openapi/detect.go +++ b/compilers/openapi/detect.go @@ -11,13 +11,13 @@ import ( // serve, so the caller can say unsupported rather than unreadable. The path is // not consulted. // -// Recognition parses all of src, whatever its format, and hands the parse to -// Compile in Recognition.Parsed. The byte budget bounds that cost: a source -// past it is declined unread with Compile's own refusal. +// It parses all of src, whatever its format, and hands Compile the parse in +// Recognition.Parsed. The byte budget bounds that cost: a source past it is +// declined unread with Compile's own refusal. // -// Unreadable bytes are declined silently unless they declare a version under a -// discriminating key; then the parser's complaint is reported, since that -// source is this compiler's own and broken. +// Other declines say why only when the source is this compiler's own: it +// declares a version under a discriminating key and will not parse, or its +// root merges deeper than detection reads. func (*Compiler) Detect(src compilers.Source, opts compilers.Options) (compilers.Recognition, []ir.Diagnostic, bool) { // Source 0 is src: the one source detection is handed is the whole of the // table a declined diagnostic can index (compilers.Compiler.Detect). diff --git a/compilers/openapi/internal/document/doc.go b/compilers/openapi/internal/document/doc.go index ae93698d..3c60bae0 100644 --- a/compilers/openapi/internal/document/doc.go +++ b/compilers/openapi/internal/document/doc.go @@ -1,13 +1,13 @@ -// Package document lowers what a document says about itself rather than about -// its types or operations: its info, root externalDocs and servers, and, kept -// verbatim, what the objects around them write that lowers to no node of its -// own (docs/compilers/openapi.md#document-metadata, +// Package document lowers what a document says about itself rather than its +// types or operations: its info, root externalDocs and servers, and, kept +// verbatim, what the objects around them write that lowers to no node +// (docs/compilers/openapi.md#document-metadata, // docs/compilers/openapi.md#unreferenced-components). // -// It runs after every other lowering, since only they know which components -// they reached, and returns a Meta for the compiler to assign. +// It runs after every other lowering, which alone know the components they +// reached, and returns a Meta for the compiler to assign. // -// Servers are lowered here while the IR holds only the document's own. A field -// for a path item's or an operation's (GitHub #285) would move their lowering -// to a package both walks reach. +// Servers are lowered here while the IR holds only the document's. A field for +// a path item's or an operation's (GitHub #285) may move their lowering to a +// package both walks reach, or have the operation walk reach this one. package document From f55bae19000693488ce793abc30c114ad215df01 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 06:03:09 +0300 Subject: [PATCH 07/21] docs(compilers/openapi): correct comments the moves left behind Comments still described the old layout, or named nothing in the new: - load's server-repeats test placed TestDuplicateServerName_CanonicalCollisionsAreNotReported in the openapi package; it is the document package's now. - resolve.Reached said no lowering reads it and the compiler asks it. Its one reader is document.Lower, a lowering itself, which asks once every other lowering has returned; lowering.Ctx.Reached says the same. - Compile's step comment named the last lowering meta. - serverVariables likened its direct field read to an unqualified XMLHints, which names nothing in its package: annotation.XMLHints. TestLowerServers_EveryEntrySkippedIsNil also spoke of "the same guard" with nothing beside it to say which, a reference the move put further away. It names the schemes' test whose guard it mirrors. --- compilers/openapi/internal/document/servers.go | 2 +- .../openapi/internal/document/servers_internal_test.go | 7 ++++--- compilers/openapi/internal/load/repeats_internal_test.go | 2 +- compilers/openapi/internal/lowering/lowering.go | 3 ++- compilers/openapi/internal/resolve/reached.go | 6 +++--- compilers/openapi/openapi.go | 3 ++- 6 files changed, 13 insertions(+), 10 deletions(-) diff --git a/compilers/openapi/internal/document/servers.go b/compilers/openapi/internal/document/servers.go index 077ab1b5..bff9fbf6 100644 --- a/compilers/openapi/internal/document/servers.go +++ b/compilers/openapi/internal/document/servers.go @@ -94,7 +94,7 @@ func serverVariables(c lowering.Ctx, s *soa.Server, sptr jsontext.Pointer) ([]ir } vptr := sptr + ids.Ptr("variables", name) // ServerVariable exposes no GetExtensions at this library version, so the - // field is read directly — as XMLHints already reads its own. + // field is read directly — as annotation.XMLHints already reads its own. ext, extDiags := annotation.ExtensionsFrom(v.Extensions, c.ProvenanceAt, vptr) diags = append(diags, extDiags...) one := ir.ServerVariable{ diff --git a/compilers/openapi/internal/document/servers_internal_test.go b/compilers/openapi/internal/document/servers_internal_test.go index 21e102dd..4b8a4645 100644 --- a/compilers/openapi/internal/document/servers_internal_test.go +++ b/compilers/openapi/internal/document/servers_internal_test.go @@ -83,9 +83,10 @@ func TestServerVariables_NilEntrySkipped(t *testing.T) { assert.Equal(t, "keep", srv.Variables[0].Name) } -// TestLowerServers_EveryEntrySkippedIsNil pins the same guard on the servers -// side: when no entry survives, the field stays unset rather than becoming an -// empty list the source never declared. +// TestLowerServers_EveryEntrySkippedIsNil pins, for the servers, the guard +// TestLowerSecuritySchemes_NothingLoweredIsNilNotEmpty pins for the schemes: +// when no entry survives, the field stays unset rather than becoming an empty +// list the source never declared. func TestLowerServers_EveryEntrySkippedIsNil(t *testing.T) { t.Parallel() doc := &soa.OpenAPI{Servers: []*soa.Server{nil, nil}} diff --git a/compilers/openapi/internal/load/repeats_internal_test.go b/compilers/openapi/internal/load/repeats_internal_test.go index 17add209..30a3c217 100644 --- a/compilers/openapi/internal/load/repeats_internal_test.go +++ b/compilers/openapi/internal/load/repeats_internal_test.go @@ -382,7 +382,7 @@ paths: {} // entry with no name or an empty one holds none, and a name may recur across // lists, since a path item's or an operation's servers replace the ones above // them rather than join them. Names equal only once canonicalized are the -// openapi package's TestDuplicateServerName_CanonicalCollisionsAreNotReported, +// document package's TestDuplicateServerName_CanonicalCollisionsAreNotReported, // which proves each collision is real. func TestServerRepeats_ReportsOnlyANameOneListRepeatsExactly(t *testing.T) { t.Parallel() diff --git a/compilers/openapi/internal/lowering/lowering.go b/compilers/openapi/internal/lowering/lowering.go index b957b6d8..4ef677be 100644 --- a/compilers/openapi/internal/lowering/lowering.go +++ b/compilers/openapi/internal/lowering/lowering.go @@ -209,7 +209,8 @@ func New(srcIndex int, doc *soa.OpenAPI, src ir.SourceInfo, grouping GroupingStr // Reached reports whether a lowering run under this context, or any copy of it, // resolved a reference to pointer in the source. It is meant to be asked once -// every lowering has returned: before that it answers for those that have. +// every other lowering has returned: before that it answers for those that +// have. func (c Ctx) Reached(pointer jsontext.Pointer) bool { return c.reached.Has(pointer) } diff --git a/compilers/openapi/internal/resolve/reached.go b/compilers/openapi/internal/resolve/reached.go index 977d505d..1d72911d 100644 --- a/compilers/openapi/internal/resolve/reached.go +++ b/compilers/openapi/internal/resolve/reached.go @@ -8,9 +8,9 @@ import ( // to, each hop of a chain included: the declarations the IR carries through // their uses rather than on a node of their own (GitHub #616). // -// No lowering reads it. The compiler asks it once the last lowering has -// returned, so what it holds cannot change a lowering's answer, and as a set it -// cannot depend on the order lowerings ran in. That is why sharing it between +// Only the document's lowering reads it, and only once every other lowering +// has returned, so what it holds cannot change their answers, and as a set it +// cannot depend on the order they ran in. That is why sharing it between // copies of a context is safe, as sharing a memo is. type Reached struct { at map[jsontext.Pointer]bool diff --git a/compilers/openapi/openapi.go b/compilers/openapi/openapi.go index 878ec42e..c1fbe02f 100644 --- a/compilers/openapi/openapi.go +++ b/compilers/openapi/openapi.go @@ -89,7 +89,8 @@ func (c *Compiler) Compile(ctx context.Context, sources []compilers.Source, opts if err != nil || loadedDoc == nil { return nil, diags, err } - // components schemas → auth → service/operations → meta; assembles Document + // components schemas → auth → service/operations → document metadata; + // assembles Document out, lowerDiags, err := run(ctx, loweringCtx(loadedDoc, formatOpts), compile.NewTypes()) // Load diagnostics precede lowering diagnostics. all := slices.Concat(diags, withoutRereported(lowerDiags, diags)) From 6cb272779810c00bc0636f99b48485e0d1adaa0a Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 16:34:32 +0300 Subject: [PATCH 08/21] docs: say who reads the reached record now micro-compiler-design.md's note on resolve.Reached, and the field comment in lowering.Ctx, both said no lowering reads the record and the compiler reads it once the last lowering returns. Its one reader is document.Lower, itself a lowering, which reads it once every other lowering has returned. Both now say that, as resolve.Reached's own comment does. The note on where meta and auth landed also mixed two kinds of note. The document marks how the restructuring landed with "As landed" and what changed after it with "Later", so the part #1091 did is now a "Later" note of its own. --- compilers/openapi/internal/lowering/lowering.go | 6 +++--- docs/micro-compiler-design.md | 16 +++++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/compilers/openapi/internal/lowering/lowering.go b/compilers/openapi/internal/lowering/lowering.go index 4ef677be..546ce4a6 100644 --- a/compilers/openapi/internal/lowering/lowering.go +++ b/compilers/openapi/internal/lowering/lowering.go @@ -147,9 +147,9 @@ type Ctx struct { hints map[scopedPosition]string // reached records where each reference a lowering resolved landed in the - // source. Copies share it on purpose: no lowering reads it, so it cannot - // change any lowering's answer, and the compiler reads it once the last one - // has returned (Reached). Nil in a zero context, which records nothing. + // source. Copies share it on purpose: only the document's lowering reads it, + // once every other lowering has returned (Reached), so it cannot change their + // answers. Nil in a zero context, which records nothing. reached *resolve.Reached } diff --git a/docs/micro-compiler-design.md b/docs/micro-compiler-design.md index ad2fa777..dacc903d 100644 --- a/docs/micro-compiler-design.md +++ b/docs/micro-compiler-design.md @@ -295,9 +295,9 @@ the claim as written does not admit it. *Later:* a second shared record does accumulate decisions. `resolve.Reached`, held by `Ctx` and written by every reference resolution, records where each one landed, so the compiler can keep the component entries no lowering reached (GitHub #616). -It is not a handle in the sense rejected above for diagnostics: no lowering reads -it, so it cannot change a lowering's answer, and it is read once, after the last -lowering returns, as a set whose contents do not depend on the order they were +It is not a handle in the sense rejected above for diagnostics: only the document's +lowering reads it, once every other lowering has returned, so it cannot change their +answers, and it is read as a set whose contents do not depend on the order they were written in. Only the lowering knows what it followed, so the alternatives were a reading of the source that guesses — which dropped entries and gave others two homes — or threading a return value through every lowering. @@ -448,10 +448,12 @@ go list -f '{{$p := .ImportPath}}{{range .Imports}}{{$p}} -> {{.}} *As landed:* `operation/` took neither `auth` nor `meta`. `auth` founded a package of its own, because both halves of the compiler reach it and neither may reach the other. Document-level -metadata, its servers included, stayed in the root, as did detection's reading of a source, until -#1091 gave each a package: `document`, which is no part of lowering an operation and runs after both -walks, reaching neither, and `detect`, which runs before anything has decided a source is this -compiler's. The root now holds the face this layout names, with `Detect` a call into `detect`. +metadata, its servers included, stayed in the root, as did detection's reading of a source. + +*Later:* #1091 gave each of those a package: `document`, which is no part of lowering an operation +and runs after both walks, reaching neither, and `detect`, which runs before anything has decided a +source is this compiler's. The root now holds the face this layout names, with `Detect` a call into +`detect`. `diag` sits at the bottom because its constructor is the single one that populates severity, code and provenance, and eight files call it. The OpenAPI diagnostic codes are format-specific strings, so From 6f8543e8bc1fc8078a03b1e63bff229ce3bb71f3 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 18:09:09 +0300 Subject: [PATCH 09/21] docs(compilers/openapi): say which declines report whatever the format Detect's and Recognize's comments said a decline explains itself only when the source is this compiler's own, and counted a root merged past the depth detection reads among those. It is not one. Unless the root's own keys settle the version, it is declined with an openapi/budget-exceeded error whatever its format, since a version past the bound may outrank one read above it, as docs/compilers/openapi.md#yaml-decoding says. A YAML file of another format whose root merges 65 levels deep and declares no version anywhere draws the error, on main as here. Both comments now name the two declines that report whatever the format, the byte budget and the cut-short read, and keep the source's being this compiler's own for the one that declares a version and cannot be read. The recognize test helper said it calls Recognize as Detect does, with no byte budget; Detect passes the caller's. It now says the helper reads Detect's source with no budget. --- compilers/openapi/detect.go | 13 ++++++------- compilers/openapi/internal/detect/helpers_test.go | 4 ++-- compilers/openapi/internal/detect/recognize.go | 12 +++++++----- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/compilers/openapi/detect.go b/compilers/openapi/detect.go index 3ec23668..dfb0b76b 100644 --- a/compilers/openapi/detect.go +++ b/compilers/openapi/detect.go @@ -11,13 +11,12 @@ import ( // serve, so the caller can say unsupported rather than unreadable. The path is // not consulted. // -// It parses all of src, whatever its format, and hands Compile the parse in -// Recognition.Parsed. The byte budget bounds that cost: a source past it is -// declined unread with Compile's own refusal. -// -// Other declines say why only when the source is this compiler's own: it -// declares a version under a discriminating key and will not parse, or its -// root merges deeper than detection reads. +// It parses all of src, handing Compile the parse in Recognition.Parsed. Two +// sources are declined with an error whatever their format: one past the byte +// budget, unread, with Compile's own refusal, and one whose root merges deeper +// than detection reads, its own keys leaving the version open. Any other +// decline is silent unless src declares a version under a discriminating key +// and is unreadable. func (*Compiler) Detect(src compilers.Source, opts compilers.Options) (compilers.Recognition, []ir.Diagnostic, bool) { // Source 0 is src: the one source detection is handed is the whole of the // table a declined diagnostic can index (compilers.Compiler.Detect). diff --git a/compilers/openapi/internal/detect/helpers_test.go b/compilers/openapi/internal/detect/helpers_test.go index 0ace385c..e870c7e7 100644 --- a/compilers/openapi/internal/detect/helpers_test.go +++ b/compilers/openapi/internal/detect/helpers_test.go @@ -7,8 +7,8 @@ import ( "github.com/dexpace/morphic/ir" ) -// recognize is Recognize as Detect calls it on the one source it is handed, -// source 0, with no byte budget: no case here comes near the default, and +// recognize is Recognize on source 0, the one source Detect hands it, with no +// byte budget: no case here comes near the default, and // TestRecognize_HoldsTheParseToItsByteBudget sets one of its own. func recognize(src string) (compilers.Recognition, []ir.Diagnostic, bool) { return Recognize(0, []byte(src), 0) diff --git a/compilers/openapi/internal/detect/recognize.go b/compilers/openapi/internal/detect/recognize.go index 2c371f7b..9ebd3522 100644 --- a/compilers/openapi/internal/detect/recognize.go +++ b/compilers/openapi/internal/detect/recognize.go @@ -10,12 +10,14 @@ import ( // Recognize reads the dialect data declares, data being the source at srcIndex: // the discriminating key it writes and its version's major.minor prefix, with -// the parse that read them. maxBytes bounds that parse, zero being none: data -// past it is declined unread, with the refusal Load gives it. +// the parse that read them. maxBytes bounds that parse, zero being none. // -// A decline says why only when the source is this compiler's own: its root -// merges past the depth the read expands, or it will not parse and declares a -// version where a document declares one (declaresProbeKey). +// Two sources are declined with an error whatever their format: one past +// maxBytes, unread, with the refusal Load gives it, and one whose root merges +// past the depth the read expands, its own keys leaving the version open +// (settledByOwnPairs). Any other decline is silent unless the source is +// unreadable and declares a version where a document declares one +// (declaresProbeKey). func Recognize(srcIndex int, data []byte, maxBytes int) (compilers.Recognition, []ir.Diagnostic, bool) { atSource := ir.Provenance{Source: srcIndex} if d, over := load.OverByteBudget(atSource, data, maxBytes); over { From 3ad8c3421f587c9308bf3ae2c8809947924a4fa9 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 18:09:09 +0300 Subject: [PATCH 10/21] test(compilers/openapi): pin the cut-short decline for another format TestRecognize_AReadCutShortIsReported held the bound to a chain with an openapi version at its foot, so a decline that reported only bytes naming that key passed it. A second row puts another format's keys there instead, and reporting the cut only when the bytes name openapi now turns it red. --- .../openapi/internal/detect/recognize_test.go | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/compilers/openapi/internal/detect/recognize_test.go b/compilers/openapi/internal/detect/recognize_test.go index 488779d6..08e9ab34 100644 --- a/compilers/openapi/internal/detect/recognize_test.go +++ b/compilers/openapi/internal/detect/recognize_test.go @@ -331,17 +331,26 @@ func TestRecognize_ReadsAVersionKeyThroughAMergeKey(t *testing.T) { // TestRecognize_AReadCutShortIsReported pins that the bound never passes for a // decline: the source may be this compiler's own, so detection says why it -// could not tell. +// could not tell. That holds whatever the chain holds, a version or another +// format's keys, since what lies past the bound is what went unread. func TestRecognize_AReadCutShortIsReported(t *testing.T) { t.Parallel() - src := openapitest.RootMergeChain(nodeview.MergeDepthLimit+1, "openapi: 3.1.0", "") - _, diags, ok := recognize(string(src)) + for name, leaf := range map[string]string{ + "a version past the bound": "openapi: 3.1.0", + "another format past the bound": "kind: Deployment", + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + src := openapitest.RootMergeChain(nodeview.MergeDepthLimit+1, leaf, "") + _, diags, ok := recognize(string(src)) - assert.False(t, ok) - require.Len(t, diags, 1) - assert.Equal(t, diag.BudgetExceeded, diags[0].Code) - assert.Equal(t, ir.SeverityError, diags[0].Severity, "it is the whole reason the source went unclaimed") - assert.Contains(t, diags[0].Message, fmt.Sprint(nodeview.MergeDepthLimit)) + assert.False(t, ok) + require.Len(t, diags, 1) + assert.Equal(t, diag.BudgetExceeded, diags[0].Code) + assert.Equal(t, ir.SeverityError, diags[0].Severity, "it is the whole reason the source went unclaimed") + assert.Contains(t, diags[0].Message, fmt.Sprint(nodeview.MergeDepthLimit)) + }) + } } // TestRecognize_AMergeCycleHidesNoKey pins the other way a root's merges can stop From 8bfcae8ca4156de968d216d31eb7aca4748c785b Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 18:09:09 +0300 Subject: [PATCH 11/21] test(compilers/openapi): hold each document site to the object it reads TestSites_ListEveryObjectAroundTheMetadata named every site's scope and pointer but not the object it holds, so a site reading the wrong object passed it and only the root package's conformance tests noticed. It now checks each by identity, the one way to tell the root's externalDocs from a tag's. Giving the contact's site the license, a tag's externalDocs site the root's, or the components site the root each turns it red. --- .../internal/document/sites_internal_test.go | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/compilers/openapi/internal/document/sites_internal_test.go b/compilers/openapi/internal/document/sites_internal_test.go index adc9dda5..cb19e87d 100644 --- a/compilers/openapi/internal/document/sites_internal_test.go +++ b/compilers/openapi/internal/document/sites_internal_test.go @@ -4,20 +4,29 @@ import ( "encoding/json/jsontext" "testing" + "github.com/speakeasy-api/openapi/jsonschema/oas3" soa "github.com/speakeasy-api/openapi/openapi" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/dexpace/morphic/compilers/openapi/internal/lowering" ) // TestSites_ListEveryObjectAroundTheMetadata pins the one list the extension // and undeclared-key censuses share: each object, keyed by its path from the -// root, at its own pointer. A nil tag contributes no site, the guard -// lowerTagDefs keeps on the other side of the tag list, so nothing -// dereferences it and no site is keyed at an index holding no tag. +// root, at its own pointer, holding that object and no other. A nil tag +// contributes no site, the guard operation's lowerTagDefs keeps over the same +// list, so nothing dereferences it and no site is keyed at an index holding no +// tag. func TestSites_ListEveryObjectAroundTheMetadata(t *testing.T) { t.Parallel() - doc := &soa.OpenAPI{Tags: []*soa.Tag{nil, {Name: "kept"}}} + kept := &soa.Tag{Name: "kept", ExternalDocs: &oas3.ExternalDocumentation{}} + doc := &soa.OpenAPI{ + Info: soa.Info{Contact: &soa.Contact{}, License: &soa.License{}}, + ExternalDocs: &oas3.ExternalDocumentation{}, + Components: &soa.Components{}, + Tags: []*soa.Tag{nil, kept}, + } type keyed struct { scope string @@ -39,4 +48,13 @@ func TestSites_ListEveryObjectAroundTheMetadata(t *testing.T) { {"tags/1", "/tags/1"}, {"tags/1/externalDocs", "/tags/1/externalDocs"}, }, got, "the surviving tag is keyed at its own index, not its position among the survivors") + + // The two externalDocs objects are alike in type and content, so only their + // identity tells a site holding the other one's apart. + want := []extensible{doc, &doc.Info, doc.Info.Contact, doc.Info.License, + doc.ExternalDocs, doc.Components, kept, kept.ExternalDocs} + require.Len(t, all, len(want)) + for i, s := range all { + assert.Same(t, want[i], s.model, "the site at %q holds the object written there", s.owner) + } } From e2be7b33b05a68457748589fd42bdadd88560a5b Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 18:09:09 +0300 Subject: [PATCH 12/21] docs: give auth's package a reason that holds auth's package comment, and its archtest rule, said the document walk lowers the schemes. With internal/document a package, that reads as document.Lower, which lowers none; the root's run does. They also said neither half of the compiler may reach the other, and that putting either half's lowering with its caller would make the other import it. The root imports the operation walk, and the two halves share no code, so neither holds. The comment now gives the reason that does: the requirements cannot sit in the root, which the operation walk may not import, and the schemes are the document's, no part of lowering an operation. The as-landed note in micro-compiler-design.md, which repeated the old reason, now says which part of the compiler reaches each half instead. --- compilers/openapi/internal/auth/doc.go | 10 +++++----- docs/micro-compiler-design.md | 5 +++-- internal/archtest/arch_test.go | 6 +++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/compilers/openapi/internal/auth/doc.go b/compilers/openapi/internal/auth/doc.go index aa2652ce..780c99ec 100644 --- a/compilers/openapi/internal/auth/doc.go +++ b/compilers/openapi/internal/auth/doc.go @@ -1,9 +1,9 @@ // Package auth lowers what a document says about authentication: the security // schemes it declares, and the requirements that name them. // -// It is a package of its own because both halves of the compiler reach it and -// neither may reach the other. The document walk lowers the schemes once, before -// anything references them; the service and operation walks lower requirements -// against the result. Putting either half's lowering with its caller would make -// the other import it. +// It is a package of its own because both halves of the compiler reach it. The +// compiler's run lowers the schemes once, before anything references them; the +// service and operation walks lower requirements against the result. The +// requirements cannot sit in the root, which those walks may not import, and +// the schemes are the document's, no part of lowering an operation. package auth diff --git a/docs/micro-compiler-design.md b/docs/micro-compiler-design.md index dacc903d..e16b9bf5 100644 --- a/docs/micro-compiler-design.md +++ b/docs/micro-compiler-design.md @@ -447,8 +447,9 @@ go list -f '{{$p := .ImportPath}}{{range .Imports}}{{$p}} -> {{.}} ``` *As landed:* `operation/` took neither `auth` nor `meta`. `auth` founded a package of its own, -because both halves of the compiler reach it and neither may reach the other. Document-level -metadata, its servers included, stayed in the root, as did detection's reading of a source. +which the compiler's run reaches for the security schemes and the operation walk for the +requirements. Document-level metadata, its servers included, stayed in the root, as did detection's +reading of a source. *Later:* #1091 gave each of those a package: `document`, which is no part of lowering an operation and runs after both walks, reaching neither, and `detect`, which runs before anything has decided a diff --git a/internal/archtest/arch_test.go b/internal/archtest/arch_test.go index f342abcf..b9ce9767 100644 --- a/internal/archtest/arch_test.go +++ b/internal/archtest/arch_test.go @@ -228,9 +228,9 @@ var rules = map[string][]string{ module + "/compilers/openapi/internal/resolve", "github.com/speakeasy-api/openapi/openapi"}, // The security schemes a document declares and the requirements that name - // them. It is its own package because both sides of the compiler reach it and - // neither may reach the other: the document walk lowers the schemes, and the - // service and operation walks lower the requirements against them. + // them. It is its own package because both sides of the compiler reach it: + // the compiler's run lowers the schemes, and the service and operation walks + // lower the requirements against them. "compilers/openapi/internal/auth": {module + "/ir", module + "/compilers/compile", module + "/compilers/openapi/internal/annotation", module + "/compilers/openapi/internal/diag", From 7a59e020c6d820e55e17b1e885052d016bd67ee1 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 18:12:32 +0300 Subject: [PATCH 13/21] docs(compilers/openapi): say why the lowering context is a package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lowering's package comment gave the reason auth's did, that the schema walk and the operation walk both need it and neither may reach the other. The operation walk imports the schema walk, as micro-compiler-design.md §5.1, which the comment cites, lays out. What holds is that every lowering package reads the context and only a package beneath them all serves each: the schema walk may not reach the operation walk, and auth and document reach neither. --- compilers/openapi/internal/lowering/lowering.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compilers/openapi/internal/lowering/lowering.go b/compilers/openapi/internal/lowering/lowering.go index 546ce4a6..bb2c9ebd 100644 --- a/compilers/openapi/internal/lowering/lowering.go +++ b/compilers/openapi/internal/lowering/lowering.go @@ -6,8 +6,10 @@ // identity, the indexes derived once at entry, and the constructors that stamp // provenance. // -// It is a package because the schema walk and the operation walk both need -// those answers and neither may reach the other (micro-compiler-design §5.1). +// It is a package because every lowering package needs those answers, and only +// one beneath them all serves each: the schema walk may not reach the +// operation walk, which reaches it, and auth and document reach neither +// (micro-compiler-design §5.1). package lowering import ( From 9e4fe1e2b235572496d9b1d3b18c033d0ac328f3 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 18:29:40 +0300 Subject: [PATCH 14/21] test(compilers/openapi): pin the budget detection reads from the options detectionBudget gives options of another compiler's type this compiler's default byte budget, as compilers.Compiler.Detect requires. Nothing held it to that: reading them as no budget at all passed every test, because Detect's tests exceed only a budget the caller sets, and a source past the 64 MiB default costs more than the gate's race run should pay. A table now asks detectionBudget directly for no options, another compiler's, a budget of the caller's own and one turned off, and both that change and leaving a negative budget untranslated turn it red. --- compilers/openapi/detect_internal_test.go | 34 +++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 compilers/openapi/detect_internal_test.go diff --git a/compilers/openapi/detect_internal_test.go b/compilers/openapi/detect_internal_test.go new file mode 100644 index 00000000..fba0ebe2 --- /dev/null +++ b/compilers/openapi/detect_internal_test.go @@ -0,0 +1,34 @@ +package openapi + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/dexpace/morphic/compilers" +) + +// TestDetectionBudget_ReadsTheCallersOptions pins the budget detection takes +// from each kind of options, another compiler's included, which mean this +// one's default as compilers.Compiler.Detect requires rather than no budget. +// Detect's own tests exceed only a budget the caller sets: a source past the +// default costs more than the gate's race run should pay, so another +// compiler's options read as no budget would pass them all. +func TestDetectionBudget_ReadsTheCallersOptions(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + opts compilers.Options + want int + }{ + {"no options", compilers.Options{}, DefaultMaxSourceBytes}, + {"another compiler's options", compilers.Options{FormatOptions: "another compiler's options"}, DefaultMaxSourceBytes}, + {"a budget of its own", compilers.Options{FormatOptions: Options{Limits: Limits{MaxSourceBytes: 7}}}, 7}, + {"the budget turned off", compilers.Options{FormatOptions: Options{Limits: Limits{MaxSourceBytes: -1}}}, 0}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, detectionBudget(tc.opts)) + }) + } +} From 5dba18152d4913b783c7f55af1374f2ea2f4199f Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 18:29:40 +0300 Subject: [PATCH 15/21] docs(compilers/openapi): say which package the named auth tests are in Two document tests name an auth test as their twin, TestAuth_ and TestLowerSecuritySchemes_, as though it sat beside them. Both say auth's now, as load's test already says the document package's of the one it names. --- .../openapi/internal/document/servers_internal_test.go | 6 +++--- compilers/openapi/internal/document/sites_test.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/compilers/openapi/internal/document/servers_internal_test.go b/compilers/openapi/internal/document/servers_internal_test.go index 4b8a4645..7149d198 100644 --- a/compilers/openapi/internal/document/servers_internal_test.go +++ b/compilers/openapi/internal/document/servers_internal_test.go @@ -84,9 +84,9 @@ func TestServerVariables_NilEntrySkipped(t *testing.T) { } // TestLowerServers_EveryEntrySkippedIsNil pins, for the servers, the guard -// TestLowerSecuritySchemes_NothingLoweredIsNilNotEmpty pins for the schemes: -// when no entry survives, the field stays unset rather than becoming an empty -// list the source never declared. +// auth's TestLowerSecuritySchemes_NothingLoweredIsNilNotEmpty pins for the +// schemes: when no entry survives, the field stays unset rather than becoming +// an empty list the source never declared. func TestLowerServers_EveryEntrySkippedIsNil(t *testing.T) { t.Parallel() doc := &soa.OpenAPI{Servers: []*soa.Server{nil, nil}} diff --git a/compilers/openapi/internal/document/sites_test.go b/compilers/openapi/internal/document/sites_test.go index 387ef62f..4d232ecc 100644 --- a/compilers/openapi/internal/document/sites_test.go +++ b/compilers/openapi/internal/document/sites_test.go @@ -11,10 +11,10 @@ import ( ) // TestMeta_UnserializableExtensionStillWarns pins the document-level twin of -// TestAuth_UnserializableExtensionStillWarns: when every top-level x-* extension -// fails to serialize, Lower ends up with an empty Unmodeled map, so the -// warning must come from the extension reader rather than from a branch guarded -// on what was kept — the shape that used to drop it silently. +// auth's TestAuth_UnserializableExtensionStillWarns: when every top-level x-* +// extension fails to serialize, Lower ends up with an empty Unmodeled map, so +// the warning must come from the extension reader rather than from a branch +// guarded on what was kept — the shape that used to drop it silently. func TestMeta_UnserializableExtensionStillWarns(t *testing.T) { t.Parallel() spec := `openapi: 3.1.0 From a6cbccab56124cf62803194a22ce545df7fae0a8 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 19:06:28 +0300 Subject: [PATCH 16/21] docs(compilers/openapi): qualify names comments borrow from elsewhere serverName likened its hint to operationName, which is the operation package's. Recognize and its budget test named the refusal Load gives, which is load.Load's. Two detect test comments said what Detect passes, which is the compiler's, and a document test said Load's tests for the load package's. Each now names where the thing lives, as serverVariables already says annotation.XMLHints. --- compilers/openapi/internal/detect/helpers_test.go | 4 ++-- compilers/openapi/internal/detect/recognize.go | 6 +++--- compilers/openapi/internal/detect/recognize_test.go | 9 +++++---- compilers/openapi/internal/document/servers.go | 11 ++++++----- compilers/openapi/internal/document/servers_test.go | 2 +- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/compilers/openapi/internal/detect/helpers_test.go b/compilers/openapi/internal/detect/helpers_test.go index e870c7e7..1613d76e 100644 --- a/compilers/openapi/internal/detect/helpers_test.go +++ b/compilers/openapi/internal/detect/helpers_test.go @@ -7,8 +7,8 @@ import ( "github.com/dexpace/morphic/ir" ) -// recognize is Recognize on source 0, the one source Detect hands it, with no -// byte budget: no case here comes near the default, and +// recognize is Recognize on source 0, the one source the compiler's Detect +// hands it, with no byte budget: no case here comes near the default, and // TestRecognize_HoldsTheParseToItsByteBudget sets one of its own. func recognize(src string) (compilers.Recognition, []ir.Diagnostic, bool) { return Recognize(0, []byte(src), 0) diff --git a/compilers/openapi/internal/detect/recognize.go b/compilers/openapi/internal/detect/recognize.go index 9ebd3522..cc700857 100644 --- a/compilers/openapi/internal/detect/recognize.go +++ b/compilers/openapi/internal/detect/recognize.go @@ -13,9 +13,9 @@ import ( // the parse that read them. maxBytes bounds that parse, zero being none. // // Two sources are declined with an error whatever their format: one past -// maxBytes, unread, with the refusal Load gives it, and one whose root merges -// past the depth the read expands, its own keys leaving the version open -// (settledByOwnPairs). Any other decline is silent unless the source is +// maxBytes, unread, with the refusal load.Load gives it, and one whose root +// merges past the depth the read expands, its own keys leaving the version +// open (settledByOwnPairs). Any other decline is silent unless the source is // unreadable and declares a version where a document declares one // (declaresProbeKey). func Recognize(srcIndex int, data []byte, maxBytes int) (compilers.Recognition, []ir.Diagnostic, bool) { diff --git a/compilers/openapi/internal/detect/recognize_test.go b/compilers/openapi/internal/detect/recognize_test.go index 08e9ab34..56015631 100644 --- a/compilers/openapi/internal/detect/recognize_test.go +++ b/compilers/openapi/internal/detect/recognize_test.go @@ -19,8 +19,8 @@ import ( // TestRecognize_HoldsTheParseToItsByteBudget pins the bound on what recognition // reads: the parse is of the whole source, so a source past maxBytes is -// declined unread, with the refusal Load gives the same source. One at the -// budget is read, and so is any source under no budget at all. +// declined unread, with the refusal load.Load gives the same source. One at +// the budget is read, and so is any source under no budget at all. func TestRecognize_HoldsTheParseToItsByteBudget(t *testing.T) { t.Parallel() const src = "openapi: 3.1.0\ninfo: {title: T, version: \"1\"}\npaths: {}\n" @@ -40,8 +40,9 @@ func TestRecognize_HoldsTheParseToItsByteBudget(t *testing.T) { } // TestRecognize_ADeclineNamesTheSourceItWasHanded holds every decline that -// says why to the index its caller passed. Detect passes 0, so a recognition -// that named source 0 whatever it was handed would pass every other test here. +// says why to the index its caller passed. The compiler's Detect passes 0, so +// a recognition that named source 0 whatever it was handed would pass every +// other test here. func TestRecognize_ADeclineNamesTheSourceItWasHanded(t *testing.T) { t.Parallel() const srcIndex = 3 diff --git a/compilers/openapi/internal/document/servers.go b/compilers/openapi/internal/document/servers.go index bff9fbf6..ce5f9a47 100644 --- a/compilers/openapi/internal/document/servers.go +++ b/compilers/openapi/internal/document/servers.go @@ -61,11 +61,12 @@ func lowerServer(c lowering.Ctx, s *soa.Server, sptr jsontext.Pointer) (ir.Serve } // serverName builds a server's neutral naming: the declared name (only OpenAPI -// 3.2 has one), else a hint derived from the URL template, as operationName does -// for an operation with no operationId (GitHub #258). It names by the URL, not -// the position in servers[], which shifts when the list is reordered, and by the -// whole template, not a part: choosing a part is a policy and drops what the -// rest distinguishes, since one host serving /v1 and /v2 is two servers. +// 3.2 has one), else a hint derived from the URL template, as operation's +// operationName does for an operation with no operationId (GitHub #258). It +// names by the URL, not the position in servers[], which shifts when the list +// is reordered, and by the whole template, not a part: choosing a part is a +// policy and drops what the rest distinguishes, since one host serving /v1 and +// /v2 is two servers. // // Distinct servers can collide (".../v1" and ".../v-1" reduce to one word // sequence), as two enum members can; an emitter uniquifies. diff --git a/compilers/openapi/internal/document/servers_test.go b/compilers/openapi/internal/document/servers_test.go index 99e9ad6c..a8b4fa8d 100644 --- a/compilers/openapi/internal/document/servers_test.go +++ b/compilers/openapi/internal/document/servers_test.go @@ -14,7 +14,7 @@ import ( // TestDuplicateServerName_BothServersLower is GitHub #631's repro compiled end // to end: the repeat is reported, a warning at its own name naming the first, // and nothing is dropped or merged, each server lowering with the name it -// declares. Load's tests cover every list the rule reads. +// declares. The load package's tests cover every list the rule reads. func TestDuplicateServerName_BothServersLower(t *testing.T) { t.Parallel() doc, diags := parseFull(t, `openapi: 3.2.0 From 56b4a523b67f6337dddce5b6f666bfc6c3fbd9cc Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 20:14:39 +0300 Subject: [PATCH 17/21] refactor(compilers/openapi): read the root's own keys through nodeview settledByOwnPairs walked the root mapping's raw pairs by hand, asking nodeview.KeyOf for each key, to learn which keys the root writes itself. nodeview.WrittenChildren, which #1106 added, answers exactly that: the keys a mapping writes, no merge read, each named as KeyOf names it. The probe now asks it, so the compiler has one reading of a mapping's own keys; #1106 left this site alone only because detection was moving. Nothing held the function to its own keys. Reading the root's keys through its merges instead passed every test, because the row named "a merged version a deeper one may outrank" defined that version but never merged it, and so repeated "a chain past it". It now merges the version after the chain, where the read finds it and the bound may hide one that outranks it, and the merged reading turns it red. --- compilers/openapi/internal/detect/probe.go | 11 ++++------- compilers/openapi/internal/detect/probe_test.go | 9 +++++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/compilers/openapi/internal/detect/probe.go b/compilers/openapi/internal/detect/probe.go index ebe65826..9bda9a88 100644 --- a/compilers/openapi/internal/detect/probe.go +++ b/compilers/openapi/internal/detect/probe.go @@ -131,13 +131,10 @@ func probeFromMapping(root *yaml.Node) (sniffProbe, error) { // when that value is a version, and with `swagger` written too whatever it is. // A key root leaves to its merges may lie past the bound. func settledByOwnPairs(root *yaml.Node, probe sniffProbe) bool { - own := map[string]bool{} - for i := 0; i+1 < len(root.Content); i += 2 { - if key, ok := nodeview.KeyOf(root.Content[i]); ok { - own[key] = true - } - } - return own["openapi"] && (isVersion(probe.OpenAPI) || own["swagger"]) + own := nodeview.WrittenChildren(root) + _, writesOpenAPI := own["openapi"] + _, writesSwagger := own["swagger"] + return writesOpenAPI && (isVersion(probe.OpenAPI) || writesSwagger) } // probeVersion returns the version string a probe key's value declares, and an diff --git a/compilers/openapi/internal/detect/probe_test.go b/compilers/openapi/internal/detect/probe_test.go index b0e69d02..f46b595a 100644 --- a/compilers/openapi/internal/detect/probe_test.go +++ b/compilers/openapi/internal/detect/probe_test.go @@ -1,6 +1,7 @@ package detect import ( + "fmt" "strings" "testing" @@ -70,6 +71,11 @@ func TestSniff_ReadsALargeDocumentWhole(t *testing.T) { func TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt(t *testing.T) { t.Parallel() const limit = nodeview.MergeDepthLimit + // A version merged in beside the chain, listed after it, so the chain's + // keys outrank it: the read finds it, and what the bound hid may still win. + beside := strings.Replace(string(openapitest.RootMergeChain(limit+1, "openapi: 3.1.0", "")), + fmt.Sprintf("<<: *a%d\n", limit), fmt.Sprintf("s: &s {openapi: 3.0.3}\n<<: [*a%d, *s]\n", limit), 1) + require.Contains(t, beside, "<<: [*a", "the root merges the chain and then s") cases := map[string]struct { src []byte version string @@ -79,8 +85,7 @@ func TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt(t *testing.T) { "a chain past it": {openapitest.RootMergeChain(limit+1, "openapi: 3.0.3", ""), "", true}, "a version the root writes itself": { openapitest.RootMergeChain(limit+1, "openapi: 3.0.3", "openapi: 3.1.0\n"), "3.1.0", false}, - "a merged version a deeper one may outrank": { - []byte(string(openapitest.RootMergeChain(limit+1, "openapi: 3.1.0", "")) + "s: &s {openapi: 3.0.3}\n"), "", true}, + "a merged version a deeper one may outrank": {[]byte(beside), "", true}, "a root writing no version and a swagger one": { openapitest.RootMergeChain(limit+1, "openapi: 3.0.3", "openapi: prose words\nswagger: '2.0'\n"), "", false}, "a root writing no version and leaving swagger to its merges": { From 64f837cdf7053116a851040a186c39d1f201ecc9 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 20:14:51 +0300 Subject: [PATCH 18/21] refactor(compilers/openapi): read jsonSchemaDialect as its siblings do keepJSONSchemaDialect unwrapped the document node before asking nodeview.Child, which unwraps it itself. Every other caller, its sibling keepComponentMediaTypes included, hands Child the root node as the parser gives it; this one now does too. --- compilers/openapi/internal/document/dialect.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compilers/openapi/internal/document/dialect.go b/compilers/openapi/internal/document/dialect.go index 5a8639e6..85253980 100644 --- a/compilers/openapi/internal/document/dialect.go +++ b/compilers/openapi/internal/document/dialect.go @@ -16,7 +16,7 @@ import ( // read in the OpenAPI dialect whatever it names. func keepJSONSchemaDialect(c lowering.Ctx, p *ir.Unmodeled) []ir.Diagnostic { at := ids.Ptr("jsonSchemaDialect") - node := nodeview.Child(nodeview.DocumentRoot(c.Doc.GetRootNode()), "jsonSchemaDialect") + node := nodeview.Child(c.Doc.GetRootNode(), "jsonSchemaDialect") kept, diags := annotation.PreserveNodeInto(p, annotation.EntryKey("", "jsonSchemaDialect"), node, ir.ReasonOutOfScope, c.ProvenanceAt(at)) if !kept { From a523663fe5e003c7178d376febd0ca66af35e65c Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 20:14:39 +0300 Subject: [PATCH 19/21] docs(compilers/openapi): say which roots past the merge bound are kept The yaml-decoding rule declined a root that merges past the depth detection reads only when it writes no version of its own. One that writes swagger itself is declined too: detection reads openapi first, so an openapi a merge past the bound may hold would outrank it. The rule now states the condition settledByOwnPairs checks, that the root writes openapi with a version or writes both keys, and that the decline holds whatever the source's format. A row of TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt pins the swagger case, and the rule names that test. --- .../openapi/internal/detect/probe_test.go | 2 ++ docs/compilers/openapi.md | 23 +++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/compilers/openapi/internal/detect/probe_test.go b/compilers/openapi/internal/detect/probe_test.go index f46b595a..f81ad4e3 100644 --- a/compilers/openapi/internal/detect/probe_test.go +++ b/compilers/openapi/internal/detect/probe_test.go @@ -90,6 +90,8 @@ func TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt(t *testing.T) { openapitest.RootMergeChain(limit+1, "openapi: 3.0.3", "openapi: prose words\nswagger: '2.0'\n"), "", false}, "a root writing no version and leaving swagger to its merges": { openapitest.RootMergeChain(limit+1, "swagger: '2.0'", "openapi: prose words\n"), "", true}, + "a root writing swagger and leaving openapi to its merges": { + openapitest.RootMergeChain(limit+1, "x: 1", "swagger: '2.0'\n"), "", true}, } for name, tc := range cases { t.Run(name, func(t *testing.T) { diff --git a/docs/compilers/openapi.md b/docs/compilers/openapi.md index 36f9c113..384bb834 100644 --- a/docs/compilers/openapi.md +++ b/docs/compilers/openapi.md @@ -58,21 +58,24 @@ as an `openapi/undecodable-source` error, and nothing compiles. A YAML stream lo its documents that holds content; the documents after it that hold content reach the IR in no form, reported once as an `openapi/stream-documents-dropped` error at the first of them, and the first still lowers. A document holding nothing, a bare separator or an explicit null, is not counted. -Detection reads the version off the root mapping as the parser does, through aliases, `<<` merges and -a repeated key's last value, so the compile reads the version the source is routed by. A root that -merges deeper than that reading expands, writing no version of its own, is declined as an -`openapi/budget-exceeded` error, since a version past the bound may outrank one found above it. A -merge cycle hides no key, and is the compile's to refuse. A -mapping tagged anything but `!!map`, which OpenAPI's limit on YAML tags forbids, refuses the source -before the parser reads it, as an `openapi/tagged-mapping` error at the tagged mapping with the -least pointer, and it compiles to no Document. A tagged scalar is not refused here; what it lowers -to is [`values`](#values)'. +Detection reads the version off the root mapping as the parser does, through aliases, `<<` merges +and a repeated key's last value, so the compile reads the version the source is routed by. A root +that merges deeper than that reading expands is declined as an `openapi/budget-exceeded` error, +whatever its format, since a version past the bound may outrank one found above it, unless the +root's own keys settle the version: it writes `openapi` with a version, or writes both keys. A +`swagger` it writes alone settles nothing, as an `openapi` past the bound would outrank it. A merge +cycle hides no key, and is the compile's to refuse. A mapping tagged anything but `!!map`, which +OpenAPI's limit on YAML tags forbids, refuses the source before the parser reads it, as an +`openapi/tagged-mapping` error at the tagged mapping with the least pointer, and it compiles to no +Document. A tagged scalar is not refused here; what it lowers to is [`values`](#values)'. - Codes: `openapi/undecodable-source`, `openapi/stream-documents-dropped`, `openapi/tagged-mapping`, `openapi/budget-exceeded` - Tests: `detect.TestRecognize_ABrokenDocumentIsReportedAtAnySize`, `openapi.TestDetect_ReadsTheVersionTheCompileReads`, - `detect.TestRecognize_AReadCutShortIsReported`, `detect.TestRecognize_AMergeCycleHidesNoKey`, + `detect.TestRecognize_AReadCutShortIsReported`, + `detect.TestSniff_ReadsAMergeChainAsFarAsTheViewExpandsIt`, + `detect.TestRecognize_AMergeCycleHidesNoKey`, `load.TestLoad_AStreamLowersItsFirstDocumentAndReportsTheRest`, `load.TestLoad_ATaggedMappingIsRefusedBeforeParsing` - Status: partly built: a JSON source holding a surrogate-pair escape is refused as undecodable From 335a75b330720de21c8dd68a6bc3cdcd5dccf4dc Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 20:14:39 +0300 Subject: [PATCH 20/21] docs(architecture): list every package under the OpenAPI compiler The layout listed the compiler's internal packages as "its own packages", but left out anchors, which #1106 added, and fragment, navigation, overlay, sourceindex and spelling before it. nodeview's line still read "as the resolver reads it", where its own package comment now says the parser's reading and one bounded walk. The list now names every package, the test-only ones on a line of their own, in an order where each comes after every package it imports, which value importing nodeview and load importing resolve had broken. --- docs/architecture.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 678c32b8..775e0821 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -388,22 +388,29 @@ morphic/ │ │ └── internal/ # Its own packages, each below that face: │ │ ├── diag/ # diagnostic codes and the constructor │ │ ├── ids/ # pointer construction; pointer → TypeID +│ │ ├── fragment/ # a $ref fragment read as a pointer, and spelled for the resolver +│ │ ├── navigation/ # a pointer read through the parsed model as the library reads it +│ │ ├── spelling/ # the primitive kinds OpenAPI spells +│ │ ├── nodeview/ # the raw YAML as the parser reads it, and one bounded walk │ │ ├── value/ # scalar and BigVal lowering -│ │ ├── nodeview/ # the raw source as the resolver reads it +│ │ ├── sourceindex/ # what one walk of a decoded source tree answers │ │ ├── scan/ # pre-parse refusals (alias cycles and fan-out, nesting) │ │ ├── bind/ # every schema $ref bound where it lands, loops found │ │ ├── annotation/ # what a schema says about itself, not its shape │ │ ├── oas/ # the OpenAPI Specification's rules by minor, and their check +│ │ ├── overlay/ # an Overlay applied, and the positions it answers for +│ │ ├── resolve/ # what a $ref names; reference-or-inline entries │ │ ├── load/ # parse, validate, resolve │ │ ├── detect/ # a source recognized from its bytes, on load's parse -│ │ ├── resolve/ # what a $ref names; reference-or-inline entries │ │ ├── merge/ # allOf property reconciliation │ │ ├── naming/ # each node's name, from its coordinate alone +│ │ ├── anchors/ # the $dynamicAnchor index a $dynamicRef expands against │ │ ├── lowering/ # the immutable context every lowering reads │ │ ├── schema/ # the schema walk, composition, preservation │ │ ├── auth/ # security schemes and requirements │ │ ├── operation/ # path items, webhooks, callbacks, params, content -│ │ └── document/ # info, servers, and what the document keeps verbatim +│ │ ├── document/ # info, servers, and what the document keeps verbatim +│ │ └── canary/ openapitest/ ynode/ # tests only: library canaries, fixtures, YAML nodes │ ├── typespec/ smithy/ graphql/ asyncapi/ protobuf/ otp/ (future) ├── pass/ # Layer 1 — IR → IR passes (validate, dedup, filter, slice, overlay). ├── emitters/ # Layer 2 — emitter contract, plan layer, registry (future). From 44ed4779212d9152dd253d2ca2e6b5237ec03d68 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Fri, 23 Oct 2026 20:28:37 +0300 Subject: [PATCH 21/21] docs: name every package at the top of a compiler's internal order CLAUDE.md said the order among a compiler's internal packages runs from diag up to operation. operation was the one package nothing else internal imports while the root held document lowering and detection; document and detect sit beside it at the top now, so the line names all three. --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index ad23f981..90c39030 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -130,7 +130,7 @@ compilers/ Layer 1 — the Compiler contract and the format-keyed registry. Im other, never emitters/engine. internal/ Layer 1 — that compiler's own packages, each with its own allowlist rather than the compiler's. The ordering among them is real and enforced, from diag (reaches - only ir) up to operation. None may reach the compiler above it. + only ir) up to operation, document and detect. None may reach the compiler above it. pass/ Layer 1 — IR → IR passes. Imports ir and ir/irfacts. emitters/* Layer 2 — imports ir, ir/irfacts + emitter contract; never compiler. (Not built yet.) engine/ Layer 3 — orchestration; imports everything below. It owns every read, write and