changelog

Azure · Jun 3, 2024 · 2001122 · 2001122
1 parent 270c22e
commit 2001122
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 58 deletions.
diff --git a/.chronus/changes/tsp_clid-2024-5-3-18-13-18.md b/.chronus/changes/tsp_clid-2024-5-3-18-13-18.md
@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@azure-tools/typespec-client-generator-core"
+---
+
+enhance cross language definition id logic
diff --git a/packages/typespec-client-generator-core/src/public-utils.ts b/packages/typespec-client-generator-core/src/public-utils.ts
@@ -25,6 +25,7 @@ import {
   getHttpOperation,
   getPathParamName,
   getQueryParamName,
+  isMetadata,
   isStatusCode,
 } from "@typespec/http";
 import { Version, getVersions } from "@typespec/versioning";
@@ -36,7 +37,6 @@ import {
   listOperationGroups,
   listOperationsInOperationGroup,
 } from "./decorators.js";
-import { SdkClient } from "./interfaces.js";
 import {
   TCGCContext,
   TspLiteralType,
@@ -231,36 +231,30 @@ export function getWireName(context: TCGCContext, type: Type & { name: string })
  */
 export function getCrossLanguageDefinitionId(
   context: TCGCContext,
-  type: Union | Model | Enum | ModelProperty | Operation | Namespace | Interface | Scalar,
+  type: Union | Model | Enum | Scalar | ModelProperty | Operation | Namespace | Interface,
   appendNamespace: boolean = true
 ): string {
   let retval = type.name || "anonymous";
   const namespace = type.kind === "ModelProperty" ? type.model?.namespace : type.namespace;
   switch (type.kind) {
-    case "Model":
     case "Union":
-      let contextPath: string | undefined = undefined;
-      // we do it with separate calls to findContextPath because we want to prioritize finding
-      // the context path through models over the context path through operations
-      for (const client of listClients(context)) {
-        contextPath = findContextPathThroughModels(context, client, type)
-          ?.map((n) => n.name)
-          .join(".");
-        if (contextPath) {
-          break;
-        }
-      }
-      contextPath =
-        contextPath ||
-        findContextPath(context, type)
-          .map((n) => n.name)
-          .join(".");
-      if (retval === "anonymous") {
-        retval = contextPath + "." + retval;
-      } else if (contextPath) {
-        retval = contextPath;
+    case "Model":
+      // Enum and Scalar will always have a name
+      if (type.name) {
+        break;
       }
-
+      const contextPath = findContextPath(context, type);
+      retval =
+        contextPath
+          .slice(findLastNonAnonymousModelNode(contextPath))
+          .map((x) =>
+            x.type?.kind === "Model" || x.type?.kind === "Union"
+              ? x.type.name || x.name
+              : x.name || "anonymous"
+          )
+          .join(".") +
+        "." +
+        retval;
       break;
     case "ModelProperty":
       if (type.model) {
@@ -324,22 +318,6 @@ export function getGeneratedName(
   return createdName;
 }
 
-function findContextPathThroughModels(
-  context: TCGCContext,
-  client: SdkClient,
-  type: Model | Union | TspLiteralType
-): ContextNode[] | undefined {
-  if (client.service) {
-    for (const model of client.service.models.values()) {
-      const result = getContextPath(context, model, type);
-      if (result.length > 0) {
-        return result;
-      }
-    }
-  }
-  return undefined;
-}
-
 /**
  * Traverse each operation and model to find one possible context path for the given type.
  * @param context
@@ -351,6 +329,19 @@ function findContextPath(
   type: Model | Union | TspLiteralType
 ): ContextNode[] {
   for (const client of listClients(context)) {
+    // orphan models
+    if (client.service) {
+      for (const model of client.service.models.values()) {
+        if (
+          [...model.properties.values()].filter((p) => !isMetadata(context.program, p)).length === 0
+        )
+          continue;
+        const result = getContextPath(context, model, type);
+        if (result.length > 0) {
+          return result;
+        }
+      }
+    }
     for (const operation of listOperationsInOperationGroup(context, client)) {
       const result = getContextPath(context, operation, type);
       if (result.length > 0) {
@@ -365,11 +356,6 @@ function findContextPath(
         }
       }
     }
-    // orphan models
-    const modelContextPath = findContextPathThroughModels(context, client, type);
-    if (modelContextPath) {
-      return modelContextPath;
-    }
   }
   return [];
 }
@@ -557,6 +543,23 @@ function getContextPath(
   }
 }
 
+function findLastNonAnonymousModelNode(contextPath: ContextNode[]): number {
+  let lastNonAnonymousModelNodeIndex = contextPath.length - 1;
+  while (lastNonAnonymousModelNodeIndex >= 0) {
+    const currType = contextPath[lastNonAnonymousModelNodeIndex].type;
+    if (
+      !contextPath[lastNonAnonymousModelNodeIndex].type ||
+      (currType?.kind === "Model" && currType.name)
+    ) {
+      // it's nonanonymous model node (if no type defined, it's the operation node)
+      break;
+    } else {
+      --lastNonAnonymousModelNodeIndex;
+    }
+  }
+  return lastNonAnonymousModelNodeIndex;
+}
+
 /**
  * The logic is basically three steps:
  * 1. find the last nonanonymous model node, this node can be operation node or model node which is not anonymous
@@ -576,19 +579,7 @@ function buildNameFromContextPaths(
   }
 
   // 1. find the last nonanonymous model node
-  let lastNonAnonymousModelNodeIndex = contextPath.length - 1;
-  while (lastNonAnonymousModelNodeIndex >= 0) {
-    const currType = contextPath[lastNonAnonymousModelNodeIndex].type;
-    if (
-      !contextPath[lastNonAnonymousModelNodeIndex].type ||
-      (currType?.kind === "Model" && currType.name)
-    ) {
-      // it's nonanonymous model node (if no type defined, it's the operation node)
-      break;
-    } else {
-      --lastNonAnonymousModelNodeIndex;
-    }
-  }
+  const lastNonAnonymousModelNodeIndex = findLastNonAnonymousModelNode(contextPath);
   // 2. build name
   let createName: string = "";
   for (let j = lastNonAnonymousModelNodeIndex; j < contextPath.length; j++) {