diff --git a/crates/weksa/src/lower.rs b/crates/weksa/src/lower.rs index 619eb41..9f90239 100644 --- a/crates/weksa/src/lower.rs +++ b/crates/weksa/src/lower.rs @@ -161,7 +161,11 @@ pub enum LowerError { /// Phones were requested for an entity whose ruled IPA holds a symbol the /// language's phone inventory does not list (or the language has no /// inventory). The lowerer does not guess a phone. - UnknownPhone { entity: EntityId, at: usize, text: String }, + UnknownPhone { + entity: EntityId, + at: usize, + text: String, + }, /// Phones were requested for an entity whose ruled IPA has a stress mark /// with no vowel after it. DanglingStress { entity: EntityId, at: usize }, @@ -177,7 +181,24 @@ pub enum LowerError { impl fmt::Display for LowerError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{self:?}") + f.write_str(match self { + LowerError::Unrendered { .. } => "packet carries a slot the lowerer does not render", + LowerError::NoRow { .. } => "no table row for an inventory id the packet uses", + LowerError::GlossMismatch { .. } => "table row does not match the inventory's gloss", + LowerError::UnknownRole { .. } => "frame template names a role the inventory lacks", + LowerError::MissingRole { .. } => "predication fills no role its frame needs", + LowerError::UnknownEntity { .. } => "referent cites an entity the port does not hold", + LowerError::NoForm { .. } => "entity has no form in the target language", + LowerError::NoPronunciation { .. } => "entity has no ruled pronunciation", + LowerError::NoEntityReferent => "packet cites no entity", + LowerError::UnknownPhone { .. } => { + "ruled pronunciation holds a symbol with no listed phone" + } + LowerError::DanglingStress { .. } => "stress mark with no vowel after it", + LowerError::ConsentMissing { .. } => "request lacks the addressee's consent", + LowerError::AskerNotAddressee { .. } => "request agent is not the addressee", + LowerError::EntityRead { .. } => "entity port failed to read", + }) } } @@ -324,7 +345,10 @@ pub fn pronounce( input: &LoweringInput, entities: &dyn EntityForms, ) -> Result, LowerError> { - Ok(ruled(input, entities)?.into_iter().map(|(_, ipa)| ipa).collect()) + Ok(ruled(input, entities)? + .into_iter() + .map(|(_, ipa)| ipa) + .collect()) } /// One entity's ruled pronunciation as the phones line a sound consumer reads: @@ -347,15 +371,24 @@ pub fn phones( ruled(input, entities)? .into_iter() .map(|(entity, source)| { - let unknown = |at, text| LowerError::UnknownPhone { entity: entity.clone(), at, text }; + let unknown = |at, text| LowerError::UnknownPhone { + entity: entity.clone(), + at, + text, + }; let inventory = inventory.ok_or_else(|| unknown(0, source.clone()))?; let phones = inventory.segment(&source).map_err(|e| match e { SegmentError::UnknownPhone { at, text } => unknown(at, text), - SegmentError::DanglingStress { at } => { - LowerError::DanglingStress { entity: entity.clone(), at } - } + SegmentError::DanglingStress { at } => LowerError::DanglingStress { + entity: entity.clone(), + at, + }, })?; - Ok(PhoneLine { language: TARGET_LANGUAGE, source, phones }) + Ok(PhoneLine { + language: TARGET_LANGUAGE, + source, + phones, + }) }) .collect() } diff --git a/crates/weksa/src/main.rs b/crates/weksa/src/main.rs index e923303..d6832e7 100644 --- a/crates/weksa/src/main.rs +++ b/crates/weksa/src/main.rs @@ -325,6 +325,7 @@ fn parse_date(s: &str) -> std::result::Result { /// Parses the command line. A rejected value is reported by the flag that took /// it and the reason, never by the value: values are the caller's identifiers. +/// The same holds for the unknown flags and subcommands clap would quote. fn parse_cli() -> Cli { use std::error::Error; match Cli::try_parse() { @@ -345,10 +346,21 @@ fn parse_cli() -> Cli { eprintln!("error: invalid value for {arg}{reason}"); std::process::exit(2); } - Err(e) => e.exit(), + Err(e) => match e.kind() { + ErrorKind::UnknownArgument => refuse_cli("unknown argument"), + ErrorKind::InvalidSubcommand => refuse_cli("unknown subcommand"), + _ => e.exit(), + }, } } +/// A clap error that would quote the caller's token, as a fixed message that +/// names the kind only. Usage is `--help`'s to print. +fn refuse_cli(what: &str) -> ! { + eprintln!("error: {what}\n\nFor more information, try '--help'."); + std::process::exit(2); +} + fn need(value: Option, flag: &str) -> Result { value.with_context(|| format!("this source needs --{flag}")) } @@ -615,7 +627,12 @@ fn main() -> Result<()> { .map_err(refused)?; println!("admitted {key}"); } - Command::Lower { store, key, ipa, phones } => { + Command::Lower { + store, + key, + ipa, + phones, + } => { let inventory_store = InventoryStore::open(&cli.inventory)?; let inventory = inventory_store.inventory()?; let mut packets = PacketStore::open(&store)?; diff --git a/crates/weksa/src/validate.rs b/crates/weksa/src/validate.rs index e8aaf2f..6a0075f 100644 --- a/crates/weksa/src/validate.rs +++ b/crates/weksa/src/validate.rs @@ -36,15 +36,24 @@ use crate::packet::{ #[derive(Clone, Debug, PartialEq, Eq)] pub enum Refusal { /// V1: the packet's schema_version is not the one these types define. - V1SchemaVersion { found: u32 }, + V1SchemaVersion { + found: u32, + }, /// V2: no referents, predications or relations. V2PayloadRequired, /// V3: a node id repeats across referents and predications. - V3DuplicateNode { id: NodeId }, + V3DuplicateNode { + id: NodeId, + }, /// V4: a reference names no node. - V4DanglingRef { id: NodeId, at: &'static str }, + V4DanglingRef { + id: NodeId, + at: &'static str, + }, /// V5: an inventory id does not resolve at the packet's revision. - V5Unresolved { id: InvId }, + V5Unresolved { + id: InvId, + }, /// V6: an inventory id is of the wrong family for its slot. V6WrongFamily { id: InvId, @@ -52,21 +61,40 @@ pub enum Refusal { at: &'static str, }, /// V7: a feature or extension value does not fit its value kind. - V7ValueKind { what: String }, + V7ValueKind { + what: String, + }, /// V8: a feature is not declared for the slot it sits on. - V8NotApplicable { feature: InvId, slot: Slot }, + V8NotApplicable { + feature: InvId, + slot: Slot, + }, /// V10: the packet's inventory revision has no record. - V10UnknownRevision { rev: u32 }, + V10UnknownRevision { + rev: u32, + }, /// V11: a feature id repeats in one slot's list (`slot` is `None` for an /// extension id repeated in the packet). - V11DuplicateFeature { feature: InvId, slot: Option }, + V11DuplicateFeature { + feature: InvId, + slot: Option, + }, /// V11: a node id repeats in `participants`, `audience` or `preserve` /// (`at` names the list). - V11DuplicateNodeRef { id: NodeId, at: &'static str }, + V11DuplicateNodeRef { + id: NodeId, + at: &'static str, + }, /// V11: a role id repeats in one predication's roles. - V11DuplicateRole { role: InvId, predication: NodeId }, + V11DuplicateRole { + role: InvId, + predication: NodeId, + }, /// V12: more referents, predications and relations than `Packet::MAX_NODES`. - V12TooManyNodes { count: usize, max: usize }, + V12TooManyNodes { + count: usize, + max: usize, + }, /// V13: scene features were given for a packet with no addressee. They /// state the speaker's relation to the addressee, so with none they /// describe nobody. @@ -75,11 +103,15 @@ pub enum Refusal { /// names a node that is not a predication. It is what the addressee asked /// for or agreed to, an act, so a referent cannot be its object. V14ConsentWithoutAddressee, - V14ConsentNotAPredication { id: NodeId }, + V14ConsentNotAPredication { + id: NodeId, + }, /// V15: an addressee consent set by a source that only read wording. /// A decomposition of text, or imported corpus text, sees "you asked her /// to" as words; only authoring or game telemetry saw the request. - V15ConsentSource { source: ProvenanceSource }, + V15ConsentSource { + source: ProvenanceSource, + }, } impl Refusal { @@ -109,7 +141,28 @@ impl Refusal { impl fmt::Display for Refusal { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}: {self:?}", self.rule()) + let what = match self { + Refusal::V1SchemaVersion { .. } => "unsupported schema version", + Refusal::V2PayloadRequired => "packet has no referents, predications or relations", + Refusal::V3DuplicateNode { .. } => "a node id repeats", + Refusal::V4DanglingRef { .. } => "a reference names no node", + Refusal::V5Unresolved { .. } => { + "an inventory id does not resolve at the packet's revision" + } + Refusal::V6WrongFamily { .. } => "an inventory id is of the wrong family for its slot", + Refusal::V7ValueKind { .. } => "a value does not fit its value kind", + Refusal::V8NotApplicable { .. } => "a feature is not declared for its slot", + Refusal::V10UnknownRevision { .. } => "the inventory revision has no record", + Refusal::V11DuplicateFeature { .. } => "a feature id repeats in one list", + Refusal::V11DuplicateNodeRef { .. } => "a node id repeats in one list", + Refusal::V11DuplicateRole { .. } => "a role id repeats in one predication", + Refusal::V12TooManyNodes { .. } => "too many nodes", + Refusal::V13SceneWithoutAddressee => "scene features given without an addressee", + Refusal::V14ConsentWithoutAddressee => "consent given without an addressee", + Refusal::V14ConsentNotAPredication { .. } => "consent names no predication", + Refusal::V15ConsentSource { .. } => "consent set by a source that only read wording", + }; + write!(f, "{}: {what}", self.rule()) } } diff --git a/crates/weksa/tests/cli.rs b/crates/weksa/tests/cli.rs index e588c5e..18edb84 100644 --- a/crates/weksa/tests/cli.rs +++ b/crates/weksa/tests/cli.rs @@ -279,7 +279,13 @@ fn validate_reports_each_packet() { .unwrap(); let refused = validate(&short, &[]); assert!( - !refused.status.success() && out(&refused).contains("V10"), + !refused.status.success() + && out(&refused).contains("V10: the inventory revision has no record"), + "{}", + out(&refused) + ); + assert!( + !out(&refused).contains("V10UnknownRevision"), "{}", out(&refused) ); @@ -930,10 +936,13 @@ fn proof_packet_lowers_and_the_lowering_scores_against_items() { let args = ["lower", "./p.cc", "packet:6e6962752d303031", "--ipa"]; let unruled = weksa(d.path(), &args); assert!( - !unruled.status.success() && out(&unruled).contains("NoPronunciation"), + !unruled.status.success() && out(&unruled).contains("entity has no ruled pronunciation"), "{}", out(&unruled) ); + for leak in ["NoPronunciation", "entity:nibu", "entity {"] { + assert!(!out(&unruled).contains(leak), "{leak}: {}", out(&unruled)); + } let ruled = weksa( d.path(), &[ @@ -966,7 +975,10 @@ fn proof_packet_lowers_and_the_lowering_scores_against_items() { assert_eq!(String::from_utf8(ipa.stdout).unwrap(), "ˈnibu\n"); // The phones line is the committed fixture, byte for byte, and is a different // output from --ipa, so the two flags cannot be combined. - let phones = weksa(d.path(), &["lower", "./p.cc", "packet:6e6962752d303031", "--phones"]); + let phones = weksa( + d.path(), + &["lower", "./p.cc", "packet:6e6962752d303031", "--phones"], + ); assert!(phones.status.success(), "{}", out(&phones)); let fixture = std::fs::read_to_string( std::path::Path::new(env!("CARGO_MANIFEST_DIR")) @@ -974,7 +986,16 @@ fn proof_packet_lowers_and_the_lowering_scores_against_items() { ) .unwrap(); assert_eq!(String::from_utf8(phones.stdout).unwrap(), fixture); - let both = weksa(d.path(), &["lower", "./p.cc", "packet:6e6962752d303031", "--phones", "--ipa"]); + let both = weksa( + d.path(), + &[ + "lower", + "./p.cc", + "packet:6e6962752d303031", + "--phones", + "--ipa", + ], + ); assert!(!both.status.success(), "{}", out(&both)); let again = weksa(d.path(), &["lower", "./p.cc", "packet:6e6962752d303031"]); assert_eq!(String::from_utf8(again.stdout).unwrap(), text); @@ -1234,3 +1255,59 @@ fn admitted_on_must_be_a_real_calendar_date() { } assert!(!out(&weksa(d.path(), &["corpus", "dump"])).contains("item:")); } + +#[test] +fn clap_errors_never_echo_the_token() { + let d = tempfile::tempdir().unwrap(); + for args in [ + &["--sentinelflagzz"][..], + &["validate", "--sentinelflagzz"], + &["sentinelsubzz"], + &["corpus", "sentinelsubzz"], + &["--inventory"], + ] { + let o = weksa(d.path(), args); + let text = out(&o); + assert_eq!(o.status.code(), Some(2), "{args:?}: {text}"); + assert!(text.contains("error:"), "{args:?}: {text}"); + assert!( + !text.to_lowercase().contains("sentinel"), + "{args:?} echoes the token: {text}" + ); + } +} + +#[test] +fn pronunciation_language_is_never_echoed() { + let d = tempfile::tempdir().unwrap(); + let o = weksa( + d.path(), + &[ + "entity", + "put", + "entity:nibu", + "--form", + "en-US=NIBU", + "--origin", + "cast", + "--persona", + "nibu", + "--fixture", + "f.yaml", + "--pronunciation", + "sentinel-LANG=ˈnibu", + "--ruled-by", + "N", + "--ruled-on", + "2026-11-07", + "--ruling-source", + "t", + ], + ); + let text = out(&o); + assert!( + !o.status.success() && text.contains("names a language that has no --form"), + "{text}" + ); + assert!(!text.to_lowercase().contains("sentinel"), "{text}"); +} diff --git a/crates/weksa/tests/display.rs b/crates/weksa/tests/display.rs new file mode 100644 index 0000000..3c1ba63 --- /dev/null +++ b/crates/weksa/tests/display.rs @@ -0,0 +1,135 @@ +//! Error text is a fixed message naming the rule or variant. The caller's +//! identifiers (entity, node and inventory ids, IPA text, value strings) stay +//! on the typed value and never reach Display. + +use std::collections::HashSet; +use weksa::packet::{EntityId, NodeId, ProvenanceSource}; +use weksa::{Family, InvId, LowerError, Refusal, Slot}; + +const SENTINEL: &str = "sentinelzz"; + +fn inv() -> InvId { + InvId::parse("concept:4242").unwrap() +} +fn node() -> NodeId { + NodeId::parse("n4242").unwrap() +} +fn entity() -> EntityId { + EntityId::parse(&format!("entity:{SENTINEL}")).unwrap() +} + +fn assert_clean_and_distinct(all: Vec) { + let mut seen = HashSet::new(); + for e in all { + let text = e.to_string(); + assert!(!text.is_empty(), "{e:?}"); + for leak in [SENTINEL, "4242", "{", "}"] { + assert!(!text.contains(leak), "{e:?} leaks {leak:?}: {text}"); + } + assert!(seen.insert(text.clone()), "{e:?} repeats {text:?}"); + } +} + +#[test] +fn lower_error_display_names_the_variant_and_no_identifier() { + assert_clean_and_distinct(vec![ + LowerError::Unrendered { what: SENTINEL }, + LowerError::NoRow { id: inv() }, + LowerError::GlossMismatch { + id: inv(), + row: SENTINEL, + }, + LowerError::UnknownRole { + frame: inv(), + role: SENTINEL.into(), + }, + LowerError::MissingRole { + predication: node(), + role: SENTINEL.into(), + }, + LowerError::UnknownEntity { entity: entity() }, + LowerError::NoForm { entity: entity() }, + LowerError::NoPronunciation { entity: entity() }, + LowerError::NoEntityReferent, + LowerError::UnknownPhone { + entity: entity(), + at: 4242, + text: SENTINEL.into(), + }, + LowerError::DanglingStress { + entity: entity(), + at: 4242, + }, + LowerError::ConsentMissing { + predication: node(), + }, + LowerError::AskerNotAddressee { + predication: node(), + }, + LowerError::EntityRead { + entity: entity(), + why: SENTINEL.into(), + }, + ]); + assert_eq!( + LowerError::NoPronunciation { entity: entity() }.to_string(), + "entity has no ruled pronunciation" + ); +} + +#[test] +fn refusal_display_names_the_rule_and_no_identifier() { + let all = vec![ + Refusal::V1SchemaVersion { found: 4242 }, + Refusal::V2PayloadRequired, + Refusal::V3DuplicateNode { id: node() }, + Refusal::V4DanglingRef { + id: node(), + at: SENTINEL, + }, + Refusal::V5Unresolved { id: inv() }, + Refusal::V6WrongFamily { + id: inv(), + expected: Family::Role, + at: SENTINEL, + }, + Refusal::V7ValueKind { + what: SENTINEL.into(), + }, + Refusal::V8NotApplicable { + feature: inv(), + slot: Slot::Scene, + }, + Refusal::V10UnknownRevision { rev: 4242 }, + Refusal::V11DuplicateFeature { + feature: inv(), + slot: Some(Slot::Scene), + }, + Refusal::V11DuplicateNodeRef { + id: node(), + at: SENTINEL, + }, + Refusal::V11DuplicateRole { + role: inv(), + predication: node(), + }, + Refusal::V12TooManyNodes { + count: 4242, + max: 4242, + }, + Refusal::V13SceneWithoutAddressee, + Refusal::V14ConsentWithoutAddressee, + Refusal::V14ConsentNotAPredication { id: node() }, + Refusal::V15ConsentSource { + source: ProvenanceSource::ImportedCorpus, + }, + ]; + for r in &all { + assert!(r.to_string().starts_with(r.rule()), "{r:?}"); + } + assert_clean_and_distinct(all); + assert_eq!( + Refusal::V14ConsentNotAPredication { id: node() }.to_string(), + "V14: consent names no predication" + ); +}