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 common layer refine #517

Merged
merged 21 commits into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
abfd0f4
fix wrong union `generatedName` flag and refine templated model naming
tadelesh Mar 26, 2024
f552052
changelog
tadelesh Mar 26, 2024
cb1265b
renaming
tadelesh Mar 26, 2024
3b91eea
Merge branch 'main' into fix_tcgc_type_issue
tadelesh Mar 27, 2024
c01e284
format
tadelesh Mar 27, 2024
d712d63
Merge remote-tracking branch 'origin/main' into fix_tcgc_type_issue
tadelesh Mar 28, 2024
d373883
spread method parameter feature
tadelesh Mar 28, 2024
09a9adc
Update packages/typespec-client-generator-core/test/package.test.ts
tadelesh Mar 28, 2024
7dbf2c5
Merge branch 'main' of https://github.com/Azure/typespec-azure into f…
Mar 28, 2024
16fb405
Merge branch 'main' into fix_tcgc_type_issue
tadelesh Mar 29, 2024
0a563a2
Merge branch 'main' of https://github.com/Azure/typespec-azure into f…
Mar 29, 2024
f59d016
handle case with multiple method param properties mapping to body param
Mar 29, 2024
c52e0a3
Merge branch 'fix_tcgc_type_issue' of https://github.com/Azure/typesp…
Mar 29, 2024
7758cb2
fix api version test
Mar 29, 2024
e1ed5b4
Merge branch 'main' into fix_tcgc_type_issue
iscai-msft Mar 29, 2024
0c77ac7
can repro
Mar 29, 2024
4753a2a
get tests passing
Mar 30, 2024
19b0372
Merge branch 'fix_tcgc_type_issue' of https://github.com/Azure/typesp…
Mar 30, 2024
098b85b
Merge branch 'main' of https://github.com/Azure/typespec-azure into f…
Mar 30, 2024
1486b52
fix build errors
Mar 31, 2024
7e9d3cb
fix case of @body in spread model
Mar 31, 2024
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/fix_tcgc_type_issue-2024-2-26-17-43-28.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@azure-tools/typespec-client-generator-core"
---

fix wrong union `generatedName` flag and refine templated model naming
69 changes: 47 additions & 22 deletions packages/typespec-client-generator-core/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
isPathParam,
isQueryParam,
} from "@typespec/http";
import { camelCase } from "change-case";
import {
CollectionFormat,
SdkBodyParameter,
Expand All @@ -29,6 +30,7 @@ import {
SdkHttpResponse,
SdkMethodParameter,
SdkModelPropertyType,
SdkModelType,
SdkParameter,
SdkPathParameter,
SdkQueryParameter,
Expand Down Expand Up @@ -123,10 +125,14 @@ function getSdkHttpParameters(
if (getParamResponse.kind !== "body") throw new Error("blah");
retval.bodyParam = getParamResponse;
} else {
const type = diagnostics.pipe(
getClientTypeWithDiagnostics(context, tspBody.type, httpOperation.operation)
);
const name = camelCase((type as { name: string }).name ?? "body");
retval.bodyParam = {
kind: "body",
name: "body",
nameInClient: "body",
name,
nameInClient: name,
isGeneratedName: true,
description: getDocHelper(context, tspBody.type).description,
details: getDocHelper(context, tspBody.type).details,
Expand All @@ -135,9 +141,7 @@ function getSdkHttpParameters(
defaultContentType: "application/json", // actual content type info is added later
isApiVersionParam: false,
apiVersions: getAvailableApiVersions(context, tspBody.type),
type: diagnostics.pipe(
getClientTypeWithDiagnostics(context, tspBody.type, httpOperation.operation)
),
type,
optional: false,
nullable: isNullable(tspBody.type),
correspondingMethodParams,
Expand Down Expand Up @@ -393,33 +397,54 @@ export function getCorrespondingMethodParams(
serviceParam: SdkHttpParameter
): SdkModelPropertyType[] {
if (serviceParam.isApiVersionParam) {
if (!context.__api_version_parameter) throw new Error("No api version on the client");
if (!context.__api_version_parameter) {
const apiVersionParam = methodParameters.find((x) => x.name.includes("apiVersion"));
if (!apiVersionParam) {
throw new Error("Can't find corresponding api version parameter");
}
if (apiVersionParam.type.kind === "model") throw new Error(apiVersionParam.type.name);
throw new Error(apiVersionParam.type.kind);
}
return [context.__api_version_parameter];
}
const correspondingMethodParameter = methodParameters.find((x) => x.name === serviceParam.name);
if (correspondingMethodParameter) {
return [correspondingMethodParameter];
}
function paramInProperties(param: SdkModelPropertyType, type: SdkType): boolean {
if (type.kind !== "model") return false;
return Array.from(type.properties.values())
.filter((x) => x.kind === "property")
.map((x) => x.name)
.includes(param.name);
}

const serviceParamType = serviceParam.type;
if (serviceParam.kind === "body" && serviceParamType.kind === "model") {
// Here we have a spread body parameter
const correspondingProperties = methodParameters.filter((x) =>
paramInProperties(x, serviceParamType)
);
const bodyPropertyNames = serviceParamType.properties.filter((x) =>
paramInProperties(x, serviceParamType)
const serviceParamPropertyNames = Array.from(serviceParamType.properties.values())
.filter((x) => x.kind === "property")
.map((x) => x.name);
// Here we have a spread method parameter

// easy case is if the spread method parameter directly has the entire body as a property
const directBodyProperty = methodParameters
.map((x) => x.type)
.filter((x): x is SdkModelType => x.kind === "model")
.flatMap((x) => x.properties)
.find((x) => x.type === serviceParamType);
if (directBodyProperty) return [directBodyProperty];
let correspondingProperties: SdkModelPropertyType[] = methodParameters.filter((x) =>
serviceParamPropertyNames.includes(x.name)
);
if (correspondingProperties.length !== bodyPropertyNames.length) {
throw new Error("Can't find corresponding properties for spread body parameter");
for (const methodParam of methodParameters) {
const methodParamIterable =
methodParam.type.kind === "model" ? methodParam.type.properties : [methodParam];
correspondingProperties = correspondingProperties.concat(
methodParamIterable.filter(
(x) =>
serviceParamPropertyNames.includes(x.name) &&
!correspondingProperties.find((e) => e.name === x.name)
)
);
}
return correspondingProperties;
if (correspondingProperties.length === serviceParamType.properties.length)
return correspondingProperties;
throw new Error(
`Can't find corresponding parameter for ${serviceParam.name} out of ${methodParameters.map((m) => m.name).join(", ")}`
);
}
for (const methodParam of methodParameters) {
if (methodParam.type.kind === "model") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ export interface SdkEnumValueType extends SdkTypeBase {
name: string;
value: string | number;
enumType: SdkEnumType;
valueType: SdkType;
valueType: SdkBuiltInType;
description?: string;
details?: string;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { getUnionAsEnum } from "@azure-tools/typespec-azure-core";
import {
Model,
ModelProperty,
Namespace,
Operation,
Program,
Expand Down Expand Up @@ -82,7 +81,7 @@ export function getClientNamespaceStringHelper(
*/
export function updateWithApiVersionInformation(
context: TCGCContext,
type: ModelProperty
type: { name: string }
): {
isApiVersionParam: boolean;
clientDefaultValue?: unknown;
Expand Down
47 changes: 43 additions & 4 deletions packages/typespec-client-generator-core/src/package.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import { getLroMetadata, getPagedResult } from "@azure-tools/typespec-azure-core";
import {
Diagnostic,
Model,
ModelProperty,
Operation,
createDiagnosticCollector,
getNamespaceFullName,
getService,
isKey,
} from "@typespec/compiler";
import { getServers } from "@typespec/http";
import { resolveVersions } from "@typespec/versioning";
import { camelCase } from "change-case";
import {
getAccess,
listClients,
Expand Down Expand Up @@ -53,6 +56,7 @@ import {
getHashForType,
getSdkTypeBaseHelper,
isNullable,
updateWithApiVersionInformation,
} from "./internal-utils.js";
import { createDiagnostic } from "./lib.js";
import {
Expand Down Expand Up @@ -218,9 +222,24 @@ function getSdkBasicServiceMethod<
): [SdkServiceMethod<TServiceOperation>, readonly Diagnostic[]] {
const diagnostics = createDiagnosticCollector();
// when we spread, all of the inputtable properties of our model get flattened onto the method
const methodParameters = Array.from(operation.parameters.properties.values())
.map((x) => diagnostics.pipe(getSdkMethodParameter(context, x)))
.filter((x): x is SdkMethodParameter => x.kind === "method");
const methodParameters: SdkMethodParameter[] = [];
const spreadModelNames: string[] = [];
for (const prop of operation.parameters.properties.values()) {
if (prop.sourceProperty?.model?.name && !isKey(context.program, prop.sourceProperty)) {
if (!spreadModelNames.includes(prop.sourceProperty.model.name)) {
spreadModelNames.push(prop.sourceProperty.model.name);
methodParameters.push(
diagnostics.pipe(getSdkMethodParameter(context, prop.sourceProperty.model))
);
}
} else {
const methodParameter = diagnostics.pipe(getSdkMethodParameter(context, prop));
if (methodParameter.kind === "method") {
methodParameters.push(methodParameter);
}
}
}

// if there's an api version parameter, we want to bubble it up to the client
// we don't want it on the method level, but we will keep it on the service operation level
const apiVersionParam = methodParameters.find((x) => x.isApiVersionParam);
Expand Down Expand Up @@ -331,9 +350,29 @@ function getSdkInitializationType<

function getSdkMethodParameter(
context: TCGCContext,
type: ModelProperty
type: ModelProperty | Model
): [SdkMethodParameter, readonly Diagnostic[]] {
const diagnostics = createDiagnosticCollector();
if (type.kind === "Model") {
const libraryName = getLibraryName(context, type);
const name = camelCase(libraryName ?? "body");
const propertyType = diagnostics.pipe(getClientTypeWithDiagnostics(context, type));
return diagnostics.wrap({
kind: "method",
description: getDocHelper(context, type).description,
details: getDocHelper(context, type).details,
apiVersions: getAvailableApiVersions(context, type),
type: propertyType,
nameInClient: name,
name,
isGeneratedName: Boolean(libraryName),
optional: false,
nullable: false,
discriminator: false,
serializedName: name,
...updateWithApiVersionInformation(context, type),
});
}
return diagnostics.wrap({
...diagnostics.pipe(getSdkModelPropertyType(context, type)),
kind: "method",
Expand Down
17 changes: 10 additions & 7 deletions packages/typespec-client-generator-core/src/public-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
} from "@typespec/compiler";
import {
HttpOperation,
HttpOperationParameter,
getHeaderFieldName,
getHttpOperation,
getPathParamName,
Expand Down Expand Up @@ -62,12 +61,10 @@ export function getDefaultApiVersion(
* @param parameter
* @returns
*/
export function isApiVersion(
context: TCGCContext,
parameter: HttpOperationParameter | ModelProperty
): boolean {
export function isApiVersion(context: TCGCContext, type: { name: string }): boolean {
return (
parameter.name.toLowerCase() === "apiversion" || parameter.name.toLowerCase() === "api-version"
type.name.toLowerCase().includes("apiversion") ||
type.name.toLowerCase().includes("api-version")
);
}

Expand Down Expand Up @@ -174,7 +171,13 @@ export function getLibraryName(

// 5. if type is derived from template and name is the same as template, add template parameters' name as suffix
if (typeof type.name === "string" && type.kind === "Model" && type.templateMapper?.args) {
return type.name + type.templateMapper.args.map((arg) => (arg as Model).name).join("");
return (
type.name +
type.templateMapper.args
.filter((arg): arg is Model => arg.kind === "Model" && arg.name.length > 0)
.map((arg) => pascalCase(arg.name))
.join("")
);
}

return typeof type.name === "string" ? type.name : "";
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-client-generator-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,7 @@ function getSdkCredentialType(
values: credentialTypes,
nullable: false,
name: createGeneratedName(client.service, "CredentialUnion"),
isGeneratedName: false,
isGeneratedName: true,
};
}
return credentialTypes[0];
Expand Down
Loading
Loading