Hide private fields in tagged enums and errors (#84)

According to [The Big O of Code
Reviews](https://www.egorand.dev/the-big-o-of-code-reviews/), this is a
O(_m * n_) change.

Potentially fixes #75.

This PR does a couple of things for TaggedEnums:

- we eliminate the `Variant__data` intermediate type and replace it with
a macro to render the type explicitly. This is to make the values and
their types show up in autocomplete.
- we move or remove `__uniffiTypeName`, `__variant` and `__variantName`
from `UniffiEnum` and `UniffiError`. I think that this fixes the bug
#75. If not, I don't think it's harmful to keep this fix.
- we introduce a `playground` directory into the `typescript/tests`
directory: this is explicitly for experimenting with hand writing
generated code before touching the templates.

crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/ErrorTemplate.ts

@@ -9,8 +9,10 @@ export const {{ decl_type_name }} = (() => {
     {%-   call ts::docstring(variant, 4) %}
     {%-   let variant_name = variant.name()|class_name(ci) %}
     class {{ variant_name }} extends UniffiError {
+        private readonly __uniffiTypeName = "{{ type_name }}";
+        private readonly __variant = {{ loop.index }};
         constructor(message: string) {
-            super("{{ type_name }}", "{{ variant_name }}", {{ loop.index }}, message);
+            super("{{ type_name }}", "{{ variant_name }}", message);
         }
 
         static {{ instance_of }}(e: any): e is {{ variant_name }} {

crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/TaggedEnumTemplate.ts

@@ -1,13 +1,30 @@
 
 // Enum: {{ type_name }}
-{%- let kind_type_name = format!("{type_name}_Tags") %}
-export enum {{ kind_type_name }} {
+{%- let type_name__Tags = format!("{type_name}_Tags") %}
+export enum {{ type_name__Tags }} {
     {%- for variant in e.variants() %}
     {{ variant|variant_name }} = "{{ variant.name() }}"
     {%- if !loop.last %},{% endif -%}
     {% endfor %}
 }
 
+{%- macro variant_data_type(variant) %}Readonly<
+{%-   let fields = variant.fields() %}
+{%-   if !is_tuple %}{
+{%-     for field in fields %}
+{{-       field.name()|var_name }}: {{ field|type_name(self) }}
+{%-       if !loop.last %}; {% endif -%}
+{%-     endfor %}}
+{%-   else %}
+[
+{%-     for field in fields %}
+{{-       field|type_name(self) }}
+{%-       if !loop.last %}, {% endif -%}
+{%-     endfor %}
+]
+{%-   endif %}>
+{%- endmacro %}
+
 {%- call ts::docstring(e, 0) %}
 export const {{ decl_type_name }} = (() => {
   {%- for variant in e.variants() %}
@@ -25,35 +42,27 @@ export const {{ decl_type_name }} = (() => {
     #}
     {%- 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}.{external_name}") %}
+    {%- let variant_tag = format!("{type_name__Tags}.{external_name}") %}
     {%- let has_fields = !variant.fields().is_empty() %}
     {%- let is_tuple = variant.has_nameless_fields() %}
-    {%- if has_fields %}
-    type {{ variant_data }} = {# space #}
-    {%-   if !is_tuple %}{
-    {%-     for field in variant.fields() %}
-    {{-       field.name()|var_name }}: {{ field|type_name(self) }}
-    {%-       if !loop.last %}; {% endif -%}
-    {%-     endfor %}}
-    {%-   else %}[
-    {%-     for field in variant.fields() %}
-    {{-       field|type_name(self) }}
-    {%-       if !loop.last %}, {% endif -%}
-    {%-     endfor %}]
-    {%-   endif %};
-    {%- endif %}
-    type {{ variant_interface }} = { tag: {{ variant_tag }} {%- if has_fields %}; inner: Readonly<{{ variant_data }}> {%- endif %}};
+
+    type {{ variant_interface }} = {
+        tag: {{ variant_tag }}
+        {%- if has_fields %};
+        inner: {% call variant_data_type(variant) %}
+        {%- endif %}
+    };
 
     {% call ts::docstring(variant, 4) %}
     class {{ variant_name }} extends {{ superclass }} implements {{ variant_interface }} {
+        private readonly __uniffiTypeName = "{{ type_name }}";
         readonly tag = {{ variant_tag }};
         {%- if has_fields %}
-        readonly inner: Readonly<{{ variant_data }}>;
+        readonly inner: {% call variant_data_type(variant) %};
         {%-   if !is_tuple %}
         constructor(inner: { {% call ts::field_list_decl(variant, false) %} }) {
-            super("{{ type_name }}", "{{ external_name }}", {{ loop.index }});
+            super("{{ type_name }}", "{{ external_name }}");
             this.inner = Object.freeze(inner);
         }
 
@@ -62,7 +71,7 @@ export const {{ decl_type_name }} = (() => {
         }
         {%-   else %}
         constructor({%- call ts::field_list_decl(variant, true) -%}) {
-            super("{{ type_name }}", "{{ external_name }}", {{ loop.index }});
+            super("{{ type_name }}", "{{ external_name }}");
             this.inner = Object.freeze([{%- call ts::field_list(variant, true) -%}]);
         }
 
@@ -72,7 +81,7 @@ export const {{ decl_type_name }} = (() => {
         {%-   endif %}
         {%- else %}
         constructor() {
-            super("{{ type_name }}", "{{ external_name }}", {{ loop.index }});
+            super("{{ type_name }}", "{{ external_name }}");
         }
 
         static new(): {{ variant_name }} {
@@ -90,7 +99,7 @@ export const {{ decl_type_name }} = (() => {
             return {{ variant_name }}.instanceOf(obj);
         }
 
-        static getInner(obj: {{ variant_name }}): Readonly<{{ variant_data }}> {
+        static getInner(obj: {{ variant_name }}): {% call variant_data_type(variant) %} {
             return obj.inner;
         }
         {%- else %}
@@ -103,11 +112,7 @@ export const {{ decl_type_name }} = (() => {
     }
   {%- endfor %}
 
-    function instanceOf(obj: any): obj is {# space #}
-  {%- for variant in e.variants() %}
-  {{-  variant.name()|class_name(ci)|fmt("{}_") }}
-  {%-  if !loop.last %}| {% endif -%}
-  {%- endfor %} {
+    function instanceOf(obj: any): obj is {{ type_name }} {
         return obj.__uniffiTypeName === "{{ type_name }}";
     }
 
@@ -135,9 +140,11 @@ const {{ ffi_converter_name }} = (() => {
         read(from: RustBuffer): TypeName {
             switch (ordinalConverter.read(from)) {
             {%- for variant in e.variants() %}
+            {%-   let has_fields = !variant.fields().is_empty() %}
+            {%-   let is_tuple = variant.has_nameless_fields() %}
                 case {{ loop.index }}: return new {{ type_name }}.{{ variant|variant_name }}(
-            {%-   if !variant.fields().is_empty() %}
-            {%-     if !variant.has_nameless_fields() %}{
+            {%-   if has_fields %}
+            {%-     if !is_tuple %}{
             {%-     for field in variant.fields() %}
             {{-       field.name()|var_name }}: {{ field|ffi_converter_name(self) }}.read(from)
             {%-       if !loop.last -%}, {% endif %}
@@ -156,9 +163,11 @@ const {{ ffi_converter_name }} = (() => {
         write(value: TypeName, into: RustBuffer): void {
             switch (value.tag) {
                 {%- for variant in e.variants() %}
-                case {{ kind_type_name }}.{{ variant|variant_name }}: {
+                {%-   let has_fields = !variant.fields().is_empty() %}
+                {%-   let is_tuple = variant.has_nameless_fields() %}
+                case {{ type_name__Tags }}.{{ variant|variant_name }}: {
                     ordinalConverter.write({{ loop.index }}, into);
-                    {%- if !variant.fields().is_empty() %}
+                    {%- if has_fields %}
                     const inner = value.inner;
                     {%-   for field in variant.fields() %}
                     {{ field|ffi_converter_name(self) }}.write({% call ts::field_name("inner", field, loop.index0) %}, into);
@@ -168,24 +177,26 @@ const {{ ffi_converter_name }} = (() => {
                 }
                 {%- endfor %}
                 default:
-                    // Throwing from here means that {{ kind_type_name }} hasn't matched an ordinal.
+                    // Throwing from here means that {{ type_name__Tags }} hasn't matched an ordinal.
                     throw new UniffiInternalError.UnexpectedEnumCase();
             }
         }
         allocationSize(value: TypeName): number {
             switch (value.tag) {
                 {%- for variant in e.variants() %}
-                case {{ kind_type_name }}.{{ variant|variant_name }}: {
-                    {%- if !variant.fields().is_empty() %}
+                {%-   let has_fields = !variant.fields().is_empty() %}
+                {%-   let is_tuple = variant.has_nameless_fields() %}
+                case {{ type_name__Tags }}.{{ variant|variant_name }}: {
+                {%-   if has_fields %}
                     const inner = value.inner;
                     let size = ordinalConverter.allocationSize({{ loop.index }});
-                    {%- for field in variant.fields() %}
+                {%-     for field in variant.fields() %}
                     size += {{ field|ffi_converter_name(self) }}.allocationSize({% call ts::field_name("inner", field, loop.index0) %});
-                    {%- endfor %}
+                {%-     endfor %}
                     return size;
-                    {%- else %}
+                {%-   else %}
                     return ordinalConverter.allocationSize({{ loop.index }});
-                    {%- endif %}
+                {%-   endif %}
                 }
                 {%- endfor %}
                 default: throw new UniffiInternalError.UnexpectedEnumCase();

typescript/src/enums.ts

@@ -4,9 +4,5 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/
  */
 export class UniffiEnum {
-  constructor(
-    private readonly __uniffiTypeName: string,
-    private readonly __variantName: string,
-    private readonly __variant: number,
-  ) {}
+  protected constructor(...args: any) {}
 }

typescript/src/errors.ts

@@ -10,28 +10,14 @@
 // in tests and in the generated callback code, and more locally the FfiConverters
 // for each error.
 export class UniffiError extends Error {
-  constructor(
-    private readonly __uniffiTypeName: string,
-    private readonly __variantName: string,
-    private readonly __variant: number,
-    message?: string,
-  ) {
+  constructor(enumTypeName: string, variantName: string, message?: string) {
     // We append the error type and variant to the message because we cannot override `toString()`—
     // in errors.test.ts, we see that the overridden `toString()` method is not called.
-    super(UniffiError.createMessage(__uniffiTypeName, __variantName, message));
-  }
-
-  // Current implementations of hermes errors do not repect instance methods or calculated properties.
-  toString(): string {
-    return UniffiError.createMessage(
-      this.__uniffiTypeName,
-      this.__variantName,
-      this.message,
-    );
+    super(UniffiError.createMessage(enumTypeName, variantName, message));
   }
 
-  static instanceOf(err: any): err is UniffiError {
-    return err instanceof Error && (err as any).__uniffiTypeName !== undefined;
+  static instanceOf(obj: any): obj is UniffiError {
+    return obj.__uniffiTypeName !== undefined && obj instanceof Error;
   }
 
   private static createMessage(
@@ -52,22 +38,13 @@ export class UniffiThrownObject<T> extends Error {
   private static __baseTypeName = "UniffiThrownObject";
   private readonly __baseTypeName: string = UniffiThrownObject.__baseTypeName;
   constructor(
-    private readonly __uniffiTypeName: string,
+    typeName: string,
     public readonly inner: T,
     message?: string,
   ) {
     // We append the error type and variant to the message because we cannot override `toString()`—
     // in errors.test.ts, we see that the overridden `toString()` method is not called.
-    super(UniffiThrownObject.createMessage(__uniffiTypeName, inner, message));
-  }
-
-  // Current implementations of hermes errors do not repect instance methods or calculated properties.
-  toString(): string {
-    return UniffiThrownObject.createMessage(
-      this.__uniffiTypeName,
-      this.inner,
-      this.message,
-    );
+    super(UniffiThrownObject.createMessage(typeName, inner, message));
   }
 
   static instanceOf(err: any): err is UniffiThrownObject<unknown> {

typescript/tests/enums.test.ts

@@ -0,0 +1,38 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/
+ */
+
+import { test } from "../testing/asserts";
+import { MyEnum } from "./playground/enums";
+
+test("Enums have private fields", (t) => {
+  const v1: MyEnum = new MyEnum.Variant1({ myValue: "string" });
+  const v2: MyEnum = new MyEnum.Variant2(42, "string");
+
+  function switchGetTag(obj: any): string {
+    if (!MyEnum.instanceOf(obj)) {
+      t.fail("Obj should be a MyEnum");
+      return "";
+    }
+    const v = obj;
+    switch (v.tag) {
+      case "Variant1": {
+        const { myValue } = v.inner;
+        t.assertEqual(myValue, "string");
+        t.assertTrue(MyEnum.Variant1.instanceOf(v));
+        return v.tag;
+      }
+      case "Variant2": {
+        const [p1, p2] = v.inner;
+        t.assertEqual(p1, 42);
+        t.assertEqual(p2, "string");
+        t.assertTrue(MyEnum.Variant2.instanceOf(v));
+        return v.tag;
+      }
+    }
+  }
+  switchGetTag(v1);
+  switchGetTag(v2);
+});

typescript/tests/importing.test.ts

@@ -11,7 +11,7 @@ import {
   MyObject,
   type MyObjectInterface,
   type MyRecord,
-} from "./exported";
+} from "./playground/exported";
 
 import { test } from "../testing/asserts";
 

typescript/tests/playground/enums.ts

@@ -0,0 +1,57 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/
+ */
+import { UniffiEnum } from "../../src/enums";
+
+export const MyEnum = (() => {
+  const typeName = "MyEnum";
+  type Variant1__interface = {
+    tag: "Variant1";
+    inner: Readonly<{ myValue: string }>;
+  };
+  class Variant1_ extends UniffiEnum implements Variant1__interface {
+    private readonly __uniffiTypeName = typeName;
+    readonly tag = "Variant1";
+    readonly inner: Readonly<{ myValue: string }>;
+    constructor(inner: { myValue: string }) {
+      super(typeName, "Variant1");
+      this.inner = Object.freeze(inner);
+    }
+    static instanceOf(obj: any): obj is Variant1_ {
+      return obj.tag === "Variant1" && instanceOf(obj);
+    }
+  }
+
+  type Variant2__interface = {
+    tag: "Variant2";
+    inner: Readonly<[number, string]>;
+  };
+  class Variant2_ extends UniffiEnum implements Variant2__interface {
+    private readonly __uniffiTypeName = typeName;
+    readonly tag = "Variant2";
+    readonly inner: Readonly<[number, string]>;
+    constructor(p1: number, p2: string) {
+      super(typeName, "Variant2");
+      this.inner = Object.freeze([p1, p2]);
+    }
+    static instanceOf(obj: any): obj is Variant2_ {
+      return obj.tag === "Variant2" && instanceOf(obj);
+    }
+  }
+
+  function instanceOf(obj: any): obj is MyEnum {
+    return obj.__uniffiTypeName === "MyEnum";
+  }
+
+  return Object.freeze({
+    Variant1: Variant1_,
+    Variant2: Variant2_,
+    instanceOf,
+  });
+})();
+
+export type MyEnum = InstanceType<
+  (typeof MyEnum)[keyof Omit<typeof MyEnum, "instanceOf">]
+>;