From 5109542ddadbf8172e85c05b0a0c72294305d546 Mon Sep 17 00:00:00 2001 From: jhugman Date: Sun, 18 Aug 2024 19:13:25 +0100 Subject: [PATCH] Allow enum variant names that collide with existing types (#70) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to [The Big O of Code Reviews](https://www.egorand.dev/the-big-o-of-code-reviews/), this is a O(_n_) change. This small PR tweaks the Variant classes for our typescript version of Tagged Enums, to allow Variants to have names of existing types. For example, it's not uncommon for Rust programs to have enums constructed like so: ```rs struct Dog {} struct Cat {} enum Animal { Dog(Dog), Cat(Cat), } ``` The way we've modeled tagged enums are with a frozen object containing a class for each Variant. So this would translate to: ```typescript type Dog = …; // Dog defined elsewhere type Cat = …; const Animal = (() => { class Dog { constructor(public value: Dog) { // This Dog value is the wrong type! // value is the type of the Variant class. } } class Cat { constructor(public value: Cat) {} } return Object.freeze({ Dog, Cat }); })(); ``` This commit changes the naming of the Variant class: ```typescript type Dog = …; // Dog defined elsewhere type Cat = …; const Animal = (() => { class Dog_ { constructor(public value: Dog) { // There is no variant class called Dog yet, just Dog_, // so value is the correct type. } } class Cat_ { constructor(public value: Cat) {} } // We rename the Dog_ variant class as Dog here, // so the only way to access this class is via Animal.Dog. return Object.freeze({ Dog: Dog_, Cat: Cat_ }); })(); ``` --- .../templates/TaggedEnumTemplate.ts | 33 ++++++--- fixtures/enum-types/src/lib.rs | 14 ++++ .../tests/bindings/test_enum_types.ts | 73 +++++++++++++++++++ 3 files changed, 111 insertions(+), 9 deletions(-) diff --git a/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/TaggedEnumTemplate.ts b/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/TaggedEnumTemplate.ts index 5e0440a5..c90f3c56 100644 --- a/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/TaggedEnumTemplate.ts +++ b/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/TaggedEnumTemplate.ts @@ -11,10 +11,23 @@ export enum {{ kind_type_name }} { {%- call ts::docstring(e, 0) %} export const {{ decl_type_name }} = (() => { {%- for variant in e.variants() %} - {%- let variant_name = variant.name()|class_name(ci) %} + {# + // We have an external name and an internal name so that variants of the enum can + // have names the same as other types. Since we're building Tagged Enums from + // scratch in Typescript, we should make effort to make enums match what is possible + // or common in Rust. + // Internally to this IIFE, we use a generated name which is impossible to express + // in a different type name in Rust (we append a `_`, which would be sifted out by + // UpperCamelCasing of type names into Typescript). This lets use the class without + // colliding with the outside world. e.g. `new Variant_(record: Variant)`. + // Externally, just as we return, we name it with the Variant name, so now + // client code can use `new MyEnum.Variant(record: Variant)`. + #} + {%- let external_name = variant.name()|class_name(ci) %} + {%- let variant_name = external_name|fmt("{}_") %} {%- let variant_data = variant_name|fmt("{}_data") %} {%- let variant_interface = variant_name|fmt("{}_interface") %} - {%- let variant_tag = format!("{kind_type_name}.{variant_name}") %} + {%- let variant_tag = format!("{kind_type_name}.{external_name}") %} {%- let has_fields = !variant.fields().is_empty() %} {%- let is_tuple = variant.has_nameless_fields() %} {%- if has_fields %} @@ -40,7 +53,7 @@ export const {{ decl_type_name }} = (() => { readonly inner: Readonly<{{ variant_data }}>; {%- if !is_tuple %} constructor(inner: { {% call ts::field_list_decl(variant, false) %} }) { - super("{{ type_name }}", "{{ variant_name }}", {{ loop.index }}); + super("{{ type_name }}", "{{ external_name }}", {{ loop.index }}); this.inner = Object.freeze(inner); } @@ -49,7 +62,7 @@ export const {{ decl_type_name }} = (() => { } {%- else %} constructor({%- call ts::field_list_decl(variant, true) -%}) { - super("{{ type_name }}", "{{ variant_name }}", {{ loop.index }}); + super("{{ type_name }}", "{{ external_name }}", {{ loop.index }}); this.inner = Object.freeze([{%- call ts::field_list(variant, true) -%}]); } @@ -59,7 +72,7 @@ export const {{ decl_type_name }} = (() => { {%- endif %} {%- else %} constructor() { - super("{{ type_name }}", "{{ variant_name }}", {{ loop.index }}); + super("{{ type_name }}", "{{ external_name }}", {{ loop.index }}); } static new(): {{ variant_name }} { @@ -92,19 +105,21 @@ export const {{ decl_type_name }} = (() => { function instanceOf(obj: any): obj is {# space #} {%- for variant in e.variants() %} - {{- variant.name()|class_name(ci) }} + {{- variant.name()|class_name(ci)|fmt("{}_") }} {%- if !loop.last %}| {% endif -%} {%- endfor %} { return obj.__uniffiTypeName === "{{ type_name }}"; } - return { + return Object.freeze({ instanceOf, {%- for variant in e.variants() %} - {{ variant.name()|class_name(ci) }} + {%- let external_name = variant.name()|class_name(ci) %} + {%- let variant_name = external_name|fmt("{}_") %} + {{ external_name }}: {{ variant_name }} {%- if !loop.last %}, {% endif -%} {%- endfor %} - }; + }); })(); diff --git a/fixtures/enum-types/src/lib.rs b/fixtures/enum-types/src/lib.rs index e09a43fe..fbed4a50 100644 --- a/fixtures/enum-types/src/lib.rs +++ b/fixtures/enum-types/src/lib.rs @@ -97,6 +97,20 @@ fn get_animal(a: Option) -> Animal { a.unwrap_or(Animal::Dog) } +#[derive(uniffi::Enum)] +pub(crate) enum CollidingVariants { + AnimalRecord(AnimalRecord), + AnimalObjectInterface(Arc), + AnimalObject(Arc), + Animal(Animal), + CollidingVariants, +} + +#[uniffi::export] +fn identity_colliding_variants(value: CollidingVariants) -> CollidingVariants { + value +} + uniffi::include_scaffolding!("enum_types"); #[cfg(test)] diff --git a/fixtures/enum-types/tests/bindings/test_enum_types.ts b/fixtures/enum-types/tests/bindings/test_enum_types.ts index 9606adc1..19c6b635 100644 --- a/fixtures/enum-types/tests/bindings/test_enum_types.ts +++ b/fixtures/enum-types/tests/bindings/test_enum_types.ts @@ -13,11 +13,15 @@ import { AnimalNamedAssociatedType_Tags, AnimalNoReprInt, AnimalObject, + AnimalRecord, AnimalSignedInt, AnimalUInt, getAnimal, identityEnumWithAssociatedType, identityEnumWithNamedAssociatedType, + CollidingVariants, + CollidingVariants_Tags, + identityCollidingVariants, } from "../../generated/enum_types"; test("Enum disriminant", (t) => { @@ -100,3 +104,72 @@ test("Roundtripping enums with name values", (t) => { assertEqual(v, identityEnumWithNamedAssociatedType(v)); } }); + +test("Variant naming cam collide with existing types", (t) => { + { + const record = AnimalRecord.create({ value: 5 }); + const variant1 = CollidingVariants.AnimalRecord.new(record); + const variant2 = new CollidingVariants.AnimalRecord(record); + + t.assertEqual(variant1.tag, CollidingVariants_Tags.AnimalRecord); + t.assertEqual(variant2.tag, CollidingVariants_Tags.AnimalRecord); + t.assertEqual(variant1, variant2); + t.assertEqual(identityCollidingVariants(variant1), variant2); + + t.assertTrue(CollidingVariants.instanceOf(variant1)); + t.assertTrue(CollidingVariants.AnimalRecord.instanceOf(variant1)); + } + { + const obj = new AnimalObject(1); + const variant1 = CollidingVariants.AnimalObject.new(obj); + const variant2 = new CollidingVariants.AnimalObject(obj); + + t.assertEqual(variant1.tag, CollidingVariants_Tags.AnimalObject); + t.assertEqual(variant2.tag, CollidingVariants_Tags.AnimalObject); + t.assertEqual(variant1, variant2); + t.assertEqual(identityCollidingVariants(variant1), variant2); + + t.assertTrue(CollidingVariants.instanceOf(variant1)); + t.assertTrue(CollidingVariants.AnimalObject.instanceOf(variant1)); + } + + { + const obj = new AnimalObject(1); + const variant1 = CollidingVariants.AnimalObjectInterface.new(obj); + const variant2 = new CollidingVariants.AnimalObjectInterface(obj); + + t.assertEqual(variant1.tag, CollidingVariants_Tags.AnimalObjectInterface); + t.assertEqual(variant2.tag, CollidingVariants_Tags.AnimalObjectInterface); + t.assertEqual(variant1, variant2); + t.assertEqual(identityCollidingVariants(variant1), variant2); + + t.assertTrue(CollidingVariants.instanceOf(variant1)); + t.assertTrue(CollidingVariants.AnimalObjectInterface.instanceOf(variant1)); + } + { + const animal = Animal.Dog; + const variant1 = CollidingVariants.Animal.new(animal); + const variant2 = new CollidingVariants.Animal(animal); + + t.assertEqual(variant1.tag, CollidingVariants_Tags.Animal); + t.assertEqual(variant2.tag, CollidingVariants_Tags.Animal); + t.assertEqual(variant1, variant2); + t.assertEqual(identityCollidingVariants(variant1), variant2); + + t.assertTrue(CollidingVariants.instanceOf(variant1)); + t.assertTrue(CollidingVariants.Animal.instanceOf(variant1)); + } + + { + const variant1 = CollidingVariants.CollidingVariants.new(); + const variant2 = new CollidingVariants.CollidingVariants(); + + t.assertEqual(variant1.tag, CollidingVariants_Tags.CollidingVariants); + t.assertEqual(variant2.tag, CollidingVariants_Tags.CollidingVariants); + t.assertEqual(variant1, variant2); + t.assertEqual(identityCollidingVariants(variant1), variant2); + + t.assertTrue(CollidingVariants.instanceOf(variant1)); + t.assertTrue(CollidingVariants.CollidingVariants.instanceOf(variant1)); + } +});