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] add generated names for constants #766

Merged
merged 2 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .chronus/changes/add_naming_constant-2024-3-30-17-2-35.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@azure-tools/typespec-client-generator-core"
---

add generated names for constants
7 changes: 5 additions & 2 deletions packages/typespec-client-generator-core/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ function getSdkHttpParameters(
) {
// if we have a body param and no content type header, we add one
const contentTypeBase = {
...createContentTypeOrAcceptHeader(retval.bodyParam),
...createContentTypeOrAcceptHeader(httpOperation, retval.bodyParam),
description: `Body parameter's content type. Known values are ${retval.bodyParam.contentTypes}`,
};
if (!methodParameters.some((m) => m.__raw && isContentTypeHeader(context.program, m.__raw))) {
Expand All @@ -197,7 +197,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(responseBody),
...createContentTypeOrAcceptHeader(httpOperation, responseBody),
};
if (!methodParameters.some((m) => m.name === "accept")) {
methodParameters.push({
Expand All @@ -221,6 +221,7 @@ function getSdkHttpParameters(
}

function createContentTypeOrAcceptHeader(
httpOperation: HttpOperation,
bodyObject: SdkBodyParameter | SdkHttpResponse
): Omit<SdkMethodParameter, "kind"> {
const name = bodyObject.kind === "body" ? "contentType" : "accept";
Expand All @@ -247,6 +248,8 @@ function createContentTypeOrAcceptHeader(
kind: "constant",
value: bodyObject.contentTypes[0],
valueType: type,
name: `${httpOperation.operation.name}ContentType`,
iscai-msft marked this conversation as resolved.
Show resolved Hide resolved
isGeneratedName: true,
};
}
// No need for clientDefaultValue because it's a constant, it only has one value
Expand Down
2 changes: 2 additions & 0 deletions packages/typespec-client-generator-core/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,8 @@ export interface SdkConstantType extends SdkTypeBase {
kind: "constant";
value: string | number | boolean | null;
valueType: SdkBuiltInType;
name: string;
isGeneratedName: boolean;
}

export interface SdkUnionType extends SdkTypeBase {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { getUnionAsEnum } from "@azure-tools/typespec-azure-core";
import {
BooleanLiteral,
Diagnostic,
Model,
Namespace,
NumericLiteral,
Operation,
Program,
ProjectedProgram,
StringLiteral,
Type,
Union,
createDiagnosticCollector,
Expand Down Expand Up @@ -253,6 +256,9 @@ export function isMultipartOperation(context: TCGCContext, operation?: Operation
export function isHttpOperation(context: TCGCContext, obj: any): obj is HttpOperation {
return obj?.kind === "Operation" && getHttpOperationWithCache(context, obj) !== undefined;
}

export type TspLiteralType = StringLiteral | NumericLiteral | BooleanLiteral;

export interface TCGCContext {
program: Program;
emitterName: string;
Expand All @@ -264,7 +270,7 @@ export interface TCGCContext {
arm?: boolean;
modelsMap?: Map<Type, SdkModelType | SdkEnumType>;
operationModelsMap?: Map<Operation, Map<Type, SdkModelType | SdkEnumType>>;
generatedNames?: Map<Union | Model, string>;
generatedNames?: Map<Union | Model | TspLiteralType, string>;
httpOperationCache?: Map<Operation, HttpOperation>;
unionsMap?: Map<Union, SdkUnionType>;
__api_version_parameter?: SdkParameter;
Expand Down
48 changes: 33 additions & 15 deletions packages/typespec-client-generator-core/src/public-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,20 @@ import {
isStatusCode,
} from "@typespec/http";
import { Version, getVersions } from "@typespec/versioning";
import { pascalCase } from "change-case";
import { capitalCase, pascalCase } from "change-case";
import pluralize from "pluralize";
import {
getClientNameOverride,
listClients,
listOperationGroups,
listOperationsInOperationGroup,
} from "./decorators.js";
import { TCGCContext, getClientNamespaceStringHelper, parseEmitterName } from "./internal-utils.js";
import {
TCGCContext,
TspLiteralType,
getClientNamespaceStringHelper,
parseEmitterName,
} from "./internal-utils.js";
import { createDiagnostic } from "./lib.js";

/**
Expand Down Expand Up @@ -256,11 +261,11 @@ export function getCrossLanguagePackageId(context: TCGCContext): [string, readon
*/
export function getGeneratedName(
context: TCGCContext,
type: Model | Union,
type: Model | Union | TspLiteralType,
operation?: Operation
): string {
if (!context.generatedNames) {
context.generatedNames = new Map<Union | Model, string>();
context.generatedNames = new Map<Union | Model | TspLiteralType, string>();
}
const generatedName = context.generatedNames.get(type);
if (generatedName) return generatedName;
Expand All @@ -278,7 +283,10 @@ export function getGeneratedName(
* @param type
* @returns
*/
function findContextPath(context: TCGCContext, type: Model | Union): ContextNode[] {
function findContextPath(
context: TCGCContext,
type: Model | Union | TspLiteralType
): ContextNode[] {
for (const client of listClients(context)) {
for (const operation of listOperationsInOperationGroup(context, client)) {
const result = getContextPath(context, operation, type);
Expand Down Expand Up @@ -309,7 +317,7 @@ function findContextPath(context: TCGCContext, type: Model | Union): ContextNode

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

/**
Expand All @@ -322,7 +330,7 @@ interface ContextNode {
function getContextPath(
context: TCGCContext,
root: Operation | Model,
typeToFind: Model | Union
typeToFind: Model | Union | TspLiteralType
): ContextNode[] {
// use visited set to avoid cycle model reference
const visited: Set<Type> = new Set<Type>();
Expand Down Expand Up @@ -392,7 +400,7 @@ function getContextPath(
* @returns
*/
function dfsModelProperties(
expectedType: Model | Union,
expectedType: Model | Union | TspLiteralType,
currentType: Type,
displayName: string
): boolean {
Expand All @@ -401,7 +409,7 @@ function getContextPath(
return false;
}

if (!(currentType.kind === "Model" || currentType.kind === "Union")) {
if (!["Model", "Union", "String", "Number", "Boolean"].includes(currentType.kind)) {
return false;
}

Expand Down Expand Up @@ -473,7 +481,7 @@ function getContextPath(
if (result) return true;
}
return false;
} else {
} else if (currentType.kind === "Union") {
// handle union
for (const unionType of currentType.variants.values()) {
// traverse union type
Expand All @@ -482,6 +490,8 @@ function getContextPath(
if (result) return true;
}
return false;
} else {
return false;
}
}
}
Expand All @@ -496,7 +506,7 @@ function getContextPath(
*/
function buildNameFromContextPaths(
context: TCGCContext,
type: Union | Model,
type: Union | Model | TspLiteralType,
contextPath: ContextNode[]
): string {
// fallback to empty name for corner case
Expand All @@ -507,9 +517,10 @@ 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 ||
contextPath[lastNonAnonymousModelNodeIndex].type?.name
(currType?.kind === "Model" && currType.name)
) {
// it's nonanonymous model node (if no type defined, it's the operation node)
break;
Expand All @@ -520,12 +531,19 @@ function buildNameFromContextPaths(
// 2. build name
let createName: string = "";
for (let j = lastNonAnonymousModelNodeIndex; j < contextPath.length; j++) {
if (!contextPath[j]?.type?.name) {
const currContextPathType = contextPath[j]?.type;
if (
currContextPathType?.kind === "String" ||
currContextPathType?.kind === "Number" ||
currContextPathType?.kind === "Boolean"
) {
createName = `${createName}${contextPath[j].displayName}${capitalCase(String(currContextPathType.value))}`;
iscai-msft marked this conversation as resolved.
Show resolved Hide resolved
} else if (!currContextPathType?.name) {
// is anonymous model node
createName = `${createName}${contextPath[j].displayName}`;
} else {
// is non-anonymous model, use type name
createName = `${createName}${contextPath[j]!.type!.name!}`;
createName = `${createName}${currContextPathType!.name!}`;
}
}
// 3. simplely handle duplication
Expand All @@ -538,7 +556,7 @@ function buildNameFromContextPaths(
if (context.generatedNames) {
context.generatedNames.set(type, createName);
} else {
context.generatedNames = new Map<Union | Model, string>([[type, createName]]);
context.generatedNames = new Map<Union | Model | TspLiteralType, string>([[type, createName]]);
}
return createName;
}
Expand Down
10 changes: 7 additions & 3 deletions packages/typespec-client-generator-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,8 @@ export function getSdkUnionWithDiagnostics(

function getSdkConstantWithDiagnostics(
context: TCGCContext,
type: StringLiteral | NumericLiteral | BooleanLiteral
type: StringLiteral | NumericLiteral | BooleanLiteral,
operation?: Operation
): [SdkConstantType, readonly Diagnostic[]] {
const diagnostics = createDiagnosticCollector();
switch (type.kind) {
Expand All @@ -399,15 +400,18 @@ function getSdkConstantWithDiagnostics(
...getSdkTypeBaseHelper(context, type, "constant"),
value: type.value,
valueType,
name: getGeneratedName(context, type, operation),
isGeneratedName: true,
});
}
}

export function getSdkConstant(
context: TCGCContext,
type: StringLiteral | NumericLiteral | BooleanLiteral
type: StringLiteral | NumericLiteral | BooleanLiteral,
operation?: Operation
): SdkConstantType {
return ignoreDiagnostics(getSdkConstantWithDiagnostics(context, type));
return ignoreDiagnostics(getSdkConstantWithDiagnostics(context, type, operation));
}

function addDiscriminatorToModelType(
Expand Down
6 changes: 6 additions & 0 deletions packages/typespec-client-generator-core/test/types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1941,6 +1941,8 @@ describe("typespec-client-generator-core: types", () => {
strictEqual(sdkType.kind, "constant");
strictEqual(sdkType.valueType.kind, "string");
strictEqual(sdkType.value, "json");
strictEqual(sdkType.name, "TestPropJson");
strictEqual(sdkType.isGeneratedName, true);
});
it("boolean", async function () {
await runner.compileWithBuiltInService(`
Expand All @@ -1955,6 +1957,8 @@ describe("typespec-client-generator-core: types", () => {
strictEqual(sdkType.kind, "constant");
strictEqual(sdkType.valueType.kind, "boolean");
strictEqual(sdkType.value, true);
strictEqual(sdkType.name, "TestPropTrue");
strictEqual(sdkType.isGeneratedName, true);
});
it("number", async function () {
await runner.compileWithBuiltInService(`
Expand All @@ -1969,6 +1973,8 @@ describe("typespec-client-generator-core: types", () => {
strictEqual(sdkType.kind, "constant");
strictEqual(sdkType.valueType.kind, "int32");
strictEqual(sdkType.value, 4);
strictEqual(sdkType.name, "TestProp4");
strictEqual(sdkType.isGeneratedName, true);
});
});
describe("SdkModelType", () => {
Expand Down
Loading