Skip to content

Commit

Permalink
[tcgc] add generated names for constants (#766)
Browse files Browse the repository at this point in the history
fixes #553

---------

Co-authored-by: iscai-msft <[email protected]>
  • Loading branch information
2 people authored and tadelesh committed May 10, 2024
1 parent 088863a commit 38e4293
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 21 deletions.
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 @@ -187,7 +187,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(retval.bodyParam),
...createContentTypeOrAcceptHeader(httpOperation, retval.bodyParam),
description: `Body parameter's content type. Known values are ${retval.bodyParam.contentTypes}`,
};
if (!methodParameters.some((m) => m.name === "contentType")) {
Expand All @@ -206,7 +206,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 @@ -230,6 +230,7 @@ function getSdkHttpParameters(
}

function createContentTypeOrAcceptHeader(
httpOperation: HttpOperation,
bodyObject: SdkBodyParameter | SdkHttpResponse
): Omit<SdkMethodParameter, "kind"> {
const name = bodyObject.kind === "body" ? "contentType" : "accept";
Expand All @@ -256,6 +257,8 @@ function createContentTypeOrAcceptHeader(
kind: "constant",
value: bodyObject.contentTypes[0],
valueType: type,
name: `${httpOperation.operation.name}ContentType`,
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,12 +1,15 @@
import { getUnionAsEnum } from "@azure-tools/typespec-azure-core";
import {
BooleanLiteral,
Diagnostic,
Interface,
Model,
Namespace,
NumericLiteral,
Operation,
Program,
ProjectedProgram,
StringLiteral,
Type,
Union,
createDiagnosticCollector,
Expand Down Expand Up @@ -282,6 +285,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 @@ -293,7 +299,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>;
__namespaceToApiVersionParameter: Map<Interface | Namespace, 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))}`;
} 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 @@ -389,7 +389,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 @@ -401,15 +402,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

0 comments on commit 38e4293

Please sign in to comment.