Skip to content

Commit

Permalink
Refine tcgc logic for spread (#572)
Browse files Browse the repository at this point in the history
  • Loading branch information
tadelesh authored Apr 2, 2024
1 parent 560f2e4 commit 2dc52a7
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 28 deletions.
7 changes: 7 additions & 0 deletions .chronus/changes/refine_tcgc_logic-2024-3-2-16-27-30.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@azure-tools/typespec-client-generator-core"
---

Set spread model with none usage
7 changes: 7 additions & 0 deletions .chronus/changes/refine_tcgc_logic-2024-3-2-16-27-52.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-client-generator-core"
---

Workaround for arm provider method parameter
1 change: 1 addition & 0 deletions packages/typespec-client-generator-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
"pluralize": "^8.0.0"
},
"peerDependencies": {
"@azure-tools/typespec-azure-core": "workspace:~",
"@typespec/compiler": "workspace:~",
"@typespec/http": "workspace:~",
"@typespec/rest": "workspace:~",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,7 @@ export function isAzureCoreModel(t: Type): boolean {
return (
t.kind === "Model" &&
t.namespace !== undefined &&
["Azure.Core", "Azure.Core.Foundations", "Azure.ResourceManager"].includes(
getNamespaceFullName(t.namespace)
)
["Azure.Core", "Azure.Core.Foundations"].includes(getNamespaceFullName(t.namespace))
);
}

Expand Down
6 changes: 3 additions & 3 deletions packages/typespec-client-generator-core/src/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,9 @@ function getSdkBasicServiceMethod<
);
}
} else {
const methodParameter = diagnostics.pipe(getSdkMethodParameter(context, prop));
if (methodParameter.kind === "method") {
methodParameters.push(methodParameter);
// workaround for the provider parameter in arm, need to refine method design in tcgc later
if (!context.arm || prop.name !== "provider") {
methodParameters.push(diagnostics.pipe(getSdkMethodParameter(context, prop)));
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion packages/typespec-client-generator-core/src/public-utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
Diagnostic,
Enum,
Interface,
Model,
ModelProperty,
Expand Down Expand Up @@ -175,7 +176,7 @@ export function getLibraryName(
type.name +
type.templateMapper.args
.filter(
(arg): arg is Model =>
(arg): arg is Model | Enum =>
(arg.kind === "Model" || arg.kind === "Enum") && arg.name.length > 0
)
.map((arg) => pascalCase(arg.name))
Expand Down
21 changes: 9 additions & 12 deletions packages/typespec-client-generator-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1127,7 +1127,7 @@ function checkAndGetClientType(

interface ModelUsageOptions {
seenModelNames?: Set<SdkType>;
recurseThroughProperties?: boolean;
propagation?: boolean;
}

function updateUsageOfModel(
Expand All @@ -1137,7 +1137,7 @@ function updateUsageOfModel(
options?: ModelUsageOptions
): void {
options = options ?? {};
options.recurseThroughProperties = options?.recurseThroughProperties ?? true;
options.propagation = options?.propagation ?? true;
if (!type || !["model", "enum", "array", "dict", "union", "enumvalue"].includes(type.kind))
return;
if (options?.seenModelNames === undefined) {
Expand Down Expand Up @@ -1168,10 +1168,7 @@ function updateUsageOfModel(
}

if (type.kind === "enum") return;
if (type.baseModel && (type.baseModel.usage & usage) === 0) {
// if it has a base model and the base model doesn't currently have that usage
type.baseModel.usage |= usage;
}
if (!options.propagation) return;
if (type.baseModel) {
updateUsageOfModel(context, usage, type.baseModel, options);
}
Expand All @@ -1180,13 +1177,11 @@ function updateUsageOfModel(
updateUsageOfModel(context, usage, discriminatedSubtype, options);
}
}
if (type.additionalProperties && options.recurseThroughProperties) {
if (type.additionalProperties) {
updateUsageOfModel(context, usage, type.additionalProperties, options);
}
if (options.recurseThroughProperties) {
for (const property of type.properties) {
updateUsageOfModel(context, usage, property.type, options);
}
for (const property of type.properties) {
updateUsageOfModel(context, usage, property.type, options);
}
}

Expand Down Expand Up @@ -1221,6 +1216,8 @@ function updateTypesFromOperation(
const bodies = diagnostics.pipe(checkAndGetClientType(context, httpBody.type, operation));
if (generateConvenient) {
bodies.forEach((body) => {
// spread body model should be none usage
if (body.kind === "model" && body.isGeneratedName) return;
updateUsageOfModel(context, UsageFlags.Input, body);
});
if (httpBody.contentTypes.includes("application/merge-patch+json")) {
Expand All @@ -1232,7 +1229,7 @@ function updateTypesFromOperation(
if (isMultipartFormData(context, httpBody.type, operation)) {
bodies.forEach((body) => {
updateUsageOfModel(context, UsageFlags.MultipartFormData, body, {
recurseThroughProperties: false,
propagation: false,
});
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2757,7 +2757,7 @@ describe("typespec-client-generator-core: package", () => {
`);
const sdkPackage = runner.context.experimental_sdkPackage;
const method = getServiceMethodOfClient(sdkPackage);
strictEqual(sdkPackage.models.length, 1);
strictEqual(sdkPackage.models.length, 0);
strictEqual(method.name, "myOp");
strictEqual(method.kind, "basic");
strictEqual(method.parameters.length, 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1284,8 +1284,7 @@ describe("typespec-client-generator-core: public-utils", () => {
`
);
const models = runner.context.experimental_sdkPackage.models;
strictEqual(models.length, 1);
ok(models.find((x) => x.name === "TestRequest" && x.isGeneratedName));
strictEqual(models.length, 0);
});

it("anonymous model for body parameter", async () => {
Expand All @@ -1295,8 +1294,7 @@ describe("typespec-client-generator-core: public-utils", () => {
`
);
const models = runner.context.experimental_sdkPackage.models;
strictEqual(models.length, 1);
ok(models.find((x) => x.name === "TestRequest" && x.isGeneratedName));
strictEqual(models.length, 0);
});

it("anonymous union in response header", async () => {
Expand Down
5 changes: 1 addition & 4 deletions packages/typespec-client-generator-core/test/types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2025,17 +2025,14 @@ describe("typespec-client-generator-core: types", () => {
interface StringExtensible extends GetAndSend<string | "b" | "c"> {}
`);
const sdkPackage = runner.context.experimental_sdkPackage;
strictEqual(sdkPackage.models.length, 2);
strictEqual(sdkPackage.models.length, 1);
strictEqual(sdkPackage.enums.length, 1);
const prop = sdkPackage.enums.find((x) => x.name === "GetResponseProp" && x.isGeneratedName);
ok(prop);
strictEqual(prop.isFixed, false);
strictEqual(prop.valueType.kind, "string");
const req = sdkPackage.models.find((x) => x.name === "SendRequest" && x.isGeneratedName);
const resp = sdkPackage.models.find((x) => x.name === "GetResponse" && x.isGeneratedName);
ok(req);
ok(resp);
strictEqual(req.properties[0].type, prop);
strictEqual(resp.properties[0].type, prop);
});

Expand Down

0 comments on commit 2dc52a7

Please sign in to comment.