diff --git a/crates/improvise-core/src/model/id.rs b/crates/improvise-core/src/model/id.rs index 7448a647..5340cc09 100644 --- a/crates/improvise-core/src/model/id.rs +++ b/crates/improvise-core/src/model/id.rs @@ -1,6 +1,7 @@ -//! How an id is made: `slugify`, and the document's `IdAllocator` (ADR 007 clause 4). -//! Reasoning: `context/code/core.md` §20. -use std::collections::HashSet; +//! How an id is made: `slugify`, and the document's `IdAllocator` (ADR 007 clause 4 +//! and its amendment). Reasoning: `context/code/core.md` §20. +use std::cmp::Ordering; +use std::collections::{HashMap, HashSet}; /// What a generated id names; spells the `` of an empty slug's `-`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -9,6 +10,7 @@ pub enum IdKind { Item, Formula, View, + Group, } impl IdKind { @@ -18,38 +20,111 @@ impl IdKind { IdKind::Item => "item", IdKind::Formula => "formula", IdKind::View => "view", + IdKind::Group => "group", } } } -/// The only ids that may begin with `_` (answer 23). -pub const VIRTUAL_IDS: [&str; 3] = ["_index", "_dim", "_measure"]; +/// The only ids that may begin with `_` (amendment 1, clause 1). +pub const VIRTUAL_IDS: [&str; 5] = ["_index", "_dim", "_measure", "_records", "_drill"]; -/// A display name's slug; empty when nothing survives (answers 16, 18, 21, 23, 26). +/// A display name's slug; empty when nothing survives (amendment 1, clause 4). pub fn slugify(display: &str) -> String { let folded = unicase::UniCase::new(display).to_folded_case(); - let kept: String = underscore_space_runs(&folded) + let mut slug = String::new(); + for c in folded .chars() - .filter(|&c| in_id_alphabet(c)) - .collect(); - kept.trim_start_matches('_').to_string() + .map(|c| if in_slug_alphabet(c) { c } else { '_' }) + { + if !(c == '_' && slug.ends_with('_')) { + slug.push(c); + } + } + let trimmed = slug.trim_matches(|c| c == '_' || c == '-'); + if trimmed.starts_with(|c: char| c.is_ascii_digit()) { + format!("n{trimmed}") + } else { + trimmed.to_string() + } } -fn underscore_space_runs(s: &str) -> String { - let (out, _) = s - .chars() - .fold((String::new(), false), |(mut out, prev), c| { - let space = c == ' '; - if !(space && prev) { - out.push(if space { '_' } else { c }); - } - (out, space) - }); - out +fn in_slug_alphabet(c: char) -> bool { + matches!(c, 'a'..='z' | '0'..='9' | '-') } -fn in_id_alphabet(c: char) -> bool { - matches!(c, 'a'..='z' | '0'..='9' | '-' | '_') +/// Whether `id` has the syntax of a generated id: a letter, then `a`-`z`, `0`-`9`, +/// `-` and single underscores, never ending in `_` or `-`. A virtual id fails it. +pub fn is_id_syntax(id: &str) -> bool { + let b = id.as_bytes(); + let edges = matches!(b.first(), Some(b'a'..=b'z')) && !matches!(b.last(), Some(b'_' | b'-')); + edges && b.windows(2).all(|w| w != b"__") && id.chars().all(|c| in_slug_alphabet(c) || c == '_') +} + +/// Ids in natural order: digit runs by value, then fewer leading zeros first. +pub fn natural_cmp(a: &str, b: &str) -> Ordering { + let (mut ra, mut rb) = (runs(a), runs(b)); + loop { + match (ra.next(), rb.next()) { + (None, None) => return Ordering::Equal, + (None, Some(_)) => return Ordering::Less, + (Some(_), None) => return Ordering::Greater, + (Some(x), Some(y)) => match run_cmp(x, y) { + Ordering::Equal => {} + other => return other, + }, + } + } +} + +fn runs(s: &str) -> impl Iterator { + let mut rest = s; + std::iter::from_fn(move || { + let digit = rest.chars().next()?.is_ascii_digit(); + let end = rest + .find(|c: char| c.is_ascii_digit() != digit) + .unwrap_or(rest.len()); + let (run, tail) = rest.split_at(end); + rest = tail; + Some(run) + }) +} + +fn run_cmp(x: &str, y: &str) -> Ordering { + let digits = |r: &str| r.starts_with(|c: char| c.is_ascii_digit()); + if digits(x) && digits(y) { + let (vx, vy) = (x.trim_start_matches('0'), y.trim_start_matches('0')); + vx.len() + .cmp(&vy.len()) + .then_with(|| vx.cmp(vy)) + .then_with(|| x.len().cmp(&y.len())) + } else { + x.cmp(y) + } +} + +/// What each `(id, display)` pair is shown as: equal displays get ` #2`, ` #3`, … +/// in natural id order, skipping numbers a literal display already holds. +pub fn display_suffixes(items: &[(&str, &str)]) -> Vec { + let literal: HashSet<&str> = items.iter().map(|&(_, d)| d).collect(); + let mut groups: HashMap<&str, Vec> = HashMap::new(); + for (i, &(_, d)) in items.iter().enumerate() { + groups.entry(d).or_default().push(i); + } + let mut shown: Vec = items.iter().map(|&(_, d)| d.to_string()).collect(); + for (display, mut at) in groups { + at.sort_by(|&i, &j| natural_cmp(items[i].0, items[j].0)); + let mut n = 2; + for &i in at.iter().skip(1) { + shown[i] = loop { + let candidate = format!("{display} #{n}"); + n += 1; + if !literal.contains(candidate.as_str()) { + break candidate; + } + }; + } + } + shown } /// Why a hand-written id cannot enter the document. @@ -81,12 +156,13 @@ impl IdAllocator { Self::default() } - /// Admit a hand-written id verbatim: it is canonical and never renamed (answer 29). + /// Admit a hand-written id verbatim: it is canonical and never renamed. pub fn claim(&mut self, id: &str) -> Result<(), ClaimError> { - let refusal = if id.is_empty() || !id.chars().all(in_id_alphabet) { - Some(ClaimError::Malformed(id.to_string())) - } else if id.starts_with('_') && !VIRTUAL_IDS.contains(&id) { + let virtual_id = VIRTUAL_IDS.contains(&id); + let refusal = if id.starts_with('_') && !virtual_id { Some(ClaimError::Reserved(id.to_string())) + } else if !virtual_id && !is_id_syntax(id) { + Some(ClaimError::Malformed(id.to_string())) } else if self.seen.contains(id) { Some(ClaimError::Duplicate(id.to_string())) } else { @@ -132,16 +208,26 @@ mod tests { use proptest::prelude::*; #[test] - fn slugify_follows_the_adrs_examples() { + fn slugify_follows_the_amendments_table() { let cases = [ - ("East", "east"), ("Time Period", "time_period"), - ("Coastal_East", "coastal_east"), - ("Q1-2024", "q1-2024"), - ("Region(code)", "regioncode"), - ("Café", "caf"), - ("_Draft", "draft"), + ("Time\tPeriod", "time_period"), + ("Time \u{200B} Period", "time_period"), + ("a_ b", "a_b"), + ("a__b", "a_b"), + ("Q1 - 2024", "q1_-_2024"), + ("East ", "east"), + ("East_", "east"), + ("-East-", "east"), + ("Region(code)", "region_code"), + ("Crème Brûlée", "cr_me_br_l_e"), + ("2024", "n2024"), + ("_2024", "n2024"), + ("2024-Q1", "n2024-q1"), ("Ü", ""), + ("-", ""), + ("%", ""), + ("", ""), ]; for (display, slug) in cases { assert_eq!(slugify(display), slug, "slugify({display:?})"); @@ -156,15 +242,21 @@ mod tests { } #[test] - fn slugify_collapses_a_run_of_spaces_to_one_underscore() { - assert_eq!(slugify("Time Period"), "time_period"); - assert_eq!(slugify("East "), "east_"); + fn slugify_trims_hyphens_and_underscores_until_neither_is_at_an_end() { + assert_eq!(slugify("_-_East-_-"), "east"); + assert_eq!(slugify("-_-"), ""); } #[test] - fn slugify_keeps_an_underscore_the_display_already_had() { - assert_eq!(slugify("a_ b"), "a__b"); - assert_eq!(slugify("a__b"), "a__b"); + fn slugify_trims_before_it_prepends_the_n() { + assert_eq!(slugify("-_7up"), "n7up"); + assert_eq!(slugify("n-7"), "n-7"); + } + + #[test] + fn slugify_keeps_a_hyphen_between_underscores() { + assert_eq!(slugify("a - b"), "a_-_b"); + assert_eq!(slugify("a_-b"), "a_-b"); } #[test] @@ -173,6 +265,8 @@ mod tests { assert_eq!(ids.generate(IdKind::Category, "Ü"), "category-1"); assert_eq!(ids.generate(IdKind::Item, "Ö"), "item-2"); assert_eq!(ids.generate(IdKind::View, ""), "view-3"); + assert_eq!(ids.generate(IdKind::Group, "-"), "group-4"); + assert_eq!(ids.generate(IdKind::Formula, "%"), "formula-5"); } #[test] @@ -197,6 +291,15 @@ mod tests { assert_eq!(ids.generate(IdKind::Item, "EAST"), "east-3"); } + #[test] + fn a_hyphenated_slug_and_a_counter_suffix_never_collide() { + let mut ids = IdAllocator::new(); + assert_eq!(ids.generate(IdKind::Item, "East"), "east"); + assert_eq!(ids.generate(IdKind::Item, "East-1"), "east-1"); + assert_eq!(ids.generate(IdKind::Item, "east"), "east-2"); + assert_eq!(ids.generate(IdKind::Item, "East-1"), "east-1-3"); + } + #[test] fn a_claimed_id_keeps_its_spelling_and_generation_steps_past_it() { let mut ids = IdAllocator::new(); @@ -224,10 +327,21 @@ mod tests { ); } + #[test] + fn claim_refuses_what_a_generated_id_could_not_be() { + let mut ids = IdAllocator::new(); + for bad in ["-x", "x-", "2024", "a__b", "x_", "-", "a b", "é"] { + assert_eq!(ids.claim(bad), Err(ClaimError::Malformed(bad.into()))); + } + for good in ["east-1", "item-4", "a-7", "q1_-_2024", "a_b", "x"] { + assert_eq!(ids.claim(good), Ok(()), "{good}"); + } + } + #[test] fn claim_refuses_an_id_that_only_begins_like_a_virtual_one() { let mut ids = IdAllocator::new(); - for lookalike in ["_indexer", "_measure-1", "_dim_2", "_measures"] { + for lookalike in ["_indexer", "_measure-1", "_dim_2", "_measures", "_recordsx"] { assert_eq!( ids.claim(lookalike), Err(ClaimError::Reserved(lookalike.into())) @@ -235,10 +349,20 @@ mod tests { } } + #[test] + fn claim_admits_each_virtual_id_once() { + let mut ids = IdAllocator::new(); + for v in VIRTUAL_IDS { + assert_eq!(ids.claim(v), Ok(())); + assert_eq!(ids.claim(v), Err(ClaimError::Duplicate(v.into()))); + } + } + #[test] fn a_display_with_leading_spaces_never_slugs_into_the_reserved_space() { assert_eq!(slugify(" Draft"), "draft"); assert_eq!(slugify(" _ Draft"), "draft"); + assert_eq!(slugify("_Drill"), "drill"); } #[test] @@ -272,7 +396,138 @@ mod tests { assert_eq!(run(), run()); } + #[test] + fn natural_order_puts_a_counter_suffix_in_numeric_order() { + let mut ids = vec!["profit-10", "profit-2", "profit", "profit-1"]; + ids.sort_by(|a, b| natural_cmp(a, b)); + assert_eq!(ids, ["profit", "profit-1", "profit-2", "profit-10"]); + } + + #[test] + fn natural_order_ties_on_value_go_to_fewer_leading_zeros() { + assert_eq!(natural_cmp("a-7", "a-007"), Ordering::Less); + assert_eq!(natural_cmp("a-007", "a-7"), Ordering::Greater); + assert_eq!(natural_cmp("a-07", "a-007"), Ordering::Less); + assert_eq!(natural_cmp("a-7", "a-7"), Ordering::Equal); + } + + #[test] + fn natural_order_compares_digit_runs_of_any_length_by_value() { + let big = "9".repeat(40); + let bigger = format!("1{}", "0".repeat(40)); + assert_eq!( + natural_cmp(&format!("x{big}"), &format!("x{bigger}")), + Ordering::Less + ); + assert_eq!(natural_cmp("x9", "x10"), Ordering::Less); + assert_eq!(natural_cmp("x000", "x0"), Ordering::Greater); + } + + #[test] + fn natural_order_puts_the_id_that_runs_out_of_runs_first() { + assert_eq!(natural_cmp("a", "a-1"), Ordering::Less); + assert_eq!(natural_cmp("a-1", "a-1-b"), Ordering::Less); + assert_eq!(natural_cmp("a-1b", "a-1"), Ordering::Greater); + } + + #[test] + fn natural_order_compares_non_digit_runs_by_bytes() { + assert_eq!(natural_cmp("b1", "a2"), Ordering::Greater); + assert_eq!(natural_cmp("a-1", "a_1"), Ordering::Less); + } + + #[test] + fn a_suffix_numbers_equal_displays_in_natural_id_order() { + let items = [ + ("profit-10", "Profit"), + ("profit", "Profit"), + ("profit-2", "Profit"), + ("profit-1", "Profit"), + ("cost", "Cost"), + ]; + assert_eq!( + display_suffixes(&items), + [ + "Profit #4", + "Profit", + "Profit #3", + "Profit #2", + "Cost" + ] + ); + } + + #[test] + fn a_suffix_skips_a_number_a_literal_display_already_holds() { + let items = [ + ("profit", "Profit"), + ("profit-1", "Profit"), + ("profit-2", "Profit"), + ("p2", "Profit #2"), + ]; + assert_eq!( + display_suffixes(&items), + ["Profit", "Profit #3", "Profit #4", "Profit #2"] + ); + } + + #[test] + fn a_suffix_is_gapless_when_a_thing_goes_away() { + let three = [("p", "Profit"), ("p-1", "Profit"), ("p-2", "Profit")]; + assert_eq!(display_suffixes(&three), ["Profit", "Profit #2", "Profit #3"]); + let two = [("p", "Profit"), ("p-2", "Profit")]; + assert_eq!(display_suffixes(&two), ["Profit", "Profit #2"]); + } + + #[test] + fn a_unique_display_is_shown_unchanged() { + let items = [("a", "East"), ("b", "West")]; + assert_eq!(display_suffixes(&items), ["East", "West"]); + assert!(display_suffixes(&[]).is_empty()); + } + + fn id_text() -> impl Strategy { + "[a-c0-3_-]{0,7}" + } + proptest! { + #[test] + fn slugify_output_has_the_id_syntax_and_is_a_fixed_point(display in ".{0,12}") { + let slug = slugify(&display); + prop_assert!(slug.is_empty() || is_id_syntax(&slug), "{slug:?}"); + prop_assert_eq!(slugify(&slug), slug); + } + + #[test] + fn id_syntax_agrees_with_its_regular_expression(id in "[a-c0-9_-]{0,8}") { + let spelled = id.starts_with(|c: char| c.is_ascii_lowercase()) + && !id.ends_with(['_', '-']) + && !id.contains("__"); + prop_assert_eq!(is_id_syntax(&id), spelled); + } + + #[test] + fn natural_order_is_a_total_order(a in id_text(), b in id_text(), c in id_text()) { + prop_assert_eq!(natural_cmp(&a, &b), natural_cmp(&b, &a).reverse()); + prop_assert_eq!(natural_cmp(&a, &b) == Ordering::Equal, a == b); + if natural_cmp(&a, &b) != Ordering::Greater && natural_cmp(&b, &c) != Ordering::Greater { + prop_assert!(natural_cmp(&a, &c) != Ordering::Greater); + } + } + + #[test] + fn suffixed_displays_are_distinct_when_the_literals_are_not_already_clashing( + ids in proptest::collection::hash_set("[a-c][0-9]{0,2}", 0..8), + display in "[AB]( #[2-4])?" + ) { + let mut ids: Vec = ids.into_iter().collect(); + ids.sort_by(|a, b| natural_cmp(a, b)); + let items: Vec<(&str, &str)> = ids.iter().map(|i| (i.as_str(), display.as_str())).collect(); + let shown = display_suffixes(&items); + let distinct: HashSet<&String> = shown.iter().collect(); + prop_assert_eq!(distinct.len(), shown.len()); + } + #[test] fn generated_ids_are_distinct_well_formed_and_never_reserved( displays in proptest::collection::vec(".{0,6}", 0..24) @@ -281,8 +536,7 @@ mod tests { let mut out = HashSet::new(); for d in &displays { let id = ids.generate(IdKind::Item, d); - prop_assert!(!id.is_empty() && !id.starts_with('_'), "{id:?}"); - prop_assert!(id.chars().all(|c| matches!(c, 'a'..='z' | '0'..='9' | '-' | '_'))); + prop_assert!(is_id_syntax(&id), "{id:?}"); prop_assert!(out.insert(id.clone()), "reissued {id:?}"); } }