diff --git a/compilers/openapi/internal/annotation/annotation.go b/compilers/openapi/internal/annotation/annotation.go index 518973bc..a2125148 100644 --- a/compilers/openapi/internal/annotation/annotation.go +++ b/compilers/openapi/internal/annotation/annotation.go @@ -544,7 +544,7 @@ func Read(st Site, pointer string, srcIndex int) (Set, []ir.Diagnostic) { out.Examples = examples ext, extDiags := ExtensionsFrom(st.Node.GetExtensions(), srcIndex, pointer) - sub, subDiags := subObjectExtensions(st.Node, pointer, srcIndex) + sub, subDiags := subObjectKeys(st.Node, pointer, srcIndex) kept, keptDiags := unmodeledAt(st.Node, pointer, srcIndex) diags := make([]ir.Diagnostic, 0, len(exDiags)+len(extDiags)+len(subDiags)+len(keptDiags)) @@ -557,27 +557,37 @@ func Read(st Site, pointer string, srcIndex int) (Set, []ir.Diagnostic) { return out, diags } -// subObjectExtensions collects the x-* the sub-objects of a schema declare — -// its xml, its discriminator and its externalDocs. Each is an OpenAPI object -// that admits extensions, and none of ir.XMLHints, ir.Discriminator or ir.Link -// holds an Unmodeled map, so the entries ride on the node the schema itself -// lowers to; the keyword each was written under is what keeps three objects' -// extensions apart on that one map. -func subObjectExtensions(s *oas3.Schema, pointer string, srcIndex int) (ir.Unmodeled, []ir.Diagnostic) { +// subObjectKeys collects what the sub-objects of a schema declare that reaches +// no IR field — the x-* they carry and the keys the specification defines for +// none of them — over its xml, its discriminator and its externalDocs. +// +// Each is an OpenAPI object with its own closed key set, and none of +// ir.XMLHints, ir.Discriminator or ir.Link holds an Unmodeled map, so the +// entries ride on the node the schema itself lowers to; the keyword each was +// written under is what keeps three objects' entries apart on that one map. +// +// The census is graded as an OpenAPI object's rather than as a schema keyword's, +// even though these hang off a schema: the JSON Schema rule that an unrecognized +// keyword is legal governs the schema itself, and these three are OpenAPI +// objects that the schema vocabulary says nothing about. +func subObjectKeys(s *oas3.Schema, pointer string, srcIndex int) (ir.Unmodeled, []ir.Diagnostic) { subs := []struct { keyword string + obj any ext *extensions.Extensions }{ - {"xml", s.GetXML().GetExtensions()}, - {"discriminator", s.GetDiscriminator().GetExtensions()}, - {"externalDocs", s.GetExternalDocs().GetExtensions()}, + {"xml", s.GetXML(), s.GetXML().GetExtensions()}, + {"discriminator", s.GetDiscriminator(), s.GetDiscriminator().GetExtensions()}, + {"externalDocs", s.GetExternalDocs(), s.GetExternalDocs().GetExtensions()}, } var out ir.Unmodeled var diags []ir.Diagnostic for _, sub := range subs { - ext, extDiags := ExtensionsUnder(sub.ext, srcIndex, pointer+ids.Ptr(sub.keyword), sub.keyword) + owner := pointer + ids.Ptr(sub.keyword) + ext, extDiags := ExtensionsUnder(sub.ext, srcIndex, owner, sub.keyword) out = MergeUnmodeled(out, ext) diags = append(diags, extDiags...) + diags = append(diags, UnknownKeysUnder(&out, sub.obj, srcIndex, owner, sub.keyword)...) } return out, diags } diff --git a/compilers/openapi/internal/annotation/unknown.go b/compilers/openapi/internal/annotation/unknown.go new file mode 100644 index 00000000..bab5accd --- /dev/null +++ b/compilers/openapi/internal/annotation/unknown.go @@ -0,0 +1,207 @@ +package annotation + +import ( + "reflect" + "slices" + + oas3 "github.com/speakeasy-api/openapi/jsonschema/oas3" + yaml "gopkg.in/yaml.v3" + + "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/compilers/openapi/internal/ids" + "github.com/dexpace/morphic/ir" +) + +// MaxUnknownKeys bounds how many keys one object contributes to the IR. +// +// The key set is the document's to choose the size of, and every collection in +// this compiler is bounded, so this one is too. It sits far above what a +// document writes by accident, so an object reaching it is generated or hostile +// rather than merely sloppy, and what it discards is announced under +// diag.UnknownKeyBudget rather than dropped in silence. +const MaxUnknownKeys = 64 + +// DecidedKeywords are the JSON Schema keywords the library's schema model names +// no field for and this compiler has already decided about, so the census must +// not claim them as unread. Each decision is recorded where it was made, and the +// schema walk's 2020-12 vocabulary test fails if one starts being carried: +// +// - $comment — 2020-12 §8.3 forbids presenting it to end users, so no SDK +// emitter may see it. Dropped on purpose. +// - $dynamicAnchor — read by the anchor index as a reference target, which is +// what lets a $dynamicRef expand; declaring one says nothing about the shape. +// - $dynamicRef — carried by the dynamic-reference lowering, which either +// expands it into the position's type or keeps it under a reason of its own. +// An entry beside an expanded one would tell a consumer the compiler ignored +// a reference it had in fact resolved. +// +// The other 2020-12 keywords with no field of their own — $vocabulary and +// dependentRequired — need no entry here. Their readers write to the same map, +// so the census finds them already recorded and leaves them alone. +var DecidedKeywords = []string{"$comment", "$dynamicAnchor", "$dynamicRef"} + +// UnknownKeywordsIn records on p the keywords s writes that no field of the JSON +// Schema model names, and announces each. +// +// OpenAPI 3.1 schemas are JSON Schema 2020-12, where an unrecognized keyword is +// legal input: the specification requires an implementation to ignore what it +// does not recognize and allows such a keyword to carry meaning for other +// tooling. So this reports a decision rather than a fault, and is graded +// accordingly — see diag.UnknownSchemaKeyword. +// +// It keeps only what no other reader kept, which is why it runs after all of +// them: `$vocabulary` and `dependentRequired` have no field in the model either +// and are read straight off the raw node by readers with more to say about them, +// so the census finds those already recorded and leaves them alone. A keyword no +// reader leaves a trace of needs naming in DecidedKeywords instead. +func UnknownKeywordsIn(p *ir.Unmodeled, s *oas3.Schema, pointer string, srcIndex int) []ir.Diagnostic { + return census(p, s, srcIndex, pointer, "", keyClass{ + code: diag.UnknownSchemaKeyword, + severity: ir.SeverityInfo, + skip: DecidedKeywords, + message: "keyword %q has no field in the schema model this compiler lowers and no IR " + + "position of its own; kept verbatim under Unmodeled", + }) +} + +// UnknownKeysIn records on p the keys an OpenAPI object writes that the +// specification neither defines nor admits as an extension, for an object +// lowering to a node with an Unmodeled map of its own. owner is the object's own +// source pointer. +// +// Unlike its schema neighbour this reports a fault: OpenAPI gives each of its +// objects a closed key set and requires every extension to be prefixed x-, so a +// key that is neither is nothing the document is permitted to write — in +// practice a misspelling of the field beside it. It is kept all the same, +// because invariant 2 does not bend for invalid input, and a misspelt key is the +// one a reader most needs to find. +func UnknownKeysIn(p *ir.Unmodeled, model any, srcIndex int, owner string) []ir.Diagnostic { + return UnknownKeysUnder(p, model, srcIndex, owner, "") +} + +// UnknownKeysUnder is UnknownKeysIn with every entry keyed beneath scope, for +// the objects with no Unmodeled map of their own, whose keys ride on the nearest +// node that has one — an info object's on the document, a tag's on the document. +// +// scope says which object wrote them: the source path from the carrier down to +// the object. Several objects reach one map, where "openapi:status" from two of +// them would be a single key and the entry that survived would depend on which +// lowering ran last. +func UnknownKeysUnder(p *ir.Unmodeled, model any, srcIndex int, owner, scope string) []ir.Diagnostic { + return census(p, model, srcIndex, owner, scope, keyClass{ + code: diag.UnknownObjectKey, + severity: ir.SeverityWarning, + message: "key %q is not defined by the OpenAPI object it is written on and is not an " + + "x- extension; kept verbatim under Unmodeled", + }) +} + +// keyClass is how a key the model does not name is graded: which diagnostic +// announces it, and at what severity. +// +// The reason is not part of it. Both classes carry ReasonOutOfScope, because +// that is a property of the construct rather than of the document: no IR node is +// coming for a key the format does not define, nor for one a schema dialect +// defines and this compiler does not model, so an emitter policy layer is the +// only consumer either has. Which of the two a key is says something about the +// source, and the diagnostic channel is where this compiler says that. +type keyClass struct { + code string + severity ir.Severity + skip []string // keywords already decided about; see DecidedKeywords + message string // one %q, filled with the key +} + +// census records on p every key model's source object wrote that its own model +// names no field for, each under its own key beneath scope. +// +// A key p already holds is left alone and not announced: the census is the +// complement of everything the compiler read, not only of what the model names, +// and a reader with a reason of its own for a keyword has already said it +// better. +func census(p *ir.Unmodeled, model any, srcIndex int, owner, scope string, cl keyClass) []ir.Diagnostic { + keys, root := undeclaredKeys(model) + if len(keys) == 0 { + return nil + } + var diags []ir.Diagnostic + if len(keys) > MaxUnknownKeys { + diags = append(diags, budgetDiag(len(keys), owner, srcIndex)) + keys = keys[:MaxUnknownKeys] + } + for _, key := range keys { + entry := "openapi:" + scoped(scope, key) + if _, recorded := (*p)[entry]; recorded || slices.Contains(cl.skip, key) { + continue + } + at := owner + ids.Ptr(key) + kept, keptDiags := PreserveNodeInto(p, entry, RawChildNode(root, key), + ir.ReasonOutOfScope, at, srcIndex) + diags = append(diags, keptDiags...) + if !kept { + continue + } + diags = append(diags, diag.Newf(cl.severity, cl.code, + ir.Provenance{Source: srcIndex, Pointer: at}, cl.message, key)) + } + return diags +} + +// scoped spells one entry's key on the carrier holding it. +func scoped(scope, key string) string { + if scope == "" { + return key + } + return scope + "/" + key +} + +// budgetDiag reports the keys past MaxUnknownKeys, which reach the IR in no form. +func budgetDiag(total int, owner string, srcIndex int) ir.Diagnostic { + return diag.Newf(ir.SeverityWarning, diag.UnknownKeyBudget, + ir.Provenance{Source: srcIndex, Pointer: owner}, + "object writes %d keys its model names no field for, past the %d this compiler keeps; "+ + "the rest are represented in the IR in no form at all", total, MaxUnknownKeys) +} + +// parsedObject is the part of a parsed model the census reads: its core, which +// holds the census the unmarshaller took, and the mapping node the keys were +// written on. +// +// Declared here rather than taken from the library, so this package depends on +// the shape it uses rather than on the marshaller package, and so a test can +// drive the branches below with a model of its own. +type parsedObject interface { + GetCoreAny() any + GetRootNode() *yaml.Node +} + +// unknownReporter is a core model's own record of the keys it did not name. +type unknownReporter interface{ GetUnknownProperties() []string } + +// undeclaredKeys returns, sorted, the keys model's source object wrote that its +// model names no field for, and the mapping node they were written on. +// +// Sorted, and on a copy: the library fills that list from a parallel walk of the +// mapping under a mutex, so its order is neither source order nor stable, and +// the slice it hands back is the model's own. An unsorted read would order this +// compiler's diagnostics by something the source does not decide, which +// invariant 7 forbids. +// +// A model reporting no census yields nothing rather than panicking. The receiver +// may be a typed nil — an absent object is what the getters return for one the +// document omitted — and a promoted method on one of those dereferences it. +func undeclaredKeys(model any) ([]string, *yaml.Node) { + v := reflect.ValueOf(model) + if v.Kind() == reflect.Pointer && v.IsNil() { + return nil, nil + } + obj, ok := model.(parsedObject) + if !ok { + return nil, nil + } + core, ok := obj.GetCoreAny().(unknownReporter) + if !ok { + return nil, nil + } + return slices.Sorted(slices.Values(core.GetUnknownProperties())), obj.GetRootNode() +} diff --git a/compilers/openapi/internal/annotation/unknown_internal_test.go b/compilers/openapi/internal/annotation/unknown_internal_test.go new file mode 100644 index 00000000..764a402b --- /dev/null +++ b/compilers/openapi/internal/annotation/unknown_internal_test.go @@ -0,0 +1,212 @@ +package annotation + +import ( + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + yaml "gopkg.in/yaml.v3" + + "github.com/dexpace/morphic/ir" +) + +// TestUnknownKeywordsIn_KeepsWhatTheModelDoesNotName is the census at its own +// subject: a keyword with no field in the schema model reaches the IR under its +// own key, located at itself, and is announced at info. +func TestUnknownKeywordsIn_KeepsWhatTheModelDoesNotName(t *testing.T) { + t.Parallel() + s := schemaFromYAML(t, "type: string\nx-ray: kept\nnotAKeyword: 7\n") + + var got ir.Unmodeled + diags := UnknownKeywordsIn(&got, s, "/components/schemas/A", 3) + + require.Len(t, got, 1, "the x-* is the extension reader's, not the census's; got %v", got) + entry := got["openapi:notAKeyword"] + assert.Equal(t, ir.ReasonOutOfScope, entry.Reason) + assert.Equal(t, ir.RawValue("7"), entry.Value) + assert.Equal(t, ir.Provenance{Source: 3, Pointer: "/components/schemas/A/notAKeyword"}, entry.Provenance) + + require.Len(t, diags, 1) + assert.Equal(t, ir.SeverityInfo, diags[0].Severity) + assert.Equal(t, "openapi/unknown-schema-keyword", diags[0].Code) + assert.Contains(t, diags[0].Message, `"notAKeyword"`) +} + +// TestUnknownKeywordsIn_DecidedKeywordsAreLeftAlone holds the census to the +// decisions already recorded elsewhere. Each of these has no field in the schema +// model, so the census would otherwise claim all three and overrule a +// deliberate drop — or, for an expanded $dynamicRef, say the compiler ignored a +// reference it resolved. +func TestUnknownKeywordsIn_DecidedKeywordsAreLeftAlone(t *testing.T) { + t.Parallel() + assert.Equal(t, []string{"$comment", "$dynamicAnchor", "$dynamicRef"}, DecidedKeywords, + "a keyword joining or leaving the exclusion must be decided here too") + + var body strings.Builder + body.WriteString("type: string\n") + for _, keyword := range DecidedKeywords { + body.WriteString(keyword + ": v\n") + } + s := schemaFromYAML(t, body.String()) + + var got ir.Unmodeled + diags := UnknownKeywordsIn(&got, s, "/components/schemas/A", 0) + + assert.Empty(t, got, "each is decided about elsewhere, so the census keeps none of them") + assert.Empty(t, diags) +} + +// TestUnknownKeywordsIn_AlreadyRecordedKeyIsLeftAlone is why the census runs +// last. dependentRequired has no field in the model either and is kept by the +// validation-only reader under a reason that says what it is; a census that +// overwrote it would replace that with a weaker one and announce the keyword +// twice. +func TestUnknownKeywordsIn_AlreadyRecordedKeyIsLeftAlone(t *testing.T) { + t.Parallel() + s := schemaFromYAML(t, "type: object\ndependentRequired: {a: [b]}\n") + already := ir.UnmodeledEntry{ + Reason: ir.ReasonValidationOnly, + Value: ir.RawValue(`{"a":["b"]}`), + Provenance: ir.Provenance{Pointer: "/A/dependentRequired"}, + } + got := ir.Unmodeled{"openapi:dependentRequired": already} + + diags := UnknownKeywordsIn(&got, s, "/A", 0) + + assert.Equal(t, ir.Unmodeled{"openapi:dependentRequired": already}, got) + assert.Empty(t, diags, "a keyword another reader already announced is not announced twice") +} + +// TestUnknownKeywordsIn_UnpreservableValueIsReportedNotKept holds the census to +// GitHub #144's rule: a value that cannot be rendered as JSON keeps nothing, and +// says so, rather than announcing a preservation that did not happen. +func TestUnknownKeywordsIn_UnpreservableValueIsReportedNotKept(t *testing.T) { + t.Parallel() + s := schemaFromYAML(t, "type: string\nnotAKeyword: .nan\n") + + var got ir.Unmodeled + diags := UnknownKeywordsIn(&got, s, "/A", 0) + + assert.Empty(t, got) + require.Len(t, diags, 1) + assert.Equal(t, ir.SeverityError, diags[0].Severity) + assert.Equal(t, "openapi/unpreservable-construct", diags[0].Code) +} + +// TestUnknownKeysUnder_KeysBeneathTheScopeAndSorted pins the OpenAPI-object +// class: entries key under the path that says which object wrote them, the +// grading is a warning because the format admits no such key, and both the +// entries and the findings come out in key order. +// +// Sorted matters on its own. The library builds its census from a parallel walk +// of the mapping, so the order it hands back is neither source order nor stable, +// and diagnostics ordered by it would make the document non-deterministic +// (invariant 7). +func TestUnknownKeysUnder_KeysBeneathTheScopeAndSorted(t *testing.T) { + t.Parallel() + reported := []string{"zeta", "alpha"} + obj := fakeObject{core: &fakeCore{keys: reported}, root: parsedMapping(t, "zeta: 1\nalpha: 2\n")} + + var got ir.Unmodeled + diags := UnknownKeysUnder(&got, obj, 1, "/info/contact", "info/contact") + + assert.Equal(t, []string{"zeta", "alpha"}, reported, "the model's own slice is not reordered") + require.Len(t, got, 2) + assert.Equal(t, ir.RawValue("2"), got["openapi:info/contact/alpha"].Value) + assert.Equal(t, ir.Provenance{Source: 1, Pointer: "/info/contact/zeta"}, + got["openapi:info/contact/zeta"].Provenance) + + require.Len(t, diags, 2) + assert.Equal(t, ir.SeverityWarning, diags[0].Severity) + assert.Equal(t, "openapi/unknown-object-key", diags[0].Code) + assert.Contains(t, diags[0].Message, `"alpha"`, "the findings follow the sorted keys") + assert.Contains(t, diags[1].Message, `"zeta"`) +} + +// TestUnknownKeysIn_BudgetBoundsWhatOneObjectContributes exercises the bound. An +// object writing more undeclared keys than MaxUnknownKeys keeps exactly that +// many and reports the remainder, which is what stops the discarded tail from +// being the silent loss this census exists to end. +func TestUnknownKeysIn_BudgetBoundsWhatOneObjectContributes(t *testing.T) { + t.Parallel() + const over = MaxUnknownKeys + 3 + keys := make([]string, 0, over) + var body strings.Builder + for i := range over { + // Zero-padded, so sorting the census is sorting the source order too and the + // key that survives the truncation is a predictable one. + key := "k" + strconv.Itoa(1000+i) + keys = append(keys, key) + body.WriteString(key + ": " + strconv.Itoa(i) + "\n") + } + obj := fakeObject{core: &fakeCore{keys: keys}, root: parsedMapping(t, body.String())} + + var got ir.Unmodeled + diags := UnknownKeysIn(&got, obj, 0, "/x") + + assert.Len(t, got, MaxUnknownKeys) + assert.NotContains(t, got, "openapi:k"+strconv.Itoa(1000+over-1), "the tail past the bound is dropped") + require.NotEmpty(t, diags) + assert.Equal(t, "openapi/unknown-key-budget", diags[0].Code) + assert.Equal(t, ir.SeverityWarning, diags[0].Severity) + assert.Equal(t, ir.Provenance{Pointer: "/x"}, diags[0].Provenance) +} + +// TestUnknownKeysIn_ModelWithNoCensusRecordsNothing covers the shapes the reader +// must survive rather than panic on. The absent object is the one that occurs: +// the getters hand back a typed nil for an object the document omitted, and a +// promoted method on one of those dereferences it. +func TestUnknownKeysIn_ModelWithNoCensusRecordsNothing(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + model any + }{ + {"an object the document omitted", (*fakeObjectPtr)(nil)}, + {"an untyped nil", nil}, + {"a value that is no parsed model", 42}, + {"a model whose core keeps no census", fakeObject{core: "not a core"}}, + {"a model with an empty census", fakeObject{core: &fakeCore{}}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var got ir.Unmodeled + + diags := UnknownKeysIn(&got, tc.model, 0, "/x") + + assert.Nil(t, got) + assert.Empty(t, diags) + }) + } +} + +// fakeObject is a parsed model standing in for the library's, so the census can +// be driven at shapes no real document produces — an unsorted census, one past +// the bound, and a core that keeps none. +type fakeObject struct { + core any + root *yaml.Node +} + +func (f fakeObject) GetCoreAny() any { return f.core } +func (f fakeObject) GetRootNode() *yaml.Node { return f.root } + +// fakeObjectPtr is fakeObject's pointer-receiver twin, for the typed-nil case: +// a promoted method on a nil pointer is what the guard exists for, and a +// value-receiver method on a nil pointer would not reach it. +type fakeObjectPtr struct{ fakeObject } + +// fakeCore is a core model's census, reported exactly as given. +type fakeCore struct{ keys []string } + +func (c *fakeCore) GetUnknownProperties() []string { return c.keys } + +// parsedMapping parses body into the mapping node the census reads values from. +func parsedMapping(t *testing.T, body string) *yaml.Node { + t.Helper() + var doc yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(body), &doc)) + return &doc +} diff --git a/compilers/openapi/internal/auth/auth.go b/compilers/openapi/internal/auth/auth.go index e701ca3f..fdfc82af 100644 --- a/compilers/openapi/internal/auth/auth.go +++ b/compilers/openapi/internal/auth/auth.go @@ -132,40 +132,52 @@ func lowerSecurityScheme(c lowering.Ctx, name string, ss *soa.SecurityScheme, return ir.AuthScheme{}, false, []ir.Diagnostic{mechanismRefusalDiag(c, name, missing, entry)} } diags = preserveUnreadFields(c, &scheme, ss, decl) - return scheme, true, append(diags, applySchemeExtensions(c, &scheme, ss, decl)...) + diags = append(diags, applySchemeAnnotations(c, &scheme, ss, decl)...) + // Distinct from preserveUnreadFields above it: that keeps the fields OpenAPI + // defines for a securityScheme which this entry's own mechanism gives no + // meaning to, while this keeps the keys OpenAPI defines for no securityScheme + // at all. + return scheme, true, append(diags, + annotation.UnknownKeysIn(&scheme.Unmodeled, ss, c.SrcIndex, decl)...) } -// applySchemeExtensions keeps the x-* of the securitySchemes entry and, for an -// oauth2 scheme, of the flows object and of each flow inside it. ir.OAuthFlow +// applySchemeAnnotations keeps what the securitySchemes entry and, for an oauth2 +// scheme, the flows object and each flow inside it declare that reaches no IR +// field: their x-*, and the keys OpenAPI defines for none of them. ir.OAuthFlow // carries an Unmodeled map of its own; the flows object does not lower to a node -// at all, so its own extensions are kept on the scheme under the keyword. +// at all, so its entries are kept on the scheme under the keyword. // // Only oauth2 reads inside `flows`: on any other type the whole node is kept // verbatim by preserveUnreadFields, extensions and all, and no ir.OAuthFlow was // lowered for a flow's own to land on. -func applySchemeExtensions(c lowering.Ctx, scheme *ir.AuthScheme, ss *soa.SecurityScheme, decl string) []ir.Diagnostic { +func applySchemeAnnotations(c lowering.Ctx, scheme *ir.AuthScheme, ss *soa.SecurityScheme, decl string) []ir.Diagnostic { ext, diags := annotation.ExtensionsFrom(ss.GetExtensions(), c.SrcIndex, decl) scheme.Unmodeled = annotation.MergeUnmodeled(scheme.Unmodeled, ext) if scheme.Kind != ir.AuthKindOAuth2 { return diags } + flows := ss.GetFlows() flowsPtr := decl + ids.Ptr("flows") - flowsExt, flowsDiags := annotation.ExtensionsUnder(ss.GetFlows().GetExtensions(), c.SrcIndex, flowsPtr, "flows") + flowsExt, flowsDiags := annotation.ExtensionsUnder(flows.GetExtensions(), c.SrcIndex, flowsPtr, "flows") scheme.Unmodeled = annotation.MergeUnmodeled(scheme.Unmodeled, flowsExt) diags = append(diags, flowsDiags...) - return append(diags, applyFlowExtensions(c, scheme.Flows, ss.GetFlows(), flowsPtr)...) + diags = append(diags, annotation.UnknownKeysUnder(&scheme.Unmodeled, flows, c.SrcIndex, flowsPtr, "flows")...) + return append(diags, applyFlowAnnotations(c, scheme.Flows, flows, flowsPtr)...) } -// applyFlowExtensions writes each declared flow's own x-* onto the ir.OAuthFlow -// it lowered to. Both lists are one ordered reading of the same object — -// scheme.Flows came from oauthFlows, which walks presentFlows — so the i-th -// lowered flow is the i-th declared one and the two cannot differ in length. -func applyFlowExtensions(c lowering.Ctx, lowered []ir.OAuthFlow, flows *soa.OAuthFlows, flowsPtr string) []ir.Diagnostic { +// applyFlowAnnotations writes each declared flow's own x-* and undeclared keys +// onto the ir.OAuthFlow it lowered to. Both lists are one ordered reading of the +// same object — scheme.Flows came from oauthFlows, which walks presentFlows — so +// the i-th lowered flow is the i-th declared one and the two cannot differ in +// length. +func applyFlowAnnotations(c lowering.Ctx, lowered []ir.OAuthFlow, flows *soa.OAuthFlows, flowsPtr string) []ir.Diagnostic { var diags []ir.Diagnostic for i, f := range presentFlows(flows) { - ext, extDiags := annotation.ExtensionsFrom(f.src.GetExtensions(), c.SrcIndex, flowsPtr+ids.Ptr(f.keyword)) + fptr := flowsPtr + ids.Ptr(f.keyword) + ext, extDiags := annotation.ExtensionsFrom(f.src.GetExtensions(), c.SrcIndex, fptr) lowered[i].Unmodeled = annotation.MergeUnmodeled(lowered[i].Unmodeled, ext) diags = append(diags, extDiags...) + diags = append(diags, annotation.UnknownKeysIn(&lowered[i].Unmodeled, f.src, c.SrcIndex, fptr)...) } return diags } diff --git a/compilers/openapi/internal/diag/diag.go b/compilers/openapi/internal/diag/diag.go index 59027084..5a9a12b8 100644 --- a/compilers/openapi/internal/diag/diag.go +++ b/compilers/openapi/internal/diag/diag.go @@ -191,6 +191,40 @@ const ( // DegradedConstruct's constructs survive in a weaker shape, and these survive // in none (GitHub #144). UnpreservableConstruct = "openapi/unpreservable-construct" + // UnknownSchemaKeyword reports a JSON Schema keyword no field of the schema + // model names, kept verbatim under Unmodeled. + // + // Info, because the document did nothing wrong: JSON Schema requires an + // implementation to ignore a keyword it does not recognize, and says such a + // keyword may carry meaning for other tooling, so an unrecognized keyword is + // legal input rather than a defect. What is recorded is this compiler's own + // decision — that it read no meaning from the keyword and kept the text — which + // is the same thing ValidationOnlyKeyword records beside it. + UnknownSchemaKeyword = "openapi/unknown-schema-keyword" + // UnknownObjectKey reports a key on an OpenAPI object that the specification + // neither defines nor admits as an extension, kept verbatim under Unmodeled. + // + // Warning rather than info, because unlike its schema neighbour this one is a + // defect: OpenAPI gives its objects a closed key set and requires every + // extension to be prefixed x-, so a key that is neither is a document error — + // in practice a misspelling of the field beside it, which is precisely the + // class of mistake that survives when the compiler swallows the key in silence. + // + // Warning rather than error for the reason ReservedHeaderName is one: the + // document still lowers, everything the key was written beside is unaffected, + // and harness.Check stops at the first error diagnostic, which would hide every + // later finding in the same spec and make any fixture carrying a stray key + // unable to reach the invariant checks. + UnknownObjectKey = "openapi/unknown-object-key" + // UnknownKeyBudget reports an object declaring more keys the model does not + // name than the compiler keeps, so the ones past the bound reached the IR in no + // form at all. + // + // Every collection here is bounded, and this one is over a key set the document + // chooses the size of. The bound is far above what any document writes by + // accident, so tripping it is either a generated file or a hostile one; the + // diagnostic is what keeps the discarded remainder from being a silent loss. + UnknownKeyBudget = "openapi/unknown-key-budget" ) // Newf builds an ir.Diagnostic with a formatted message. It is the single diff --git a/compilers/openapi/internal/diag/diag_test.go b/compilers/openapi/internal/diag/diag_test.go index 26e351ac..8b172081 100644 --- a/compilers/openapi/internal/diag/diag_test.go +++ b/compilers/openapi/internal/diag/diag_test.go @@ -138,6 +138,7 @@ func codes() []string { diag.AliasAmplification, diag.UnattachableRequired, diag.InternalInvariant, diag.DuplicateOperationID, diag.IncompleteSecurityScheme, diag.ReservedHeaderName, diag.UnpreservableConstruct, + diag.UnknownSchemaKeyword, diag.UnknownObjectKey, diag.UnknownKeyBudget, } } diff --git a/compilers/openapi/internal/operation/content.go b/compilers/openapi/internal/operation/content.go index 67753d68..677f1109 100644 --- a/compilers/openapi/internal/operation/content.go +++ b/compilers/openapi/internal/operation/content.go @@ -89,7 +89,8 @@ func lowerContent(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex if len(ext) > 0 { content.Unmodeled = annotation.MergeUnmodeled(content.Unmodeled, ext) } - return content, diags + return content, append(diags, + annotation.UnknownKeysIn(&content.Unmodeled, media, c.SrcIndex, mediaPtr)...) } // fillSequential lowers 3.2 sequential-media fields: itemSchema becomes the @@ -354,9 +355,10 @@ func encodingConfig(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd // encodingUnmodeled keeps what an Encoding Object declares that nothing in the // IR holds: `allowReserved`, which ir.PartEncoding has no field for even though -// its neighbours style and explode do, and the object's own x-*. Neither had -// reached an IR field, an Unmodeled entry or a diagnostic, so two documents -// differing only in them compiled to one IR (GitHub #291). +// its neighbours style and explode do, the object's own x-*, and the keys the +// specification defines for no encoding at all. Neither allowReserved nor the +// extensions had reached an IR field, an Unmodeled entry or a diagnostic, so two +// documents differing only in them compiled to one IR (GitHub #291). // // Both ride on the owning ir.Content, since PartEncoding carries no Unmodeled // map, keyed under scope — "encoding/" or "itemEncoding". One content can @@ -377,7 +379,8 @@ func encodingUnmodeled(c lowering.Ctx, enc *soa.Encoding, encPtr, scope string) } ext, extDiags := schema.ExtensionsIn(c, enc.GetExtensions(), encPtr, scope) out = annotation.MergeUnmodeled(out, ext) - return out, append(diags, extDiags...) + diags = append(diags, extDiags...) + return out, append(diags, annotation.UnknownKeysUnder(&out, enc, c.SrcIndex, encPtr, scope)...) } // lowerHeaders lowers a header map into Properties in source order. Each @@ -555,7 +558,7 @@ func applyHeaderAnnotations(c lowering.Ctx, p *ir.Property, h *soa.Header, hdecl hExt, extDiags := schema.ExtensionsOf(c, h.GetExtensions(), hdecl) diags = append(diags, extDiags...) p.Unmodeled = annotation.MergeUnmodeled(p.Unmodeled, hExt) - return diags + return append(diags, annotation.UnknownKeysIn(&p.Unmodeled, h, c.SrcIndex, hdecl)...) } // exampleList lowers a single example node and a plural example map into value @@ -592,12 +595,14 @@ func exampleList(c lowering.Ctx, single *yaml.Node, plural *sequencedmap.Map[str func appendPluralExample(c lowering.Ctx, out []ir.Example, re *soa.ReferencedExample, pointer, name string) ([]ir.Example, []ir.Diagnostic) { // The declaration pointer, not the entry's, is where an Example Object's own // keywords are written: a $ref entry holds none of them. ir.Example carries an - // Unmodeled map, so the object's x-* need no scope. + // Unmodeled map, so neither the object's x-* nor its undeclared keys need a + // scope. ex, decl := resolve.ObjectAt[soa.Example](c.RefScope(), re, pointer+ids.Ptr("examples", name)) if ex == nil { return out, nil } ext, diags := schema.ExtensionsOf(c, ex.GetExtensions(), decl) + diags = append(diags, annotation.UnknownKeysIn(&ext, ex, c.SrcIndex, decl)...) proto := ir.Example{ Name: name, Summary: ex.GetSummary(), @@ -664,7 +669,7 @@ func lowerRequestBody(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorI "request body is not required; optionality kept under Unmodeled")) } // soa.RequestBody exposes no GetExtensions at this library version, so the - // field is read directly — as XMLHints already reads its own. The read sits + // field is read directly — as XMLHints already reads its own. Both reads sit // after the payload guard because ir.Payload is the body's only carrier: a // request body declaring no content lowers to nothing to hang them on, and // OpenAPI makes content REQUIRED there, so such a body is a defect in the @@ -672,6 +677,7 @@ func lowerRequestBody(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorI bodyExt, bodyExtDiags := schema.ExtensionsOf(c, rb.Extensions, bodyPtr) payload.Unmodeled = annotation.MergeUnmodeled(payload.Unmodeled, bodyExt) diags = append(diags, bodyExtDiags...) + diags = append(diags, annotation.UnknownKeysIn(&payload.Unmodeled, rb, c.SrcIndex, bodyPtr)...) op.Request = payload hb.RequestContentTypes = contentTypeKeys(rb.GetContent()) return diags diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index d9f007dd..583b964b 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -296,30 +296,38 @@ func lowerOperation(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd diags = append(diags, cbDiags...) } op.Bindings = ir.OpBindings{HTTP: []ir.HTTPBinding{hb}} - diags = append(diags, applyOperationExtensions(c, &op, src, decl)...) + diags = append(diags, applyOperationAnnotations(c, &op, src, decl)...) diags = append(diags, applyOperationServers(c, &op, src, decl)...) return op, extra, append(diags, checkOperationIDUnique(c, operationIDs, op, mount)...) } -// applyOperationExtensions keeps the operation's own x-* and those of the two -// objects beneath it that lower to no node of their own: its externalDocs, -// since ir.Link holds no Unmodeled map, and its Responses Object, whose -// extensions are the map's own rather than any one response's. +// applyOperationAnnotations keeps the operation's own x-* and undeclared keys, +// and those of the two objects beneath it that lower to no node of their own: +// its externalDocs, since ir.Link holds no Unmodeled map, and its Responses +// Object, whose extensions are the map's own rather than any one response's. +// +// The Responses Object contributes extensions but no census. Its key set is the +// status codes the document chooses, which the library models as a map, so an +// undeclared key there is read as one more response rather than reported as +// unknown — there is nothing for a census to say about it. // // It merges rather than assigns. The operation's map already carries whatever // its parameters or callbacks wrote by the time this runs, and an assignment // here would drop them — which is why the servers preservation used to have to // run after it. -func applyOperationExtensions(c lowering.Ctx, op *ir.Operation, src *soa.Operation, decl string) []ir.Diagnostic { +func applyOperationAnnotations(c lowering.Ctx, op *ir.Operation, src *soa.Operation, decl string) []ir.Diagnostic { + docsPtr := decl + ids.Ptr("externalDocs") ext, diags := annotation.ExtensionsAt(c.SrcIndex, annotation.ExtensionSite{Owner: decl, Ext: src.GetExtensions()}, - annotation.ExtensionSite{Scope: "externalDocs", Owner: decl + ids.Ptr("externalDocs"), + annotation.ExtensionSite{Scope: "externalDocs", Owner: docsPtr, Ext: src.GetExternalDocs().GetExtensions()}, annotation.ExtensionSite{Scope: "responses", Owner: decl + ids.Ptr("responses"), Ext: src.GetResponses().GetExtensions()}, ) op.Unmodeled = annotation.MergeUnmodeled(op.Unmodeled, ext) - return diags + diags = append(diags, annotation.UnknownKeysIn(&op.Unmodeled, src, c.SrcIndex, decl)...) + return append(diags, annotation.UnknownKeysUnder(&op.Unmodeled, + src.GetExternalDocs(), c.SrcIndex, docsPtr, "externalDocs")...) } // applyOperationServers preserves an operation's own `servers` verbatim under @@ -411,6 +419,25 @@ func fillOperationDocs(d *ir.Docs, src *soa.Operation) { // must reach both, and a second call beside the first is a second chance to // forget one on a route added later. That is exactly how the servers half came // to be missing on two of its three routes (GitHub #39). +// +// A path item takes no census, unlike every other object the compiler reads +// extensions from, and deliberately so. The library folds a key it does not +// recognize into the item's embedded operations map rather than recording it as +// undeclared, so GetUnknownProperties reports nothing and there is no census to +// read (speakeasy-api/openapi v1.24.0). Two consequences decide it: +// +// - The key is not lost in silence. Folding it produces a +// validation-type-mismatch at error severity naming the key at its own +// pointer, which is the losslessness property the census exists for; what is +// lost is the key's value, not the fact that it was written. +// - Recovering the value means reading the raw node against a path item's key +// vocabulary, and the only vocabulary this compiler owns is httpMethods, +// which is narrower than the library's — it has no `query`, the method +// OpenAPI 3.2 adds. A census over what httpMethods does not name would +// therefore report a valid 3.2 `query` operation as an undeclared key. +// +// That vocabulary is what GitHub #293 is about, so widening it here would settle +// that issue as a side effect of this one. Tracked separately in GitHub #377. func applyPathItem(c lowering.Ctx, op *ir.Operation, pi *soa.PathItem, declPtr string) []ir.Diagnostic { diags := applyPathServers(c, op, pi, declPtr) ext, extDiags := schema.ExtensionsIn(c, pi.GetExtensions(), declPtr, "pathItem") @@ -509,7 +536,8 @@ func lowerResponse(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde } // preserveResponseExtras keeps what a Response Object declares that has no home -// on the node it lowered to: its links map, and its own x-* extensions. +// on the node it lowered to: its links map, its own x-* extensions, and the keys +// the specification does not define at all. // // One helper for both branches on purpose. ir.Response and ir.ErrorCase are two // lowerings of the same source object, and each construct kept on only one of @@ -520,12 +548,21 @@ func lowerResponse(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde // The links entry carries ReasonNoIRHome and no diagnostic, as it always has: // nothing is degraded, the map is in the document, and the gap is one the IR can // close by growing a links field. +// +// A Link Object inside that map gets no entry and no census of its own, which is +// the decision already recorded for its extensions. This compiler lowers no Link +// Object anywhere: a response's links survive only as the verbatim node above, +// and a components/links entry nothing references is dropped whole, so a keyed +// entry at one of the two positions would be the only trace of a construct the +// IR does not model — while duplicating, for the response position alone, a +// value the node above already carries. func preserveResponseExtras(c lowering.Ctx, p *ir.Unmodeled, r *soa.Response, rptr string) []ir.Diagnostic { _, diags := schema.PreserveNode(c, p, "openapi:links", annotation.RawChildNode(r.GetRootNode(), "links"), ir.ReasonNoIRHome, rptr+ids.Ptr("links")) ext, extDiags := schema.ExtensionsOf(c, r.GetExtensions(), rptr) *p = annotation.MergeUnmodeled(*p, ext) - return append(diags, extDiags...) + diags = append(diags, extDiags...) + return append(diags, annotation.UnknownKeysIn(p, r, c.SrcIndex, rptr)...) } // responseName builds a success response's neutral naming. OpenAPI names no diff --git a/compilers/openapi/internal/operation/params.go b/compilers/openapi/internal/operation/params.go index e0875ee3..c7bcc3a6 100644 --- a/compilers/openapi/internal/operation/params.go +++ b/compilers/openapi/internal/operation/params.go @@ -201,7 +201,10 @@ func fillParamSchemaAnnotations(c lowering.Ctx, ts *compile.Types, param *ir.Par diags = append(diags, preserveParamXML(c, param, s, pointer)...) } param.Unmodeled = annotation.MergeUnmodeled(param.Unmodeled, a.Unmodeled) - return diags + // Last, per schema.PreserveUnknownKeywords: it keeps only the keywords no + // reader above it kept, and everything annotation.Read recorded is already on + // the parameter by this line. + return append(diags, schema.PreserveUnknownKeywords(c, ¶m.Unmodeled, s, pointer)...) } // preserveParamXML keeps a parameter schema's xml hints instead of dropping @@ -274,6 +277,7 @@ func fillParamDetail(c lowering.Ctx, param *ir.Parameter, p *soa.Parameter, pptr pExt, extDiags := schema.ExtensionsOf(c, p.GetExtensions(), pptr) diags = append(diags, extDiags...) param.Unmodeled = annotation.MergeUnmodeled(param.Unmodeled, pExt) + diags = append(diags, annotation.UnknownKeysIn(¶m.Unmodeled, p, c.SrcIndex, pptr)...) return append(diags, preserveAllowEmptyValue(c, param, p, pptr)...) } diff --git a/compilers/openapi/internal/schema/accumulate.go b/compilers/openapi/internal/schema/accumulate.go index 34f0262d..ea056307 100644 --- a/compilers/openapi/internal/schema/accumulate.go +++ b/compilers/openapi/internal/schema/accumulate.go @@ -73,6 +73,20 @@ func PreserveSchemaKeyword(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, keyw return PreserveNode(c, p, "openapi:"+keyword, annotation.RawPropertyNode(s, keyword), reason, pointer) } +// PreserveUnknownKeywords records every keyword s writes that no field of the +// schema model names and no reader above it already kept (GitHub #297). +// +// It is the last thing an attachment does, which is the contract +// annotation.UnknownKeywordsIn states and the reason it is called here rather +// than from inside annotation.Read with the other readers: $dynamicRef has no +// field in the schema model either and is decided by recordUnexpandedDynamicRef, +// which runs after Read and deliberately keeps nothing once the reference has +// expanded. A census running before it could not tell that from an unread +// keyword. +func PreserveUnknownKeywords(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, pointer string) []ir.Diagnostic { + return annotation.UnknownKeywordsIn(p, s, pointer, c.SrcIndex) +} + // preserveKeyword records a validation-only keyword's raw payload under key in // p and returns the one info diagnostic naming it at declPtr, the schema that // wrote it. An absent or unconvertible payload records nothing and returns diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index d06d1eb9..b5cc509f 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -894,7 +894,8 @@ func fillPropertyAnnotations(c lowering.Ctx, ts *compile.Types, anchors *AnchorI // nil node: this arm runs only when the schema lowered to no node of its own, // so nothing here can be carrying an Encoding. diags = append(diags, recordUnplacedContent(c, &p.Unmodeled, ref, nil, pointer)...) - return append(diags, recordUnexpandedDynamicRef(c, anchors, &p.Unmodeled, ref, pointer)...) + diags = append(diags, recordUnexpandedDynamicRef(c, anchors, &p.Unmodeled, ref, pointer)...) + return append(diags, PreserveUnknownKeywords(c, &p.Unmodeled, ref, pointer)...) } // LoweredToOwnNode reports whether the declaration at pointer lowered to a type @@ -978,7 +979,8 @@ func attachDeclaredAnnotations(c lowering.Ctx, ts *compile.Types, anchors *Ancho common.Examples = a.Examples } diags = append(diags, recordUnplacedContent(c, &common.Unmodeled, s, td, pointer)...) - return append(diags, recordUnexpandedDynamicRef(c, anchors, &common.Unmodeled, s, pointer)...) + diags = append(diags, recordUnexpandedDynamicRef(c, anchors, &common.Unmodeled, s, pointer)...) + return append(diags, PreserveUnknownKeywords(c, &common.Unmodeled, s, pointer)...) } // fillAdditional lowers additionalProperties, patternProperties, and diff --git a/compilers/openapi/meta.go b/compilers/openapi/meta.go index a282d889..ae6aafdb 100644 --- a/compilers/openapi/meta.go +++ b/compilers/openapi/meta.go @@ -47,7 +47,9 @@ func lowerMeta(c lowering.Ctx) (docMeta, []ir.Diagnostic) { servers, serverDiags := lowerServers(c) m.Servers = servers - return m, append(diags, serverDiags...) + diags = append(diags, serverDiags...) + // After the extensions assignment, which would otherwise overwrite the map. + return m, append(diags, documentUnknownKeys(c, &m.Unmodeled)...) } // documentExtensions collects the x-* of every object that lowers to no IR node @@ -104,6 +106,77 @@ func tagExtensions(c lowering.Ctx) []annotation.ExtensionSite { 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 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 keys are scoped by the source path they were written at. One unscoped +// "openapi:status" would be a single key for six objects, 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.SrcIndex, 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 string + 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 rather than a map with nothing to say: +// its own key set is the fixed list of component kinds, which the library models +// as named fields and takes a census over, and only the map *under* each of +// those keys is the document's to name. +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 { + 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 +} + // 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. @@ -163,13 +236,15 @@ func lowerServers(c lowering.Ctx) ([]ir.Server, []ir.Diagnostic) { func lowerServer(c lowering.Ctx, s *soa.Server, sptr string) (ir.Server, []ir.Diagnostic) { vars, diags := serverVariables(c, s, sptr) ext, extDiags := annotation.ExtensionsFrom(s.GetExtensions(), c.SrcIndex, sptr) - return ir.Server{ + out := ir.Server{ Name: serverName(s), URLTemplate: s.GetURL(), Description: ir.Docs{Description: s.GetDescription()}, Variables: vars, Unmodeled: ext, - }, append(diags, extDiags...) + } + diags = append(diags, extDiags...) + return out, append(diags, annotation.UnknownKeysIn(&out.Unmodeled, s, c.SrcIndex, sptr)...) } // serverName builds a server's neutral naming: the declared name when the source @@ -211,18 +286,20 @@ func serverVariables(c lowering.Ctx, s *soa.Server, sptr string) ([]ir.ServerVar if v == nil { continue } + 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. - ext, extDiags := annotation.ExtensionsFrom(v.Extensions, c.SrcIndex, - sptr+ids.Ptr("variables", name)) + ext, extDiags := annotation.ExtensionsFrom(v.Extensions, c.SrcIndex, vptr) diags = append(diags, extDiags...) - out = append(out, ir.ServerVariable{ + one := ir.ServerVariable{ Name: name, Default: v.GetDefault(), Enum: v.GetEnum(), Docs: ir.Docs{Description: v.GetDescription()}, Unmodeled: ext, - }) + } + diags = append(diags, annotation.UnknownKeysIn(&one.Unmodeled, v, c.SrcIndex, vptr)...) + out = append(out, one) } return out, diags } diff --git a/compilers/openapi/unknownkeys_test.go b/compilers/openapi/unknownkeys_test.go new file mode 100644 index 00000000..34f5269c --- /dev/null +++ b/compilers/openapi/unknownkeys_test.go @@ -0,0 +1,227 @@ +// This file covers one property: a key the OpenAPI model names no field for +// reaches the IR rather than vanishing between the parser and the lowering. It +// is separate from annotations_test.go because the subject is the complement of +// what the model names, not any one annotation. +package openapi_test // external test package — exercises only the public API + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/ir" +) + +// unknownKeysSpec reads the census fixture the corpus sweeps also drive, so the +// assertions below and the six oracles run over the same bytes: putting it under +// testdata is what gets it compiled in both declaration orders, round-tripped and +// verified, none of which a spec written inline reaches. +func unknownKeysSpec(t *testing.T) string { + t.Helper() + data, err := os.ReadFile(filepath.Join("..", "..", "testdata", "openapi", "unknown_keys.yaml")) + require.NoError(t, err) + return string(data) +} + +// TestUnknownKeys_KeptAtEveryObject holds the whole rule rather than the +// positions that happened to be noticed. A key the model does not name reached +// no IR field, no Unmodeled entry and no diagnostic at every one of these +// objects, so two documents differing only in it compiled to the same IR +// (GitHub #297). +// +// Carriers are derived from the value graph rather than named: each row says +// which Unmodeled map the entry must land on by the path the walk reaches it at, +// so an entry written to the wrong carrier fails here rather than passing +// because the assertion looked only where it expected. +func TestUnknownKeys_KeptAtEveryObject(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "unknown-keys", unknownKeysSpec(t)) + requireNoErrorDiagnostics(t, diags) + sites := unmodeledSites(doc) + + for _, tc := range []struct { + object string + key string + want string + carrier string + }{ + {"openapi root", "openapi:basePath", `"ROOT"`, "doc.Unmodeled"}, + {"info", "openapi:info/contactEmail", `"INFO"`, "doc.Unmodeled"}, + {"contact", "openapi:info/contact/slack", `"CONTACT"`, "doc.Unmodeled"}, + {"license", "openapi:info/license/spdx", `"LICENSE"`, "doc.Unmodeled"}, + {"externalDocs", "openapi:externalDocs/title", `"EXTERNALDOCS"`, "doc.Unmodeled"}, + {"server", "openapi:host", `"SERVER"`, "doc.Servers[0].Unmodeled"}, + {"server variable", "openapi:example", `"SERVERVARIABLE"`, "doc.Servers[0].Variables[0].Unmodeled"}, + {"tag", "openapi:tags/0/color", `"TAG"`, "doc.Unmodeled"}, + {"components", "openapi:components/definitions", `"COMPONENTS"`, "doc.Unmodeled"}, + {"tag externalDocs", "openapi:tags/0/externalDocs/title", `"TAGEXTERNALDOCS"`, "doc.Unmodeled"}, + {"operation", "openapi:operationid", `"OPERATION"`, ".Unmodeled"}, + // The carrier is named down to the operation rather than left at + // ".Unmodeled": the document writes this exact key too, and the row is only + // evidence of the operation's if it cannot match the document's. + {"operation externalDocs", "openapi:externalDocs/title", `"OPERATIONEXTERNALDOCS"`, + "Operations[0].Unmodeled"}, + {"parameter", "openapi:collectionFormat", `"PARAMETER"`, ".Params[0].Unmodeled"}, + {"example", "openapi:name", `"EXAMPLE"`, ".Params[0].Examples[0].Unmodeled"}, + {"request body", "openapi:schema", `"REQUESTBODY"`, ".Request.Unmodeled"}, + {"media type", "openapi:format", `"MEDIATYPE"`, ".Contents[0].Unmodeled"}, + // On the content rather than on the part: ir.PartEncoding holds no + // Unmodeled map, and one content can carry an entry per part, so the part + // name is what tells two encodings' keys apart on that one map. + {"encoding", "openapi:encoding/part/contentEncoding", `"ENCODING"`, ".Contents[1].Unmodeled"}, + {"response", "openapi:status", `"RESPONSE"`, ".Responses[0].Unmodeled"}, + {"error response", "openapi:status", `"ERRORRESPONSE"`, ".Errors[0].Unmodeled"}, + {"header", "openapi:in", `"HEADER"`, ".Headers[0].Unmodeled"}, + {"security scheme", "openapi:tokenUrl", `"SECURITYSCHEME"`, + "doc.Auth[auth/openapi/components/securitySchemes/k].Unmodeled"}, + {"oauth flows", "openapi:flows/application", `"OAUTHFLOWS"`, + "doc.Auth[auth/openapi/components/securitySchemes/o].Unmodeled"}, + {"oauth flow", "openapi:scope", `"OAUTHFLOW"`, + "doc.Auth[auth/openapi/components/securitySchemes/o].Flows[0].Unmodeled"}, + {"schema", "openapi:additionalItems", `"SCHEMA"`, + "doc.Types[t/openapi/components/schemas/S].Unmodeled"}, + // The property's schema reduced to a shared primitive, so it owns no node + // and its keywords stay on the declaring property — the carrier rule + // fillPropertyAnnotations already applies to every other annotation. + {"property schema", "openapi:divisibleBy", `"PROPERTYSCHEMA"`, + "doc.Types[t/openapi/components/schemas/S].Properties[0].Unmodeled"}, + } { + site, found := findUnmodeled(sites, tc.key, tc.want) + if !assert.True(t, found, "%s drops %s = %s: it is nowhere in the document", + tc.object, tc.key, tc.want) { + continue + } + assert.Contains(t, site.path, tc.carrier, "%s key lands on the wrong carrier", tc.object) + } +} + +// TestUnknownKeys_SchemaAndObjectAreGradedApart pins the one distinction the +// census turns on, at the two keys the fixture picks for it: a draft-07 +// additionalItems in a schema, and an operationId with the case wrong on an +// operation. +// +// JSON Schema states that an implementation must ignore a keyword it does not +// recognize, so an unrecognized keyword in a schema is legal input and may carry +// meaning for other tooling; OpenAPI states that an extension key must be +// prefixed x-, so an undefined key on one of its objects is not an extension but +// a defect in the document. +// +// Both are kept — invariant 2 does not bend for invalid input — and both carry +// the same reason, because "no IR node is coming" is true of each. What differs +// is what the document did, which is the diagnostic channel's subject: an +// unrecognized keyword records a decision at info, and an undefined key reports +// a fault at warning. +func TestUnknownKeys_SchemaAndObjectAreGradedApart(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "unknown-keys", unknownKeysSpec(t)) + requireNoErrorDiagnostics(t, diags) + + schema, ok := doc.Types[ir.TypeID("t/openapi/components/schemas/S")] + require.True(t, ok) + keyword := unmodeledEntry(t, schema.Common().Unmodeled, "openapi:additionalItems") + assert.Equal(t, ir.ReasonOutOfScope, keyword.Reason, + "no IR node is coming for a keyword this compiler does not model") + assert.Equal(t, "/components/schemas/S/additionalItems", keyword.Provenance.Pointer) + assert.Equal(t, []ir.Severity{ir.SeverityInfo}, + diagsAt(diags, "openapi/unknown-schema-keyword", "/components/schemas/S/additionalItems")) + + op, ok := opByName(doc, "listWidgets") + require.True(t, ok) + key := unmodeledEntry(t, op.Unmodeled, "openapi:operationid") + assert.Equal(t, ir.ReasonOutOfScope, key.Reason, + "OpenAPI defines no such key, so no IR node is coming for it either") + assert.Equal(t, "/paths/~1widgets/get/operationid", key.Provenance.Pointer) + assert.Equal(t, []ir.Severity{ir.SeverityWarning}, + diagsAt(diags, "openapi/unknown-object-key", "/paths/~1widgets/get/operationid")) +} + +// TestUnknownKeys_WellFormedDocumentRecordsNothing is the control the census +// needs: a document writing only what the model names keeps no entry and reports +// no finding, so the rows above are evidence of the keys they name rather than +// of a sweep that fires on everything. +func TestUnknownKeys_WellFormedDocumentRecordsNothing(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "clean", `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: + /widgets: + get: + operationId: listWidgets + responses: {"200": {description: ok}} +components: + schemas: + S: {type: object, properties: {a: {type: string}}} +`) + requireNoErrorDiagnostics(t, diags) + + assert.Empty(t, unmodeledSites(doc), "nothing undeclared, so nothing kept") + for _, d := range diags { + assert.NotContains(t, d.Code, "unknown-", "a well-formed document reports no unknown key: %+v", d) + } +} + +// TestUnknownKeys_SchemaSubObjects covers the three objects that hang off a +// schema — its xml, its discriminator and its externalDocs — which the fixture +// above deliberately leaves out. +// +// It leaves them out because the OpenAPI dialect meta-schema closes all three to +// anything but an x- key, so the library reports a validation error on each and +// harness.Check returns at the first one, before the oracles that fixture exists +// to reach. The keys are kept and announced all the same, which is what this +// asserts: an invalid document is still not a document whose keys may vanish. +// +// Graded as an OpenAPI object's keys rather than as schema keywords, at warning +// rather than info: JSON Schema's rule that an unrecognized keyword is legal +// governs the schema, and these three are OpenAPI objects the schema vocabulary +// says nothing about. All three ride on the schema's own map, since none of +// ir.XMLHints, ir.Discriminator or ir.Link holds one, and the keyword each was +// written under is what keeps them apart there. +func TestUnknownKeys_SchemaSubObjects(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "schema-sub-objects", `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: {} +components: + schemas: + S: + type: object + xml: {name: s, attribute2: XML} + discriminator: {propertyName: k, mapping2: DISCRIMINATOR} + externalDocs: {url: 'https://d.example', title: SCHEMAEXTERNALDOCS} + properties: {k: {type: string}} +`) + schema, ok := doc.Types[ir.TypeID("t/openapi/components/schemas/S")] + require.True(t, ok) + + // assert rather than require on the lookup, so a keyword missing from the map + // reports itself and leaves the other two still checked. The three share one + // reader, and a row that never runs is no evidence about the keyword it names. + for _, tc := range []struct{ keyword, key, want string }{ + {"xml", "openapi:xml/attribute2", `"XML"`}, + {"discriminator", "openapi:discriminator/mapping2", `"DISCRIMINATOR"`}, + {"externalDocs", "openapi:externalDocs/title", `"SCHEMAEXTERNALDOCS"`}, + } { + entry, found := schema.Common().Unmodeled[tc.key] + if !assert.True(t, found, "schema %s drops %s: it is nowhere on the schema", tc.keyword, tc.key) { + continue + } + assert.Equal(t, tc.want, string(entry.Value), "%s key keeps what the source wrote", tc.keyword) + assert.Equal(t, ir.ReasonOutOfScope, entry.Reason, "%s key reason", tc.keyword) + at := "/components/schemas/S/" + tc.keyword + "/" + tc.key[len("openapi:"+tc.keyword+"/"):] + assert.Equal(t, at, entry.Provenance.Pointer, "%s key provenance", tc.keyword) + assert.Equal(t, []ir.Severity{ir.SeverityWarning}, + diagsAt(diags, "openapi/unknown-object-key", at), + "%s key is announced as an object's, not as a schema keyword's", tc.keyword) + } +} + +// requireNoErrorDiagnostics fails the test on the first error-severity +// diagnostic, naming it. +func requireNoErrorDiagnostics(t *testing.T, diags []ir.Diagnostic) { + t.Helper() + d, ok := ir.FirstError(diags) + require.False(t, ok, "unexpected error diagnostic: %+v", d) +} diff --git a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json index c43e9937..3218cb4f 100644 --- a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json +++ b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json @@ -44,7 +44,7 @@ }, "anonymous": true, "docs": { - "description": "named by position" + "description": "named by position, not by target" }, "sensitive": false, "provenance": { @@ -366,7 +366,7 @@ "eventPayload": false, "secret": false, "docs": { - "description": "named by position" + "description": "named by position, not by target" }, "provenance": { "source": 0, @@ -548,7 +548,7 @@ { "format": "openapi@3.1", "path": "allof-oneof-cooccurrence.yaml", - "hash": "cb7fb6bc613222b0211fc363b9028e7739b44a1d14b04f74cf709b8d603655f2" + "hash": "580c2ffd45b8b24e175b4c5b7f1af6c237a920de7d1942d84b10a00ad2dc1e33" } ] } diff --git a/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml b/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml index f085da90..042727df 100644 --- a/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml +++ b/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml @@ -56,5 +56,5 @@ components: branch: {$ref: '#/components/schemas/InlineHost/oneOf/0'} InlineHost: oneOf: - - {type: integer, description: named by position, not by target} + - {type: integer, description: 'named by position, not by target'} - {type: string} diff --git a/testdata/openapi/unknown_keys.yaml b/testdata/openapi/unknown_keys.yaml new file mode 100644 index 00000000..ba1668b1 --- /dev/null +++ b/testdata/openapi/unknown_keys.yaml @@ -0,0 +1,93 @@ +# One key the OpenAPI model names no field for at every object the compiler takes +# a census from, each valued with the object it was written on so an entry found +# at the wrong carrier cannot pass for the right one. +# +# Every key here is one real documents carry rather than invented nonsense: a +# Swagger 2.0 field with no OpenAPI 3 equivalent (basePath, host, definitions, a +# body parameter's schema, a flow named `application`), a keyword from an older +# JSON Schema draft (additionalItems, divisibleBy), a field belonging to a +# neighbouring object (a flow's tokenUrl on the scheme, a parameter's `in` on a +# header, a documentation page's `title` on an externalDocs), or a field with the +# case or the number wrong (operationid, scope). None of them reached an IR field, +# an Unmodeled entry or a diagnostic. +# +# `title` is written on all three externalDocs objects and `scope` on the one +# flow, so the entries a document can write more than once are here in more than +# one copy: an unscoped key would leave a single entry and the survivor would +# depend on which lowering ran last. +# +# A schema's own xml, discriminator and externalDocs are censused too but are not +# here. The OpenAPI dialect meta-schema closes those three to undeclared keys, so +# one draws a library validation error, and an error diagnostic stops +# harness.Check before the oracles this fixture exists to reach — see +# TestUnknownKeys_SchemaSubObjects for where they are asserted instead. +openapi: 3.1.0 +basePath: ROOT +info: + title: T + version: "1" + contactEmail: INFO + contact: {name: n, slack: CONTACT} + license: {name: MIT, spdx: LICENSE} +externalDocs: {url: 'https://d.example', title: EXTERNALDOCS} +servers: + - url: 'https://a.example/{region}' + host: SERVER + variables: + region: {default: us, example: SERVERVARIABLE} +tags: + - name: t1 + color: TAG + externalDocs: {url: 'https://t.example', title: TAGEXTERNALDOCS} +paths: + /widgets: + get: + operationId: listWidgets + tags: [t1] + operationid: OPERATION + externalDocs: {url: 'https://o.example', title: OPERATIONEXTERNALDOCS} + parameters: + - name: shape + in: query + schema: {type: string} + collectionFormat: PARAMETER + examples: + round: {value: circle, name: EXAMPLE} + requestBody: + required: true + schema: REQUESTBODY + content: + application/json: + schema: {$ref: '#/components/schemas/S'} + format: MEDIATYPE + multipart/form-data: + schema: {type: object, properties: {part: {type: string}}} + encoding: + part: {contentType: text/plain, contentEncoding: ENCODING} + responses: + "200": + description: ok + status: RESPONSE + headers: + X-Trace: {schema: {type: string}, in: HEADER} + "404": + description: gone + status: ERRORRESPONSE +components: + definitions: COMPONENTS + securitySchemes: + k: {type: apiKey, in: header, name: X-Key, tokenUrl: SECURITYSCHEME} + o: + type: oauth2 + flows: + application: OAUTHFLOWS + implicit: + authorizationUrl: 'https://auth.example' + scopes: {} + scope: OAUTHFLOW + schemas: + S: + type: object + additionalItems: SCHEMA + properties: + a: {type: string, divisibleBy: PROPERTYSCHEMA}