Skip to content

Commit 1a55215

Browse files
committed
Add placeholder normalization and validation utilities
Implements methods for normalizing placeholders across translations and collecting placeholder issues, with strict and non-strict validation modes. Updates the roadmap to mark placeholder normalization and validation as complete, and adds tests for the new methods.
1 parent 8e315a6 commit 1a55215

2 files changed

Lines changed: 138 additions & 15 deletions

File tree

ROADMAP.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ Legend: [ ] todo, [x] done, [~] in progress
1717

1818
## M1. Quality & Safety
1919

20-
- [ ] Placeholder normalization and validation
21-
- [ ] Mapping between iOS (`%1$@`, `%d`) and Android (`%1$s`, `%d`)
22-
- [ ] Detect placeholder mismatches across languages; fail in strict mode, warn otherwise
23-
- [ ] Auto‑fix option for common cases (`%@``%s`, `%1$@``%1$s`)
24-
- [ ] Tests across `.strings`, Android, `.xcstrings`
20+
- [x] Placeholder normalization and validation
21+
- [x] Mapping between iOS (`%1$@`, `%@`, `%ld`) and Android (`%1$s`, `%s`, `%d/%u`)
22+
- [x] Detect placeholder mismatches across languages; strict vs non‑strict modes
23+
- [x] Auto‑fix option for common cases (`normalize_placeholders_in_place`)
24+
- [x] Tests across singular and plural entries; cross‑language normalization
2525
- [ ] Plural rules engine
2626
- [ ] CLDR‑driven required category sets per locale (few/many/etc.)
2727
- [ ] Validation pass: flag missing categories per key+locale

langcodec/src/codec.rs

Lines changed: 133 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -683,18 +683,93 @@ impl Codec {
683683
}
684684
}
685685

686-
if problems.is_empty() || !strict {
687-
if strict && !problems.is_empty() {
688-
// unreachable given condition, retained for clarity
686+
if problems.is_empty() {
687+
return Ok(());
688+
}
689+
if strict {
690+
return Err(Error::validation_error(format!(
691+
"Placeholder issues: {}",
692+
problems.join(" | ")
693+
)));
694+
}
695+
// Non-strict mode: treat as success
696+
Ok(())
697+
}
698+
699+
/// Collect placeholder issues without failing.
700+
/// Returns a list of human-readable messages; empty if none.
701+
pub fn collect_placeholder_issues(&self) -> Vec<String> {
702+
use std::collections::HashMap;
703+
use crate::placeholder::signature;
704+
use crate::types::Translation;
705+
706+
let mut map: HashMap<String, HashMap<String, Vec<Vec<String>>>> = HashMap::new();
707+
for res in &self.resources {
708+
for entry in &res.entries {
709+
let sigs: Vec<Vec<String>> = match &entry.value {
710+
Translation::Singular(v) => vec![signature(v)],
711+
Translation::Plural(p) => p
712+
.forms
713+
.iter()
714+
.map(|(_c, v)| signature(v))
715+
.collect(),
716+
};
717+
map.entry(entry.id.clone())
718+
.or_default()
719+
.entry(res.metadata.language.clone())
720+
.or_default()
721+
.push(sigs.into_iter().flatten().collect());
689722
}
690-
if problems.is_empty() {
691-
Ok(())
692-
} else {
693-
// Non-strict mode: return Ok but could be logged by caller. For now, include in error.
694-
Err(Error::validation_error(format!("Placeholder issues: {}", problems.join(" | "))))
723+
}
724+
725+
let mut problems = Vec::new();
726+
for (key, langs) in map {
727+
let mut per_lang_sig: HashMap<String, Vec<String>> = HashMap::new();
728+
for (lang, sig_lists) in langs {
729+
if let Some(first) = sig_lists.first() {
730+
if sig_lists.iter().any(|s| s != first) {
731+
problems.push(format!(
732+
"Key '{}' in '{}': inconsistent placeholders across forms: {:?}",
733+
key, lang, sig_lists
734+
));
735+
}
736+
per_lang_sig.insert(lang, first.clone());
737+
}
738+
}
739+
if let Some((base_lang, base_sig)) = per_lang_sig.iter().next() {
740+
for (lang, sig) in &per_lang_sig {
741+
if sig != base_sig {
742+
problems.push(format!(
743+
"Key '{}' mismatch: {} {:?} vs {} {:?}",
744+
key, base_lang, base_sig, lang, sig
745+
));
746+
}
747+
}
748+
}
749+
}
750+
problems
751+
}
752+
753+
/// Normalize placeholders in all entries (mutates in place).
754+
/// Converts iOS patterns like `%@`, `%1$@`, `%ld` to canonical forms (%s, %1$s, %d/%u).
755+
pub fn normalize_placeholders_in_place(&mut self) {
756+
use crate::placeholder::normalize_placeholders;
757+
use crate::types::Translation;
758+
for res in &mut self.resources {
759+
for entry in &mut res.entries {
760+
match &mut entry.value {
761+
Translation::Singular(v) => {
762+
let nv = normalize_placeholders(v);
763+
*v = nv;
764+
}
765+
Translation::Plural(p) => {
766+
for (_c, v) in p.forms.iter_mut() {
767+
let nv = normalize_placeholders(v);
768+
*v = nv;
769+
}
770+
}
771+
}
695772
}
696-
} else {
697-
Err(Error::validation_error(format!("Placeholder issues: {}", problems.join(" | "))))
698773
}
699774
}
700775

@@ -1716,4 +1791,52 @@ mod tests {
17161791
});
17171792
assert!(codec.validate_placeholders(true).is_err());
17181793
}
1794+
1795+
#[test]
1796+
fn test_collect_placeholder_issues_non_strict_ok() {
1797+
let mut codec = Codec::new();
1798+
codec.add_resource(Resource {
1799+
metadata: Metadata { language: "en".into(), domain: "d".into(), custom: HashMap::new() },
1800+
entries: vec![Entry {
1801+
id: "count".into(),
1802+
value: Translation::Singular("%d files".into()),
1803+
comment: None,
1804+
status: EntryStatus::Translated,
1805+
custom: HashMap::new(),
1806+
}],
1807+
});
1808+
codec.add_resource(Resource {
1809+
metadata: Metadata { language: "fr".into(), domain: "d".into(), custom: HashMap::new() },
1810+
entries: vec![Entry {
1811+
id: "count".into(),
1812+
value: Translation::Singular("%s fichiers".into()),
1813+
comment: None,
1814+
status: EntryStatus::Translated,
1815+
custom: HashMap::new(),
1816+
}],
1817+
});
1818+
// Non-strict should be Ok but issues present
1819+
assert!(codec.validate_placeholders(false).is_ok());
1820+
let issues = codec.collect_placeholder_issues();
1821+
assert!(!issues.is_empty());
1822+
}
1823+
1824+
#[test]
1825+
fn test_normalize_placeholders_in_place() {
1826+
let mut codec = Codec::new();
1827+
codec.add_resource(Resource {
1828+
metadata: Metadata { language: "en".into(), domain: "d".into(), custom: HashMap::new() },
1829+
entries: vec![Entry {
1830+
id: "g".into(),
1831+
value: Translation::Singular("Hello %@ and %1$@".into()),
1832+
comment: None,
1833+
status: EntryStatus::Translated,
1834+
custom: HashMap::new(),
1835+
}],
1836+
});
1837+
codec.normalize_placeholders_in_place();
1838+
let v = match &codec.resources[0].entries[0].value { Translation::Singular(v) => v.clone(), _ => String::new() };
1839+
assert!(v.contains("%s"));
1840+
assert!(v.contains("%1$s"));
1841+
}
17191842
}

0 commit comments

Comments
 (0)