diff --git a/crates/ogar-from-ruff/src/lib.rs b/crates/ogar-from-ruff/src/lib.rs index d81f2635..ef25ae23 100644 --- a/crates/ogar-from-ruff/src/lib.rs +++ b/crates/ogar-from-ruff/src/lib.rs @@ -781,22 +781,47 @@ fn lift_attribute(a: &AttrDecl) -> Option { /// Lift a Rails `enum :status, { open: 0, closed: 1 }` declaration /// to an OGAR [`EnumDecl`]. Returns `None` for non-Enum [`AttrDecl`] -/// kinds. +/// kinds, and for an enum that declares the same key twice (refused, +/// like every other `lift_*` that cannot represent its input +/// faithfully; the column is not claimed to be enum-backed with a +/// guessed variant list). /// -/// `ruff_ruby_spo::walk` drops the variant-list hash on the floor -/// (see the walker comment: "`enum :status, { active: 0 }` — 1 attr -/// (skip Hash)"), so the lifted `EnumDecl::source` is always -/// `EnumSource::Static(empty)` for Rails today. The column name is -/// still useful — downstream consumers know "this column is -/// enum-backed", even without the variant list. A future -/// `ruff_ruby_spo` enrichment could pass the variants through -/// `AttrDecl::options` and this lift would extend. +/// The ruby walker keeps the variant hash of the `enum :col, { k: n }` +/// form in `AttrDecl::options`, in source order. This lift carries the +/// **keys**, in that order, into [`EnumSource::Static`]. +/// +/// - **Label slot.** [`EnumSource::Static`] holds `(key, label)` pairs +/// and Rails source declares no label (labels live in i18n files +/// outside the model). No "absent label" representation exists in +/// the vocab, so the label is the key, the same self-label +/// `ogar-from-schema` uses. The Rails integer is deliberately **not** +/// put there: the emitter publishes the label slot as `variantValue`, +/// and the integer is a source-local physical id, not a canonical +/// value. +/// - **Empty options.** The walker also yields empty options for forms +/// whose variants it cannot see: the array form +/// (`enum :status, [:a, :b]`), a constant or other dynamic argument, +/// and a declaration with no variant argument at all. These are +/// indistinguishable from each other here, and no [`EnumSource`] +/// variant can say "variants not recoverable" (`Computed` needs a +/// verbatim body the walker does not keep). The lift therefore +/// returns `Static` with no variants, which means "the variant list +/// was not recovered", not "the enum has no values". The kwarg form +/// `enum status: { .. }` produces no `AttrDecl` at all and never +/// reaches this function. fn lift_enum(a: &AttrDecl) -> Option { if !matches!(a.kind, AttrKind::Enum) { return None; } + let mut variants: Vec<(String, String)> = Vec::with_capacity(a.options.len()); + for (key, _source_integer) in &a.options { + if variants.iter().any(|(seen, _)| seen == key) { + return None; + } + variants.push((key.clone(), key.clone())); + } let mut out = EnumDecl::new(&a.name); - out.source = EnumSource::Static(Vec::new()); + out.source = EnumSource::Static(variants); Some(out) } @@ -1579,6 +1604,9 @@ mod tests { // Enum lifts to enums slot only. assert_eq!(class.enums.len(), 1); assert_eq!(class.enums[0].column, "status"); + // The fixture has no options (the walker's output for an enum whose + // variants it could not see), so the variant list stays empty. The + // variant-bearing case is pinned by the `lift_enum_*` tests below. assert!(matches!(class.enums[0].source, EnumSource::Static(ref v) if v.is_empty())); // Attribute lifts to attributes slot only — enum does NOT // double-emit. @@ -1587,6 +1615,101 @@ mod tests { assert!(!class.attributes.iter().any(|a| a.name == "status")); } + fn enum_decl(name: &str, options: &[(&str, &str)]) -> AttrDecl { + AttrDecl { + kind: AttrKind::Enum, + name: name.to_string(), + options: options + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(), + } + } + + fn lifted_keys(a: &AttrDecl) -> Vec { + match lift_enum(a).expect("enum lifts").source { + EnumSource::Static(v) => v.into_iter().map(|(k, _)| k).collect(), + other => panic!("expected Static, got {other:?}"), + } + } + + /// `enum :status, { shipped: 3, cancelled: 7 }` keeps both keys in + /// declared order (was `Static(empty)`: the lift never read options). + #[test] + fn lift_enum_keeps_variant_keys_in_declared_order() { + let a = enum_decl("status", &[("shipped", "3"), ("cancelled", "7")]); + assert_eq!(lifted_keys(&a), ["shipped", "cancelled"]); + } + + /// Reordering the declarations changes the order, not the key set. + #[test] + fn lift_enum_reorder_changes_order_not_key_identity() { + let a = enum_decl("status", &[("shipped", "3"), ("cancelled", "7")]); + let b = enum_decl("status", &[("cancelled", "7"), ("shipped", "3")]); + assert_eq!(lifted_keys(&b), ["cancelled", "shipped"]); + let mut ka = lifted_keys(&a); + let mut kb = lifted_keys(&b); + ka.sort(); + kb.sort(); + assert_eq!(ka, kb); + } + + /// Label convention: no label is declared in Rails source, so the label + /// is the key (as in `ogar-from-schema`). The Rails integer must never + /// appear in the label slot: the emitter publishes it as `variantValue`. + #[test] + fn lift_enum_label_is_the_key_never_the_rails_integer() { + let a = enum_decl("status", &[("shipped", "3"), ("cancelled", "7")]); + match lift_enum(&a).unwrap().source { + EnumSource::Static(v) => assert_eq!( + v, + [ + ("shipped".to_string(), "shipped".to_string()), + ("cancelled".to_string(), "cancelled".to_string()), + ] + ), + other => panic!("expected Static, got {other:?}"), + } + } + + /// A duplicate key is refused (`None`, like the other `lift_*` that cannot + /// represent their input), never collapsed or lifted with a guessed list. + #[test] + fn lift_enum_refuses_duplicate_key() { + let a = enum_decl("status", &[("shipped", "3"), ("shipped", "7")]); + assert!(lift_enum(&a).is_none()); + let mut m = Model::new("Order"); + m.attributes.push(a); + assert!(lift_model(&m).enums.is_empty()); + } + + /// Empty options (array form, constant argument, no variant argument) + /// stay a key-less `Static`: no variant is invented, and no value is + /// claimed. Pins the documented limitation. + #[test] + fn lift_enum_without_visible_variants_claims_no_values() { + let a = enum_decl("status", &[]); + assert!(lifted_keys(&a).is_empty()); + assert_eq!(lift_enum(&a).unwrap().column, "status"); + } + + /// Ordinary attributes are unchanged and an enum-less class has no enums. + #[test] + fn lift_enum_leaves_ordinary_attributes_and_enum_less_classes_alone() { + let mut m = Model::new("Order"); + m.attributes.push(AttrDecl { + kind: AttrKind::Attribute, + name: "age".to_string(), + options: vec![("type".to_string(), "integer".to_string())], + }); + let class = lift_model(&m); + assert!(class.enums.is_empty()); + assert_eq!(class.attributes.len(), 1); + assert_eq!(class.attributes[0].name, "age"); + assert_eq!(class.attributes[0].type_name.as_deref(), Some("integer")); + assert!(lift_enum(&m.attributes[0]).is_none()); + } + /// **Codex P2 on #52** — block-form callbacks (`before_save { ... }` /// or `before_save do ... end`) arrive with `target` empty. /// Routing them through `Callback::method` produces