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

Fix union as enum issues #328

Merged
merged 7 commits into from
Feb 28, 2024
Merged

Fix union as enum issues #328

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ebc8e89
fix wrong type for union as enum variant
tadelesh Feb 28, 2024
4474b54
support anonymous union as enum
tadelesh Feb 28, 2024
1fd6bc3
format
tadelesh Feb 28, 2024
2bce5f1
changelog
tadelesh Feb 28, 2024
4148a50
Merge branch 'main' into fix_union_variant
tadelesh Feb 28, 2024
81ee2aa
fix wrong enum value type of union as enum
tadelesh Feb 28, 2024
504ec87
Merge branch 'main' into fix_union_variant
iscai-msft Feb 28, 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_union_variant-2024-1-28-18-32-18.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"
---

add support for anonymous union as enum and fix union as enum variant discriminator typing problem
1 change: 1 addition & 0 deletions packages/typespec-client-generator-core/src/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ export interface SdkDictionaryType extends SdkTypeBase {
export interface SdkEnumType extends SdkTypeBase {
kind: "enum";
name: string;
generatedName?: string;
valueType: SdkBuiltInType;
values: SdkEnumValueType[];
isFixed: boolean;
Expand Down
23 changes: 13 additions & 10 deletions 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 @@ -450,11 +450,7 @@ function addDiscriminatorToModelType(
if (discriminatorProperty.type.kind === "constant") {
discriminatorType = { ...discriminatorProperty.type.valueType };
} else if (discriminatorProperty.type.kind === "enumvalue") {
discriminatorType = getSdkEnum(
context,
(discriminatorProperty.type.__raw as EnumMember).enum,
operation
);
discriminatorType = discriminatorProperty.type.enumType;
}
} else {
discriminatorType = {
Expand Down Expand Up @@ -650,11 +646,12 @@ function getSdkUnionEnum(context: TCGCContext, type: UnionEnum, operation?: Oper
sdkType = {
...getSdkTypeBaseHelper(context, type.union, "enum"),
name: getLibraryName(context, type.union),
generatedName: type.union.name ? undefined : getGeneratedName(context, type.union),
description: docWrapper.description,
details: docWrapper.details,
valueType: { ...getSdkTypeBaseHelper(context, type.kind, "string"), encode: "string" },
valueType: getSdkEnumValueType(context, type.flattenedMembers.values().next().value),
values: [],
nullable: false,
nullable: type.nullable,
isFixed: !type.open,
isFlags: false,
usage: UsageFlags.None, // We will add usage as we loop through the operations
Expand Down Expand Up @@ -761,9 +758,8 @@ export function getClientTypeWithDiagnostics(
retval = getSdkEnum(context, type, operation);
break;
case "Union":
// start off with just handling nullable type
const unionAsEnum = diagnostics.pipe(getUnionAsEnum(type));
if (unionAsEnum && type.name) {
if (unionAsEnum) {
retval = getSdkUnionEnum(context, unionAsEnum, operation);
} else {
retval = diagnostics.pipe(getSdkUnionWithDiagnostics(context, type, operation));
Expand All @@ -778,7 +774,14 @@ export function getClientTypeWithDiagnostics(
retval = getKnownValuesEnum(context, type, operation) ?? innerType;
break;
case "UnionVariant":
retval = diagnostics.pipe(getClientTypeWithDiagnostics(context, type.type, operation));
const unionType = diagnostics.pipe(
getClientTypeWithDiagnostics(context, type.union, operation)
);
if (unionType.kind === "enum") {
retval = unionType.values.find((x) => x.name === getLibraryName(context, type))!;
} else {
retval = diagnostics.pipe(getClientTypeWithDiagnostics(context, type.type, operation));
}
break;
case "EnumMember":
const enumType = getSdkEnum(context, type.enum, operation);
Expand Down
18 changes: 7 additions & 11 deletions packages/typespec-client-generator-core/test/public-utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1139,25 +1139,21 @@ describe("typespec-client-generator-core: public-utils", () => {
`
);
const models = getAllModels(runner.context);
strictEqual(models.length, 1);
strictEqual(models.length, 2);
const unionName = ((models[0] as SdkModelType).properties[0].type as SdkUnionType)
.generatedName;
strictEqual(unionName, "AStatus");
strictEqual(models[0].kind, "model");
const statusProp = models[0].properties[0];
strictEqual(statusProp.kind, "property");
strictEqual(statusProp.type.kind, "union");
strictEqual(statusProp.type.kind, "enum");
strictEqual(statusProp.type.values.length, 2);
const startVal = statusProp.type.values.find(
(x) => x.kind === "constant" && x.value === "start"
)!;
strictEqual(startVal.kind, "constant");
const startVal = statusProp.type.values.find((x) => x.name === "start")!;
strictEqual(startVal.kind, "enumvalue");
strictEqual(startVal.valueType.kind, "string");

const stopVal = statusProp.type.values.find(
(x) => x.kind === "constant" && x.value === "stop"
)!;
strictEqual(stopVal.kind, "constant");
const stopVal = statusProp.type.values.find((x) => x.name === "stop")!;
strictEqual(stopVal.kind, "enumvalue");
strictEqual(stopVal.valueType.kind, "string");
});

Expand Down Expand Up @@ -1196,7 +1192,7 @@ describe("typespec-client-generator-core: public-utils", () => {
`
);
const models = getAllModels(runner.context);
strictEqual(models.length, 2);
strictEqual(models.length, 3);
const test1 = models.find((x) => (x as SdkModelType).generatedName === "AChoice")!;
ok(test1);
const unionName = ((test1 as SdkModelType).properties[0].type as SdkUnionType)
Expand Down
36 changes: 35 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 @@ -1427,7 +1427,9 @@ describe("typespec-client-generator-core: types", () => {
it("union to extensible enum values", async () => {
await runner.compileWithBuiltInService(`
union PetKind {
@doc("Cat")
Cat: "cat",
@doc("Dog")
Dog: "dog",
string,
}
Expand All @@ -1450,17 +1452,40 @@ describe("typespec-client-generator-core: types", () => {

const catValue = values.find((x) => x.name === "Cat")!;
strictEqual(catValue.value, "cat");
strictEqual(catValue.description, "Cat");
strictEqual(catValue.enumType, petKind);
strictEqual(catValue.valueType, petKind.valueType);
strictEqual(catValue.kind, "enumvalue");

const dogValue = values.find((x) => x.name === "Dog")!;
strictEqual(dogValue.value, "dog");
strictEqual(dogValue.description, "Dog");
strictEqual(dogValue.enumType, petKind);
strictEqual(dogValue.valueType, petKind.valueType);
strictEqual(dogValue.kind, "enumvalue");
});

it("property of anonymous union as enum", async () => {
await runner.compileWithBuiltInService(`
model Pet {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't want to create an enum from an anonymous union, we only do it if the union is named. let me roll back this part of the pr, rest looks good as far as i can see

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this not true anymore?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didn't know we changed this, i know @lmazuel before really didn't want to generate a named enum from anonymous unions. can you approve it as is and @tadelesh depending on what @lmazuel says can you make a separate PR if we do want to name anonymous unions?

kind: string | "cat" | "dog";
}

@route("/extensible-enum")
@put
op putPet(@body pet: Pet): void;
`);
const models = getAllModels(runner.context);
strictEqual(models.length, 2);
const pet = models.find((x) => x.name === "Pet")! as SdkModelType;
const kind = models.find((x) => x.name === "")!;
strictEqual(kind.generatedName, "PetKind");
const kindProperty = pet.properties.find(
(x) => (x.nameInClient = "kind")
)! as SdkBodyModelPropertyType;
strictEqual(kindProperty.type, kind);
});

it("enum discriminator model without base discriminator property", async () => {
await runner.compileWithBuiltInService(`
enum DogKind {
Expand Down Expand Up @@ -1568,11 +1593,20 @@ describe("typespec-client-generator-core: types", () => {
op getModel(): Fish;
`);
const models = getAllModels(runner.context);
strictEqual(models.length, 3);
strictEqual(models.length, 4);
const fish = models.find((x) => x.name === "Fish")! as SdkModelType;
let kindTypeProperty = fish.properties.find((x) => x.nameInClient === "kind")!;
strictEqual(kindTypeProperty.type.kind, "enum");
const shark = models.find((x) => x.name === "Shark")! as SdkModelType;
strictEqual(shark.discriminatorValue, "shark");
kindTypeProperty = shark.properties.find((x) => x.nameInClient === "kind")!;
strictEqual(kindTypeProperty.type.kind, "enumvalue");
const salmon = models.find((x) => x.name === "Salmon")! as SdkModelType;
kindTypeProperty = salmon.properties.find((x) => x.nameInClient === "kind")!;
strictEqual(kindTypeProperty.type.kind, "enumvalue");
strictEqual(salmon.discriminatorValue, "salmon");
const kindType = models.find((x) => x.name === "KindType")! as SdkEnumType;
strictEqual(kindType.isFixed, false);
});

it("filterOutCoreModels true", async () => {
Expand Down
Loading