Improve declaration of untagged unions to allow for generator optimizations (elastic#2548)

Author: flobernd

compiler-rs/clients_schema/src/lib.rs

@@ -380,6 +380,7 @@ pub enum ServerDefault {
 pub enum Variants {
     ExternalTag(ExternalTag),
     InternalTag(InternalTag),
+    Untagged(Untagged),
     Container(Container),
 }
 
@@ -404,6 +405,13 @@ pub struct InternalTag {
     pub default_tag: Option<String>,
 }
 
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct Untagged {
+    #[serde(default)]
+    pub non_exhaustive: bool,
+}
+
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct Container {
@@ -799,6 +807,7 @@ impl TypeAlias {
 pub enum TypeAliasVariants {
     ExternalTag(ExternalTag),
     InternalTag(InternalTag),
+    Untagged(Untagged),
 }
 
 //------------------------------------------------------------------------------------------------------------

compiler-rs/clients_schema_to_openapi/src/main.rs

@@ -16,6 +16,7 @@
 // under the License.
 
 use std::path::{Path, PathBuf};
+use anyhow::bail;
 
 use clap::{Parser, ValueEnum};
 use clients_schema::{Availabilities, Visibility};
@@ -71,7 +72,10 @@ impl Cli {
             std::fs::read_to_string(self.schema)?
         };
 
-        let mut model: clients_schema::IndexedModel = serde_json::from_str(&json)?;
+        let mut model: clients_schema::IndexedModel = match serde_json::from_str(&json) {
+            Ok(indexed_model) => indexed_model,
+            Err(e) => bail!("cannot parse schema json: {}", e)
+        };
 
         if let Some(flavor) = self.flavor {
             if flavor != SchemaFlavor::All {

compiler-rs/clients_schema_to_openapi/src/schemas.rs

@@ -395,6 +395,8 @@ impl<'a> TypesAndComponents<'a> {
                     extensions: Default::default(),
                 });
             }
+            Some(TypeAliasVariants::Untagged(_tag)) => {
+            }
         };
 
         Ok(schema)

compiler-rs/openapi_to_clients_schema/src/types.rs

@@ -323,7 +323,7 @@ fn generate_schema_kind_one_of(
     let mut variants: Option<TypeAliasVariants> = None;
 
     // TODO: do we want to allow untagged unions (those that are disambiguated by inspecting property names)?
-
+    
     if let Some(discriminator) = discriminator {
         variants = Some(TypeAliasVariants::InternalTag(InternalTag {
             default_tag: None,

compiler/src/model/metamodel.ts

@@ -180,7 +180,7 @@ export abstract class BaseType {
   specLocation: string
 }
 
-export type Variants = ExternalTag | InternalTag | Container
+export type Variants = ExternalTag | InternalTag | Container | Untagged
 
 export class VariantBase {
   /**
@@ -207,6 +207,11 @@ export class Container extends VariantBase {
   kind: 'container'
 }
 
+export class Untagged extends VariantBase {
+  kind: 'untagged'
+  untypedVariant: Inherits
+}
+
 /**
  * Inherits clause (aka extends or implements) for an interface or request
  */
@@ -364,8 +369,11 @@ export class TypeAlias extends BaseType {
   type: ValueOf
   /** generic parameters: either concrete types or open parameters from the enclosing type */
   generics?: TypeName[]
-  /** Only applicable to `union_of` aliases: identify typed_key unions (external) and variant inventories (internal) */
-  variants?: InternalTag | ExternalTag
+  /**
+   * Only applicable to `union_of` aliases: identify typed_key unions (external), variant inventories (internal)
+   * and untagged variants
+   */
+  variants?: InternalTag | ExternalTag | Untagged
 }
 
 // ------------------------------------------------------------------------------------------------

compiler/src/model/utils.ts

@@ -536,8 +536,8 @@ export function modelTypeAlias (declaration: TypeAliasDeclaration): model.TypeAl
     if (variants != null) {
       assert(
         declaration.getJsDocs(),
-        variants.kind === 'internal_tag' || variants.kind === 'external_tag',
-        'Type Aliases can only have internal or external variants'
+        variants.kind === 'internal_tag' || variants.kind === 'external_tag' || variants.kind === 'untagged',
+        'Type Aliases can only have internal, external or untagged variants'
       )
       typeAlias.variants = variants
     }
@@ -1110,17 +1110,34 @@ export function parseVariantsTag (jsDoc: JSDoc[]): model.Variants | undefined {
     }
   }
 
-  assert(jsDoc, type === 'internal', `Bad variant type: ${type}`)
-
-  const pairs = parseKeyValues(jsDoc, values, 'tag', 'default')
-  assert(jsDoc, typeof pairs.tag === 'string', 'Internal variant requires a tag definition')
+  if (type === 'internal') {
+    const pairs = parseKeyValues(jsDoc, values, 'tag', 'default')
+    assert(jsDoc, typeof pairs.tag === 'string', 'Internal variant requires a tag definition')
+    return {
+      kind: 'internal_tag',
+      nonExhaustive: nonExhaustive,
+      tag: pairs.tag,
+      defaultTag: pairs.default
+    }
+  }
 
-  return {
-    kind: 'internal_tag',
-    nonExhaustive: nonExhaustive,
-    tag: pairs.tag,
-    defaultTag: pairs.default
+  if (type === 'untagged') {
+    const pairs = parseKeyValues(jsDoc, values, 'untyped')
+    assert(jsDoc, typeof pairs.untyped === 'string', 'Untagged variant requires an untyped definition')
+    const fqn = pairs.untyped.split('.')
+    return {
+      kind: 'untagged',
+      nonExhaustive: nonExhaustive,
+      untypedVariant: {
+        type: {
+          namespace: fqn.slice(0, fqn.length - 1).join('.'),
+          name: fqn[fqn.length - 1]
+        }
+      }
+    }
   }
+
+  assert(jsDoc, false, `Bad variant type: ${type}`)
 }
 
 /**

compiler/src/steps/validate-model.ts

@@ -559,14 +559,14 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
       if (typeDef.type.kind !== 'union_of') {
         modelError('The "variants" tag only applies to unions')
       } else {
-        validateTaggedUnion(typeDef.type, typeDef.variants)
+        validateTaggedUnion(typeDef.name, typeDef.type, typeDef.variants)
       }
     } else {
       validateValueOf(typeDef.type, openGenerics)
     }
   }
 
-  function validateTaggedUnion (valueOf: model.UnionOf, variants: model.InternalTag | model.ExternalTag): void {
+  function validateTaggedUnion (parentName: TypeName, valueOf: model.UnionOf, variants: model.InternalTag | model.ExternalTag | model.Untagged): void {
     if (variants.kind === 'external_tag') {
       // All items must have a 'variant' attribute
       const items = flattenUnionMembers(valueOf)
@@ -610,6 +610,72 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
       }
 
       validateValueOf(valueOf, new Set())
+    } else if (variants.kind === 'untagged') {
+      if (fqn(parentName) !== '_types.query_dsl:DecayFunction' &&
+          fqn(parentName) !== '_types.query_dsl:DistanceFeatureQuery' &&
+          fqn(parentName) !== '_types.query_dsl:RangeQuery') {
+        throw new Error(`Please contact the devtools team before adding new untagged variant ${fqn(parentName)}`)
+      }
+
+      const untypedVariant = getTypeDef(variants.untypedVariant.type)
+      if (untypedVariant == null) {
+        modelError(`Type ${fqn(variants.untypedVariant.type)} not found`)
+      }
+
+      const items = flattenUnionMembers(valueOf)
+      const baseTypes = new Set<string>()
+      let foundUntyped = false
+
+      for (const item of items) {
+        if (item.kind !== 'instance_of') {
+          modelError('Items of type untagged unions must be type references')
+        } else {
+          validateTypeRef(item.type, undefined, new Set<string>())
+          const type = getTypeDef(item.type)
+          if (type == null) {
+            modelError(`Type ${fqn(item.type)} not found`)
+          } else {
+            if (type.kind !== 'interface') {
+              modelError(`Type ${fqn(item.type)} must be an interface to be used in an untagged union`)
+              continue
+            }
+
+            if (untypedVariant != null && fqn(item.type) === fqn(untypedVariant.name)) {
+              foundUntyped = true
+            }
+
+            if (type.inherits == null) {
+              modelError(`Type ${fqn(item.type)} must derive from a base type to be used in an untagged union`)
+              continue
+            }
+
+            baseTypes.add(fqn(type.inherits.type))
+
+            const baseType = getTypeDef(type.inherits.type)
+            if (baseType == null) {
+              modelError(`Type ${fqn(type.inherits.type)} not found`)
+              continue
+            }
+
+            if (baseType.kind !== 'interface') {
+              modelError(`Type ${fqn(type.inherits.type)} must be an interface to be used as the base class of another interface`)
+              continue
+            }
+
+            if (baseType.generics == null || baseType.generics.length === 0) {
+              modelError('The common base type of an untagged union must accept at least one generic type argument')
+            }
+          }
+        }
+      }
+
+      if (baseTypes.size !== 1) {
+        modelError('All items of an untagged union must derive from the same common base type')
+      }
+
+      if (!foundUntyped) {
+        modelError('The untyped variant of an untagged variant must be contained in the union items')
+      }
     }
   }
 

docs/modeling-guide.md

@@ -418,7 +418,7 @@ An annotation allows distinguishing these properties from container variants:
 
 For example:
 
-```
+```ts
 /**
  * @variants container
  */
@@ -435,6 +435,39 @@ class AggregationContainer {
   ...
 ```
 
+#### Untagged
+
+The untagged variant is used for unions that can only be distinguished by the type of one or more fields.
+
+> [!WARNING]
+> This variant should only be used for legacy types and should otherwise be avoided as far as possible, as it leads to less optimal code generation in the client libraries.
+
+The syntax is:
+
+```ts
+/** @variants untagged */
+```
+
+Untagged variants must exactly follow a defined pattern.
+
+For example:
+
+```ts
+export class MyTypeBase<T1, T2, ...> { ... }
+
+export class MyTypeUntyped extends MyTypeBase<UserDefinedValue> {}
+export class MyTypeSpecialized1 extends MyTypeBase<int> {}
+export class MyTypeSpecialized2 extends MyTypeBase<string> {}
+export class MyTypeSpecialized3 extends MyTypeBase<bool> {}
+
+/**
+ * @codegen_names untyped, mytype1, mytypet2, mytype3 
+ * @variant untagged untyped=_types.MyTypeUntyped
+ */
+// Note: deserialization depends on value types
+export type MyType = MyTypeUntyped | MyTypeSpecialized1 | MyTypeSpecialized2 | MyTypeSpecialized3 
+```
+
 ### Shortcut properties
 
 In many places Elasticsearch accepts a property value to be either a complete data structure or a single value, that value being a shortcut for a property in the data structure.