Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tcgc] update crossLanguageDefinitionId to correspond to apiview #936

Merged
merged 9 commits into from
Jun 3, 2024
7 changes: 7 additions & 0 deletions .chronus/changes/tsp_clid-2024-5-3-18-13-18.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@azure-tools/typespec-client-generator-core"
---

enhance cross language definition id logic
9 changes: 6 additions & 3 deletions packages/typespec-client-generator-core/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import {
isSubscriptionId,
} from "./internal-utils.js";
import { createDiagnostic } from "./lib.js";
import { getEffectivePayloadType } from "./public-utils.js";
import { getCrossLanguageDefinitionId, getEffectivePayloadType } from "./public-utils.js";
import {
addEncodeInfo,
addFormatInfo,
Expand Down Expand Up @@ -165,6 +165,7 @@ function getSdkHttpParameters(
type,
optional: false,
correspondingMethodParams,
crossLanguageDefinitionId: `${getCrossLanguageDefinitionId(context, httpOperation.operation)}.body`,
};
}
if (retval.bodyParam) {
Expand All @@ -182,7 +183,7 @@ function getSdkHttpParameters(
if (retval.bodyParam && !headerParams.some((h) => isContentTypeHeader(h))) {
// if we have a body param and no content type header, we add one
const contentTypeBase = {
...createContentTypeOrAcceptHeader(httpOperation, retval.bodyParam),
...createContentTypeOrAcceptHeader(context, httpOperation, retval.bodyParam),
description: `Body parameter's content type. Known values are ${retval.bodyParam.contentTypes}`,
};
if (!methodParameters.some((m) => m.name === "contentType")) {
Expand All @@ -201,7 +202,7 @@ function getSdkHttpParameters(
if (responseBody && !headerParams.some((h) => isAcceptHeader(h))) {
// If our operation returns a body, we add an accept header if none exist
const acceptBase = {
...createContentTypeOrAcceptHeader(httpOperation, responseBody),
...createContentTypeOrAcceptHeader(context, httpOperation, responseBody),
};
if (!methodParameters.some((m) => m.name === "accept")) {
methodParameters.push({
Expand All @@ -225,6 +226,7 @@ function getSdkHttpParameters(
}

function createContentTypeOrAcceptHeader(
context: TCGCContext,
httpOperation: HttpOperation,
bodyObject: SdkBodyParameter | SdkHttpResponse
): Omit<SdkMethodParameter, "kind"> {
Expand Down Expand Up @@ -264,6 +266,7 @@ function createContentTypeOrAcceptHeader(
isApiVersionParam: false,
onClient: false,
optional: false,
crossLanguageDefinitionId: `${getCrossLanguageDefinitionId(context, httpOperation.operation)}.${name}`,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ export interface SdkModelPropertyTypeBase {
clientDefaultValue?: any;
isApiVersionParam: boolean;
optional: boolean;
crossLanguageDefinitionId: string;
}

export interface SdkEndpointParameter extends SdkModelPropertyTypeBase {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,12 @@ export function getAllResponseBodies(
* Otherwise, you should use the `getGeneratedName` function.
* @param context
*/
export function createGeneratedName(type: Namespace | Operation, suffix: string): string {
return `${getCrossLanguageDefinitionId(type).split(".").at(-1)}${suffix}`;
export function createGeneratedName(
context: TCGCContext,
type: Namespace | Operation,
suffix: string
): string {
return `${getCrossLanguageDefinitionId(context, type).split(".").at(-1)}${suffix}`;
}

function isOperationBodyType(context: TCGCContext, type: Type, operation?: Operation): boolean {
Expand Down
17 changes: 12 additions & 5 deletions packages/typespec-client-generator-core/src/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,11 @@ function getSdkLroServiceMethod<
});
}

function getSdkMethodResponse(
function getSdkMethodResponse<
TOptions extends object,
TServiceOperation extends SdkServiceOperation,
>(
context: SdkContext<TOptions, TServiceOperation>,
operation: Operation,
sdkOperation: SdkServiceOperation
): SdkMethodResponse {
Expand All @@ -208,7 +212,7 @@ function getSdkMethodResponse(
__raw: operation,
kind: "union",
values: allResponseBodies,
name: createGeneratedName(operation, "UnionResponse"),
name: createGeneratedName(context, operation, "UnionResponse"),
isGeneratedName: true,
};
} else if (responseTypes) {
Expand Down Expand Up @@ -275,7 +279,7 @@ function getSdkBasicServiceMethod<
const serviceOperation = diagnostics.pipe(
getSdkServiceOperation<TOptions, TServiceOperation>(context, operation, methodParameters)
);
const response = getSdkMethodResponse(operation, serviceOperation);
const response = getSdkMethodResponse(context, operation, serviceOperation);
const name = getLibraryName(context, operation);
return diagnostics.wrap({
__raw: operation,
Expand All @@ -298,7 +302,7 @@ function getSdkBasicServiceMethod<
getResponseMapping: function getResponseMapping(): string | undefined {
return undefined; // currently we only return a value for paging or lro
},
crossLanguageDefintionId: getCrossLanguageDefinitionId({ ...operation, name }),
crossLanguageDefintionId: getCrossLanguageDefinitionId(context, operation),
});
}

Expand Down Expand Up @@ -413,6 +417,7 @@ function getSdkMethodParameter(
serializedName: name,
isApiVersionParam: false,
onClient: false,
crossLanguageDefinitionId: "anonymous",
});
}
return diagnostics.wrap({
Expand Down Expand Up @@ -445,7 +450,7 @@ function getSdkMethods<TOptions extends object, TServiceOperation extends SdkSer
access: "internal",
response: operationGroupClient,
apiVersions: getAvailableApiVersions(context, operationGroup.type, client.type),
crossLanguageDefintionId: getCrossLanguageDefinitionId({ ...operationGroup.type, name }),
crossLanguageDefintionId: getCrossLanguageDefinitionId(context, operationGroup.type),
});
}
return diagnostics.wrap(retval);
Expand Down Expand Up @@ -484,6 +489,7 @@ function getSdkEndpointParameter(
},
isApiVersionParam: false,
apiVersions: context.__tspTypeToApiVersions.get(client.type)!,
crossLanguageDefinitionId: `${getCrossLanguageDefinitionId(context, client.service)}.endpoint`,
},
],
};
Expand Down Expand Up @@ -532,6 +538,7 @@ function getSdkEndpointParameter(
apiVersions: context.__tspTypeToApiVersions.get(client.type)!,
optional,
isApiVersionParam: false,
crossLanguageDefinitionId: `${getCrossLanguageDefinitionId(context, client.service)}.endpoint`,
});
}

Expand Down
123 changes: 80 additions & 43 deletions packages/typespec-client-generator-core/src/public-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ModelProperty,
Namespace,
Operation,
Scalar,
Type,
Union,
createDiagnosticCollector,
Expand All @@ -24,6 +25,7 @@ import {
getHttpOperation,
getPathParamName,
getQueryParamName,
isMetadata,
isStatusCode,
} from "@typespec/http";
import { Version, getVersions } from "@typespec/versioning";
Expand Down Expand Up @@ -228,20 +230,45 @@ export function getWireName(context: TCGCContext, type: Type & { name: string })
* @returns
*/
export function getCrossLanguageDefinitionId(
type: {
name?: string;
kind: string;
interface?: Interface;
namespace?: Namespace;
},
name?: string
context: TCGCContext,
type: Union | Model | Enum | Scalar | ModelProperty | Operation | Namespace | Interface,
appendNamespace: boolean = true
): string {
let retval = type.name ? type.name : name ?? "";
if (type.kind === "Operation" && type.interface) {
retval = `${type.interface.name}.${retval}`;
let retval = type.name || "anonymous";
const namespace = type.kind === "ModelProperty" ? type.model?.namespace : type.namespace;
switch (type.kind) {
case "Union":
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) {
retval = `${getCrossLanguageDefinitionId(context, type.model, false)}.${retval}`;
}
break;
case "Operation":
if (type.interface) {
retval = `${getCrossLanguageDefinitionId(context, type.interface, false)}.${retval}`;
}
break;
}
if (type.namespace) {
retval = `${getNamespaceFullName(type.namespace!)}.${retval}`;
if (appendNamespace && namespace && getNamespaceFullName(namespace)) {
retval = `${getNamespaceFullName(namespace)}.${retval}`;
}
return retval;
}
Expand Down Expand Up @@ -302,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) {
Expand All @@ -316,21 +356,12 @@ function findContextPath(
}
}
}
// orphan models
if (client.service) {
for (const model of client.service.models.values()) {
const result = getContextPath(context, model, type);
if (result.length > 0) {
return result;
}
}
}
}
return [];
}

interface ContextNode {
displayName: string;
name: string;
type?: Model | Union | TspLiteralType;
}

Expand All @@ -355,15 +386,15 @@ function getContextPath(

if (httpOperation.parameters.body) {
visited.clear();
result = [{ displayName: pascalCase(root.name) }];
result = [{ name: root.name }];
if (dfsModelProperties(typeToFind, httpOperation.parameters.body.type, "Request")) {
return result;
}
}

for (const parameter of Object.values(httpOperation.parameters.parameters)) {
visited.clear();
result = [{ displayName: pascalCase(root.name) }];
result = [{ name: root.name }];
if (
dfsModelProperties(typeToFind, parameter.param.type, `Request${pascalCase(parameter.name)}`)
) {
Expand All @@ -375,7 +406,7 @@ function getContextPath(
for (const innerResponse of response.responses) {
if (innerResponse.body?.type) {
visited.clear();
result = [{ displayName: pascalCase(root.name) }];
result = [{ name: root.name }];
if (dfsModelProperties(typeToFind, innerResponse.body.type, "Response")) {
return result;
}
Expand All @@ -384,7 +415,7 @@ function getContextPath(
if (innerResponse.headers) {
for (const header of Object.values(innerResponse.headers)) {
visited.clear();
result = [{ displayName: pascalCase(root.name) }];
result = [{ name: root.name }];
if (dfsModelProperties(typeToFind, header.type, `Response${pascalCase(header.name)}`)) {
return result;
}
Expand Down Expand Up @@ -432,7 +463,7 @@ function getContextPath(
visited.add(currentType);

if (currentType === expectedType) {
result.push({ displayName: pascalCase(displayName), type: currentType });
result.push({ name: displayName, type: currentType });
return true;
} else if (
currentType.kind === "Model" &&
Expand All @@ -447,7 +478,7 @@ function getContextPath(
} else if (currentType.kind === "Model") {
currentType = getEffectivePayloadType(context, currentType);
// handle model
result.push({ displayName: pascalCase(displayName), type: currentType });
result.push({ name: displayName, type: currentType });
for (const property of currentType.properties.values()) {
// traverse model property
// use property.name as displayName
Expand Down Expand Up @@ -512,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
Expand All @@ -531,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++) {
Expand All @@ -553,10 +589,11 @@ function buildNameFromContextPaths(
currContextPathType?.kind === "Number" ||
currContextPathType?.kind === "Boolean"
) {
createName = `${createName}${contextPath[j].displayName}`;
// constant type
createName = `${createName}${pascalCase(contextPath[j].name)}`;
} else if (!currContextPathType?.name) {
// is anonymous model node
createName = `${createName}${contextPath[j].displayName}`;
createName = `${createName}${pascalCase(contextPath[j].name)}`;
} else {
// is non-anonymous model, use type name
createName = `${createName}${currContextPathType!.name!}`;
Expand Down
Loading
Loading