[OpenAPI] Lift enum member descriptions in property descriptions (#4313) (#4355)

(cherry picked from commit 4fb0647)

Co-authored-by: Sylvain Wallez <[email protected]>
Co-authored-by: Laura Trotta <[email protected]>

Author: 3 people

compiler-rs/Cargo.lock

@@ -17,7 +17,7 @@
 
 use clients_schema::TypeName;
 use openapiv3::{Components, Parameter, ReferenceOr, RequestBody, Response, Schema, StatusCode};
-
+use crate::Configuration;
 use crate::utils::SchemaName;
 
 // Separator used to combine parts of a component path.
@@ -29,13 +29,14 @@ use crate::utils::SchemaName;
 pub const SEPARATOR: char = '-';
 
 pub struct TypesAndComponents<'a> {
+    pub config: &'a Configuration,
     pub model: &'a clients_schema::IndexedModel,
     pub components: &'a mut Components,
 }
 
 impl<'a> TypesAndComponents<'a> {
-    pub fn new(model: &'a clients_schema::IndexedModel, components: &'a mut Components) -> TypesAndComponents<'a> {
-        TypesAndComponents { model, components }
+    pub fn new(config: &'a Configuration, model: &'a clients_schema::IndexedModel, components: &'a mut Components) -> TypesAndComponents<'a> {
+        TypesAndComponents { config, model, components }
     }
 
     pub fn add_request_body(&mut self, endpoint: &str, body: RequestBody) -> ReferenceOr<RequestBody> {

compiler-rs/clients_schema_to_openapi/src/components.rs

@@ -27,13 +27,27 @@ use openapiv3::{Components, OpenAPI};
 use clients_schema::transform::ExpandConfig;
 use crate::components::TypesAndComponents;
 
+pub struct Configuration {
+    pub flavor: Option<Flavor>,
+    pub lift_enum_descriptions: bool,
+}
+
+impl Default for Configuration {
+    fn default() -> Self {
+        Self {
+            flavor: None,
+            lift_enum_descriptions: true,
+        }
+    }
+}
+
 /// Convert an API model into an OpenAPI v3 schema, optionally filtered for a given flavor
-pub fn convert_schema(mut schema: IndexedModel, flavor: Option<Flavor>) -> anyhow::Result<OpenAPI> {
+pub fn convert_schema(mut schema: IndexedModel, config: Configuration) -> anyhow::Result<OpenAPI> {
     // Expand generics
     schema = clients_schema::transform::expand_generics(schema, ExpandConfig::default())?;
 
     // Filter flavor
-    let filter: Option<fn(&Option<Availabilities>) -> bool> = match flavor {
+    let filter: Option<fn(&Option<Availabilities>) -> bool> = match config.flavor {
         None => None,
         Some(Flavor::Stack) => Some(|a| {
             // Generate only public items for Stack
@@ -49,7 +63,7 @@ pub fn convert_schema(mut schema: IndexedModel, flavor: Option<Flavor>) -> anyho
         schema = clients_schema::transform::filter_availability(schema, filter)?;
     }
 
-    convert_expanded_schema(&schema)
+    convert_expanded_schema(&schema, &config)
 }
 
 /// Convert an API model into an OpenAPI v3 schema. The input model must have all generics expanded, conversion
@@ -58,7 +72,7 @@ pub fn convert_schema(mut schema: IndexedModel, flavor: Option<Flavor>) -> anyho
 /// Note: there are ways to represent [generics in JSON Schema], but its unlikely that tooling will understand it.
 ///
 /// [generics in JSON Schema]: https://json-schema.org/blog/posts/dynamicref-and-generics
-pub fn convert_expanded_schema(model: &IndexedModel) -> anyhow::Result<OpenAPI> {
+pub fn convert_expanded_schema(model: &IndexedModel, config: &Configuration) -> anyhow::Result<OpenAPI> {
     let mut openapi = OpenAPI {
         openapi: "3.0.3".into(),
         info: info(model),
@@ -87,7 +101,7 @@ pub fn convert_expanded_schema(model: &IndexedModel) -> anyhow::Result<OpenAPI>
         extensions: Default::default(),
     };
 
-    let mut tac = TypesAndComponents::new(model, openapi.components.as_mut().unwrap());
+    let mut tac = TypesAndComponents::new(config, model, openapi.components.as_mut().unwrap());
 
     // Endpoints
     for endpoint in &model.endpoints {

compiler-rs/clients_schema_to_openapi/src/lib.rs

@@ -23,6 +23,7 @@ use clients_schema::Flavor;
 use tracing::Level;
 use tracing_subscriber::fmt::format::FmtSpan;
 use tracing_subscriber::FmtSubscriber;
+use clients_schema_to_openapi::Configuration;
 
 fn main() -> anyhow::Result<()> {
     let cli = Cli::parse();
@@ -83,7 +84,12 @@ impl Cli {
             Some(SchemaFlavor::Serverless) => Some(Flavor::Serverless),
         };
 
-        let openapi = clients_schema_to_openapi::convert_schema(model, flavor)?;
+        let config = Configuration {
+            flavor,
+            ..Default::default()
+        };
+
+        let openapi = clients_schema_to_openapi::convert_schema(model, config)?;
 
         let output: Box<dyn std::io::Write> = {
             if let Some(output) = self.output {

compiler-rs/clients_schema_to_openapi/src/main.rs

@@ -62,7 +62,7 @@ pub fn add_endpoint(
     fn parameter_data(prop: &Property, in_path: bool, tac: &mut TypesAndComponents) -> anyhow::Result<ParameterData> {
         Ok(ParameterData {
             name: prop.name.clone(),
-            description: prop.description.clone(),
+            description: tac.property_description(prop)?,
             required: in_path || prop.required, // Path parameters are always required
             deprecated: Some(prop.deprecation.is_some()),
             format: ParameterSchemaOrContent::Schema(tac.convert_value_of(&prop.typ)?),

compiler-rs/clients_schema_to_openapi/src/paths.rs

@@ -15,18 +15,15 @@
 // specific language governing permissions and limitations
 // under the License.
 
+use std::fmt::Write;
 use anyhow::bail;
-use clients_schema::{
-    Body, Enum, Interface, LiteralValueValue, PropertiesBody, Property, Request, Response, TypeAlias,
-    TypeAliasVariants, TypeDefinition, TypeName, ValueOf,
-};
+use clients_schema::{ArrayOf, Body, Enum, EnumMember, Interface, LiteralValueValue, PropertiesBody, Property, Request, Response, TypeAlias, TypeAliasVariants, TypeDefinition, TypeName, ValueOf};
 use indexmap::IndexMap;
 use openapiv3::{
     AdditionalProperties, ArrayType, Discriminator, ExternalDocumentation, NumberType, ObjectType, ReferenceOr, Schema,
     SchemaData, SchemaKind, StringType, Type,
 };
 use openapiv3::SchemaKind::AnyOf;
-
 use crate::components::TypesAndComponents;
 use crate::utils::{IntoSchema, ReferenceOrBoxed, SchemaName};
 
@@ -249,7 +246,7 @@ impl<'a> TypesAndComponents<'a> {
         let mut result = self.convert_value_of(&prop.typ)?;
         // TODO: how can we just wrap a reference so that we can add docs?
         if let ReferenceOr::Item(ref mut schema) = &mut result {
-            self.fill_data_with_prop(&mut schema.schema_data, prop);
+            self.fill_data_with_prop(&mut schema.schema_data, prop)?;
         }
         Ok(result)
     }
@@ -468,15 +465,171 @@ impl<'a> TypesAndComponents<'a> {
         // TODO: base.codegen_names as extension?
     }
 
-    fn fill_data_with_prop(&self, data: &mut SchemaData, prop: &Property) {
+    fn fill_data_with_prop(&self, data: &mut SchemaData, prop: &Property) -> anyhow::Result<()> {
         data.external_docs = self.convert_external_docs(prop);
         data.deprecated = prop.deprecation.is_some();
-        data.description = prop.description.clone();
+        data.description = self.property_description(prop)?;
         data.extensions = crate::availability_as_extensions(&prop.availability);
         // TODO: prop.aliases as extensions
         // TODO: prop.server_default as extension
         // TODO: prop.doc_id as extension (new representation of since and stability)
         // TODO: prop.es_quirk as extension?
         // TODO: prop.codegen_name as extension?
+
+        Ok(())
+    }
+
+    pub fn property_description(&self, prop: &Property) -> anyhow::Result<Option<String>> {
+        if self.config.lift_enum_descriptions {
+            Ok(lift_enum_descriptions(prop, &self.model)?.or_else(|| prop.description.clone()))
+        } else {
+            Ok(prop.description.clone())
+        }
+    }
+}
+
+/// Unwraps aliases from a value definition, recursively.
+///
+/// Returns the end value definition of the alias chain or `None` if the value definition isn't an alias.
+fn unwrap_alias<'a> (value: &ValueOf, model: &'a clients_schema::IndexedModel) -> anyhow::Result<Option<&'a ValueOf>> {
+    let ValueOf::InstanceOf(io) = value else {
+        return Ok(None);
+    };
+
+    if io.typ.is_builtin() {
+        return Ok(None);
+    }
+
+    let TypeDefinition::TypeAlias(alias) = model.get_type(&io.typ)? else {
+        return Ok(None);
+    };
+
+    // Try to unwrap further or else return the current alias
+    let result = match unwrap_alias(&alias.typ, model)? {
+        Some(alias_value) => Some(alias_value),
+        None => Some(&alias.typ),
+    };
+
+    Ok(result)
+}
+
+/// Checks if a value_of is a lenient array definition (i.e. `Foo | Foo[]`) and
+/// if successful, returns the value definition.
+fn unwrap_lenient_array(value: &ValueOf) -> Option<&ValueOf> {
+    // Is this a union
+    let ValueOf::UnionOf(u) = value else {
+        return None
+    };
+
+    // of a value and array_of (in any order)
+    let (single_value, array_value) = match &u.items.as_slice() {
+        [v, ValueOf::ArrayOf(ao)] |
+        [ValueOf::ArrayOf(ao), v] => (v, &*ao.value),
+        _ => return None,
+    };
+
+    // and both value types are the same
+    if single_value == array_value {
+        return Some(single_value);
+    }
+
+    None
+}
+
+fn unwrap_array(value: &ValueOf) -> Option<&ValueOf> {
+    match value {
+        ValueOf::ArrayOf(ArrayOf { value }) => Some(value),
+        _ => None,
+    }
+}
+
+/// If a property value is an enumeration (possibly via aliases and arrays)
+fn lift_enum_descriptions(prop: &Property, model: &clients_schema::IndexedModel) -> anyhow::Result<Option<String>> {
+
+    // FIXME: could be memoized on `prop.typ` as we'll redo this work every time we encounter the same value definition
+    let value = &prop.typ;
+
+    // Maybe an alias pointing to an array or lenient array
+    let value = unwrap_alias(value, model)?.unwrap_or(value);
+
+    // Unwrap lenient array
+    let (lenient_array, value) = match unwrap_lenient_array(value) {
+        Some(lenient_array) => (true, lenient_array),
+        None => (false, value),
+    };
+
+    // Unwrap array to get to the enum type
+    let value = unwrap_array(value).unwrap_or(value);
+
+    // Unwrap aliases again, in case the array value was itself an alias
+    let value = unwrap_alias(value, model)?.unwrap_or(value);
+
+    // Is this an enum?
+    let ValueOf::InstanceOf(inst) = value else {
+        return Ok(None);
+    };
+
+    if inst.typ.is_builtin() {
+        return Ok(None);
     }
+
+    let TypeDefinition::Enum(enum_def) = model.get_type(&inst.typ)? else {
+        return Ok(None);
+    };
+
+    let mut result: String = match &prop.description {
+        Some(desc) => desc.clone(),
+        None => String::new(),
+    };
+
+    // Do we have at least one enum member description?
+    if enum_def.members.iter().any(|m| m.description.is_some()) {
+        // Some descriptions: output a list with descriptions
+
+        // Close description paragraph and add an empty line to start a new paragraph
+        writeln!(result)?;
+        writeln!(result)?;
+
+        writeln!(result, "Supported values include:")?;
+        for member in &enum_def.members {
+            write!(result, "  - ")?;
+            value_and_aliases(&mut result, member)?;
+            if let Some(desc) = &member.description {
+                write!(result, ": {}", desc)?;
+            }
+            writeln!(result)?;
+        }
+        writeln!(result)?;
+
+    } else {
+        // No description: inline list of values, only if this wasn't a lenient array.
+        // Otherwise (enum or enum array), bump.sh will correctly output a list of possible values.
+        if !lenient_array {
+            return Ok(None);
+        }
+
+        // Close description paragraph and add an empty line to start a new paragraph
+        writeln!(result)?;
+        writeln!(result)?;
+
+        write!(result, "Supported values include: ")?;
+        for (idx, member) in enum_def.members.iter().enumerate() {
+            if idx > 0 {
+                write!(result, ", ")?;
+            }
+            value_and_aliases(&mut result, member)?;
+        }
+        write!(result, "\n\n")?;
+    }
+
+    fn value_and_aliases(out: &mut String, member: &EnumMember) -> anyhow::Result<()> {
+        write!(out, "`{}`", member.name)?;
+        if !member.aliases.is_empty() {
+            write!(out, " (or `{}`)", member.aliases.join("`, `"))?;
+        }
+
+        Ok(())
+    }
+
+    Ok(Some(result))
 }

compiler-rs/clients_schema_to_openapi/src/schemas.rs

@@ -17,6 +17,7 @@ clients_schema = {path="../clients_schema"}
 clients_schema_to_openapi = {path="../clients_schema_to_openapi"}
 serde_json = { workspace = true }
 anyhow = { workspace = true }
+tracing = "0.1"
 
 console_error_panic_hook = { workspace = true, optional = true }
 tracing-wasm = "0.2.1"

compiler-rs/compiler-wasm-lib/Cargo.toml

@@ -18,6 +18,7 @@
 use anyhow::bail;
 use clients_schema::{Flavor, IndexedModel};
 use wasm_bindgen::prelude::*;
+use clients_schema_to_openapi::Configuration;
 
 #[wasm_bindgen]
 pub fn convert_schema_to_openapi(json: &str, flavor: &str) -> Result<String, String> {
@@ -33,8 +34,12 @@ fn convert0(json: &str, flavor: &str) -> anyhow::Result<String> {
         _ => bail!("Unknown flavor {}", flavor),
     };
 
+    let config = Configuration {
+        flavor,
+        ..Default::default()
+    };
     let schema = IndexedModel::from_reader(json.as_bytes())?;
-    let openapi = clients_schema_to_openapi::convert_schema(schema, flavor)?;
+    let openapi = clients_schema_to_openapi::convert_schema(schema, config)?;
     let result = serde_json::to_string_pretty(&openapi)?;
     Ok(result)
 }