diff --git a/CLAUDE.md b/CLAUDE.md index ad23f981a..90c390306 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 diff --git a/compilers/openapi/detect.go b/compilers/openapi/detect.go index 78d7e3630..dfb0b76b2 100644 --- a/compilers/openapi/detect.go +++ b/compilers/openapi/detect.go @@ -1,74 +1,26 @@ 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 // 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. -// -// 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. +// 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). - 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 +34,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 23f356dc9..000000000 --- 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_internal_test.go b/compilers/openapi/detect_internal_test.go new file mode 100644 index 000000000..fba0ebe25 --- /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)) + }) + } +} diff --git a/compilers/openapi/detect_test.go b/compilers/openapi/detect_test.go index 10bbd8425..d81079fe8 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/doc.go b/compilers/openapi/doc.go index a6bfdbacc..86b24af3f 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/helpers_test.go b/compilers/openapi/helpers_test.go index 3336d3b3f..b5d2b7ff8 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/auth/doc.go b/compilers/openapi/internal/auth/doc.go index aa2652ce2..780c99ecc 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/compilers/openapi/internal/detect/declares.go b/compilers/openapi/internal/detect/declares.go new file mode 100644 index 000000000..4f5e1cdd7 --- /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. 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 +// 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 000000000..f3045629b --- /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 000000000..40b80d7de --- /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 000000000..1613d76e9 --- /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 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) +} + +// 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 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 +// 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 000000000..9bda9a88f --- /dev/null +++ b/compilers/openapi/internal/detect/probe.go @@ -0,0 +1,169 @@ +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 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 Recognize +// 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 := 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 +// 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 000000000..f81ad4e3e --- /dev/null +++ b/compilers/openapi/internal/detect/probe_test.go @@ -0,0 +1,264 @@ +package detect + +import ( + "fmt" + "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 +// 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 + // 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 + 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(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": { + 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) { + 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 +// 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 }{ + {"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 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 }{ + {"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 79% rename from compilers/openapi/detect_reading_test.go rename to compilers/openapi/internal/detect/reading_test.go index 62cc6d84b..16e072488 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" @@ -14,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 ( @@ -22,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 ) @@ -46,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{ @@ -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 -// 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) { +// 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"} 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 000000000..cc7008576 --- /dev/null +++ b/compilers/openapi/internal/detect/recognize.go @@ -0,0 +1,60 @@ +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. +// +// Two sources are declined with an error whatever their format: one past +// 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) { + 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 000000000..56015631e --- /dev/null +++ b/compilers/openapi/internal/detect/recognize_test.go @@ -0,0 +1,413 @@ +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.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. 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 + 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. 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() + 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)) + }) + } +} + +// 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 000000000..3e9c90bab --- /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 000000000..8873597bf --- /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/document/components.go b/compilers/openapi/internal/document/components.go new file mode 100644 index 000000000..e05a6f9dd --- /dev/null +++ b/compilers/openapi/internal/document/components.go @@ -0,0 +1,68 @@ +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 { + mediaTypes := nodeview.Child(nodeview.Child(c.Doc.GetRootNode(), "components"), "mediaTypes") + at := ids.Ptr("components", "mediaTypes") + kept, diags := annotation.PreserveNodeInto(p, annotation.EntryKey("components", "mediaTypes"), + 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 12cb1a770..d25269ac2 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 000000000..85253980d --- /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.Child(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 000000000..11737850a --- /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 000000000..3c60bae0b --- /dev/null +++ b/compilers/openapi/internal/document/doc.go @@ -0,0 +1,13 @@ +// 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, 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. 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 diff --git a/compilers/openapi/internal/document/document.go b/compilers/openapi/internal/document/document.go new file mode 100644 index 000000000..fee2d9f6c --- /dev/null +++ b/compilers/openapi/internal/document/document.go @@ -0,0 +1,50 @@ +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 lowering in this package 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 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) + + 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 000000000..979f6b87a --- /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 000000000..550791479 --- /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 000000000..f601d3d86 --- /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 89% rename from compilers/openapi/servers.go rename to compilers/openapi/internal/document/servers.go index 783a43add..ce5f9a47c 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" @@ -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. @@ -94,7 +95,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 new file mode 100644 index 000000000..7149d1981 --- /dev/null +++ b/compilers/openapi/internal/document/servers_internal_test.go @@ -0,0 +1,97 @@ +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, for the servers, the guard +// 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}} + + 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 51% rename from compilers/openapi/servers_test.go rename to compilers/openapi/internal/document/servers_test.go index 3f178c83b..a8b4fa8db 100644 --- a/compilers/openapi/servers_test.go +++ b/compilers/openapi/internal/document/servers_test.go @@ -1,106 +1,20 @@ -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 -// 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 diff --git a/compilers/openapi/internal/document/sites.go b/compilers/openapi/internal/document/sites.go new file mode 100644 index 000000000..18b4bd971 --- /dev/null +++ b/compilers/openapi/internal/document/sites.go @@ -0,0 +1,93 @@ +package document + +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 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) { + 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()}) + } + return annotation.ExtensionsAt(c.ProvenanceAt, ext...) +} + +// documentUnknownKeys collects the keys the OpenAPI model names no field for +// 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 { + 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 +} + +// 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 extensible +} + +// 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 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 site. +func sites(c lowering.Ctx) []site { + info := c.Doc.GetInfo() + infoPtr := ids.Ptr("info") + out := []site{ + {"", "", 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()}, + } + for i, t := range c.Doc.GetTags() { + if t == nil { + continue + } + index := strconv.Itoa(i) + ptr := ids.Ptr("tags", index) + out = append(out, + 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 new file mode 100644 index 000000000..cb19e87d2 --- /dev/null +++ b/compilers/openapi/internal/document/sites_internal_test.go @@ -0,0 +1,60 @@ +package document + +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, 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() + 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 + 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") + + // 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) + } +} diff --git a/compilers/openapi/internal/document/sites_test.go b/compilers/openapi/internal/document/sites_test.go new file mode 100644 index 000000000..6c83fba56 --- /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 +// 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 +info: {title: T, version: "1"} +paths: {} +x-bad: {n: .nan} +` + 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/internal/load/repeats_internal_test.go b/compilers/openapi/internal/load/repeats_internal_test.go index 17add209a..30a3c2173 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 f94d4168b..65a7e5f26 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 ( @@ -147,9 +149,9 @@ type Ctx struct { anchors *anchorMemo // 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 } @@ -211,7 +213,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/openapitest/spec.go b/compilers/openapi/internal/openapitest/spec.go index 62a74a7c6..ac69f6770 100644 --- a/compilers/openapi/internal/openapitest/spec.go +++ b/compilers/openapi/internal/openapitest/spec.go @@ -1,6 +1,7 @@ package openapitest import ( + "fmt" "strings" oas3 "github.com/speakeasy-api/openapi/jsonschema/oas3" @@ -139,3 +140,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 78e5d4dbb..d88ff9089 100644 --- a/compilers/openapi/internal/openapitest/spec_test.go +++ b/compilers/openapi/internal/openapitest/spec_test.go @@ -128,6 +128,21 @@ func TestInlineProbeBody_WritesEveryKeywordTheProbeAssertsOn(t *testing.T) { } } +// 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") +} + // TestNestedMerges_WritesOneMergePerLevel pins the spelling: a body merged in // through as many `<<` keys as asked, a level of none being the body itself. func TestNestedMerges_WritesOneMergePerLevel(t *testing.T) { diff --git a/compilers/openapi/internal/resolve/reached.go b/compilers/openapi/internal/resolve/reached.go index 977d505de..1d72911d8 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/meta.go b/compilers/openapi/meta.go deleted file mode 100644 index 74c38ea56..000000000 --- a/compilers/openapi/meta.go +++ /dev/null @@ -1,289 +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.Child(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 { - mediaTypes := nodeview.Child(nodeview.Child(c.Doc.GetRootNode(), "components"), "mediaTypes") - at := ids.Ptr("components", "mediaTypes") - kept, diags := annotation.PreserveNodeInto(p, annotation.EntryKey("components", "mediaTypes"), - 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 d0b5b884a..000000000 --- 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: {n: .nan} -` - 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 d3703f284..d5284a029 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" @@ -88,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)) @@ -199,7 +201,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/openapi_test.go b/compilers/openapi/openapi_test.go index 99e12475b..8ff78a92e 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/compilers/openapi/overlay_test.go b/compilers/openapi/overlay_test.go index 0211d7253..44c8ebfda 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/architecture.md b/docs/architecture.md index 9aaa82ad2..775e08219 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,25 +383,34 @@ 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 +│ │ ├── 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 -│ │ ├── load/ # parse, validate, resolve +│ │ ├── 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 │ │ ├── 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 +│ │ ├── operation/ # path items, webhooks, callbacks, params, content +│ │ ├── 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). diff --git a/docs/compilers/openapi.md b/docs/compilers/openapi.md index 51db74ec9..384bb834b 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` @@ -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: `openapi.TestDetect_ABrokenDocumentIsReportedAtAnySize`, - `openapi.TestDetect_ReadsTheVersionTheCompileReads`, `openapi.TestDetect_AReadCutShortIsReported`, - `openapi.TestDetect_AMergeCycleHidesNoKey`, +- Tests: `detect.TestRecognize_ABrokenDocumentIsReportedAtAnySize`, + `openapi.TestDetect_ReadsTheVersionTheCompileReads`, + `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 @@ -503,7 +506,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` @@ -617,9 +620,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). @@ -1912,7 +1915,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` @@ -2588,7 +2591,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/docs/micro-compiler-design.md b/docs/micro-compiler-design.md index f4e2b3b94..efeb408d5 100644 --- a/docs/micro-compiler-design.md +++ b/docs/micro-compiler-design.md @@ -300,9 +300,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. @@ -451,6 +451,16 @@ 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, +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 +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 diff --git a/internal/archtest/arch_test.go b/internal/archtest/arch_test.go index 5fb31e392..f5293b5d3 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}, // 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 @@ -190,6 +189,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 @@ -244,10 +251,10 @@ 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. It reads - // a field the model names no member for off the raw node, through nodeview. + // 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. It reads a field the model names no + // member for off the raw node, through nodeview. "compilers/openapi/internal/auth": {module + "/ir", module + "/compilers/compile", module + "/compilers/openapi/internal/annotation", module + "/compilers/openapi/internal/diag", @@ -292,6 +299,18 @@ 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/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 // outside these rules. @@ -457,7 +476,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 c879fde8f..ec793f3f4 100644 --- a/internal/archtest/recursion_test.go +++ b/internal/archtest/recursion_test.go @@ -92,6 +92,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", }