tsp-openapi3 - scope top-level parameters to Parameters namespace (mi…

…crosoft#4216) Fixes microsoft#4151 This PR updates tsp-openapi3's model generation so that all top-level parameters (`#/components/parameters`) are nested in a `Parameters` block namespace. Prior to this change, if top-level parameter had the same name as a top-level schema, we would attempt to merge the two. This worked OK if the schema was an object type, but led to broken results if the schema was anything else. Note: In the linked issue, it was suggested that top-level schemas not be scoped to their own namespace, so if a schema is referenced by a parameter, it will now qualify it with the file-level namespace. This PR introduces a `context` object that contains some state that can be passed around. This is useful for keeping track of the file-level namespace and using it when necessary, but the context will also be useful in cases where we need to look at the definition of a referenced schema from another schema. --------- Co-authored-by: Christopher Radek <Christopher.Radek@microsoft.com>
weidongxu-microsoft · Sep 3, 2024 · b0b6a0a · b0b6a0a
1 parent 8227a8d
commit b0b6a0a
Show file tree

Hide file tree

Showing 20 changed files with 714 additions and 358 deletions.
diff --git a/.chronus/changes/tsp-openapi3-add-context-2024-7-20-9-44-3.md b/.chronus/changes/tsp-openapi3-add-context-2024-7-20-9-44-3.md
@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@typespec/openapi3"
+---
+
+Fixes issue in tsp-openapi3 that resulted in component schemas and parameters with the same name being merged into a single TypeSpec data type.
diff --git a/packages/openapi3/src/cli/actions/convert/convert-file.ts b/packages/openapi3/src/cli/actions/convert/convert-file.ts
@@ -6,15 +6,17 @@ import { handleInternalCompilerError } from "../../utils.js";
 import { ConvertCliArgs } from "./args.js";
 import { generateMain } from "./generators/generate-main.js";
 import { transform } from "./transforms/transforms.js";
+import { createContext } from "./utils/context.js";
 
 export async function convertAction(host: CliHost, args: ConvertCliArgs) {
   // attempt to read the file
   const fullPath = resolvePath(process.cwd(), args.path);
   const model = await parseOpenApiFile(fullPath);
-  const program = transform(model);
+  const context = createContext(model);
+  const program = transform(context);
   let mainTsp: string;
   try {
-    mainTsp = generateMain(program);
+    mainTsp = generateMain(program, context);
   } catch (err) {
     handleInternalCompilerError(err);
   }

diff --git a/packages/openapi3/src/cli/actions/convert/convert.ts b/packages/openapi3/src/cli/actions/convert/convert.ts
@@ -2,10 +2,12 @@ import { formatTypeSpec } from "@typespec/compiler";
 import { OpenAPI3Document } from "../../../types.js";
 import { generateMain } from "./generators/generate-main.js";
 import { transform } from "./transforms/transforms.js";
+import { createContext } from "./utils/context.js";
 
 export async function convertOpenAPI3Document(document: OpenAPI3Document) {
-  const program = transform(document);
-  const content = generateMain(program);
+  const context = createContext(document);
+  const program = transform(context);
+  const content = generateMain(program, context);
   try {
     return await formatTypeSpec(content, {
       printWidth: 100,

diff --git a/packages/openapi3/src/cli/actions/convert/generators/generate-main.ts b/packages/openapi3/src/cli/actions/convert/generators/generate-main.ts
@@ -1,10 +1,11 @@
 import { TypeSpecProgram } from "../interfaces.js";
+import { Context } from "../utils/context.js";
 import { generateDataType } from "./generate-model.js";
 import { generateNamespace } from "./generate-namespace.js";
 import { generateOperation } from "./generate-operation.js";
 import { generateServiceInformation } from "./generate-service-info.js";
 
-export function generateMain(program: TypeSpecProgram): string {
+export function generateMain(program: TypeSpecProgram, context: Context): string {
   return `
   import "@typespec/http";
   import "@typespec/openapi";
@@ -15,12 +16,12 @@ export function generateMain(program: TypeSpecProgram): string {
 
   ${generateServiceInformation(program.serviceInfo)}
 
-  ${program.types.map(generateDataType).join("\n\n")}
+  ${program.types.map((t) => generateDataType(t, context)).join("\n\n")}
 
-  ${program.operations.map(generateOperation).join("\n\n")}
+  ${program.operations.map((o) => generateOperation(o, context)).join("\n\n")}
 
   ${Object.entries(program.namespaces)
-    .map(([name, namespace]) => generateNamespace(name, namespace))
+    .map(([name, namespace]) => generateNamespace(name, namespace, context))
     .join("\n\n")}
   `;
 }
diff --git a/packages/openapi3/src/cli/actions/convert/generators/generate-model.ts b/packages/openapi3/src/cli/actions/convert/generators/generate-model.ts
@@ -1,43 +1,38 @@
-import { OpenAPI3Schema, Refable } from "../../../../types.js";
 import {
   TypeSpecAlias,
   TypeSpecDataTypes,
   TypeSpecEnum,
   TypeSpecModel,
-  TypeSpecModelProperty,
   TypeSpecScalar,
   TypeSpecUnion,
 } from "../interfaces.js";
+import { Context } from "../utils/context.js";
 import { getDecoratorsForSchema } from "../utils/decorators.js";
 import { generateDocs } from "../utils/docs.js";
 import { generateDecorators } from "./generate-decorators.js";
-import {
-  generateTypeFromSchema,
-  getRefScopeAndName,
-  getTypeSpecPrimitiveFromSchema,
-} from "./generate-types.js";
+import { getTypeSpecPrimitiveFromSchema } from "./generate-types.js";
 
-export function generateDataType(type: TypeSpecDataTypes): string {
+export function generateDataType(type: TypeSpecDataTypes, context: Context): string {
   switch (type.kind) {
     case "alias":
-      return generateAlias(type);
+      return generateAlias(type, context);
     case "enum":
       return generateEnum(type);
     case "model":
-      return generateModel(type);
+      return generateModel(type, context);
     case "scalar":
-      return generateScalar(type);
+      return generateScalar(type, context);
     case "union":
-      return generateUnion(type);
+      return generateUnion(type, context);
   }
 }
 
-function generateAlias(alias: TypeSpecAlias): string {
+function generateAlias(alias: TypeSpecAlias, context: Context): string {
   // Since aliases are not represented in the TypeGraph,
   // generate a model so that the model name is present in emitted OpenAPI3.
   // May revisit to allow emitting actual alias.
-  const { scope, name } = getRefScopeAndName(alias.ref);
-  return `model ${alias.name} is ${[...scope, name].join(".")};`;
+  const sourceModel = context.getRefName(alias.ref, alias.scope);
+  return `model ${alias.name} is ${sourceModel};`;
 }
 
 function generateEnum(tsEnum: TypeSpecEnum): string {
@@ -61,22 +56,22 @@ function generateEnum(tsEnum: TypeSpecEnum): string {
   return definitions.join("\n");
 }
 
-function generateScalar(scalar: TypeSpecScalar): string {
+function generateScalar(scalar: TypeSpecScalar, context: Context): string {
   const definitions: string[] = [];
 
   if (scalar.doc) {
     definitions.push(generateDocs(scalar.doc));
   }
 
   definitions.push(...generateDecorators(scalar.decorators));
-  const type = generateTypeFromSchema(scalar.schema);
+  const type = context.generateTypeFromRefableSchema(scalar.schema, scalar.scope);
 
   definitions.push(`scalar ${scalar.name} extends ${type};`);
 
   return definitions.join("\n");
 }
 
-function generateUnion(union: TypeSpecUnion): string {
+function generateUnion(union: TypeSpecUnion, context: Context): string {
   const definitions: string[] = [];
 
   if (union.doc) {
@@ -92,9 +87,13 @@ function generateUnion(union: TypeSpecUnion): string {
   if (schema.enum) {
     definitions.push(...schema.enum.map((e) => `${JSON.stringify(e)},`));
   } else if (schema.oneOf) {
-    definitions.push(...schema.oneOf.map(generateUnionMember));
+    definitions.push(
+      ...schema.oneOf.map((member) => context.generateTypeFromRefableSchema(member, union.scope))
+    );
   } else if (schema.anyOf) {
-    definitions.push(...schema.anyOf.map(generateUnionMember));
+    definitions.push(
+      ...schema.anyOf.map((member) => context.generateTypeFromRefableSchema(member, union.scope))
+    );
   } else {
     // check if it's a primitive type
     const primitiveType = getTypeSpecPrimitiveFromSchema(schema);
@@ -112,11 +111,7 @@ function generateUnion(union: TypeSpecUnion): string {
   return definitions.join("\n");
 }
 
-function generateUnionMember(member: Refable<OpenAPI3Schema>): string {
-  return `${generateTypeFromSchema(member)},`;
-}
-
-export function generateModel(model: TypeSpecModel): string {
+function generateModel(model: TypeSpecModel, context: Context): string {
   const definitions: string[] = [];
   const modelDeclaration = generateModelDeclaration(model);
 
@@ -127,10 +122,25 @@ export function generateModel(model: TypeSpecModel): string {
   definitions.push(...generateDecorators(model.decorators));
   definitions.push(modelDeclaration.open);
 
-  definitions.push(...model.properties.map(generateModelProperty));
+  definitions.push(
+    ...model.properties.map((prop) => {
+      // Decorators will be a combination of top-level (parameters) and
+      // schema-level decorators.
+      const decorators = generateDecorators([
+        ...prop.decorators,
+        ...getDecoratorsForSchema(prop.schema),
+      ]).join(" ");
+
+      const doc = prop.doc ? generateDocs(prop.doc) : "";
+
+      return `${doc}${decorators} ${prop.name}${prop.isOptional ? "?" : ""}: ${context.generateTypeFromRefableSchema(prop.schema, model.scope)};`;
+    })
+  );
 
   if (model.additionalProperties) {
-    definitions.push(`...Record<${generateTypeFromSchema(model.additionalProperties)}>;`);
+    definitions.push(
+      `...Record<${context.generateTypeFromRefableSchema(model.additionalProperties, model.scope)}>;`
+    );
   }
 
   if (modelDeclaration.close) definitions.push(modelDeclaration.close);
@@ -158,16 +168,3 @@ function generateModelDeclaration(model: TypeSpecModel): ModelDeclarationOutput
 
   return { open: `model ${modelName} {`, close: "}" };
 }
-
-function generateModelProperty(property: TypeSpecModelProperty): string {
-  // Decorators will be a combination of top-level (parameters) and
-  // schema-level decorators.
-  const decorators = generateDecorators([
-    ...property.decorators,
-    ...getDecoratorsForSchema(property.schema),
-  ]).join(" ");
-
-  const doc = property.doc ? generateDocs(property.doc) : "";
-
-  return `${doc}${decorators} ${property.name}${property.isOptional ? "?" : ""}: ${generateTypeFromSchema(property.schema)};`;
-}
diff --git a/packages/openapi3/src/cli/actions/convert/generators/generate-namespace.ts b/packages/openapi3/src/cli/actions/convert/generators/generate-namespace.ts
@@ -1,16 +1,21 @@
 import { TypeSpecNamespace } from "../interfaces.js";
+import { Context } from "../utils/context.js";
 import { generateDataType } from "./generate-model.js";
 import { generateOperation } from "./generate-operation.js";
 
-export function generateNamespace(name: string, namespace: TypeSpecNamespace): string {
+export function generateNamespace(
+  name: string,
+  namespace: TypeSpecNamespace,
+  context: Context
+): string {
   const definitions: string[] = [];
   definitions.push(`namespace ${name} {`);
 
-  definitions.push(...namespace.types.map(generateDataType));
-  definitions.push(...namespace.operations.map(generateOperation));
+  definitions.push(...namespace.types.map((t) => generateDataType(t, context)));
+  definitions.push(...namespace.operations.map((o) => generateOperation(o, context)));
 
   for (const [namespaceName, nestedNamespace] of Object.entries(namespace.namespaces)) {
-    definitions.push(generateNamespace(namespaceName, nestedNamespace));
+    definitions.push(generateNamespace(namespaceName, nestedNamespace, context));
   }
 
   definitions.push("}");

diff --git a/packages/openapi3/src/cli/actions/convert/generators/generate-operation.ts b/packages/openapi3/src/cli/actions/convert/generators/generate-operation.ts
@@ -4,11 +4,11 @@ import {
   TypeSpecOperationParameter,
   TypeSpecRequestBody,
 } from "../interfaces.js";
+import { Context } from "../utils/context.js";
 import { generateDocs } from "../utils/docs.js";
 import { generateDecorators } from "./generate-decorators.js";
-import { generateTypeFromSchema, getRefName } from "./generate-types.js";
 
-export function generateOperation(operation: TypeSpecOperation): string {
+export function generateOperation(operation: TypeSpecOperation, context: Context): string {
   const definitions: string[] = [];
 
   if (operation.doc) {
@@ -21,8 +21,8 @@ export function generateOperation(operation: TypeSpecOperation): string {
 
   // generate parameters
   const parameters: string[] = [
-    ...operation.parameters.map(generateOperationParameter),
-    ...generateRequestBodyParameters(operation.requestBodies),
+    ...operation.parameters.map((p) => generateOperationParameter(operation, p, context)),
+    ...generateRequestBodyParameters(operation.requestBodies, context),
   ];
 
   const responseTypes = operation.responseTypes.length
@@ -34,14 +34,13 @@ export function generateOperation(operation: TypeSpecOperation): string {
   return definitions.join(" ");
 }
 
-function generateOperationParameter(parameter: Refable<TypeSpecOperationParameter>) {
+function generateOperationParameter(
+  operation: TypeSpecOperation,
+  parameter: Refable<TypeSpecOperationParameter>,
+  context: Context
+) {
   if ("$ref" in parameter) {
-    // check if referencing a model or a property
-    const refName = getRefName(parameter.$ref);
-    const paramName = refName.indexOf(".") >= 0 ? refName.split(".").pop() : refName;
-    // when refName and paramName match, we're referencing a model and can spread
-    // TODO: Handle optionality
-    return refName === paramName ? `...${refName}` : `${paramName}: ${refName}`;
+    return `...${context.getRefName(parameter.$ref, operation.scope)}`;
   }
 
   const definitions: string[] = [];
@@ -53,13 +52,16 @@ function generateOperationParameter(parameter: Refable<TypeSpecOperationParamete
   definitions.push(...generateDecorators(parameter.decorators));
 
   definitions.push(
-    `${parameter.name}${parameter.isOptional ? "?" : ""}: ${generateTypeFromSchema(parameter.schema)}`
+    `${parameter.name}${parameter.isOptional ? "?" : ""}: ${context.generateTypeFromRefableSchema(parameter.schema, operation.scope)}`
   );
 
   return definitions.join(" ");
 }
 
-function generateRequestBodyParameters(requestBodies: TypeSpecRequestBody[]): string[] {
+function generateRequestBodyParameters(
+  requestBodies: TypeSpecRequestBody[],
+  context: Context
+): string[] {
   if (!requestBodies.length) {
     return [];
   }
@@ -74,7 +76,11 @@ function generateRequestBodyParameters(requestBodies: TypeSpecRequestBody[]): st
 
   // Get the set of referenced types
   const body = Array.from(
-    new Set(requestBodies.filter((r) => !!r.schema).map((r) => generateTypeFromSchema(r.schema!)))
+    new Set(
+      requestBodies
+        .filter((r) => !!r.schema)
+        .map((r) => context.generateTypeFromRefableSchema(r.schema!, []))
+    )
   ).join(" | ");
 
   if (body) {

diff --git a/packages/openapi3/src/cli/actions/convert/generators/generate-service-info.ts b/packages/openapi3/src/cli/actions/convert/generators/generate-service-info.ts
@@ -1,5 +1,6 @@
 import { TypeSpecServiceInfo } from "../interfaces.js";
 import { generateDocs } from "../utils/docs.js";
+import { generateNamespaceName } from "../utils/generate-namespace-name.js";
 
 export function generateServiceInformation(serviceInfo: TypeSpecServiceInfo): string {
   const definitions: string[] = [];
@@ -21,7 +22,3 @@ export function generateServiceInformation(serviceInfo: TypeSpecServiceInfo): st
 
   return definitions.join("\n");
 }
-
-function generateNamespaceName(name: string): string {
-  return name.replaceAll(/[^\w^\d_]+/g, "");
-}