diff --git a/mlf-codegen/src/lib.rs b/mlf-codegen/src/lib.rs index 6c2aff8..a09d399 100644 --- a/mlf-codegen/src/lib.rs +++ b/mlf-codegen/src/lib.rs @@ -334,6 +334,84 @@ fn extract_docs(docs: &[DocComment]) -> String { .join("\n") } +///// Insert `key: value` into `map` only when `value` is non-empty. Several +/// ATProto fields (`description`, per-type `format`, etc.) are optional and +/// should be omitted when empty — emitting `""` causes spurious roundtrip +/// diffs against authoritative lexicons. +fn insert_opt_str(map: &mut Map, key: &str, value: &str) { + if !value.is_empty() { + map.insert(key.to_string(), Value::String(value.to_string())); + } +} + +/// Insert `key: values` only when the slice is non-empty. Mirrors the +/// ATProto convention for `required`, `enum`, and similar list fields. +fn insert_opt_list(map: &mut Map, key: &str, values: &[String]) { + if !values.is_empty() { + map.insert(key.to_string(), json!(values)); + } +} + +/// If `docs` render to a non-empty string, insert it as the `description` +/// field of `obj` (which is expected to be a JSON object). No-op for +/// non-object values or empty doc sets. +fn add_description_from_docs(obj: &mut Value, docs: &[DocComment]) { + let text = extract_docs(docs); + if text.is_empty() { + return; + } + if let Some(map) = obj.as_object_mut() { + map.insert("description".to_string(), Value::String(text)); + } +} + +/// Materialise a query/procedure/subscription parameter list into +/// `(properties, required)`. Each parameter is emitted through +/// [`generate_type_json`], annotated with its doc comment, and collected +/// into `properties`; the names of non-optional parameters are collected +/// into `required` in source order. +fn build_param_properties( + params: &[Field], + usage_counts: &HashMap, + workspace: &Workspace, + current_namespace: &str, +) -> (Map, Vec) { + let mut properties = Map::new(); + let mut required = Vec::new(); + for param in params { + if !param.optional { + required.push(param.name.name.clone()); + } + let mut param_json = generate_type_json(¶m.ty, usage_counts, workspace, current_namespace); + add_description_from_docs(&mut param_json, ¶m.docs); + properties.insert(param.name.name.clone(), param_json); + } + (properties, required) +} + +/// Build a `type: "params"` JSON object from the output of +/// [`build_param_properties`]. `required` is omitted when empty, and an +/// empty `properties` object is still emitted (queries always carry an +/// empty `parameters.properties` per ATProto convention). +fn build_params_object(properties: Map, required: &[String]) -> Value { + let mut obj = Map::new(); + obj.insert("type".to_string(), json!("params")); + insert_opt_list(&mut obj, "required", required); + obj.insert("properties".to_string(), Value::Object(properties)); + Value::Object(obj) +} + +/// Like [`build_params_object`] but returns `None` when there are no +/// parameters — used by subscriptions, where the entire `parameters` +/// field is elided rather than emitted as an empty object. +fn build_params_object_opt(properties: Map, required: &[String]) -> Option { + if properties.is_empty() { + None + } else { + Some(build_params_object(properties, required)) + } +} + fn generate_record_json(record: &Record, usage_counts: &HashMap, workspace: &Workspace, current_namespace: &str) -> Value { let mut required = Vec::new(); let mut properties = Map::new(); @@ -342,64 +420,31 @@ fn generate_record_json(record: &Record, usage_counts: &HashMap, if !field.optional { required.push(field.name.name.clone()); } - let mut field_json = generate_type_json(&field.ty, usage_counts, workspace, current_namespace); - // Add description if the field has doc comments - if !field.docs.is_empty() { - if let Some(obj) = field_json.as_object_mut() { - obj.insert("description".to_string(), json!(extract_docs(&field.docs))); - } - } + add_description_from_docs(&mut field_json, &field.docs); properties.insert(field.name.name.clone(), field_json); } - let record_obj = json!({ - "type": "object", - "required": required, - "properties": properties - }); + let mut record_obj = Map::new(); + record_obj.insert("type".to_string(), json!("object")); + insert_opt_list(&mut record_obj, "required", &required); + record_obj.insert("properties".to_string(), Value::Object(properties)); // Check for @key annotation, default to "tid" let key = get_annotation_string_value(&record.annotations, "key").unwrap_or_else(|| "tid".to_string()); - json!({ - "type": "record", - "description": extract_docs(&record.docs), - "key": key, - "record": record_obj - }) + let mut record_top = Map::new(); + record_top.insert("type".to_string(), json!("record")); + insert_opt_str(&mut record_top, "description", &extract_docs(&record.docs)); + record_top.insert("key".to_string(), Value::String(key)); + record_top.insert("record".to_string(), Value::Object(record_obj)); + Value::Object(record_top) } fn generate_query_json(query: &Query, usage_counts: &HashMap, workspace: &Workspace, current_namespace: &str) -> Value { - let mut params_properties = Map::new(); - let mut params_required = Vec::new(); - - for param in &query.params { - if !param.optional { - params_required.push(param.name.name.clone()); - } - let mut param_json = generate_type_json(¶m.ty, usage_counts, workspace, current_namespace); - // Add description if the parameter has doc comments - if !param.docs.is_empty() { - if let Some(obj) = param_json.as_object_mut() { - obj.insert("description".to_string(), json!(extract_docs(¶m.docs))); - } - } - params_properties.insert(param.name.name.clone(), param_json); - } - - let params = if !params_properties.is_empty() { - let mut params_obj = Map::new(); - params_obj.insert("type".to_string(), json!("params")); - params_obj.insert("required".to_string(), json!(params_required)); - params_obj.insert("properties".to_string(), json!(params_properties)); - Value::Object(params_obj) - } else { - let mut params_obj = Map::new(); - params_obj.insert("type".to_string(), json!("params")); - params_obj.insert("properties".to_string(), json!({})); - Value::Object(params_obj) - }; + let (params_properties, params_required) = + build_param_properties(&query.params, usage_counts, workspace, current_namespace); + let params = build_params_object(params_properties, ¶ms_required); // Check for @encoding annotation (output only for queries), default to "application/json" let output_encoding = get_encoding_annotation(&query.annotations, "output") @@ -437,7 +482,7 @@ fn generate_query_json(query: &Query, usage_counts: &HashMap, wor let mut query_obj = Map::new(); query_obj.insert("type".to_string(), json!("query")); - query_obj.insert("description".to_string(), json!(extract_docs(&query.docs))); + insert_opt_str(&mut query_obj, "description", &extract_docs(&query.docs)); query_obj.insert("parameters".to_string(), params); if let Some(output_val) = output { query_obj.insert("output".to_string(), output_val); @@ -449,22 +494,8 @@ fn generate_query_json(query: &Query, usage_counts: &HashMap, wor } fn generate_procedure_json(procedure: &Procedure, usage_counts: &HashMap, workspace: &Workspace, current_namespace: &str) -> Value { - let mut params_properties = Map::new(); - let mut params_required = Vec::new(); - - for param in &procedure.params { - if !param.optional { - params_required.push(param.name.name.clone()); - } - let mut param_json = generate_type_json(¶m.ty, usage_counts, workspace, current_namespace); - // Add description if the parameter has doc comments - if !param.docs.is_empty() { - if let Some(obj) = param_json.as_object_mut() { - obj.insert("description".to_string(), json!(extract_docs(¶m.docs))); - } - } - params_properties.insert(param.name.name.clone(), param_json); - } + let (params_properties, params_required) = + build_param_properties(&procedure.params, usage_counts, workspace, current_namespace); // Check for @encoding annotation with "input" parameter, default to "application/json" let input_encoding = get_encoding_annotation(&procedure.annotations, "input") @@ -473,8 +504,8 @@ fn generate_procedure_json(procedure: &Procedure, usage_counts: &HashMap Value { - let mut params_properties = Map::new(); - let mut params_required = Vec::new(); + let (params_properties, params_required) = + build_param_properties(&subscription.params, usage_counts, workspace, current_namespace); - for param in &subscription.params { - if !param.optional { - params_required.push(param.name.name.clone()); - } - let mut param_json = generate_type_json(¶m.ty, usage_counts, workspace, current_namespace); - // Add description if the parameter has doc comments - if !param.docs.is_empty() { - if let Some(obj) = param_json.as_object_mut() { - obj.insert("description".to_string(), json!(extract_docs(¶m.docs))); - } - } - params_properties.insert(param.name.name.clone(), param_json); - } - - let parameters = if !params_properties.is_empty() { - json!({ - "type": "params", - "required": params_required, - "properties": params_properties - }) - } else { - Value::Null - }; - - let mut result = json!({ - "type": "subscription", - "description": extract_docs(&subscription.docs) - }); + let mut result = Map::new(); + result.insert("type".to_string(), json!("subscription")); + insert_opt_str(&mut result, "description", &extract_docs(&subscription.docs)); + let mut result = Value::Object(result); if let Some(messages) = &subscription.messages { let message = json!({ @@ -578,7 +585,7 @@ fn generate_subscription_json( result["message"] = message; } - if !parameters.is_null() { + if let Some(parameters) = build_params_object_opt(params_properties, ¶ms_required) { result["parameters"] = parameters; } @@ -587,10 +594,12 @@ fn generate_subscription_json( fn generate_def_type_json(def_type: &DefType, usage_counts: &HashMap, workspace: &Workspace, current_namespace: &str) -> Value { let mut field_json = generate_type_json(&def_type.ty, usage_counts, workspace, current_namespace); - // Add description if the type definition has doc comments - if !def_type.docs.is_empty() { + // def types insert `description` at position 1 (right after `type`) to + // match the canonical field order in published lexicons. + let text = extract_docs(&def_type.docs); + if !text.is_empty() { if let Some(obj) = field_json.as_object_mut() { - obj.shift_insert(1, "description".to_string(), json!(extract_docs(&def_type.docs))); + obj.shift_insert(1, "description".to_string(), Value::String(text)); } } field_json @@ -698,25 +707,19 @@ fn generate_type_json(ty: &Type, usage_counts: &HashMap, workspac Type::Object { fields, .. } => { let mut required = Vec::new(); let mut properties = Map::new(); - for field in fields { if !field.optional { required.push(field.name.name.clone()); } let mut field_json = generate_type_json(&field.ty, usage_counts, workspace, current_namespace); - // Add description if the field has doc comments - if !field.docs.is_empty() { - if let Some(obj) = field_json.as_object_mut() { - obj.insert("description".to_string(), json!(extract_docs(&field.docs))); - } - } + add_description_from_docs(&mut field_json, &field.docs); properties.insert(field.name.name.clone(), field_json); } let mut obj = Map::new(); obj.insert("type".to_string(), json!("object")); - obj.insert("required".to_string(), json!(required)); - obj.insert("properties".to_string(), json!(properties)); + insert_opt_list(&mut obj, "required", &required); + obj.insert("properties".to_string(), Value::Object(properties)); Value::Object(obj) } Type::Parenthesized { inner, .. } => { diff --git a/tests/codegen/lexicon/annotations/expected.json b/tests/codegen/lexicon/annotations/expected.json index f083d35..bc59188 100644 --- a/tests/codegen/lexicon/annotations/expected.json +++ b/tests/codegen/lexicon/annotations/expected.json @@ -5,7 +5,6 @@ "defs": { "main": { "type": "record", - "description": "", "key": "custom-key", "record": { "type": "object", diff --git a/tests/codegen/lexicon/basic_record/expected.json b/tests/codegen/lexicon/basic_record/expected.json index dc9c511..4daf9b2 100644 --- a/tests/codegen/lexicon/basic_record/expected.json +++ b/tests/codegen/lexicon/basic_record/expected.json @@ -5,7 +5,6 @@ "defs": { "main": { "type": "record", - "description": "", "key": "tid", "record": { "type": "object", diff --git a/tests/codegen/lexicon/implicit_main/expected.json b/tests/codegen/lexicon/implicit_main/expected.json index bd2ef7d..8389864 100644 --- a/tests/codegen/lexicon/implicit_main/expected.json +++ b/tests/codegen/lexicon/implicit_main/expected.json @@ -5,7 +5,6 @@ "defs": { "main": { "type": "record", - "description": "", "key": "tid", "record": { "type": "object", diff --git a/tests/codegen/lexicon/integer_constraints/expected.json b/tests/codegen/lexicon/integer_constraints/expected.json index 2dd553f..eedaccb 100644 --- a/tests/codegen/lexicon/integer_constraints/expected.json +++ b/tests/codegen/lexicon/integer_constraints/expected.json @@ -5,7 +5,6 @@ "defs": { "main": { "type": "record", - "description": "", "key": "tid", "record": { "type": "object", diff --git a/tests/codegen/lexicon/local_references/expected.json b/tests/codegen/lexicon/local_references/expected.json index 5e17757..1bbdbc7 100644 --- a/tests/codegen/lexicon/local_references/expected.json +++ b/tests/codegen/lexicon/local_references/expected.json @@ -5,7 +5,6 @@ "defs": { "main": { "type": "record", - "description": "", "key": "tid", "record": { "type": "object", diff --git a/tests/codegen/lexicon/nested_objects/expected.json b/tests/codegen/lexicon/nested_objects/expected.json index 2cb16fe..33aeafb 100644 --- a/tests/codegen/lexicon/nested_objects/expected.json +++ b/tests/codegen/lexicon/nested_objects/expected.json @@ -5,7 +5,6 @@ "defs": { "main": { "type": "record", - "description": "", "key": "tid", "record": { "type": "object", diff --git a/tests/codegen/lexicon/record_key_any/expected.json b/tests/codegen/lexicon/record_key_any/expected.json index f2d0120..d2b37e5 100644 --- a/tests/codegen/lexicon/record_key_any/expected.json +++ b/tests/codegen/lexicon/record_key_any/expected.json @@ -5,7 +5,6 @@ "defs": { "main": { "type": "record", - "description": "", "key": "any", "record": { "type": "object", diff --git a/tests/codegen/lexicon/record_key_literal/expected.json b/tests/codegen/lexicon/record_key_literal/expected.json index ca15751..716770e 100644 --- a/tests/codegen/lexicon/record_key_literal/expected.json +++ b/tests/codegen/lexicon/record_key_literal/expected.json @@ -5,7 +5,6 @@ "defs": { "main": { "type": "record", - "description": "", "key": "literal:self", "record": { "type": "object", diff --git a/tests/codegen/lexicon/string_constraints/expected.json b/tests/codegen/lexicon/string_constraints/expected.json index 26faa8f..a763df2 100644 --- a/tests/codegen/lexicon/string_constraints/expected.json +++ b/tests/codegen/lexicon/string_constraints/expected.json @@ -5,7 +5,6 @@ "defs": { "main": { "type": "record", - "description": "", "key": "tid", "record": { "type": "object", diff --git a/tests/codegen/lexicon/union_types/expected.json b/tests/codegen/lexicon/union_types/expected.json index 5d53bb8..f627bf7 100644 --- a/tests/codegen/lexicon/union_types/expected.json +++ b/tests/codegen/lexicon/union_types/expected.json @@ -5,7 +5,6 @@ "defs": { "main": { "type": "record", - "description": "", "key": "tid", "record": { "type": "object",