fix(analyzer): inline bodies of the same shape took a type name from another operation

internal/analyzer/analyzer.go

@@ -26,6 +26,10 @@ type Analyzer struct {
 	// inlineMultipartBodies holds the naming hints of multipart bodies written
 	// inline, which have no schema name to record instead.
 	inlineMultipartBodies map[string]bool
+	// bodyScope names the body being converted while it is one an operation
+	// writes inline. Such a body has no name in the spec beyond the operation's,
+	// so what it synthesizes is kept out of the scope every other body draws on.
+	bodyScope string
 	// goNameBySchema maps every component schema to its Go type name, filled in
 	// before any conversion so a reference to a schema that has not been converted
 	// yet still resolves to the name it will end up with.

internal/analyzer/operations.go

@@ -483,14 +483,31 @@ func (a *Analyzer) convertRequestBody(rb *v3high.RequestBody, nameHint string) (
 		return nil, nil
 	}
 
+	end := a.enterBodyScope(nameHint, rb.GoLow().IsReference())
+	goType := a.resolveMediaTypeSchema(mediaType, nameHint)
+	end()
+
 	return &ir.RequestBodyDef{
 		Required:    rb.Required != nil && *rb.Required,
 		Description: rb.Description,
 		ContentType: contentType,
-		TypeName:    bodyGoType(contentType, a.resolveMediaTypeSchema(mediaType, nameHint), mediaTypeSchema(mediaType)),
+		TypeName:    bodyGoType(contentType, goType, mediaTypeSchema(mediaType)),
 	}, nil
 }
 
+// enterBodyScope scopes the conversion of a body an operation writes inline,
+// returning the func that ends that scope. A body the spec declares as a
+// component is named by the spec, so it is converted in the shared scope and the
+// operations referencing it land on one type.
+func (a *Analyzer) enterBodyScope(nameHint string, component bool) func() {
+	prev := a.bodyScope
+	a.bodyScope = nameHint
+	if component {
+		a.bodyScope = ""
+	}
+	return func() { a.bodyScope = prev }
+}
+
 // formEncodedContentType reports whether a body is sent as form data, whose
 // encoders walk the value property by property.
 func formEncodedContentType(contentType string) bool {
@@ -627,6 +644,7 @@ func (a *Analyzer) convertSingleResponse(code string, resp *v3high.Response, nam
 	}
 
 	if resp.Content != nil {
+		end := a.enterBodyScope(nameHint, resp.GoLow().IsReference())
 		for contentType, mediaType := range resp.Content.FromOldest() {
 			if strings.Contains(contentType, "json") {
 				rd.ContentType = contentType
@@ -645,6 +663,7 @@ func (a *Analyzer) convertSingleResponse(code string, resp *v3high.Response, nam
 				break
 			}
 		}
+		end()
 	}
 
 	return rd
@@ -666,7 +685,12 @@ func (a *Analyzer) eventStreamType(resp *v3high.Response, nameHint string) strin
 	if !ok || mediaType == nil {
 		return ""
 	}
+	// The event payload is scoped to the response, not to its own hint: the same
+	// schema is also resolved as the response body, and one schema declares one
+	// type.
+	end := a.enterBodyScope(nameHint, resp.GoLow().IsReference())
 	goType := a.resolveMediaTypeSchema(mediaType, nameHint+"Event")
+	end()
 	if goType == "" {
 		// Events with no schema are still events; their data arrives as text.
 		return "string"

internal/analyzer/schemas.go

@@ -722,8 +722,9 @@ func (a *Analyzer) resolveGoType(schema *highbase.Schema, nameHint string) strin
 // synthesizeInlineUnion creates a named union TypeDef for an inline
 // oneOf/anyOf schema so its $ref variants keep their generated types instead
 // of degrading to any. Identical unions (same variants and discriminator) are
-// synthesized once and reuse the first occurrence's name; the resulting types
-// are emitted after the component schemas. Returns false when the schema is
+// synthesized once and reuse the first occurrence's name, unless they sit in
+// bodies different operations write inline; the resulting types are emitted
+// after the component schemas. Returns false when the schema is
 // not a union, has no $ref variants worth naming, or no nameHint is available.
 func (a *Analyzer) synthesizeInlineUnion(schema *highbase.Schema, nameHint string) (string, bool) {
 	variants := schema.OneOf
@@ -773,6 +774,7 @@ func (a *Analyzer) synthesizeInlineUnion(schema *highbase.Schema, nameHint strin
 	if schema.Discriminator != nil {
 		key += "|" + schema.Discriminator.PropertyName
 	}
+	key = a.synthesisKey(key, schema.Title != "")
 	if existing, ok := a.synthesizedByKey[key]; ok {
 		return existing.Name, true
 	}
@@ -789,14 +791,16 @@ func (a *Analyzer) synthesizeInlineUnion(schema *highbase.Schema, nameHint strin
 
 // synthesizeInlineObject declares a named struct for an object written inline, so
 // the properties it lists stay typed instead of collapsing into any. Two
-// declarations of one shape share a type.
+// declarations of one shape share a type, unless they sit in bodies different
+// operations write inline.
 func (a *Analyzer) synthesizeInlineObject(schema *highbase.Schema, nameHint string) (string, bool) {
 	// Multipart is a property of where the schema is used, so it is looked up
 	// under the hint the request body passed, before a title renames it.
 	multipartBody := a.inlineMultipartBodies[nameHint]
 
 	// A titled schema names itself, which keeps the generated name stable when
 	// the property that reaches it first is renamed.
+	selfNamed := true
 	switch {
 	case schema.Title != "":
 		nameHint = schema.Title
@@ -805,12 +809,14 @@ func (a *Analyzer) synthesizeInlineObject(schema *highbase.Schema, nameHint stri
 		// so it is named for itself rather than for whichever property in this
 		// document happened to reach it first.
 		nameHint = externalRefName(schema)
+	default:
+		selfNamed = false
 	}
 	if nameHint == "" {
 		return "", false
 	}
 
-	key := a.inlineObjectKey(schema, nameHint)
+	key := a.synthesisKey(a.inlineObjectKey(schema, nameHint), selfNamed)
 	if existing, ok := a.synthesizedByKey[key]; ok {
 		return existing.Name, true
 	}
@@ -833,14 +839,15 @@ func (a *Analyzer) synthesizeInlineAllOf(schema *highbase.Schema, nameHint strin
 	// properties are file parts, not base64 text.
 	multipartBody := a.inlineMultipartBodies[nameHint]
 
-	if schema.Title != "" {
+	selfNamed := schema.Title != ""
+	if selfNamed {
 		nameHint = schema.Title
 	}
 	if nameHint == "" {
 		return "", false
 	}
 
-	key := a.inlineAllOfKey(schema, nameHint)
+	key := a.synthesisKey(a.inlineAllOfKey(schema, nameHint), selfNamed)
 	if existing, ok := a.synthesizedByKey[key]; ok {
 		return existing.Name, true
 	}
@@ -855,6 +862,18 @@ func (a *Analyzer) synthesizeInlineAllOf(schema *highbase.Schema, nameHint strin
 	return goName, true
 }
 
+// synthesisKey identifies a synthesized type. A schema inside a body an
+// operation writes inline is keyed by that body as well as by its shape, so two
+// operations that happen to declare the same body get a type each, named for the
+// operation declaring it, rather than sharing one named for whichever was
+// converted first.
+func (a *Analyzer) synthesisKey(key string, selfNamed bool) string {
+	if a.bodyScope == "" || selfNamed {
+		return key
+	}
+	return "at:" + a.bodyScope + "|" + key
+}
+
 // inlineAllOfKey identifies a composition by what it composes, so the same one
 // written twice lands on one type.
 func (a *Analyzer) inlineAllOfKey(schema *highbase.Schema, nameHint string) string {

internal/analyzer/schemas_operation_body_test.go

@@ -0,0 +1,204 @@
+package analyzer
+
+import "testing"
+
+const operationBodySpec = `openapi: 3.1.0
+info: { title: t, version: "1" }
+paths:
+  /avatar:
+    post:
+      operationId: uploadUserAvatar
+      requestBody:
+        content:
+          multipart/form-data:
+            schema:
+              type: object
+              properties:
+                file: { type: string, format: binary }
+              required: [file]
+      responses:
+        "200":
+          description: ok
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  url: { type: string }
+  /icon:
+    post:
+      operationId: uploadWorkflowIcon
+      requestBody:
+        content:
+          multipart/form-data:
+            schema:
+              type: object
+              properties:
+                file: { type: string, format: binary }
+              required: [file]
+      responses:
+        "200":
+          description: ok
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  url: { type: string }
+`
+
+func TestOperationBody_InlineBodiesOfOneShapeStayPerOperation(t *testing.T) {
+	pkg, typeMap := analyzeSpec(t, operationBodySpec)
+
+	want := map[string]struct{ body, response string }{
+		"UploadUserAvatar":   {"UploadUserAvatarBody", "UploadUserAvatarResponse"},
+		"UploadWorkflowIcon": {"UploadWorkflowIconBody", "UploadWorkflowIconResponse"},
+	}
+	for _, op := range pkg.Operations {
+		w, ok := want[op.Name]
+		if !ok {
+			t.Fatalf("unexpected operation %s", op.Name)
+		}
+		if op.RequestBody == nil || op.RequestBody.TypeName != w.body {
+			t.Errorf("%s body = %v, want %s", op.Name, op.RequestBody, w.body)
+		}
+		if got := op.Responses[0].TypeName; got != w.response {
+			t.Errorf("%s response = %q, want %q", op.Name, got, w.response)
+		}
+		if typeMap[w.body] == nil {
+			t.Errorf("%s not generated", w.body)
+		}
+		if typeMap[w.response] == nil {
+			t.Errorf("%s not generated", w.response)
+		}
+	}
+}
+
+const sharedComponentBodySpec = `openapi: 3.1.0
+info: { title: t, version: "1" }
+paths:
+  /a:
+    post:
+      operationId: createA
+      requestBody:
+        $ref: '#/components/requestBodies/Upload'
+      responses:
+        "400": { $ref: '#/components/responses/Failure' }
+  /b:
+    post:
+      operationId: createB
+      requestBody:
+        $ref: '#/components/requestBodies/Upload'
+      responses:
+        "400": { $ref: '#/components/responses/Failure' }
+components:
+  requestBodies:
+    Upload:
+      content:
+        application/json:
+          schema:
+            type: object
+            properties:
+              file: { type: string }
+  responses:
+    Failure:
+      description: nope
+      content:
+        application/json:
+          schema:
+            type: object
+            properties:
+              message: { type: string }
+`
+
+func TestOperationBody_ComponentBodyStaysShared(t *testing.T) {
+	pkg, _ := analyzeSpec(t, sharedComponentBodySpec)
+
+	var bodies, responses []string
+	for _, op := range pkg.Operations {
+		bodies = append(bodies, op.RequestBody.TypeName)
+		responses = append(responses, op.Responses[0].TypeName)
+	}
+	if len(bodies) != 2 || bodies[0] != bodies[1] {
+		t.Errorf("component request body types = %v, want both operations on one type", bodies)
+	}
+	if len(responses) != 2 || responses[0] != responses[1] {
+		t.Errorf("component response types = %v, want both operations on one type", responses)
+	}
+}
+
+const titledBodySpec = `openapi: 3.1.0
+info: { title: t, version: "1" }
+paths:
+  /a:
+    post:
+      operationId: createA
+      requestBody:
+        content:
+          application/json:
+            schema:
+              type: object
+              title: Upload
+              properties:
+                file: { type: string }
+      responses:
+        "204": { description: ok }
+  /b:
+    post:
+      operationId: createB
+      requestBody:
+        content:
+          application/json:
+            schema:
+              type: object
+              title: Upload
+              properties:
+                file: { type: string }
+      responses:
+        "204": { description: ok }
+`
+
+func TestOperationBody_TitledBodyStaysShared(t *testing.T) {
+	pkg, _ := analyzeSpec(t, titledBodySpec)
+
+	for _, op := range pkg.Operations {
+		if got := op.RequestBody.TypeName; got != "Upload" {
+			t.Errorf("%s body = %q, want Upload", op.Name, got)
+		}
+	}
+}
+
+const inlineEventStreamSpec = `openapi: 3.1.0
+info: { title: t, version: "1" }
+paths:
+  /events:
+    get:
+      operationId: streamEvents
+      responses:
+        "200":
+          description: ok
+          content:
+            text/event-stream:
+              schema:
+                type: object
+                properties:
+                  id: { type: string }
+`
+
+// An event payload is resolved twice, once as the response body and once as the
+// event, so scoping it to the response keeps the two on one declared type.
+func TestOperationBody_EventPayloadDeclaresOneType(t *testing.T) {
+	pkg, _ := analyzeSpec(t, inlineEventStreamSpec)
+
+	op := pkg.Operations[0]
+	if op.EventType != op.Responses[0].TypeName {
+		t.Errorf("event type %q, response type %q, want one type", op.EventType, op.Responses[0].TypeName)
+	}
+	var synthesized []string
+	for _, td := range pkg.Types {
+		synthesized = append(synthesized, td.Name)
+	}
+	if len(synthesized) != 1 {
+		t.Errorf("generated types = %v, want one", synthesized)
+	}
+}

internal/analyzer/webhooks.go

@@ -100,7 +100,9 @@ func (a *Analyzer) inboundPayloadType(op *v3high.Operation, nameHint string) (st
 	if !strings.Contains(contentType, "json") || mediaType == nil {
 		return "", false
 	}
+	end := a.enterBodyScope(nameHint+"Payload", op.RequestBody.GoLow().IsReference())
 	goType := a.resolveMediaTypeSchema(mediaType, nameHint+"Payload")
+	end()
 	if goType == "" || goType == "any" {
 		return "", false
 	}