cmounce/marzipan
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Overhaul label sanitization logic

Browse Source 
Besides code cleanup, this commit changes the sanitization logic to
avoid numeric digits altogether. Previously, they had been allowed at
the end of a label name, but this is susceptible to a ZZT parser bug:
`#foo` matches `:foo2` because ZZT allows a prefix match if the next
character is a digit.
This commit is contained in:
Chris Mounce
2025-07-19 15:22:27 -07:00
parent 85ba7b63c8
commit 84f35c8b44
4 changed files with 145 additions and 184 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/labels/sanitize.rs
+104 -143
View File
@@ -1,187 +1,151 @@
use std::collections::{HashMap, HashSet};
use compact_str::CompactString; use compact_str::CompactString;
use rustc_hash::{FxHashMap, FxHashSet};
#[derive(PartialEq, Eq, Hash)] pub struct Registry {
struct LabelId(CompactString); key_to_suffix: FxHashMap<Lowercase, Suffix>,
names: FxHashSet<Lowercase>,
anonymous_counter: CompactString,
}
impl LabelId { #[derive(Clone, PartialEq, Eq, Hash)]
fn new(label: &str) -> Self { struct Lowercase(CompactString);
let mut result = CompactString::new(label);
impl Lowercase {
fn new(key: &str) -> Self {
let mut result = CompactString::new(key);
result.make_ascii_lowercase(); result.make_ascii_lowercase();
Self(result) Self(result)
} }
} }
pub struct Registry { #[derive(Default)]
label_transforms: HashMap<LabelId, Transform>, struct Suffix(CompactString);
existing: HashSet<CompactString>,
anonymous_counter: CompactString,
}
enum Transform { impl Suffix {
Preferred, fn apply(&self, key: &str) -> CompactString {
FilteredWithSuffix(CompactString), let mut result = preferred_label_name(key);
} result.push_str(&self.0);
impl Transform {
fn apply(&self, label: &str) -> CompactString {
let preferred = preferred_label_name(label);
match self {
Transform::Preferred => preferred,
Transform::FilteredWithSuffix(suffix) => {
let mut result = preferred;
result = Registry::filter_label_chars(&result);
result.push_str(&suffix);
result result
} }
} }
}
}
impl Registry { impl Registry {
pub fn new() -> Self { pub fn new() -> Self {
let builtin_labels = ["bombed", "energize", "shot", "thud", "touch"]; let builtin_labels = ["bombed", "energize", "shot", "thud", "touch"];
let mut label_transforms = HashMap::new(); let mut key_to_suffix = FxHashMap::default();
let mut existing = HashSet::new(); let mut taken = FxHashSet::default();
for name in builtin_labels { for name in builtin_labels {
let cs = CompactString::new(name); let name = Lowercase(name.into());
label_transforms.insert(LabelId(cs.clone()), Transform::Preferred); key_to_suffix.insert(name.clone(), Suffix::default());
existing.insert(cs); taken.insert(name);
} }
Self { Self {
label_transforms, key_to_suffix,
existing, names: taken,
anonymous_counter: "_".into(), anonymous_counter: CompactString::const_new(""),
} }
} }
pub fn sanitize(&mut self, label: &str) -> CompactString { pub fn sanitize(&mut self, key: &str) -> CompactString {
// Use existing sanitization if one exists // Use existing sanitization if one exists
let id = LabelId::new(label); let key_lower = Lowercase::new(key);
if let Some(transform) = self.label_transforms.get(&id) { if let Some(transform) = self.key_to_suffix.get(&key_lower) {
return transform.apply(label); return transform.apply(key);
} }
// Try to use label's preferred name as-is // Append suffixes until we find a name that's not taken yet
let preferred = Transform::Preferred.apply(label); let mut candidate = Lowercase::new(&preferred_label_name(key));
if Registry::is_valid_label(&preferred) { let mut suffix = CompactString::const_new("");
let key = preferred.to_ascii_lowercase(); let original_len = candidate.0.len();
if !self.existing.contains(&key) {
self.label_transforms.insert(id, Transform::Preferred);
self.existing.insert(key);
return preferred;
}
}
// Generate a new name by appending suffixes
let base = Registry::filter_label_chars(&preferred);
let base_len = base.len();
let mut candidate = base;
let mut suffix = CompactString::with_capacity(0);
loop { loop {
candidate.push_str(&suffix); candidate.0.push_str(&suffix);
let key = candidate.to_ascii_lowercase(); if !self.names.contains(&candidate) {
if !self.existing.contains(&key) { self.key_to_suffix.insert(key_lower, Suffix(suffix));
self.label_transforms self.names.insert(candidate.clone());
.insert(id, Transform::FilteredWithSuffix(suffix));
self.existing.insert(key);
break; break;
} else { } else {
candidate.truncate(base_len); candidate.0.truncate(original_len);
increment(&mut suffix); increment(&mut suffix);
} }
} }
candidate candidate.0
} }
pub fn gen_anonymous(&mut self) -> CompactString { pub fn gen_anonymous(&mut self) -> CompactString {
while self.existing.contains(&self.anonymous_counter) { increment(&mut self.anonymous_counter);
while self
.names
.contains(&Lowercase(self.anonymous_counter.clone()))
{
increment(&mut self.anonymous_counter); increment(&mut self.anonymous_counter);
} }
let result = self.anonymous_counter.clone(); self.anonymous_counter.clone()
increment(&mut self.anonymous_counter); }
result
} }
fn is_valid_label(s: &CompactString) -> bool { /// Given the key string identifying a label, generate the first-pick name we'd
if s.len() == 0 { /// like to assign it. (Results returned by this function are subject to veto if
return false; /// they are already taken.)
} fn preferred_label_name(key: &str) -> CompactString {
let (most, last) = s.split_at(s.len() - 1); // Extract the last part of the key: "ns~global$123.local" becomes "local"
if !most.chars().all(|c| c == '_' || c.is_ascii_alphabetic()) { let base_name = key
return false; .rsplit_once(|c: char| !c.is_ascii_alphanumeric() && c != '_')
} .map(|(_, base_name)| base_name)
last.chars().all(|c| c == '_' || c.is_ascii_alphanumeric()) .unwrap_or(key);
}
fn filter_label_chars(s: &CompactString) -> CompactString { // Collapse runs of numeric digits/underscores to a single underscore
let mut result = CompactString::with_capacity(0); let mut result = CompactString::default();
let mut run_of_digits = false; let mut run = false;
for c in s.chars() { for c in base_name.chars() {
if c.is_ascii_digit() { if c.is_ascii_alphabetic() {
if !run_of_digits {
result.push('_');
run_of_digits = true;
}
} else {
result.push(c); result.push(c);
run_of_digits = false; run = false;
} else if !run {
result.push('_');
run = true;
} }
} }
result result
} }
}
/// Given an unsanitized full name, generate the first-pick name we'd like to
/// assign this label. (Results returned by this function are subject to veto
/// if they are invalid or already taken.)
fn preferred_label_name(s: &str) -> CompactString {
if let Some((_, suffix)) = s.rsplit_once(|c: char| !c.is_ascii_alphanumeric() && c != '_') {
// Prevent namespaces and local labels from starting with a digit.
// This ensures stuff like `#take gems 100.99orless` won't compile
// to `#take gems 10099orless` (would parse incorrectly).
let mut result = CompactString::const_new("_");
result.push_str(suffix);
result
} else {
CompactString::new(s)
}
}
/// Increments a string through label-safe characters. /// Increments a string through label-safe characters.
/// Characters loop through underscore and the letters a-z. ///
/// Additionally, the final character is allowed to loop through 0-9. /// The characters tick upward in order (underscore, a-z), odometer style,
/// growing the string on overflow. The result is similar to counting in
/// base 27, but it's not quite the same:
///
/// - The zeroth value is the empty string "" (not "_").
/// - "z" is followed by "__", "_a", "_b", etc (not "a_", "aa", "ab").
///
/// Unlike in place-value systems where "0001" and "1" are equivalent, the goal
/// of this function is to generate every possible string.
fn increment(s: &mut CompactString) { fn increment(s: &mut CompactString) {
let original_len = s.len(); let original_len = s.len();
// Find a character we can increment without carry // Find a character we can increment without carry
let mut last_char = s.pop(); let mut last_char = s.pop();
while !(last_char == None || last_char != Some('z')) { while last_char == Some('z') {
last_char = s.pop(); last_char = s.pop();
} }
if let Some(c) = last_char { if let Some(c) = last_char {
// Increment character in order: 0-9, then _, then a-z // Increment character
let incremented = match c { let incremented = match c {
'0'..'9' | 'a'..'z' => (c as u8 + 1) as char, 'a'..'z' => (c as u8 + 1) as char,
'9' => '_',
'_' => 'a', '_' => 'a',
_ => unreachable!(), _ => unreachable!(),
}; };
s.push(incremented); s.push(incremented);
// Pad to original length, like "___0" // Pad to original length
while s.len() < original_len { while s.len() < original_len {
let is_last = s.len() == original_len - 1;
s.push(if is_last { '0' } else { '_' });
}
} else {
// We reached maximum value for the string: everything rolls over
for _ in 0..original_len {
s.push('_'); s.push('_');
} }
s.push('0'); } else {
// All existing chars roll over, length grows by 1
for _ in 0..(original_len + 1) {
s.push('_');
}
} }
} }
@@ -210,29 +174,26 @@ mod test {
} }
let result = rows.join("\n"); let result = rows.join("\n");
assert_snapshot!(result, @r" assert_snapshot!(result, @r"
0, 1, 2, 3, 4, 5, 6, 7, 8, 9
_, a, b, c, d, e, f, g, h, i _, a, b, c, d, e, f, g, h, i
j, k, l, m, n, o, p, q, r, s j, k, l, m, n, o, p, q, r, s
t, u, v, w, x, y, z, _0, _1, _2 t, u, v, w, x, y, z, __, _a, _b
_3, _4, _5, _6, _7, _8, _9, __, _a, _b
_c, _d, _e, _f, _g, _h, _i, _j, _k, _l _c, _d, _e, _f, _g, _h, _i, _j, _k, _l
_m, _n, _o, _p, _q, _r, _s, _t, _u, _v _m, _n, _o, _p, _q, _r, _s, _t, _u, _v
_w, _x, _y, _z, a0, a1, a2, a3, a4, a5 _w, _x, _y, _z, a_, aa, ab, ac, ad, ae
a6, a7, a8, a9, a_, aa, ab, ac, ad, ae
af, ag, ah, ai, aj, ak, al, am, an, ao af, ag, ah, ai, aj, ak, al, am, an, ao
ap, aq, ar, as, at, au, av, aw, ax, ay
az, b_, ba, bb, bc, bd, be, bf, bg, bh
bi, bj, bk, bl, bm, bn, bo, bp, bq, br
"); ");
} }
#[test] #[test]
fn test_increment_all_len_3() { fn test_increment_all_len_3() {
// Most chars are alphabetic or underscore (26 + 1 = 27). let num_labels = 27 + 27 * 27 + 27 * 27 * 27; // 1 char + 2 chars + 3 chars
// Last char can include digits (27 + 10 = 37).
let num_labels = 37 + 27 * 37 + 27 * 27 * 37; // 1 char + 2 chars + 3 chars
let mut ctr = CompactString::new(""); let mut ctr = CompactString::new("");
let mut seen = HashSet::with_capacity(num_labels); let mut seen = HashSet::with_capacity(num_labels);
for _ in 0..num_labels { for _ in 0..num_labels {
increment(&mut ctr); increment(&mut ctr);
assert!(Registry::is_valid_label(&ctr));
assert!(!seen.contains(&ctr)); assert!(!seen.contains(&ctr));
assert!(ctr.len() <= 3); assert!(ctr.len() <= 3);
seen.insert(ctr.clone()); seen.insert(ctr.clone());
@@ -273,20 +234,20 @@ mod test {
let result = results.join("\n"); let result = results.join("\n");
assert_snapshot!(result, @r#" assert_snapshot!(result, @r#"
"foo" => foo "foo" => foo
"ns1~foo" => _foo "ns1~foo" => foo_
"ns2~foo" => _foo0 "ns2~foo" => fooa
"foo.thisloop" => _thisloop "foo.thisloop" => thisloop
"foo.thatloop" => _thatloop "foo.thatloop" => thatloop
"bar.thisloop" => _thisloop0 "bar.thisloop" => thisloop_
"bar.thatloop" => _thatloop0 "bar.thatloop" => thatloop_
"foo$1.thisloop" => _thisloop1 "foo$1.thisloop" => thisloopa
"foo$1.thatloop" => _thatloop1 "foo$1.thatloop" => thatloopa
"bar1" => bar1 "bar1" => bar_
"bar2" => bar2 "bar2" => bar__
"bar123" => bar_ "bar123" => bar_a
"BAR123" => BAR_ "BAR123" => BAR_a
"bar456" => bar_0 "bar456" => bar_b
"BAR456" => BAR_0 "BAR456" => BAR_b
"foo2bar" => foo_bar "foo2bar" => foo_bar
"#); "#);
} }
src/labels/snapshots/marzipan__labels__process__test__label_sanitization.snap
+4 -4
View File
@@ -9,14 +9,14 @@ expression: board_to_text(board)
You have hundreds of gems. You have hundreds of gems.
#end #end
:lt_ :lt_
#take gems 10 lt_0 #take gems 10 lt__
#give gems 10 #give gems 10
You have tens of gems. You have tens of gems.
#end #end
:lt_0 :lt__
#take gems 1 lt1 #take gems 1 lt_a
#give gems 1 #give gems 1
You have at least one gem. You have at least one gem.
#end #end
:lt1 :lt_a
You don't have any gems! You don't have any gems!
src/labels/snapshots/marzipan__labels__process__test__local_labels.snap
+26 -26
View File
@@ -6,70 +6,70 @@ expression: board_to_text(board)
'using the same local label ".loop" 'using the same local label ".loop"
:run_w :run_w
#walk w #walk w
:_loop :loop
#try w stop #try w stop
#_loop #loop
' '
:run_e :run_e
#walk e #walk e
:_loop0 :loop_
#try e stop #try e stop
#_loop0 #loop_
' '
:stop :stop
#walk i #walk i
--- ---
'Using locals before any globals are defined 'Using locals before any globals are defined
:_loop1 :loopa
#take gems 1 _break #take gems 1 break
#_loop1 #loopa
:_break :break
--- ---
'Multiple sections with the same name 'Multiple sections with the same name
#end #end
:touch :touch
Trying to sell you some ammo... Trying to sell you some ammo...
#take gems 10 _skip #take gems 10 skip
#give ammo 10 #give ammo 10
:_skip :skip
#zap touch #zap touch
#end #end
:touch :touch
Trying to sell you some torches... Trying to sell you some torches...
#take gems 15 _skip0 #take gems 15 skip_
#give torches 5 #give torches 5
:_skip0 :skip_
#restore touch #restore touch
--- ---
'Multiple locals with the same name 'Multiple locals with the same name
#end #end
:repeat3 :repeat_
:_z :z
:_z :z
:_z :z
#take gems 1 _skip1 #take gems 1 skipa
#give score 1 #give score 1
:_skip1 :skipa
#zap _z #zap z
#_z #z
:_z :z
--- ---
'Addressing labels with section.local 'Addressing labels with section.local
:touch :touch
:_foo :foo
#end #end
:touch :touch
:_foo0 :foo_
#end #end
:shot :shot
'sends to the first one: 'sends to the first one:
#_foo #foo
--- ---
@alice @alice
:shared :shared
'this is the 1st section named "shared" 'this is the 1st section named "shared"
'but its local should correspond with @bob's 2nd "shared" 'but its local should correspond with @bob's 2nd "shared"
:_local :local
#end #end
:shared :shared
--- ---
@@ -79,4 +79,4 @@ Trying to sell you some torches...
:shared :shared
'this is the 2nd section named "shared" 'this is the 2nd section named "shared"
'but its local should correspond with @alice's 1st "shared" 'but its local should correspond with @alice's 1st "shared"
:_local :local
src/labels/snapshots/marzipan__labels__process__test__namespaces.snap
+7 -7
View File
@@ -22,13 +22,13 @@ expression: board_to_text(board)
'Make sure locals exist in separate namespaces 'Make sure locals exist in separate namespaces
:do_stuff :do_stuff
#lock #lock
:_loop :loop
' '
:_do_stuff :do_stuff_
:_loop0 :loop_
/i#if blocked rndne _loop0 /i#if blocked rndne loop_
' '
#take time 1 _break #take time 1 break
#_loop #loop
:_break :break
#unlock #unlock