More review feedback

- Add test catching empty list of fields for targets
- Add codegen tests, and fix trailing comma bug

oximeter/impl/src/schema/codegen.rs

@@ -108,16 +108,16 @@ fn emit_one(source: FieldSource, schema: &TimeseriesSchema) -> TokenStream {
             let datum_type = emit_rust_type_for_datum_type(schema.datum_type);
             (
                 quote! { ::oximeter::Metric },
-                quote! { pub datum: #datum_type },
+                quote! { pub datum: #datum_type, },
                 schema.description.metric.as_str(),
             )
         }
     };
     quote! {
         #[doc = #type_docstring]
-        #[derive(Debug, Clone, #oximeter_trait, PartialEq)]
+        #[derive(Clone, Debug, PartialEq, #oximeter_trait)]
         pub struct #struct_name {
-            #(#field_defs),*
+            #( #field_defs, )*
             #maybe_datum
         }
     }
@@ -390,3 +390,102 @@ impl quote::ToTokens for TimeseriesSchema {
         toks.to_tokens(tokens);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::schema::TimeseriesDescription;
+    use std::{collections::BTreeSet, num::NonZeroU8};
+
+    #[test]
+    fn emit_schema_types_generates_expected_tokens() {
+        let schema = TimeseriesSchema {
+            timeseries_name: "foo:bar".parse().unwrap(),
+            description: TimeseriesDescription {
+                target: "a target".into(),
+                metric: "a metric".into(),
+            },
+            field_schema: BTreeSet::from([
+                FieldSchema {
+                    name: "f0".into(),
+                    field_type: FieldType::String,
+                    source: FieldSource::Target,
+                    description: "target field".into(),
+                },
+                FieldSchema {
+                    name: "f1".into(),
+                    field_type: FieldType::Uuid,
+                    source: FieldSource::Metric,
+                    description: "metric field".into(),
+                },
+            ]),
+            datum_type: DatumType::CumulativeU64,
+            version: NonZeroU8::new(1).unwrap(),
+            authz_scope: AuthzScope::Fleet,
+            units: Units::Bytes,
+            created: Utc::now(),
+        };
+
+        let tokens = emit_schema_types(vec![schema.clone()]);
+
+        let expected = quote! {
+            #[doc = "a target"]
+            #[derive(Clone, Debug, PartialEq, ::oximeter::Target)]
+            pub struct Foo {
+                #[doc = "target field"]
+                pub f0: ::std::borrow::Cow<'static, str>,
+            }
+
+            #[doc = "a metric"]
+            #[derive(Clone, Debug, PartialEq, ::oximeter::Metric)]
+            pub struct Bar {
+                #[doc = "metric field"]
+                pub f1: ::uuid::Uuid,
+                pub datum: ::oximeter::types::Cumulative<u64>,
+            }
+        };
+
+        assert_eq!(tokens.to_string(), expected.to_string());
+    }
+
+    #[test]
+    fn emit_schema_types_with_no_metric_fields_generates_expected_tokens() {
+        let schema = TimeseriesSchema {
+            timeseries_name: "foo:bar".parse().unwrap(),
+            description: TimeseriesDescription {
+                target: "a target".into(),
+                metric: "a metric".into(),
+            },
+            field_schema: BTreeSet::from([FieldSchema {
+                name: "f0".into(),
+                field_type: FieldType::String,
+                source: FieldSource::Target,
+                description: "target field".into(),
+            }]),
+            datum_type: DatumType::CumulativeU64,
+            version: NonZeroU8::new(1).unwrap(),
+            authz_scope: AuthzScope::Fleet,
+            units: Units::Bytes,
+            created: Utc::now(),
+        };
+
+        let tokens = emit_schema_types(vec![schema.clone()]);
+
+        let expected = quote! {
+            #[doc = "a target"]
+            #[derive(Clone, Debug, PartialEq, ::oximeter::Target)]
+            pub struct Foo {
+                #[doc = "target field"]
+                pub f0: ::std::borrow::Cow<'static, str>,
+            }
+
+            #[doc = "a metric"]
+            #[derive(Clone, Debug, PartialEq, ::oximeter::Metric)]
+            pub struct Bar {
+                pub datum: ::oximeter::types::Cumulative<u64>,
+            }
+        };
+
+        assert_eq!(tokens.to_string(), expected.to_string());
+    }
+}

oximeter/impl/src/schema/ir.rs

@@ -153,6 +153,12 @@ impl TimeseriesDefinition {
                     target_name, expected_version,
                 )));
             }
+            if fields.is_empty() {
+                return Err(MetricsError::SchemaDefinition(format!(
+                    "Target '{}' version {} must have at least one field",
+                    target_name, expected_version,
+                )));
+            }
 
             if target_fields_by_version
                 .insert(expected_version, fields)
@@ -1368,4 +1374,43 @@ mod tests {
             is supported, but found {msg:?}"
         );
     }
+
+    #[test]
+    fn ensures_target_has_at_least_one_field() {
+        let contents = r#"
+        format_version = 1
+
+        [target]
+        name = "target"
+        description = "some target"
+        authz_scope = "fleet"
+        versions = [
+            { version = 1, fields = [ ] },
+        ]
+
+        [[metrics]]
+        name = "metric"
+        description = "some metric"
+        datum_type = "u8"
+        units = "count"
+        versions = [
+            { added_in = 1, fields = [ "foo" ] },
+        ]
+
+        [fields.foo]
+        type = "string"
+        description = "a field"
+        "#;
+        let res = load_schema(contents);
+        let Err(MetricsError::SchemaDefinition(msg)) = &res else {
+            panic!(
+                "Expected to fail when target has zero fields, but found {res:#?}",
+            );
+        };
+        assert_eq!(
+            msg, "Target 'target' version 1 must have at least one field",
+            "Message should indicate that all targets must \
+            have at least one field, but found {msg:?}",
+        );
+    }
 }