Skip to content

Commit

Permalink
[tcgc] fix additional property union naming problem (#726)
Browse files Browse the repository at this point in the history 
fix: #725
  • Loading branch information
@tadelesh
tadelesh authored Apr 23, 2024
1 parent 08d3308 commit 2f3d82d
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 4 deletions.
7 changes: 7 additions & 0 deletions .chronus/changes/union_null-2024-3-23-17-23-22.md
View file
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
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
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
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

0 comments on commit 2f3d82d

Please sign in to comment.