fix(compilers/openapi): warn on a malformed path key

compilers/openapi/internal/diag/diag.go

@@ -329,6 +329,17 @@ const (
 	// Warning, not the error style gets: that is the parser's own validation
 	// refusal, whereas here the compiler has already kept the value.
 	InvalidLocationKeyword = "openapi/invalid-location-keyword"
+	// InvalidPathKey reports a Paths Object key that is not a path: one that
+	// omits the leading "/" (required at every version, so not version-gated),
+	// carries a "?" (RFC 3986 §3.4: the query is not the path; OpenAPI uses in:
+	// query), or holds a character outside RFC 3986 §3.3's pchar set plus "/" and
+	// the "{}" of path templating.
+	//
+	// Warning, as InvalidStatusKey is: the key still lowers as written
+	// (invariant 2), and an error would stop harness.Check before later findings.
+	// A "#" in the key is silent here: GitHub #602 strips and reports it, and a
+	// key needs one report, not two.
+	InvalidPathKey = "openapi/invalid-path-key"
 )
 
 // Newf builds an ir.Diagnostic with a formatted message. It is the single

compilers/openapi/internal/diag/diag_test.go

@@ -147,7 +147,7 @@ func codes() []string {
 		diag.ReservedHeaderName, diag.UnpreservableConstruct,
 		diag.UnknownSchemaKeyword, diag.UnknownObjectKey, diag.UnknownKeyBudget,
 		diag.UnknownKeyUnreachable, diag.UnknownKeyEntryTaken,
-		diag.InvalidLocationKeyword,
+		diag.InvalidLocationKeyword, diag.InvalidPathKey,
 	}
 }
 

compilers/openapi/internal/operation/operations.go

@@ -190,6 +190,7 @@ func lowerPaths(ctx context.Context, c lowering.Ctx, ts *compile.Types, anchors
 		if pi == nil {
 			continue
 		}
+		diags = append(diags, pathKeyDiags(c, path)...)
 		c := lowering.Within[soa.PathItem](c, rp)
 		diags = append(diags, lowerPathItem(c, ts, anchors, claims, groups, svc, path, pi, declPtr)...)
 	}

compilers/openapi/internal/operation/operations_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/dexpace/morphic/compilers/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/openapitest"
 	"github.com/dexpace/morphic/ir"
@@ -198,6 +199,137 @@ func TestResponses_ValidStatusKeysAreNotReported(t *testing.T) {
 		"every key here names a status; got %+v", diags)
 }
 
+// pathKeySpec wraps one Paths Object key in a minimal document. The key is
+// Go-quoted rather than written raw, so a key carrying a space, a control byte
+// or a quote is still the single YAML scalar the table means.
+func pathKeySpec(key string) string { return pathKeySpecVer("3.1.0", key) }
+
+// pathKeySpecVer is pathKeySpec under a chosen OpenAPI version.
+func pathKeySpecVer(version, key string) string {
+	return openapitest.PathsSpecVer(version, fmt.Sprintf(
+		"  %s:\n    get:\n      operationId: op\n      responses: {\"200\": {description: ok}}\n",
+		strconv.Quote(key)))
+}
+
+// TestPathKeys_MalformedAreReported is GitHub #641 at the lowering level: a
+// Paths Object key that is not a path used to lower in silence, so a typo
+// reached uriTemplate and the group name as though it were a route. One warning
+// now names the key, every defect it carries, and says the key is not rewritten
+// — and the key still lowers verbatim, which is what makes a warning the honest
+// severity (see diag.InvalidPathKey).
+func TestPathKeys_MalformedAreReported(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name  string
+		key   string
+		parts []string
+	}{
+		{
+			name:  "missing slash, space and query together",
+			key:   "a/b c?x",
+			parts: []string{`"a/b c?x"`, `missing leading "/"`, `carries a query ("?")`, `contains ' '`},
+		},
+		{name: "no leading slash", key: "widgets", parts: []string{`"widgets"`, `missing leading "/"`}},
+		{name: "empty key", key: "", parts: []string{`missing leading "/"`}},
+		{name: "space", key: "/a b", parts: []string{`contains ' '`}},
+		{name: "control character", key: "/a\tb", parts: []string{`contains '\t'`}},
+		{name: "delete", key: "/a\x7fb", parts: []string{`contains '\x7f'`}},
+		{name: "bracket", key: "/a[b", parts: []string{`contains '['`}},
+		{name: "query", key: "/a?x=1", parts: []string{`carries a query ("?")`}},
+		{name: "percent before non-hex", key: "/a/%zz", parts: []string{`begins no percent-encoded triplet`}},
+		{name: "bare percent", key: "/a/%", parts: []string{`begins no percent-encoded triplet`}},
+		{name: "one digit after percent", key: "/a/%2", parts: []string{`begins no percent-encoded triplet`}},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			_, svc, diags := lowerServiceSpec(t, pathKeySpec(tc.key))
+			openapitest.RequireNoErrorDiags(t, diags)
+
+			msg := openapitest.DiagMessageAt(t, diags, diag.InvalidPathKey, ir.SeverityWarning,
+				string(ids.Ptr("paths", tc.key)))
+			for _, part := range tc.parts {
+				assert.Contains(t, msg, part, "the message names the defect")
+			}
+			assert.Contains(t, msg, "the key is lowered as written",
+				"the message closes by stating the key is not rewritten")
+			assert.Equal(t, 1, openapitest.CountDiagsAt(diags, diag.InvalidPathKey, ir.SeverityWarning),
+				"one diagnostic per key, not per defect")
+
+			assert.Equal(t, tc.key, openapitest.FirstOp(t, svc).Bindings.HTTP[0].URITemplate,
+				"the malformed key still reaches the IR verbatim")
+		})
+	}
+}
+
+// TestPathKeys_ValidAreNotReported is the overreach guard: a rule that rejected
+// anything unusual would satisfy the test above while warning on paths that are
+// entirely correct — including the ones this compiler is most likely to be handed.
+func TestPathKeys_ValidAreNotReported(t *testing.T) {
+	t.Parallel()
+	for _, key := range []string{"/a/b", "/", "/a/{id}", "/a/%20", "/a/~b", "/a/é"} {
+		t.Run(key, func(t *testing.T) {
+			t.Parallel()
+			_, svc, diags := lowerServiceSpec(t, pathKeySpec(key))
+			openapitest.RequireNoErrorDiags(t, diags)
+			assert.False(t, openapitest.HasDiag(diags, diag.InvalidPathKey),
+				"%q is a path; got %+v", key, diags)
+			assert.Equal(t, key, openapitest.FirstOp(t, svc).Bindings.HTTP[0].URITemplate)
+		})
+	}
+}
+
+// TestPathKeys_MalformedUnderEveryVersion pins that the rule is not version-gated:
+// 3.0.3 §4.7.8.1, 3.1.0 §4.8.8.1 and 3.2.0 §4.8.1 state the same sentence, so the
+// warning cannot be read off c.Source.Format.
+func TestPathKeys_MalformedUnderEveryVersion(t *testing.T) {
+	t.Parallel()
+	for _, version := range []string{"3.0.3", "3.1.0", "3.2.0"} {
+		t.Run(version, func(t *testing.T) {
+			t.Parallel()
+			_, _, diags := lowerServiceSpec(t, pathKeySpecVer(version, "a/b c?x"))
+			openapitest.RequireNoErrorDiags(t, diags)
+			openapitest.DiagMessageAt(t, diags, diag.InvalidPathKey, ir.SeverityWarning, "/paths/a~1b c?x")
+		})
+	}
+}
+
+// TestPathKeys_FragmentIsSilentForNow pins the deliberate silence on a "#"
+// fragment. GitHub #602 will strip the fragment, set SharedRoute, regroup the
+// operations and report the key itself, and a warning here saying the key is
+// lowered as written would be false the moment it lands — so this code stays
+// quiet and #602 owns the finding.
+func TestPathKeys_FragmentIsSilentForNow(t *testing.T) {
+	t.Parallel()
+	_, svc, diags := lowerServiceSpec(t, pathKeySpec("/a#frag"))
+	openapitest.RequireNoErrorDiags(t, diags)
+	assert.False(t, openapitest.HasDiag(diags, diag.InvalidPathKey),
+		"the fragment is #602's to report; got %+v", diags)
+	assert.Equal(t, "/a#frag", openapitest.FirstOp(t, svc).Bindings.HTTP[0].URITemplate)
+}
+
+// TestPathKeys_WebhookKeysAreNotPaths pins the other half of the exemption a
+// webhook key needs: it is a name, not a path, so "newPet" is not a missing
+// slash. lowerWebhooks shares pathOperations with lowerPaths but never calls
+// pathKeyDiags, and this is what keeps that from drifting.
+func TestPathKeys_WebhookKeysAreNotPaths(t *testing.T) {
+	t.Parallel()
+	spec := `openapi: 3.1.0
+info: {title: T, version: "1"}
+paths: {}
+webhooks:
+  newPet:
+    post:
+      operationId: newPet
+      responses: {"200": {description: ok}}
+`
+	_, svc, diags := lowerServiceSpec(t, spec)
+	openapitest.RequireNoErrorDiags(t, diags)
+	assert.False(t, openapitest.HasDiag(diags, diag.InvalidPathKey),
+		"a webhook key is a name, not a path; got %+v", diags)
+	assert.Equal(t, "newPet", openapitest.FirstOp(t, svc).Bindings.HTTP[0].URITemplate)
+}
+
 // TestResponses_ErrorHeadersAreStructural pins the header half of GitHub #422.
 // A 429's Retry-After and the rate-limit family live on precisely the status
 // class that had no typed home for them: they were kept verbatim under

compilers/openapi/internal/operation/pathkeys.go

@@ -0,0 +1,123 @@
+package operation
+
+import (
+	"fmt"
+	"strings"
+	"unicode/utf8"
+
+	"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/ir"
+)
+
+// The defect clauses one warning's message joins. They are constants rather than
+// inline strings so the classifier's table can name the same text the message
+// carries.
+const (
+	// pathKeyNoSlashDefect is OpenAPI's Patterned Fields rule for the Paths
+	// Object, stated identically at every version (3.0.3 §4.7.8.1, 3.1.0
+	// §4.8.8.1, 3.2.0 §4.8.1): "The field name MUST begin with a forward slash
+	// (`/`)". The empty key omits the slash too.
+	pathKeyNoSlashDefect = `missing leading "/"`
+	// pathKeyQueryDefect is RFC 3986 §3.4's: the query is not part of the path,
+	// and OpenAPI states query data as Parameter Objects with in: query.
+	pathKeyQueryDefect = `carries a query ("?")`
+	// pathKeyCharDefectFormat is RFC 3986 §3.3's pchar set (unreserved,
+	// pct-encoded, sub-delims, ":" and "@") plus "/" and OpenAPI's "{}"
+	// templating braces.
+	pathKeyCharDefectFormat = "contains %q, which is not allowed in a URI path"
+	// pathKeyPercentDefect is the pct-encoded half of the same rule: "%" is
+	// admitted only as the start of a triplet (RFC 3986 §2.1).
+	pathKeyPercentDefect = `contains "%" that begins no percent-encoded triplet`
+)
+
+// pathKeyAllowed is every byte a URI path admits here: RFC 3986 §3.3's pchar —
+// unreserved, sub-delims, ":" and "@" — plus "/" for the separators and "{}" for
+// OpenAPI path templating. "%" is handled separately, since it is admitted only
+// as the start of a pct-encoded triplet, and "#" is left out entirely because
+// the fragment is GitHub #602's to strip and report.
+const pathKeyAllowed = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" +
+	"0123456789-._~!$&'()*+,;=:@/{}"
+
+// pctTripletLen is the width of a pct-encoded octet: the "%" and its two
+// hexadecimal digits.
+const pctTripletLen = 3
+
+// pathKeyDiags reports a Paths Object key that is not a path. It returns at most
+// one diagnostic, however many defects the key carries: the key is one position,
+// and one report naming all its defects is what a reader fixes (GitHub #641).
+//
+// The key is not rewritten — it reaches uriTemplate, the no-operationId name
+// hint and the path-prefix group exactly as written — so this says only that the
+// spelling is not a path; see diag.InvalidPathKey for why that is a warning, and
+// why "#" is #602's.
+func pathKeyDiags(c lowering.Ctx, key string) []ir.Diagnostic {
+	defects := pathKeyDefects(key)
+	if len(defects) == 0 {
+		return nil
+	}
+	return []ir.Diagnostic{c.DiagAt(ir.SeverityWarning, diag.InvalidPathKey, ids.Ptr("paths", key),
+		"path key %q is not a valid path: %s; the key is lowered as written",
+		key, strings.Join(defects, "; "))}
+}
+
+// pathKeyDefects returns why key is not a valid Paths Object key, in a fixed
+// order, or nil when it is one — or when the only thing wrong with it is a "#"
+// fragment, which #602 owns.
+func pathKeyDefects(key string) []string {
+	var defects []string
+	if !strings.HasPrefix(key, "/") {
+		defects = append(defects, pathKeyNoSlashDefect)
+	}
+	if strings.ContainsRune(key, '?') {
+		defects = append(defects, pathKeyQueryDefect)
+	}
+	if defect, bad := pathKeyCharacterDefect(key); bad {
+		defects = append(defects, defect)
+	}
+	return defects
+}
+
+// pathKeyCharacterDefect returns the defect for the first byte of key that a URI
+// path does not admit, and whether it found one. A non-ASCII byte is admitted:
+// the IR field is an RFC 6570 template (ir/bindings.go), whose §2.1 literals
+// admit ucschar and iprivate and pct-encode them on expansion.
+func pathKeyCharacterDefect(key string) (string, bool) {
+	for i := 0; i < len(key); {
+		b := key[i]
+		switch {
+		case b >= utf8.RuneSelf:
+			_, size := utf8.DecodeRuneInString(key[i:])
+			i += size
+		case b == '#':
+			// A fragment is #602's to strip and report; see diag.InvalidPathKey.
+			i++
+		case b == '?':
+			// Named by pathKeyQueryDefect instead, which says what a query is.
+			i++
+		case b == '%':
+			if !pctTriplet(key, i) {
+				return pathKeyPercentDefect, true
+			}
+			i += pctTripletLen
+		case strings.IndexByte(pathKeyAllowed, b) >= 0:
+			i++
+		default:
+			return fmt.Sprintf(pathKeyCharDefectFormat, rune(b)), true
+		}
+	}
+	return "", false
+}
+
+// pctTriplet reports whether key[i:] begins a pct-encoded octet: the "%" at i
+// followed by two hexadecimal digits (RFC 3986 §2.1).
+func pctTriplet(key string, i int) bool {
+	return i+pctTripletLen <= len(key) && isHexDigit(key[i+1]) && isHexDigit(key[i+2])
+}
+
+// isHexDigit reports whether b is one of the sixteen hexadecimal digits, in
+// either case.
+func isHexDigit(b byte) bool {
+	return b >= '0' && b <= '9' || b >= 'a' && b <= 'f' || b >= 'A' && b <= 'F'
+}

compilers/openapi/internal/operation/pathkeys_internal_test.go

@@ -0,0 +1,78 @@
+package operation
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestPathKey_Defects pins the classifier behind diag.InvalidPathKey branch for
+// branch. Every class a key may use and every class it may not is a row, because
+// the classifier is the whole of the rule: a case the table omits is a spelling
+// nothing decides about, and the exact-100% coverage gate would otherwise turn a
+// row dropped from here into a build failure rather than a missing check.
+func TestPathKey_Defects(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name string
+		key  string
+		want []string
+	}{
+		// Allowed, each silent.
+		{name: "unreserved and sub-delims", key: "/a-Z.0_9~!$&'()*+,;="},
+		{name: "colon, at and slash", key: "/a:@/b"},
+		{name: "template braces", key: "/a/{id}"},
+		{name: "root", key: "/"},
+		{name: "empty segment", key: "//a"},
+		{name: "pct-encoded space", key: "/a/%20"},
+		{name: "pct-encoded uppercase hex", key: "/a/%2F"},
+		{name: "non-ASCII rune", key: "/a/é"},
+		{name: "fragment deferred to #602", key: "/a#frag"},
+
+		// Rejected: missing leading slash.
+		{name: "empty key", key: "", want: []string{pathKeyNoSlashDefect}},
+		{name: "no leading slash", key: "widgets", want: []string{pathKeyNoSlashDefect}},
+		// Rejected: a query string.
+		{name: "query", key: "/a?x=1", want: []string{pathKeyQueryDefect}},
+		// Rejected: a character no URI path admits.
+		{name: "space", key: "/a b", want: []string{fmt.Sprintf(pathKeyCharDefectFormat, ' ')}},
+		{name: "tab", key: "/a\tb", want: []string{fmt.Sprintf(pathKeyCharDefectFormat, '\t')}},
+		{name: "delete", key: "/a\x7fb", want: []string{fmt.Sprintf(pathKeyCharDefectFormat, '\x7f')}},
+		{name: "quote", key: `/a"b`, want: []string{fmt.Sprintf(pathKeyCharDefectFormat, '"')}},
+		{name: "less-than", key: "/a<b", want: []string{fmt.Sprintf(pathKeyCharDefectFormat, '<')}},
+		{name: "greater-than", key: "/a>b", want: []string{fmt.Sprintf(pathKeyCharDefectFormat, '>')}},
+		{name: "backslash", key: `/a\b`, want: []string{fmt.Sprintf(pathKeyCharDefectFormat, '\\')}},
+		{name: "caret", key: "/a^b", want: []string{fmt.Sprintf(pathKeyCharDefectFormat, '^')}},
+		{name: "backtick", key: "/a`b", want: []string{fmt.Sprintf(pathKeyCharDefectFormat, '`')}},
+		{name: "pipe", key: "/a|b", want: []string{fmt.Sprintf(pathKeyCharDefectFormat, '|')}},
+		{name: "open bracket", key: "/a[b", want: []string{fmt.Sprintf(pathKeyCharDefectFormat, '[')}},
+		{name: "close bracket", key: "/a]b", want: []string{fmt.Sprintf(pathKeyCharDefectFormat, ']')}},
+		// Rejected: a percent that begins no triplet.
+		{name: "percent before non-hex", key: "/a/%zz", want: []string{pathKeyPercentDefect}},
+		{name: "bare percent", key: "/a/%", want: []string{pathKeyPercentDefect}},
+		{name: "one digit after percent", key: "/a/%2", want: []string{pathKeyPercentDefect}},
+
+		// More than one defect, reported together and in a fixed order.
+		{
+			name: "missing slash, query and space",
+			key:  "a/b c?x",
+			want: []string{
+				pathKeyNoSlashDefect,
+				pathKeyQueryDefect,
+				fmt.Sprintf(pathKeyCharDefectFormat, ' '),
+			},
+		},
+		{
+			name: "the first bad character is the one named",
+			key:  "/a[ b",
+			want: []string{fmt.Sprintf(pathKeyCharDefectFormat, '[')},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, tc.want, pathKeyDefects(tc.key), "key %q", tc.key)
+		})
+	}
+}