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.

Sign up for GitHub

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

Jump to bottom

[tcgc] fix additional property union naming problem #726

Merged
merged 2 commits into from
Apr 23, 2024
Merged

[tcgc] fix additional property union naming problem #726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
199beb6
fix additional property union naming problem
tadelesh Apr 23, 2024
71cc204
changelog
tadelesh Apr 23, 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/union_null-2024-3-23-17-23-22.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@azure-tools/typespec-client-generator-core"
---

fix additional property union naming problem
24 changes: 22 additions & 2 deletions packages/typespec-client-generator-core/src/public-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ function getContextPath(
currentType.indexer &&
currentType.properties.size === 0 &&
((currentType.indexer.key.name === "string" && currentType.name === "Record") ||
(currentType.indexer.key.name === "integer" && currentType.name === "Array"))
currentType.indexer.key.name === "integer")
) {
// handle array or dict
const dictOrArrayItemType: Type = currentType.indexer.value;
Expand All @@ -430,7 +430,16 @@ function getContextPath(
const result = dfsModelProperties(expectedType, property.type, property.name);
if (result) return true;
}
// handle additional properties type
// handle additional properties type: model MyModel is Record<> {}
if (currentType.sourceModel?.kind === "Model" && currentType.sourceModel?.name === "Record") {
const result = dfsModelProperties(
expectedType,
currentType.sourceModel!.indexer!.value!,
"AdditionalProperty"
);
if (result) return true;
}
// handle additional properties type: model MyModel { ...Record<>}
if (currentType.indexer) {
const result = dfsModelProperties(
expectedType,
Expand All @@ -439,6 +448,17 @@ function getContextPath(
);
if (result) return true;
}
// handle additional properties type: model MyModel extends Record<> {}
if (currentType.baseModel) {
if (currentType.baseModel.name === "Record") {
const result = dfsModelProperties(
expectedType,
currentType.baseModel.indexer!.value!,
"AdditionalProperty"
);
if (result) return true;
}
}
result.pop();
if (currentType.baseModel) {
const result = dfsModelProperties(
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-client-generator-core/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ export function getSdkUnionWithDiagnostics(
return diagnostics.wrap(getAnyType(context, type));
}

// change to a simple logic: only convert to normal type if the union is type | null, otherwise, return all the union types
// convert to normal type if the union is type | null
if (nonNullOptions.length === 1) {
const clientType = diagnostics.pipe(
getClientTypeWithDiagnostics(context, nonNullOptions[0], operation)
Expand Down
148 changes: 147 additions & 1 deletion packages/typespec-client-generator-core/test/types.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,26 @@ describe("typespec-client-generator-core: types", () => {
strictEqual(nameProp.nullable, true);
});

it("nullable with more types", async function () {
await runner.compileWithBuiltInService(`
@usage(Usage.input | Usage.output)
@access(Access.public)
model Test {
name: string | float32 | null;
}
`);

const sdkType = getSdkTypeHelper(runner);
strictEqual(sdkType.kind, "union");
strictEqual(sdkType.values.length, 2);
strictEqual(sdkType.values[0].kind, "string");
strictEqual(sdkType.values[1].kind, "float32");
// eslint-disable-next-line deprecation/deprecation
strictEqual(sdkType.nullable, true);
const nameProp = runner.context.experimental_sdkPackage.models[0].properties[0];
strictEqual(nameProp.nullable, true);
});

it("record with nullable", async function () {
await runner.compileWithBuiltInService(`
@usage(Usage.input | Usage.output)
Expand All @@ -570,6 +590,29 @@ describe("typespec-client-generator-core: types", () => {
strictEqual(sdkType.nullableValues, true);
});

it("record with nullable with more types", async function () {
await runner.compileWithBuiltInService(`
@usage(Usage.input | Usage.output)
@access(Access.public)
model Test {
name: Record<string | float32 | null>;
}
`);

const sdkType = getSdkTypeHelper(runner);
strictEqual(sdkType.kind, "dict");
const elementType = sdkType.valueType;
strictEqual(elementType.kind, "union");
strictEqual(elementType.values.length, 2);
strictEqual(elementType.values[0].kind, "string");
strictEqual(elementType.values[1].kind, "float32");
// eslint-disable-next-line deprecation/deprecation
strictEqual(elementType.nullable, true);
const nameProp = runner.context.experimental_sdkPackage.models[0].properties[0];
strictEqual(nameProp.nullable, false);
strictEqual(sdkType.nullableValues, true);
});

it("array with nullable", async function () {
await runner.compileWithBuiltInService(`
@usage(Usage.input | Usage.output)
Expand All @@ -590,6 +633,29 @@ describe("typespec-client-generator-core: types", () => {
strictEqual(sdkType.nullableValues, true);
});

it("array with nullable with more types", async function () {
await runner.compileWithBuiltInService(`
@usage(Usage.input | Usage.output)
@access(Access.public)
model Test {
name: (string | float32 | null)[];
}
`);

const sdkType = getSdkTypeHelper(runner);
strictEqual(sdkType.kind, "array");
const elementType = sdkType.valueType;
strictEqual(elementType.kind, "union");
strictEqual(elementType.values.length, 2);
strictEqual(elementType.values[0].kind, "string");
strictEqual(elementType.values[1].kind, "float32");
// eslint-disable-next-line deprecation/deprecation
strictEqual(elementType.nullable, true);
const nameProp = runner.context.experimental_sdkPackage.models[0].properties[0];
strictEqual(nameProp.nullable, false);
strictEqual(sdkType.nullableValues, true);
});

it("additional property is nullable", async function () {
await runner.compileWithBuiltInService(`
@usage(Usage.input | Usage.output)
Expand All @@ -603,10 +669,17 @@ describe("typespec-client-generator-core: types", () => {
model TestIs is Record<string|null> {
name: string;
}

@usage(Usage.input | Usage.output)
@access(Access.public)
model TestSpread {
name: string;
...Record<string|null>
}
`);

const models = runner.context.experimental_sdkPackage.models;
strictEqual(models.length, 2);
strictEqual(models.length, 3);

const extendsType = models.find((x) => x.name === "TestExtends");
ok(extendsType);
Expand All @@ -623,6 +696,79 @@ describe("typespec-client-generator-core: types", () => {
// eslint-disable-next-line deprecation/deprecation
strictEqual(isType.additionalProperties?.nullable, true);
strictEqual(isType.additionalPropertiesNullable, true);

const spreadType = models.find((x) => x.name === "TestSpread");
ok(spreadType);
strictEqual(spreadType.kind, "model");
strictEqual(spreadType.additionalProperties?.kind, "string");
// eslint-disable-next-line deprecation/deprecation
strictEqual(spreadType.additionalProperties?.nullable, true);
strictEqual(spreadType.additionalPropertiesNullable, true);
});

it("additional property nullable with more types", async function () {
await runner.compileWithBuiltInService(`
@usage(Usage.input | Usage.output)
@access(Access.public)
model TestExtends extends Record<string|float32|null> {
name: string;
}

@usage(Usage.input | Usage.output)
@access(Access.public)
model TestIs is Record<string|float32|null> {
name: string;
}

@usage(Usage.input | Usage.output)
@access(Access.public)
model TestSpread {
name: string;
...Record<string|float32|null>
}
`);

const models = runner.context.experimental_sdkPackage.models;
strictEqual(models.length, 3);

const extendsType = models.find((x) => x.name === "TestExtends");
ok(extendsType);
strictEqual(extendsType.kind, "model");
strictEqual(extendsType.additionalProperties?.kind, "union");
strictEqual(extendsType.additionalProperties?.name, "TestExtendsAdditionalProperty");
strictEqual(extendsType.additionalProperties?.isGeneratedName, true);
strictEqual(extendsType.additionalProperties?.values.length, 2);
strictEqual(extendsType.additionalProperties?.values[0].kind, "string");
strictEqual(extendsType.additionalProperties?.values[1].kind, "float32");
// eslint-disable-next-line deprecation/deprecation
strictEqual(extendsType.additionalProperties?.nullable, true);
strictEqual(extendsType.additionalPropertiesNullable, true);

const isType = models.find((x) => x.name === "TestIs");
ok(isType);
strictEqual(isType.kind, "model");
strictEqual(isType.additionalProperties?.kind, "union");
strictEqual(isType.additionalProperties?.name, "TestIsAdditionalProperty");
strictEqual(isType.additionalProperties?.isGeneratedName, true);
strictEqual(isType.additionalProperties?.values.length, 2);
strictEqual(isType.additionalProperties?.values[0].kind, "string");
strictEqual(isType.additionalProperties?.values[1].kind, "float32");
// eslint-disable-next-line deprecation/deprecation
strictEqual(isType.additionalProperties?.nullable, true);
strictEqual(isType.additionalPropertiesNullable, true);

const spreadType = models.find((x) => x.name === "TestSpread");
ok(spreadType);
strictEqual(spreadType.kind, "model");
strictEqual(spreadType.additionalProperties?.kind, "union");
strictEqual(spreadType.additionalProperties?.name, "TestSpreadAdditionalProperty");
strictEqual(spreadType.additionalProperties?.isGeneratedName, true);
strictEqual(spreadType.additionalProperties?.values.length, 2);
strictEqual(spreadType.additionalProperties?.values[0].kind, "string");
strictEqual(spreadType.additionalProperties?.values[1].kind, "float32");
// eslint-disable-next-line deprecation/deprecation
strictEqual(spreadType.additionalProperties?.nullable, true);
strictEqual(spreadType.additionalPropertiesNullable, true);
});

it("model with simple union property", async function () {
Expand Down
Loading