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

Refine tcgc logic for spread #572

Merged
merged 17 commits into from
Apr 2, 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/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
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
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
Loading