From 29a4ace7ca4aebb929b92a37933879eeac4f7bc5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 23:38:39 +0000 Subject: [PATCH 1/5] ruff_ruby_spo: record each Rails enum in the form the source wrote it The Ruby walker kept a Rails enum's variants only for the hash form `enum :status, { open: 0 }`. The array form, a constant argument and a call with no variant argument all produced the same empty option list, and the legacy keyword form `enum status: { open: 0 }` produced no declaration at all. A trailing option such as `prefix: true` after an array was captured as if it were a variant. `Model::enum_forms` now carries one `EnumForm` per enum column beside the unchanged legacy `AttrDecl`. `EnumVariants` distinguishes `Hash` (key and physical value as written), `Array` (keys; the position is the physical value), `Unresolved` (a constant or other expression, rendered verbatim) and `Missing`. Rails enum options (`prefix`, `suffix`, `scopes`, `default`, `validate` and their legacy underscore forms) are kept separately and are never read as variants. The physical values stay in this IR; OGAR never receives them as labels. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XAqKMMWgnUU9g7Ldyn6sbk --- crates/ruff_ruby_spo/src/events.rs | 2 +- crates/ruff_ruby_spo/src/lib.rs | 231 ++++++++++++++++++++++++++++- crates/ruff_ruby_spo/src/walk.rs | 153 +++++++++++++++++-- crates/ruff_spo_triplet/src/ir.rs | 93 ++++++++++++ crates/ruff_spo_triplet/src/lib.rs | 4 +- 5 files changed, 463 insertions(+), 20 deletions(-) diff --git a/crates/ruff_ruby_spo/src/events.rs b/crates/ruff_ruby_spo/src/events.rs index 61498e4ea22ff..255fd20cfa4ac 100644 --- a/crates/ruff_ruby_spo/src/events.rs +++ b/crates/ruff_ruby_spo/src/events.rs @@ -1314,7 +1314,7 @@ fn rails_kind(decl: &Declaration) -> RailsKind { Declaration::Validation(_) => RailsKind::Validation, Declaration::Callback(_) => RailsKind::Callback, Declaration::Concern(_) => RailsKind::Concern, - Declaration::Attribute(_) => RailsKind::Attribute, + Declaration::Attribute(_) | Declaration::EnumForm(_) => RailsKind::Attribute, Declaration::Delegation(_) => RailsKind::Delegation, Declaration::Scope(_) => RailsKind::Scope, Declaration::ActsAs(_) => RailsKind::ActsAs, diff --git a/crates/ruff_ruby_spo/src/lib.rs b/crates/ruff_ruby_spo/src/lib.rs index 28aa65247f7d1..d5835355c54bd 100644 --- a/crates/ruff_ruby_spo/src/lib.rs +++ b/crates/ruff_ruby_spo/src/lib.rs @@ -30,8 +30,8 @@ use std::path::Path; use ruff_spo_triplet::{ - ActsAs, AssocDecl, AttrDecl, Callback, ConcernRef, Delegation, DslCall, DynMethod, Field, - Function, GemDsl, Model, ModelGraph, ScopeDecl, StiInfo, UsingRef, Validation, + ActsAs, AssocDecl, AttrDecl, Callback, ConcernRef, Delegation, DslCall, DynMethod, EnumForm, + Field, Function, GemDsl, Model, ModelGraph, ScopeDecl, StiInfo, UsingRef, Validation, }; mod actions; @@ -136,6 +136,9 @@ pub enum Declaration { /// `store_attribute` / `store_accessor` / `define_attribute_method` / /// `undef_method`. Attribute(AttrDecl), + /// The typed form of a Rails `enum` call — one per enum column. Emitted + /// beside the legacy [`Declaration::Attribute`] of kind `Enum`. + EnumForm(EnumForm), /// `delegate :foo, :bar, to: :baz`. Delegation(Delegation), /// `scope` / `default_scope` / `scopes` (OP plural). @@ -279,6 +282,7 @@ fn merge_model(dst: &mut Model, src: Model) { dst.callbacks.extend(src.callbacks); dst.concerns.extend(src.concerns); dst.attributes.extend(src.attributes); + dst.enum_forms.extend(src.enum_forms); dst.delegations.extend(src.delegations); dst.scopes.extend(src.scopes); dst.acts_as.extend(src.acts_as); @@ -319,6 +323,7 @@ fn unpack_declaration(model: &mut Model, decl: &Declaration) { Declaration::Callback(cb) => model.callbacks.push(cb.clone()), Declaration::Concern(cr) => model.concerns.push(cr.clone()), Declaration::Attribute(a) => model.attributes.push(a.clone()), + Declaration::EnumForm(e) => model.enum_forms.push(e.clone()), Declaration::Delegation(d) => model.delegations.push(d.clone()), Declaration::Scope(s) => model.scopes.push(s.clone()), Declaration::ActsAs(aa) => model.acts_as.push(aa.clone()), @@ -348,7 +353,8 @@ fn extract_fields(_class: &RubyClass) -> Vec { mod tests { use super::*; use ruff_spo_triplet::{ - AssocDecl, AssocKind, AttrDecl, AttrKind, Callback, ConcernKind, ConcernRef, expand, + AssocDecl, AssocKind, AttrDecl, AttrKind, Callback, ConcernKind, ConcernRef, EnumVariants, + expand, }; /// Locked target shape: a hand-built `ModelGraph` matching what a @@ -601,6 +607,7 @@ end Declaration::Callback(_) => cbk += 1, Declaration::Concern(_) => concern += 1, Declaration::Attribute(_) => attr += 1, + Declaration::EnumForm(_) => {} Declaration::Delegation(_) => deleg += 1, Declaration::Scope(_) => scope += 1, Declaration::ActsAs(_) => acts_as += 1, @@ -1347,4 +1354,222 @@ end ["close_duplicates", "update_done_ratio", "force_updated"] ); } + + /// Every typed enum form the first class of `src` declares. + fn enum_forms_of(src: &str) -> Vec { + let classes = extract_from_source(src); + classes[0] + .declarations + .iter() + .filter_map(|d| match d { + Declaration::EnumForm(e) => Some(e.clone()), + _ => None, + }) + .collect() + } + + fn pairs(items: &[(&str, &str)]) -> Vec<(String, String)> { + items + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect() + } + + fn names(items: &[&str]) -> Vec { + items.iter().map(|s| (*s).to_string()).collect() + } + + #[test] + fn enum_hash_form_keeps_keys_and_values_in_source_order() { + let forms = enum_forms_of("class M\n enum :status, { open: 0, closed: 1 }\nend\n"); + assert_eq!( + forms, + [EnumForm::new( + "status", + EnumVariants::Hash(pairs(&[("open", "0"), ("closed", "1")])), + Vec::new(), + )] + ); + } + + #[test] + fn enum_array_form_keeps_order_and_reordering_changes_it() { + let ab = enum_forms_of("class M\n enum :status, [:open, :closed]\nend\n"); + let ba = enum_forms_of("class M\n enum :status, [:closed, :open]\nend\n"); + assert_eq!( + ab, + [EnumForm::new( + "status", + EnumVariants::Array(names(&["open", "closed"])), + Vec::new(), + )] + ); + assert_eq!( + ba, + [EnumForm::new( + "status", + EnumVariants::Array(names(&["closed", "open"])), + Vec::new(), + )] + ); + assert_ne!(ab, ba); + } + + #[test] + fn enum_constant_form_is_unresolved_never_hash_or_array() { + let forms = enum_forms_of("class M\n enum :status, STATUSES\nend\n"); + assert_eq!( + forms, + [EnumForm::new( + "status", + EnumVariants::Unresolved("STATUSES".to_string()), + Vec::new(), + )] + ); + } + + #[test] + fn enum_without_variant_argument_is_missing() { + let forms = enum_forms_of("class M\n enum :status\nend\n"); + assert_eq!( + forms, + [EnumForm::new("status", EnumVariants::Missing, Vec::new())] + ); + } + + #[test] + fn enum_legacy_kwarg_form_is_one_column_with_its_options() { + let forms = enum_forms_of("class M\n enum status: { open: 0 }, _prefix: true\nend\n"); + assert_eq!( + forms, + [EnumForm::new( + "status", + EnumVariants::Hash(pairs(&[("open", "0")])), + pairs(&[("_prefix", "true")]), + )] + ); + } + + #[test] + fn enum_legacy_multi_column_form_yields_one_form_per_column_in_order() { + let forms = enum_forms_of("class M\n enum status: [:a], kind: { x: 1 }\nend\n"); + assert_eq!( + forms, + [ + EnumForm::new("status", EnumVariants::Array(names(&["a"])), Vec::new()), + EnumForm::new("kind", EnumVariants::Hash(pairs(&[("x", "1")])), Vec::new(),), + ] + ); + } + + #[test] + fn enum_options_are_never_captured_as_variants() { + let forms = enum_forms_of("class M\n enum :status, [:a, :b], prefix: true\nend\n"); + assert_eq!( + forms, + [EnumForm::new( + "status", + EnumVariants::Array(names(&["a", "b"])), + pairs(&[("prefix", "true")]), + )] + ); + let EnumVariants::Array(variants) = &forms[0].variants else { + panic!("expected the array form"); + }; + assert!(!variants.iter().any(|v| v == "prefix")); + } + + #[test] + fn enum_keyword_variants_without_positional_argument_split_off_options() { + let forms = + enum_forms_of("class M\n enum :status, open: 0, closed: 1, suffix: true\nend\n"); + assert_eq!( + forms, + [EnumForm::new( + "status", + EnumVariants::Hash(pairs(&[("open", "0"), ("closed", "1")])), + pairs(&[("suffix", "true")]), + )] + ); + let only_options = enum_forms_of("class M\n enum :status, prefix: true\nend\n"); + assert_eq!( + only_options, + [EnumForm::new( + "status", + EnumVariants::Missing, + pairs(&[("prefix", "true")]), + )] + ); + } + + #[test] + fn enum_hash_form_with_trailing_options_keeps_them_apart() { + let forms = enum_forms_of("class M\n enum :status, { open: 0 }, prefix: true\nend\n"); + assert_eq!( + forms, + [EnumForm::new( + "status", + EnumVariants::Hash(pairs(&[("open", "0")])), + pairs(&[("prefix", "true")]), + )] + ); + } + + #[test] + fn enum_duplicate_keys_are_preserved_as_written() { + let forms = enum_forms_of("class M\n enum :status, { a: 0, a: 1 }\nend\n"); + assert_eq!( + forms, + [EnumForm::new( + "status", + EnumVariants::Hash(pairs(&[("a", "0"), ("a", "1")])), + Vec::new(), + )] + ); + } + + #[test] + fn model_without_enums_has_no_enum_forms() { + let classes = extract_from_source("class M\n attribute :age, :integer\nend\n"); + let models = build_models(&classes); + assert_eq!(models[0].enum_forms, Vec::::new()); + } + + #[test] + fn enum_forms_flow_into_the_model_and_survive_a_reopen() { + let classes = extract_from_source( + "class M\n enum :a, [:x]\nend\nclass M\n enum :b, { y: 1 }\nend\n", + ); + let models = build_models(&classes); + assert_eq!(models.len(), 1); + assert_eq!( + models[0].enum_forms, + [ + EnumForm::new("a", EnumVariants::Array(names(&["x"])), Vec::new()), + EnumForm::new("b", EnumVariants::Hash(pairs(&[("y", "1")])), Vec::new(),), + ] + ); + } + + #[test] + fn legacy_enum_attr_decl_is_unchanged_by_the_typed_form() { + let classes = + extract_from_source("class M\n enum :status, { active: 0, archived: 1 }\nend\n"); + let attrs: Vec<&AttrDecl> = classes[0] + .declarations + .iter() + .filter_map(|d| match d { + Declaration::Attribute(a) => Some(a), + _ => None, + }) + .collect(); + assert_eq!( + attrs, + [&AttrDecl { + kind: AttrKind::Enum, + name: "status".to_string(), + options: pairs(&[("active", "0"), ("archived", "1")]), + }] + ); + } } diff --git a/crates/ruff_ruby_spo/src/walk.rs b/crates/ruff_ruby_spo/src/walk.rs index 77540c228de6e..006d7c1d8ec46 100644 --- a/crates/ruff_ruby_spo/src/walk.rs +++ b/crates/ruff_ruby_spo/src/walk.rs @@ -16,8 +16,8 @@ use lib_ruby_parser::Node; use ruff_spo_triplet::{ ActsAs, AssocDecl, AssocKind, AttrDecl, AttrKind, Callback, ConcernKind, ConcernRef, - Delegation, DslCall, DynMethod, GemDsl, GemKind, ScopeDecl, ScopeKind, UsingRef, Validation, - ValidationKind, + Delegation, DslCall, DynMethod, EnumForm, EnumVariants, GemDsl, GemKind, ScopeDecl, ScopeKind, + UsingRef, Validation, ValidationKind, }; use crate::Declaration; @@ -222,7 +222,10 @@ fn route_send(name: &str, args: &[Node], out: &mut Vec) { // so it lands in `walk_statement`'s catch-all. Handled below. "undef_method" => emit_attr(AttrKind::UndefMethod, args, out), "serialize" => emit_attr(AttrKind::Serialize, args, out), - "enum" => emit_attr(AttrKind::Enum, args, out), + "enum" => { + emit_attr(AttrKind::Enum, args, out); + emit_enum_forms(args, out); + } "store_attribute" => emit_attr(AttrKind::StoreAttribute, args, out), "store_accessor" => emit_attr(AttrKind::StoreAccessor, args, out), "define_attribute_method" => emit_attr(AttrKind::DefineAttributeMethod, args, out), @@ -516,6 +519,115 @@ fn attr_arg_window(kind: AttrKind) -> (usize, usize) { } } +/// Rails `enum` options. The legacy kwarg form spells them with a leading +/// underscore (`_prefix`); the modern positional form without. +const ENUM_OPTION_KEYS: &[&str] = &[ + "prefix", "suffix", "scopes", "default", "validate", "_prefix", "_suffix", "_scopes", + "_default", +]; + +/// Push the typed [`EnumForm`] for every enum column a `enum` call declares. +/// +/// Modern form: `enum :status, { open: 0 }, prefix: true` — a Sym column, +/// then the variant argument, then trailing options. Legacy kwarg form: +/// `enum status: { open: 0 }, _prefix: true` — each non-option pair is one +/// column; the option pairs apply to every column of the call. +fn emit_enum_forms(args: &[Node], out: &mut Vec) { + match args.split_first() { + Some((Node::Kwargs(k), _)) => emit_enum_forms_legacy(&k.pairs, out), + Some((Node::Hash(h), _)) => emit_enum_forms_legacy(&h.pairs, out), + Some((first, rest)) => { + if let Some(column) = sym_string(first) { + emit_enum_form_modern(column, rest, out); + } + } + None => {} + } +} + +fn emit_enum_form_modern(column: String, rest: &[Node], out: &mut Vec) { + let mut variants = EnumVariants::Missing; + let mut options = Vec::new(); + let mut tail = rest; + if let Some((first, after)) = rest.split_first() { + tail = after; + match first { + // Without a positional variant argument Rails reads the keyword + // arguments as the variants (`enum :status, open: 0`), except for + // the Rails option keys, which stay options. + Node::Kwargs(k) => { + let (opts, pairs) = split_enum_options(&k.pairs); + options = opts; + if !pairs.is_empty() { + variants = EnumVariants::Hash(pairs); + } + } + other => variants = classify_enum_variants(other), + } + } + for arg in tail { + if let Some(opts) = as_hash_options(arg) { + options.extend(opts); + } + } + out.push(Declaration::EnumForm(EnumForm::new( + column, variants, options, + ))); +} + +fn emit_enum_forms_legacy(pairs: &[Node], out: &mut Vec) { + let mut options = Vec::new(); + let mut columns = Vec::new(); + for pair_node in pairs { + let Node::Pair(p) = pair_node else { continue }; + let key = pair_key(&p.key); + if ENUM_OPTION_KEYS.contains(&key.as_str()) { + options.push((key, render_node(&p.value))); + } else { + columns.push((key, classify_enum_variants(&p.value))); + } + } + for (column, variants) in columns { + out.push(Declaration::EnumForm(EnumForm::new( + column, + variants, + options.clone(), + ))); + } +} + +/// `(key, value)` pairs rendered verbatim, in source order. +type Pairs = Vec<(String, String)>; + +/// Split hash pairs into (Rails enum options, variant pairs), source order. +fn split_enum_options(pairs: &[Node]) -> (Pairs, Pairs) { + let mut options = Vec::new(); + let mut variants = Vec::new(); + for (key, value) in hash_pairs(pairs) { + if ENUM_OPTION_KEYS.contains(&key.as_str()) { + options.push((key, value)); + } else { + variants.push((key, value)); + } + } + (options, variants) +} + +/// Classify the variant argument of an `enum` call by the form the source +/// wrote it. +fn classify_enum_variants(node: &Node) -> EnumVariants { + match node { + Node::Hash(h) => EnumVariants::Hash(hash_pairs(&h.pairs)), + Node::Array(a) => EnumVariants::Array( + a.elements + .iter() + .map(|e| sym_string(e).unwrap_or_else(|| render_node(e))) + .collect(), + ), + other => EnumVariants::Unresolved(render_node(other)), + } +} + fn emit_delegation(args: &[Node], out: &mut Vec) { let mut methods = Vec::new(); let mut to = String::new(); @@ -632,18 +744,31 @@ fn as_hash_options(node: &Node) -> Option> { Node::Kwargs(k) => &k.pairs, _ => return None, }; - let mut out = Vec::new(); - for pair_node in pairs { - let Node::Pair(p) = pair_node else { continue }; - let key = match p.key.as_ref() { - Node::Sym(s) => s.name.to_string_lossy(), - Node::Str(s) => s.value.to_string_lossy(), - other => render_node(other), - }; - let value = render_node(&p.value); - out.push((key, value)); + Some(hash_pairs(pairs)) +} + +/// Render the `Pair` nodes of a hash as `(key, value)` strings, source +/// order, duplicates kept. +fn hash_pairs(pairs: &[Node]) -> Vec<(String, String)> { + pairs + .iter() + .filter_map(|pair_node| { + let Node::Pair(p) = pair_node else { + return None; + }; + Some((pair_key(&p.key), render_node(&p.value))) + }) + .collect() +} + +/// A hash key as a plain string: `:sym` and `"str"` keys unwrapped, +/// anything else rendered. +fn pair_key(key: &Node) -> String { + match key { + Node::Sym(s) => s.name.to_string_lossy(), + Node::Str(s) => s.value.to_string_lossy(), + other => render_node(other), } - Some(out) } /// Render a Hash node inline as `{k: v, k2: v2}` for the args blob. diff --git a/crates/ruff_spo_triplet/src/ir.rs b/crates/ruff_spo_triplet/src/ir.rs index 5e8182e94893d..07b4633df3849 100644 --- a/crates/ruff_spo_triplet/src/ir.rs +++ b/crates/ruff_spo_triplet/src/ir.rs @@ -137,6 +137,15 @@ pub struct Model { /// `column_override` per kind. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub attributes: Vec, + /// Typed Rails `enum` declarations, one per enum column, in source + /// order. The legacy [`AttrDecl`] of kind [`AttrKind::Enum`] stays in + /// [`Self::attributes`]; this is the lossless form that keeps the + /// variant list's shape (hash, array, unresolved, missing) and the + /// Rails enum options apart. Additive + serde-defaulted: existing + /// dumps deserialize with an empty vec. No triples are expanded from + /// it; consumers read the struct field directly. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub enum_forms: Vec, /// `delegate :foo, :bar, to: :baz`. Expanded as `delegates_to` — /// one triple per (method, to) pair. #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -479,6 +488,59 @@ pub struct AttrDecl { pub options: Vec<(String, String)>, } +/// The variant list of a Rails `enum`, in the form the source wrote it. +/// +/// Physical values stay source-owned: this IR is where they are read, they +/// are never carried as labels downstream. +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[non_exhaustive] +pub enum EnumVariants { + /// `{ open: 0, closed: 1 }` — (key, physical value verbatim), source + /// order. Duplicate keys are kept as written. + Hash(Vec<(String, String)>), + /// `[:open, :closed]` — keys in source order; the 0-based position is + /// the physical value. + Array(Vec), + /// A constant or other expression the walker cannot evaluate, rendered + /// verbatim (e.g. `STATUSES`). + Unresolved(String), + /// No variant argument at all. + Missing, +} + +/// One Rails `enum` declaration, typed. Lives beside the legacy +/// [`AttrDecl`], which is unchanged. +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[non_exhaustive] +pub struct EnumForm { + /// The enum column name. + pub column: String, + /// The variant list as written. + pub variants: EnumVariants, + /// Rails enum options (`prefix`, `suffix`, `scopes`, `default`, + /// `validate`, and the legacy `_prefix` / `_suffix` / `_scopes` / + /// `_default`), key and value verbatim, source order. Never variants. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub options: Vec<(String, String)>, +} + +impl EnumForm { + /// Build an [`EnumForm`] (the struct is `#[non_exhaustive]`, so other + /// crates cannot use a struct literal). + #[must_use] + pub fn new( + column: impl Into, + variants: EnumVariants, + options: Vec<(String, String)>, + ) -> Self { + Self { + column: column.into(), + variants, + options, + } + } +} + /// The 13 Rails attribute-declaration macros covered by the AR-shape. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum AttrKind { @@ -1009,6 +1071,7 @@ mod dto_surface_tests { "delegations", "dsl_calls", "dynamic_methods", + "enum_forms", "fields", "friends", "functions", @@ -1110,6 +1173,7 @@ mod dto_surface_tests { name: "a".into(), options: opts(), }], + enum_forms: vec![EnumForm::new("status", EnumVariants::Missing, opts())], delegations: vec![Delegation { methods: vec!["m".into()], to: "t".into(), @@ -1184,6 +1248,35 @@ mod dto_surface_tests { assert_eq!(keys(&full_model()), MODEL_DTO); } + #[test] + fn model_with_enum_forms_round_trips_through_json() { + let model = Model { + enum_forms: vec![ + EnumForm::new( + "status", + EnumVariants::Hash(vec![("open".into(), "0".into())]), + vec![("_prefix".into(), "true".into())], + ), + EnumForm::new("kind", EnumVariants::Array(vec!["a".into()]), Vec::new()), + EnumForm::new("tier", EnumVariants::Unresolved("TIERS".into()), Vec::new()), + EnumForm::new("mode", EnumVariants::Missing, Vec::new()), + ], + ..Model::new("M") + }; + let json = serde_json::to_string(&model).expect("serializes"); + let back: Model = serde_json::from_str(&json).expect("deserializes"); + assert_eq!(back, model); + } + + #[test] + fn model_json_without_enum_forms_deserializes_with_an_empty_vec() { + let model: Model = serde_json::from_str(r#"{"name":"M","fields":[],"functions":[]}"#) + .expect("old dump deserializes"); + assert_eq!(model.enum_forms, Vec::::new()); + let json = serde_json::to_string(&model).expect("serializes"); + assert!(!json.contains("enum_forms"), "empty vec is skipped: {json}"); + } + #[test] fn cpp_method_dto_surface_locked() { assert_eq!(keys(&full_cpp_method()), CPP_METHOD_DTO); diff --git a/crates/ruff_spo_triplet/src/lib.rs b/crates/ruff_spo_triplet/src/lib.rs index a0736a76114c2..1d33f957c4c42 100644 --- a/crates/ruff_spo_triplet/src/lib.rs +++ b/crates/ruff_spo_triplet/src/lib.rs @@ -97,8 +97,8 @@ pub use ir::{ ActsAs, AssocDecl, AssocKind, AttrDecl, AttrKind, Callback, ConcernKind, ConcernRef, ConstexprKind, CppAccess, CppBase, CppField, CppFriend, CppMacroUse, CppMethod, CppRefQualifier, CppStaticAssert, CppTemplate, CppTemplateKind, Delegation, DslCall, DynMethod, - Field, Function, GemDsl, GemKind, Model, ModelGraph, ScopeDecl, ScopeKind, StiInfo, UsingRef, - Validation, ValidationKind, + EnumForm, EnumVariants, Field, Function, GemDsl, GemKind, Model, ModelGraph, ScopeDecl, + ScopeKind, StiInfo, UsingRef, Validation, ValidationKind, }; pub use nav_digest::build_nav_digest; pub use ndjson::{ParseError, from_ndjson, to_ndjson}; From bd202d7b7afb019d2ea59aebe9334dc8b6efad34 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 23:49:35 +0000 Subject: [PATCH 2/5] ruff_ruby_spo: keep enum string keys, source text and splats faithful Three review findings on the typed enum forms: - `enum :status, ["open", "closed"]` stored the keys with their quotes, so a string array produced different keys from the equivalent symbol array. Array elements now unwrap `"str"` the way hash keys do. - An unresolved argument such as `STATUSES.merge(extra_statuses)` was stored as the renderer's `` placeholder, so different expressions were indistinguishable. The walker now receives the source it parsed and keeps the exact text; callers without the source (the event arm) fall back to the renderer. - A hash, keyword or array literal that splices in values (`{ **BASE, archived: 2 }`, `[*BASE, :archived]`) was published as a partial list missing the spliced values. Such a literal is now `Unresolved`, with its source text. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XAqKMMWgnUU9g7Ldyn6sbk --- crates/ruff_ruby_spo/src/events.rs | 2 +- crates/ruff_ruby_spo/src/lib.rs | 68 +++++++++++++++++++++ crates/ruff_ruby_spo/src/parse.rs | 26 ++++---- crates/ruff_ruby_spo/src/walk.rs | 95 ++++++++++++++++++++---------- 4 files changed, 147 insertions(+), 44 deletions(-) diff --git a/crates/ruff_ruby_spo/src/events.rs b/crates/ruff_ruby_spo/src/events.rs index 255fd20cfa4ac..cf572394ba49b 100644 --- a/crates/ruff_ruby_spo/src/events.rs +++ b/crates/ruff_ruby_spo/src/events.rs @@ -1343,7 +1343,7 @@ fn rails_arm(body: &Node) -> Vec { for stmt in stmts { let at = anchor(stmt); let mut decls = Vec::new(); - crate::walk::walk_class_body(stmt, &mut decls); + crate::walk::walk_class_body(stmt, None, &mut decls); for decl in decls { let seq = u32::try_from(out.len()).unwrap_or(u32::MAX); out.push(RailsEvent { diff --git a/crates/ruff_ruby_spo/src/lib.rs b/crates/ruff_ruby_spo/src/lib.rs index d5835355c54bd..41ec0803ddb32 100644 --- a/crates/ruff_ruby_spo/src/lib.rs +++ b/crates/ruff_ruby_spo/src/lib.rs @@ -1428,6 +1428,74 @@ end ); } + #[test] + fn enum_string_array_keys_match_symbol_array_keys() { + let strings = enum_forms_of("class M\n enum :status, [\"open\", \"closed\"]\nend\n"); + let symbols = enum_forms_of("class M\n enum :status, [:open, :closed]\nend\n"); + assert_eq!( + strings, + [EnumForm::new( + "status", + EnumVariants::Array(names(&["open", "closed"])), + Vec::new(), + )] + ); + assert_eq!(strings, symbols); + } + + #[test] + fn enum_unresolved_expression_keeps_its_source_text() { + let merged = + enum_forms_of("class M\n enum :status, STATUSES.merge(extra_statuses)\nend\n"); + let other = enum_forms_of("class M\n enum :status, STATUSES.merge(more)\nend\n"); + assert_eq!( + merged, + [EnumForm::new( + "status", + EnumVariants::Unresolved("STATUSES.merge(extra_statuses)".to_string()), + Vec::new(), + )] + ); + assert_ne!(merged, other); + } + + #[test] + fn enum_literal_with_a_splat_is_unresolved_never_a_partial_list() { + let hash = enum_forms_of("class M\n enum :status, { **BASE, archived: 2 }\nend\n"); + let array = enum_forms_of("class M\n enum :status, [*BASE, :archived]\nend\n"); + let kwargs = enum_forms_of("class M\n enum :status, **BASE, archived: 2\nend\n"); + assert_eq!( + hash, + [EnumForm::new( + "status", + EnumVariants::Unresolved("{ **BASE, archived: 2 }".to_string()), + Vec::new(), + )] + ); + assert_eq!( + array, + [EnumForm::new( + "status", + EnumVariants::Unresolved("[*BASE, :archived]".to_string()), + Vec::new(), + )] + ); + assert!( + matches!(&kwargs[..], [f] if matches!(f.variants, EnumVariants::Unresolved(_))), + "keyword variants with a splat must be unresolved, got {kwargs:?}" + ); + // Silence twin: the same literals without a splat stay resolved. + let plain = enum_forms_of("class M\n enum :status, { archived: 2 }\nend\n"); + assert_eq!( + plain, + [EnumForm::new( + "status", + EnumVariants::Hash(pairs(&[("archived", "2")])), + Vec::new(), + )] + ); + } + #[test] fn enum_without_variant_argument_is_missing() { let forms = enum_forms_of("class M\n enum :status\nend\n"); diff --git a/crates/ruff_ruby_spo/src/parse.rs b/crates/ruff_ruby_spo/src/parse.rs index c5096d816bc95..bf1071e9b8fee 100644 --- a/crates/ruff_ruby_spo/src/parse.rs +++ b/crates/ruff_ruby_spo/src/parse.rs @@ -45,7 +45,7 @@ pub(crate) fn parse_tree(dir: &Path) -> Vec { let Some(ast) = parse_file(&src, &path) else { continue; }; - collect_classes_from_node(&ast, &mut classes); + collect_classes_from_node(&ast, &src, &mut classes); } classes } @@ -73,7 +73,7 @@ pub(crate) fn parse_models_with_engines(source_tree: &Path) -> Vec { let Some(ast) = parse_file(&src, &path) else { continue; }; - collect_classes_from_node(&ast, &mut classes); + collect_classes_from_node(&ast, &src, &mut classes); } classes } @@ -134,7 +134,7 @@ pub(crate) fn parse_models_from_source_for_test(src: &str) -> Vec { return Vec::new(); }; let mut classes = Vec::new(); - collect_classes_from_node(&ast, &mut classes); + collect_classes_from_node(&ast, src, &mut classes); classes } @@ -152,19 +152,19 @@ fn parse_file(src: &str, path: &Path) -> Option { /// Walk an AST node and emit a [`RubyClass`] for every `class X < Y` /// discovered, recursing into `module ... end` wrappers and `Begin` /// statement blocks. -fn collect_classes_from_node(node: &Node, out: &mut Vec) { - collect_classes_with_namespace(node, &[], out); +fn collect_classes_from_node(node: &Node, src: &str, out: &mut Vec) { + collect_classes_with_namespace(node, &[], src, out); } /// Recursive class-discovery walk that threads the enclosing module /// namespace through so `module Foo; class Bar < ApplicationRecord;` /// yields `name = "Foo::Bar"` (codex P2 r3418* — module namespaces /// were being dropped on the floor). -fn collect_classes_with_namespace(node: &Node, ns: &[String], out: &mut Vec) { +fn collect_classes_with_namespace(node: &Node, ns: &[String], src: &str, out: &mut Vec) { match node { Node::Begin(b) => { for stmt in &b.statements { - collect_classes_with_namespace(stmt, ns, out); + collect_classes_with_namespace(stmt, ns, src, out); } } Node::Module(m) => { @@ -172,7 +172,7 @@ fn collect_classes_with_namespace(node: &Node, ns: &[String], out: &mut Vec { @@ -206,7 +206,7 @@ fn collect_classes_with_namespace(node: &Node, ns: &[String], out: &mut Vec {} @@ -231,17 +231,17 @@ fn collect_classes_with_namespace(node: &Node, ns: &[String], out: &mut Vec) { +fn collect_nested_classes(body: &Node, ns: &[String], src: &str, out: &mut Vec) { match body { Node::Begin(b) => { for stmt in &b.statements { if matches!(stmt, Node::Class(_) | Node::Module(_)) { - collect_classes_with_namespace(stmt, ns, out); + collect_classes_with_namespace(stmt, ns, src, out); } } } Node::Class(_) | Node::Module(_) => { - collect_classes_with_namespace(body, ns, out); + collect_classes_with_namespace(body, ns, src, out); } _ => {} } diff --git a/crates/ruff_ruby_spo/src/walk.rs b/crates/ruff_ruby_spo/src/walk.rs index 006d7c1d8ec46..248061f7c71ae 100644 --- a/crates/ruff_ruby_spo/src/walk.rs +++ b/crates/ruff_ruby_spo/src/walk.rs @@ -24,22 +24,26 @@ use crate::Declaration; /// Walk a `Class.body` node and append every recognised DSL call as a /// [`Declaration`]. -pub(crate) fn walk_class_body(body: &Node, out: &mut Vec) { +/// +/// `src` is the file the body was parsed from, when the caller has it. It +/// is used only to keep an unresolved enum argument as the exact source +/// text; without it that argument falls back to the walker's renderer. +pub(crate) fn walk_class_body(body: &Node, src: Option<&str>, out: &mut Vec) { match body { Node::Begin(b) => { for stmt in &b.statements { - walk_statement(stmt, out); + walk_statement(stmt, src, out); } } - single => walk_statement(single, out), + single => walk_statement(single, src, out), } } /// Process one class-body statement. -fn walk_statement(node: &Node, out: &mut Vec) { +fn walk_statement(node: &Node, src: Option<&str>, out: &mut Vec) { match node { - Node::Send(s) if s.recv.is_none() => route_send(&s.method_name, &s.args, out), - Node::Block(blk) => walk_block(blk, out), + Node::Send(s) if s.recv.is_none() => route_send(&s.method_name, &s.args, src, out), + Node::Block(blk) => walk_block(blk, src, out), // Ruby keyword `alias new orig` parses as Node::Alias, NOT as a // Send; pick up method-alias facts that would otherwise be lost // (codex P2 PR #6 r3418*). @@ -71,7 +75,7 @@ fn alias_target_name(node: &Node) -> String { /// DSL declaration (e.g. `scope :foo, -> { … }` parses as a Send whose /// last arg is a Block; `class_methods do ... end` parses as a Block /// whose call is `class_methods`). -fn walk_block(blk: &lib_ruby_parser::nodes::Block, out: &mut Vec) { +fn walk_block(blk: &lib_ruby_parser::nodes::Block, src: Option<&str>, out: &mut Vec) { let Node::Send(s) = &*blk.call else { return; }; @@ -120,18 +124,18 @@ fn walk_block(blk: &lib_ruby_parser::nodes::Block, out: &mut Vec) { // because that's runtime-only). "with_options" => { if let Some(body) = blk.body.as_ref() { - walk_class_body(body, out); + walk_class_body(body, src, out); } // Also record the wrapper call itself so the catch-all // coverage assertion still sees `with_options` somewhere. - route_send(s.method_name.as_str(), &s.args, out); + route_send(s.method_name.as_str(), &s.args, src, out); } other => { // Generic block: route the call as if it were a bare Send, // then drop the body. Conservative — keeps the call counted // for coverage without inventing a Declaration shape for // arbitrary block DSLs. - route_send(other, &s.args, out); + route_send(other, &s.args, src, out); } } } @@ -184,7 +188,7 @@ fn lambda_loc(node: &Node) -> Option { /// explicit arm or the `has_dsl_call` catch-all. The D-AR-4 coverage /// test fails loudly if a real corpus call slips through unmatched. #[allow(clippy::too_many_lines)] // exhaustive 78-name routing — splitting hurts readability -fn route_send(name: &str, args: &[Node], out: &mut Vec) { +fn route_send(name: &str, args: &[Node], src: Option<&str>, out: &mut Vec) { match name { // ───── Associations (5) ───── "belongs_to" => emit_assoc(AssocKind::BelongsTo, args, out), @@ -224,7 +228,7 @@ fn route_send(name: &str, args: &[Node], out: &mut Vec) { "serialize" => emit_attr(AttrKind::Serialize, args, out), "enum" => { emit_attr(AttrKind::Enum, args, out); - emit_enum_forms(args, out); + emit_enum_forms(args, src, out); } "store_attribute" => emit_attr(AttrKind::StoreAttribute, args, out), "store_accessor" => emit_attr(AttrKind::StoreAccessor, args, out), @@ -532,20 +536,25 @@ const ENUM_OPTION_KEYS: &[&str] = &[ /// then the variant argument, then trailing options. Legacy kwarg form: /// `enum status: { open: 0 }, _prefix: true` — each non-option pair is one /// column; the option pairs apply to every column of the call. -fn emit_enum_forms(args: &[Node], out: &mut Vec) { +fn emit_enum_forms(args: &[Node], src: Option<&str>, out: &mut Vec) { match args.split_first() { - Some((Node::Kwargs(k), _)) => emit_enum_forms_legacy(&k.pairs, out), - Some((Node::Hash(h), _)) => emit_enum_forms_legacy(&h.pairs, out), + Some((Node::Kwargs(k), _)) => emit_enum_forms_legacy(&k.pairs, src, out), + Some((Node::Hash(h), _)) => emit_enum_forms_legacy(&h.pairs, src, out), Some((first, rest)) => { if let Some(column) = sym_string(first) { - emit_enum_form_modern(column, rest, out); + emit_enum_form_modern(column, rest, src, out); } } None => {} } } -fn emit_enum_form_modern(column: String, rest: &[Node], out: &mut Vec) { +fn emit_enum_form_modern( + column: String, + rest: &[Node], + src: Option<&str>, + out: &mut Vec, +) { let mut variants = EnumVariants::Missing; let mut options = Vec::new(); let mut tail = rest; @@ -558,11 +567,13 @@ fn emit_enum_form_modern(column: String, rest: &[Node], out: &mut Vec { let (opts, pairs) = split_enum_options(&k.pairs); options = opts; - if !pairs.is_empty() { + if has_splat(&k.pairs) { + variants = EnumVariants::Unresolved(source_text(first, src)); + } else if !pairs.is_empty() { variants = EnumVariants::Hash(pairs); } } - other => variants = classify_enum_variants(other), + other => variants = classify_enum_variants(other, src), } } for arg in tail { @@ -575,7 +586,7 @@ fn emit_enum_form_modern(column: String, rest: &[Node], out: &mut Vec) { +fn emit_enum_forms_legacy(pairs: &[Node], src: Option<&str>, out: &mut Vec) { let mut options = Vec::new(); let mut columns = Vec::new(); for pair_node in pairs { @@ -584,7 +595,7 @@ fn emit_enum_forms_legacy(pairs: &[Node], out: &mut Vec) { if ENUM_OPTION_KEYS.contains(&key.as_str()) { options.push((key, render_node(&p.value))); } else { - columns.push((key, classify_enum_variants(&p.value))); + columns.push((key, classify_enum_variants(&p.value, src))); } } for (column, variants) in columns { @@ -615,19 +626,43 @@ fn split_enum_options(pairs: &[Node]) -> (Pairs, Pairs) { /// Classify the variant argument of an `enum` call by the form the source /// wrote it. -fn classify_enum_variants(node: &Node) -> EnumVariants { +/// +/// A literal that splices in values the walker cannot see (`{ **BASE, +/// archived: 2 }`, `[*BASE, :archived]`) is `Unresolved`, never a partial +/// list: a list that silently lacks the spliced values would read as +/// complete. +fn classify_enum_variants(node: &Node, src: Option<&str>) -> EnumVariants { match node { - Node::Hash(h) => EnumVariants::Hash(hash_pairs(&h.pairs)), - Node::Array(a) => EnumVariants::Array( - a.elements - .iter() - .map(|e| sym_string(e).unwrap_or_else(|| render_node(e))) - .collect(), - ), - other => EnumVariants::Unresolved(render_node(other)), + Node::Hash(h) if !has_splat(&h.pairs) => EnumVariants::Hash(hash_pairs(&h.pairs)), + Node::Array(a) if !a.elements.iter().any(|e| matches!(e, Node::Splat(_))) => { + EnumVariants::Array(a.elements.iter().map(array_key).collect()) + } + other => EnumVariants::Unresolved(source_text(other, src)), + } +} + +/// An array element as a key: `:sym` and `"str"` unwrapped, the same way +/// [`pair_key`] reads hash keys. +fn array_key(element: &Node) -> String { + match element { + Node::Str(s) => s.value.to_string_lossy(), + other => sym_string(other).unwrap_or_else(|| render_node(other)), } } +/// Whether hash pairs include a `**splat`, whose keys the walker cannot see. +fn has_splat(pairs: &[Node]) -> bool { + pairs.iter().any(|p| matches!(p, Node::Kwsplat(_))) +} + +/// The exact source text of `node` when the source is available, else the +/// walker's rendering of it. +fn source_text(node: &Node, src: Option<&str>) -> String { + let loc = node.expression(); + src.and_then(|s| s.get(loc.begin..loc.end)) + .map_or_else(|| render_node(node), str::to_owned) +} + fn emit_delegation(args: &[Node], out: &mut Vec) { let mut methods = Vec::new(); let mut to = String::new(); From ae6b752d4c56ef9c8083ab9a951d0c51d01feb29 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 23:54:56 +0000 Subject: [PATCH 3/5] ruff_ruby_spo: classify enum arguments the way Rails' enum does The walker split keyword arguments with one option-key list for every call form, which disagrees with Rails' `enum(name = nil, values = nil, **options)`: - With a column name and no positional values, Rails reads every keyword as a variant, so `enum :status, open: 0, default: 1` declares a variant `default`; the walker had moved it to the options. - The legacy form slices off exactly `_prefix`, `_suffix`, `_scopes`, `_default` and `_instance_methods`; the walker dropped a `default:` column and turned `_instance_methods:` into a column. - A string column name (`enum "status", ..`) produced no form at all. - Hash values were rendered by the lossy argument renderer, so `Status::OPEN.to_i` became a placeholder. The modern form now treats keywords as variants when no values argument is present and as options otherwise; the legacy form uses only the underscored option keys; a string column name is accepted; and hash values and option values keep their exact source text. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XAqKMMWgnUU9g7Ldyn6sbk --- crates/ruff_ruby_spo/src/lib.rs | 63 ++++++++++++++--- crates/ruff_ruby_spo/src/walk.rs | 115 +++++++++++++++++-------------- 2 files changed, 116 insertions(+), 62 deletions(-) diff --git a/crates/ruff_ruby_spo/src/lib.rs b/crates/ruff_ruby_spo/src/lib.rs index 41ec0803ddb32..01f3640fcb82c 100644 --- a/crates/ruff_ruby_spo/src/lib.rs +++ b/crates/ruff_ruby_spo/src/lib.rs @@ -1547,25 +1547,70 @@ end assert!(!variants.iter().any(|v| v == "prefix")); } + /// Without a positional values argument Rails reads every keyword as a + /// variant, so `default:` and `suffix:` here are variants, not options. #[test] - fn enum_keyword_variants_without_positional_argument_split_off_options() { - let forms = - enum_forms_of("class M\n enum :status, open: 0, closed: 1, suffix: true\nend\n"); + fn enum_keyword_variants_without_positional_argument_are_all_variants() { + let forms = enum_forms_of("class M\n enum :status, open: 0, default: 1, suffix: 2\nend\n"); assert_eq!( forms, [EnumForm::new( "status", - EnumVariants::Hash(pairs(&[("open", "0"), ("closed", "1")])), - pairs(&[("suffix", "true")]), + EnumVariants::Hash(pairs(&[("open", "0"), ("default", "1"), ("suffix", "2")])), + Vec::new(), )] ); - let only_options = enum_forms_of("class M\n enum :status, prefix: true\nend\n"); + } + + /// The legacy form slices off exactly the underscored Rails options; + /// every other key, including one spelled like a modern option, is a + /// column. + #[test] + fn enum_legacy_form_options_are_the_underscored_rails_keys() { + let default_column = enum_forms_of("class M\n enum default: [:open]\nend\n"); assert_eq!( - only_options, + default_column, + [EnumForm::new( + "default", + EnumVariants::Array(names(&["open"])), + Vec::new(), + )] + ); + let instance_methods = + enum_forms_of("class M\n enum status: [:open], _instance_methods: false\nend\n"); + assert_eq!( + instance_methods, [EnumForm::new( "status", - EnumVariants::Missing, - pairs(&[("prefix", "true")]), + EnumVariants::Array(names(&["open"])), + pairs(&[("_instance_methods", "false")]), + )] + ); + } + + /// Rails takes the column name as `name.to_s`. + #[test] + fn enum_string_column_name_matches_symbol_column_name() { + let string = enum_forms_of("class M\n enum \"status\", [:open, :closed]\nend\n"); + let symbol = enum_forms_of("class M\n enum :status, [:open, :closed]\nend\n"); + assert_eq!(string, symbol); + assert_eq!(string.len(), 1); + } + + #[test] + fn enum_hash_values_keep_their_source_text() { + let forms = enum_forms_of( + "class M\n enum :status, { open: Status::OPEN.to_i, closed: Status::CLOSED.to_i }\nend\n", + ); + assert_eq!( + forms, + [EnumForm::new( + "status", + EnumVariants::Hash(pairs(&[ + ("open", "Status::OPEN.to_i"), + ("closed", "Status::CLOSED.to_i"), + ])), + Vec::new(), )] ); } diff --git a/crates/ruff_ruby_spo/src/walk.rs b/crates/ruff_ruby_spo/src/walk.rs index 248061f7c71ae..4a0c65927311f 100644 --- a/crates/ruff_ruby_spo/src/walk.rs +++ b/crates/ruff_ruby_spo/src/walk.rs @@ -523,25 +523,33 @@ fn attr_arg_window(kind: AttrKind) -> (usize, usize) { } } -/// Rails `enum` options. The legacy kwarg form spells them with a leading -/// underscore (`_prefix`); the modern positional form without. -const ENUM_OPTION_KEYS: &[&str] = &[ - "prefix", "suffix", "scopes", "default", "validate", "_prefix", "_suffix", "_scopes", +/// The options of the legacy keyword form (`enum status: [..], _prefix: +/// true`). Rails' `enum` slices exactly these keys off as options; every +/// other key is a column. +const LEGACY_ENUM_OPTION_KEYS: &[&str] = &[ + "_prefix", + "_suffix", + "_scopes", "_default", + "_instance_methods", ]; -/// Push the typed [`EnumForm`] for every enum column a `enum` call declares. +/// Push the typed [`EnumForm`] for every enum column a `enum` call declares, +/// following Rails' `enum(name = nil, values = nil, **options)`: /// -/// Modern form: `enum :status, { open: 0 }, prefix: true` — a Sym column, -/// then the variant argument, then trailing options. Legacy kwarg form: -/// `enum status: { open: 0 }, _prefix: true` — each non-option pair is one -/// column; the option pairs apply to every column of the call. +/// - With a column name (`enum :status, ..` or `enum "status", ..`), a +/// positional values argument is the variants and the keyword arguments +/// are options. Without one, every keyword argument is a variant +/// (`enum :status, open: 0, default: 1` declares a variant `default`). +/// - Without a name (`enum status: [..], _prefix: true`), the keys in +/// [`LEGACY_ENUM_OPTION_KEYS`] are options and every other key is a +/// column. fn emit_enum_forms(args: &[Node], src: Option<&str>, out: &mut Vec) { match args.split_first() { Some((Node::Kwargs(k), _)) => emit_enum_forms_legacy(&k.pairs, src, out), Some((Node::Hash(h), _)) => emit_enum_forms_legacy(&h.pairs, src, out), Some((first, rest)) => { - if let Some(column) = sym_string(first) { + if let Some(column) = enum_column_name(first) { emit_enum_form_modern(column, rest, src, out); } } @@ -549,38 +557,39 @@ fn emit_enum_forms(args: &[Node], src: Option<&str>, out: &mut Vec) } } +/// An enum column name: Rails takes it as `name.to_s`, so a symbol and a +/// string literal name the same column. +fn enum_column_name(node: &Node) -> Option { + match node { + Node::Str(s) => Some(s.value.to_string_lossy()), + other => sym_string(other), + } +} + fn emit_enum_form_modern( column: String, rest: &[Node], src: Option<&str>, out: &mut Vec, ) { - let mut variants = EnumVariants::Missing; - let mut options = Vec::new(); - let mut tail = rest; - if let Some((first, after)) = rest.split_first() { - tail = after; - match first { - // Without a positional variant argument Rails reads the keyword - // arguments as the variants (`enum :status, open: 0`), except for - // the Rails option keys, which stay options. - Node::Kwargs(k) => { - let (opts, pairs) = split_enum_options(&k.pairs); - options = opts; - if has_splat(&k.pairs) { - variants = EnumVariants::Unresolved(source_text(first, src)); - } else if !pairs.is_empty() { - variants = EnumVariants::Hash(pairs); - } - } - other => variants = classify_enum_variants(other, src), - } - } - for arg in tail { - if let Some(opts) = as_hash_options(arg) { - options.extend(opts); + let (variants, options) = match rest.split_first() { + None => (EnumVariants::Missing, Vec::new()), + // No positional values: Rails reads every keyword argument as a + // variant, and there are no options. + Some((first @ Node::Kwargs(_), _)) => (classify_enum_variants(first, src), Vec::new()), + Some((first, tail)) => { + let options = tail + .iter() + .filter_map(|arg| match arg { + Node::Kwargs(k) => Some(enum_pairs(&k.pairs, src)), + Node::Hash(h) => Some(enum_pairs(&h.pairs, src)), + _ => None, + }) + .flatten() + .collect(); + (classify_enum_variants(first, src), options) } - } + }; out.push(Declaration::EnumForm(EnumForm::new( column, variants, options, ))); @@ -592,8 +601,8 @@ fn emit_enum_forms_legacy(pairs: &[Node], src: Option<&str>, out: &mut Vec, out: &mut Vec; - -/// Split hash pairs into (Rails enum options, variant pairs), source order. -fn split_enum_options(pairs: &[Node]) -> (Pairs, Pairs) { - let mut options = Vec::new(); - let mut variants = Vec::new(); - for (key, value) in hash_pairs(pairs) { - if ENUM_OPTION_KEYS.contains(&key.as_str()) { - options.push((key, value)); - } else { - variants.push((key, value)); - } - } - (options, variants) +/// The `(key, value)` pairs of an enum hash: keys unwrapped as by +/// [`pair_key`], values as their exact source text when the source is +/// available. A `**splat` contributes no pair; callers that must not drop +/// it check [`has_splat`] first. +fn enum_pairs(pairs: &[Node], src: Option<&str>) -> Vec<(String, String)> { + pairs + .iter() + .filter_map(|pair_node| { + let Node::Pair(p) = pair_node else { + return None; + }; + Some((pair_key(&p.key), source_text(&p.value, src))) + }) + .collect() } /// Classify the variant argument of an `enum` call by the form the source @@ -633,7 +641,8 @@ fn split_enum_options(pairs: &[Node]) -> (Pairs, Pairs) { /// complete. fn classify_enum_variants(node: &Node, src: Option<&str>) -> EnumVariants { match node { - Node::Hash(h) if !has_splat(&h.pairs) => EnumVariants::Hash(hash_pairs(&h.pairs)), + Node::Hash(h) if !has_splat(&h.pairs) => EnumVariants::Hash(enum_pairs(&h.pairs, src)), + Node::Kwargs(k) if !has_splat(&k.pairs) => EnumVariants::Hash(enum_pairs(&k.pairs, src)), Node::Array(a) if !a.elements.iter().any(|e| matches!(e, Node::Splat(_))) => { EnumVariants::Array(a.elements.iter().map(array_key).collect()) } From 2953c47ebfca13258ae79e23843b5b62ab000305 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 23:59:23 +0000 Subject: [PATCH 4/5] ruff_ruby_spo: treat an enum literal with a computed key as unresolved A hash key or array element that needs evaluation, such as `"open".freeze`, a constant or an interpolated string, was published as a resolved variant whose key was the renderer's `` placeholder or the constant's name. Hash and array literals now resolve only when every key is a symbol or a plain string literal and nothing is splatted in; otherwise the whole argument is `Unresolved` with its source text. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XAqKMMWgnUU9g7Ldyn6sbk --- crates/ruff_ruby_spo/src/lib.rs | 44 ++++++++++++++++++++++++++ crates/ruff_ruby_spo/src/walk.rs | 53 ++++++++++++++++++++------------ 2 files changed, 77 insertions(+), 20 deletions(-) diff --git a/crates/ruff_ruby_spo/src/lib.rs b/crates/ruff_ruby_spo/src/lib.rs index 01f3640fcb82c..9e21e97dfb9c2 100644 --- a/crates/ruff_ruby_spo/src/lib.rs +++ b/crates/ruff_ruby_spo/src/lib.rs @@ -1588,6 +1588,50 @@ end ); } + /// A key that needs evaluation (a method call, a constant, an + /// interpolated string) makes the whole literal unresolved; no + /// placeholder key is published. + #[test] + fn enum_literal_with_a_computed_key_is_unresolved() { + for (src, text) in [ + ( + "class M\n enum :status, [\"open\".freeze, :closed]\nend\n", + "[\"open\".freeze, :closed]", + ), + ( + "class M\n enum :status, { \"open\".freeze => 0 }\nend\n", + "{ \"open\".freeze => 0 }", + ), + ( + "class M\n enum :status, [OPEN, :closed]\nend\n", + "[OPEN, :closed]", + ), + ( + "class M\n enum :status, [\"#{prefix}_open\"]\nend\n", + "[\"#{prefix}_open\"]", + ), + ] { + assert_eq!( + enum_forms_of(src), + [EnumForm::new( + "status", + EnumVariants::Unresolved(text.to_string()), + Vec::new(), + )], + "{src}" + ); + } + // Silence twin: literal symbol and string keys still resolve. + assert_eq!( + enum_forms_of("class M\n enum :status, { \"open\" => 0, closed: 1 }\nend\n"), + [EnumForm::new( + "status", + EnumVariants::Hash(pairs(&[("open", "0"), ("closed", "1")])), + Vec::new(), + )] + ); + } + /// Rails takes the column name as `name.to_s`. #[test] fn enum_string_column_name_matches_symbol_column_name() { diff --git a/crates/ruff_ruby_spo/src/walk.rs b/crates/ruff_ruby_spo/src/walk.rs index 4a0c65927311f..8d07dbe1ca2d8 100644 --- a/crates/ruff_ruby_spo/src/walk.rs +++ b/crates/ruff_ruby_spo/src/walk.rs @@ -616,10 +616,9 @@ fn emit_enum_forms_legacy(pairs: &[Node], src: Option<&str>, out: &mut Vec) -> Vec<(String, String)> { pairs .iter() @@ -640,28 +639,42 @@ fn enum_pairs(pairs: &[Node], src: Option<&str>) -> Vec<(String, String)> { /// list: a list that silently lacks the spliced values would read as /// complete. fn classify_enum_variants(node: &Node, src: Option<&str>) -> EnumVariants { - match node { - Node::Hash(h) if !has_splat(&h.pairs) => EnumVariants::Hash(enum_pairs(&h.pairs, src)), - Node::Kwargs(k) if !has_splat(&k.pairs) => EnumVariants::Hash(enum_pairs(&k.pairs, src)), - Node::Array(a) if !a.elements.iter().any(|e| matches!(e, Node::Splat(_))) => { - EnumVariants::Array(a.elements.iter().map(array_key).collect()) - } - other => EnumVariants::Unresolved(source_text(other, src)), - } + let resolved = match node { + Node::Hash(h) => literal_pairs(&h.pairs, src).map(EnumVariants::Hash), + Node::Kwargs(k) => literal_pairs(&k.pairs, src).map(EnumVariants::Hash), + Node::Array(a) => a + .elements + .iter() + .map(literal_key) + .collect::>>() + .map(EnumVariants::Array), + _ => None, + }; + resolved.unwrap_or_else(|| EnumVariants::Unresolved(source_text(node, src))) } -/// An array element as a key: `:sym` and `"str"` unwrapped, the same way -/// [`pair_key`] reads hash keys. -fn array_key(element: &Node) -> String { - match element { - Node::Str(s) => s.value.to_string_lossy(), - other => sym_string(other).unwrap_or_else(|| render_node(other)), +/// A variant key the walker can read without evaluating anything: a symbol +/// or a plain string literal. A constant, a method call (`"open".freeze`), +/// an interpolated string or a splat has no key until Ruby runs, so it +/// yields `None`. +fn literal_key(node: &Node) -> Option { + match node { + Node::Str(s) => Some(s.value.to_string_lossy()), + other => sym_string(other), } } -/// Whether hash pairs include a `**splat`, whose keys the walker cannot see. -fn has_splat(pairs: &[Node]) -> bool { - pairs.iter().any(|p| matches!(p, Node::Kwsplat(_))) +/// The variant pairs of a hash literal whose every element is a pair with +/// a [`literal_key`]; `None` when any key needs evaluation or a `**splat` +/// splices in pairs the walker cannot see. +fn literal_pairs(pairs: &[Node], src: Option<&str>) -> Option> { + pairs + .iter() + .map(|pair_node| match pair_node { + Node::Pair(p) => Some((literal_key(&p.key)?, source_text(&p.value, src))), + _ => None, + }) + .collect() } /// The exact source text of `node` when the source is available, else the From 8fdb7c67fc8095e6d510bd242d2c0affa25f7e21 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 8 Nov 2026 00:05:38 +0000 Subject: [PATCH 5/5] Record enum option splats instead of dropping them A keyword splat among enum options (`**ENUM_OPTIONS`) was discarded, so the form looked complete. EnumForm now carries unresolved_options with each splat's source text, for the modern and legacy forms. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XAqKMMWgnUU9g7Ldyn6sbk --- crates/ruff_ruby_spo/src/lib.rs | 38 ++++++++++++++++ crates/ruff_ruby_spo/src/walk.rs | 72 +++++++++++++++++-------------- crates/ruff_spo_triplet/src/ir.rs | 13 ++++++ 3 files changed, 91 insertions(+), 32 deletions(-) diff --git a/crates/ruff_ruby_spo/src/lib.rs b/crates/ruff_ruby_spo/src/lib.rs index 9e21e97dfb9c2..1287992df85ea 100644 --- a/crates/ruff_ruby_spo/src/lib.rs +++ b/crates/ruff_ruby_spo/src/lib.rs @@ -1672,6 +1672,44 @@ end ); } + #[test] + fn enum_option_splats_are_recorded_not_dropped() { + let only = enum_forms_of("class M\n enum :status, [:open], **ENUM_OPTIONS\nend\n"); + assert_eq!( + only, + [EnumForm::new( + "status", + EnumVariants::Array(vec!["open".into()]), + Vec::new() + ) + .with_unresolved_options(vec!["**ENUM_OPTIONS".into()])] + ); + let mixed = + enum_forms_of("class M\n enum :status, [:open], prefix: true, **OTHER_OPTIONS\nend\n"); + assert_eq!( + mixed, + [EnumForm::new( + "status", + EnumVariants::Array(vec!["open".into()]), + pairs(&[("prefix", "true")]), + ) + .with_unresolved_options(vec!["**OTHER_OPTIONS".into()])] + ); + let legacy = + enum_forms_of("class M\n enum status: [:open], _prefix: true, **LEGACY\nend\n"); + assert_eq!(legacy.len(), 1); + assert_eq!(legacy[0].unresolved_options, vec!["**LEGACY".to_string()]); + } + + #[test] + fn enum_literal_options_leave_no_unresolved_options() { + let forms = + enum_forms_of("class M\n enum :status, [:open], prefix: true, scopes: false\nend\n"); + assert_eq!(forms.len(), 1); + assert_eq!(forms[0].options.len(), 2); + assert!(forms[0].unresolved_options.is_empty()); + } + #[test] fn enum_duplicate_keys_are_preserved_as_written() { let forms = enum_forms_of("class M\n enum :status, { a: 0, a: 1 }\nend\n"); diff --git a/crates/ruff_ruby_spo/src/walk.rs b/crates/ruff_ruby_spo/src/walk.rs index 8d07dbe1ca2d8..b08430b5dabbf 100644 --- a/crates/ruff_ruby_spo/src/walk.rs +++ b/crates/ruff_ruby_spo/src/walk.rs @@ -572,34 +572,43 @@ fn emit_enum_form_modern( src: Option<&str>, out: &mut Vec, ) { - let (variants, options) = match rest.split_first() { - None => (EnumVariants::Missing, Vec::new()), + let mut options = Vec::new(); + let mut unresolved = Vec::new(); + let variants = match rest.split_first() { + None => EnumVariants::Missing, // No positional values: Rails reads every keyword argument as a // variant, and there are no options. - Some((first @ Node::Kwargs(_), _)) => (classify_enum_variants(first, src), Vec::new()), + Some((first @ Node::Kwargs(_), _)) => classify_enum_variants(first, src), Some((first, tail)) => { - let options = tail - .iter() - .filter_map(|arg| match arg { - Node::Kwargs(k) => Some(enum_pairs(&k.pairs, src)), - Node::Hash(h) => Some(enum_pairs(&h.pairs, src)), - _ => None, - }) - .flatten() - .collect(); - (classify_enum_variants(first, src), options) + for arg in tail { + let pairs = match arg { + Node::Kwargs(k) => &k.pairs, + Node::Hash(h) => &h.pairs, + _ => continue, + }; + let (resolved, rest) = enum_pairs(pairs, src); + options.extend(resolved); + unresolved.extend(rest); + } + classify_enum_variants(first, src) } }; - out.push(Declaration::EnumForm(EnumForm::new( - column, variants, options, - ))); + out.push(Declaration::EnumForm( + EnumForm::new(column, variants, options).with_unresolved_options(unresolved), + )); } fn emit_enum_forms_legacy(pairs: &[Node], src: Option<&str>, out: &mut Vec) { let mut options = Vec::new(); let mut columns = Vec::new(); + let mut unresolved = Vec::new(); for pair_node in pairs { - let Node::Pair(p) = pair_node else { continue }; + // A splat such as `**ENUM_CONFIG` may carry columns or options; it + // cannot be read, so every form records it as an unresolved option. + let Node::Pair(p) = pair_node else { + unresolved.push(source_text(pair_node, src)); + continue; + }; let key = pair_key(&p.key); if LEGACY_ENUM_OPTION_KEYS.contains(&key.as_str()) { options.push((key, source_text(&p.value, src))); @@ -608,27 +617,26 @@ fn emit_enum_forms_legacy(pairs: &[Node], src: Option<&str>, out: &mut Vec) -> Vec<(String, String)> { - pairs - .iter() - .filter_map(|pair_node| { - let Node::Pair(p) = pair_node else { - return None; - }; - Some((pair_key(&p.key), source_text(&p.value, src))) - }) - .collect() +fn enum_pairs(pairs: &[Node], src: Option<&str>) -> (Vec<(String, String)>, Vec) { + let mut resolved = Vec::new(); + let mut unresolved = Vec::new(); + for pair_node in pairs { + match pair_node { + Node::Pair(p) => resolved.push((pair_key(&p.key), source_text(&p.value, src))), + other => unresolved.push(source_text(other, src)), + } + } + (resolved, unresolved) } /// Classify the variant argument of an `enum` call by the form the source diff --git a/crates/ruff_spo_triplet/src/ir.rs b/crates/ruff_spo_triplet/src/ir.rs index 07b4633df3849..84d4dc7bbe785 100644 --- a/crates/ruff_spo_triplet/src/ir.rs +++ b/crates/ruff_spo_triplet/src/ir.rs @@ -522,6 +522,11 @@ pub struct EnumForm { /// `_default`), key and value verbatim, source order. Never variants. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub options: Vec<(String, String)>, + /// Option expressions that could not be read as `key: value` pairs, such + /// as `**ENUM_OPTIONS`, as source text in source order. When non-empty, + /// [`Self::options`] is incomplete: the expansion may set any option. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub unresolved_options: Vec, } impl EnumForm { @@ -537,8 +542,16 @@ impl EnumForm { column: column.into(), variants, options, + unresolved_options: Vec::new(), } } + + /// Record option expressions that are not literal pairs. + #[must_use] + pub fn with_unresolved_options(mut self, unresolved: Vec) -> Self { + self.unresolved_options = unresolved; + self + } } /// The 13 Rails attribute-declaration macros covered by the AR-shape.