From 2e923db6bd58590627d1c94e13ff3edaac9cb227 Mon Sep 17 00:00:00 2001 From: RumovZ Date: Fri, 28 May 2021 10:07:31 +0200 Subject: [PATCH] Add checks for parsed templates Combine existing check for unparsable templates with a check for unknown field names and a check for front sides without any field replacement. Updating the notetype's fields now mutates the parsed templates, so the checks can run on the final templates. --- ftl/core/card-templates.ftl | 1 + rslib/src/error/mod.rs | 6 ++- rslib/src/notetype/mod.rs | 99 ++++++++++++++++++++++++++----------- rslib/src/template.rs | 24 +++++---- 4 files changed, 91 insertions(+), 39 deletions(-) diff --git a/ftl/core/card-templates.ftl b/ftl/core/card-templates.ftl index fff284210..87652d405 100644 --- a/ftl/core/card-templates.ftl +++ b/ftl/core/card-templates.ftl @@ -22,6 +22,7 @@ card-templates-add-mobile-class = Add Mobile Class card-templates-preview-settings = Options card-templates-invalid-template-number = Card template { $number } in notetype '{ $notetype }' has a problem. card-templates-identical-front = The front side is identical to card template { $number }. +card-templates-no-front-field = Expected to find a field replacement on the front of the card template. card-templates-missing-cloze = Expected to find '{ "{{" }cloze:Text{ "}}" }' or similar on the front and back of the card template. card-templates-extraneous-cloze = 'cloze:' can only be used on cloze notetypes. card-templates-see-preview = See the preview for more information. diff --git a/rslib/src/error/mod.rs b/rslib/src/error/mod.rs index 3bb79c776..54af8e768 100644 --- a/rslib/src/error/mod.rs +++ b/rslib/src/error/mod.rs @@ -65,7 +65,9 @@ impl AnkiError { let header = tr.card_templates_invalid_template_number(err.ordinal + 1, &err.notetype); let details = match err.details { - TemplateSaveErrorDetails::TemplateError => tr.card_templates_see_preview(), + TemplateSaveErrorDetails::TemplateError + | TemplateSaveErrorDetails::NoSuchField => tr.card_templates_see_preview(), + TemplateSaveErrorDetails::NoFrontField => tr.card_templates_no_front_field(), TemplateSaveErrorDetails::Duplicate(i) => { tr.card_templates_identical_front(i + 1) } @@ -154,6 +156,8 @@ pub struct TemplateSaveError { pub enum TemplateSaveErrorDetails { TemplateError, Duplicate(usize), + NoFrontField, + NoSuchField, MissingCloze, ExtraneousCloze, } diff --git a/rslib/src/notetype/mod.rs b/rslib/src/notetype/mod.rs index 9d1898468..7dbb33845 100644 --- a/rslib/src/notetype/mod.rs +++ b/rslib/src/notetype/mod.rs @@ -290,6 +290,63 @@ impl Notetype { } } + /// Ensure no templates are None, every front template contains at least one + /// field, and all used field names belong to a field of this notetype. + fn ensure_valid_parsed_templates( + &self, + templates: &[(Option, Option)], + ) -> Result<()> { + if let Some((invalid_index, details)) = + templates.iter().enumerate().find_map(|(index, sides)| { + if let (Some(q), Some(a)) = sides { + let q_fields = q.fields(); + if q_fields.is_empty() { + Some((index, TemplateSaveErrorDetails::NoFrontField)) + } else if self.unknown_field_name(q_fields.union(&a.fields())) { + Some((index, TemplateSaveErrorDetails::NoSuchField)) + } else { + None + } + } else { + Some((index, TemplateSaveErrorDetails::TemplateError)) + } + }) + { + Err(AnkiError::TemplateSaveError(TemplateSaveError { + notetype: self.name.clone(), + ordinal: invalid_index, + details, + })) + } else { + Ok(()) + } + } + + /// True if any name in names does not belong to a field of this notetype. + fn unknown_field_name(&self, names: T) -> bool + where + T: IntoIterator, + I: AsRef, + { + let special_fields = [ + "FrontSide", + "Card", + "CardFlag", + "Deck", + "Subdeck", + "Tags", + "Type", + ]; + names.into_iter().any(|name| { + !special_fields.contains(&name.as_ref()) + && self + .fields + .iter() + .map(|field| &field.name) + .all(|field_name| field_name != name.as_ref()) + }) + } + fn ensure_cloze_if_and_only_if_cloze_notetype( &self, parsed_templates: &[(Option, Option)], @@ -354,37 +411,21 @@ impl Notetype { self.fix_template_names()?; self.ensure_names_unique(); self.reposition_sort_idx(); - self.ensure_template_fronts_unique()?; - let parsed_templates = self.parsed_templates(); - let invalid_card_idx = parsed_templates - .iter() - .enumerate() - .find_map(|(idx, (q, a))| { - if q.is_none() || a.is_none() { - Some(idx) - } else { - None - } - }); - if let Some(idx) = invalid_card_idx { - return Err(AnkiError::TemplateSaveError(TemplateSaveError { - notetype: self.name.clone(), - ordinal: idx, - details: TemplateSaveErrorDetails::TemplateError, - })); - } - self.ensure_cloze_if_and_only_if_cloze_notetype(&parsed_templates)?; + let mut parsed_templates = self.parsed_templates(); let reqs = self.updated_requirements(&parsed_templates); // handle renamed+deleted fields if let Some(existing) = existing { let fields = self.renamed_and_removed_fields(existing); if !fields.is_empty() { - self.update_templates_for_renamed_and_removed_fields(fields, parsed_templates); + self.update_templates_for_renamed_and_removed_fields(fields, &mut parsed_templates); } } self.config.reqs = reqs; + self.ensure_template_fronts_unique()?; + self.ensure_valid_parsed_templates(&parsed_templates)?; + self.ensure_cloze_if_and_only_if_cloze_notetype(&parsed_templates)?; Ok(()) } @@ -422,16 +463,16 @@ impl Notetype { fn update_templates_for_renamed_and_removed_fields( &mut self, fields: HashMap>, - parsed: Vec<(Option, Option)>, + parsed: &mut [(Option, Option)], ) { - for (idx, (q, a)) in parsed.into_iter().enumerate() { - if let Some(q) = q { - let updated = q.rename_and_remove_fields(&fields); - self.templates[idx].config.q_format = updated.template_to_string(); + for (idx, (q_opt, a_opt)) in parsed.iter_mut().enumerate() { + if let Some(q) = q_opt { + q.rename_and_remove_fields(&fields); + self.templates[idx].config.q_format = q.template_to_string(); } - if let Some(a) = a { - let updated = a.rename_and_remove_fields(&fields); - self.templates[idx].config.a_format = updated.template_to_string(); + if let Some(a) = a_opt { + a.rename_and_remove_fields(&fields); + self.templates[idx].config.a_format = a.template_to_string(); } } } diff --git a/rslib/src/template.rs b/rslib/src/template.rs index 338558139..f4fec2b24 100644 --- a/rslib/src/template.rs +++ b/rslib/src/template.rs @@ -655,12 +655,9 @@ impl ParsedTemplate { impl ParsedTemplate { /// Given a map of old to new field names, update references to the new names. /// Returns true if any changes made. - pub(crate) fn rename_and_remove_fields( - self, - fields: &HashMap>, - ) -> ParsedTemplate { - let out = rename_and_remove_fields(self.0, fields); - ParsedTemplate(out) + pub(crate) fn rename_and_remove_fields(&mut self, fields: &HashMap>) { + let old_nodes = std::mem::replace(&mut self.0, vec![]); + self.0 = rename_and_remove_fields(old_nodes, fields); } } @@ -765,25 +762,34 @@ fn nodes_to_string(buf: &mut String, nodes: &[ParsedNode]) { //---------------------------------------- impl ParsedTemplate { + /// A set of all field names. Field names may not be valid. + pub(crate) fn fields(&self) -> HashSet<&str> { + let mut set = HashSet::new(); + find_fields_with_filter(&self.0, &mut set, None); + set + } + /// A set of field names with a cloze filter attached. /// Field names may not be valid. pub(crate) fn cloze_fields(&self) -> HashSet<&str> { let mut set = HashSet::new(); - find_fields_with_filter(&self.0, &mut set, "cloze"); + find_fields_with_filter(&self.0, &mut set, Some("cloze")); set } } +/// Insert all fields in 'nodes' with 'filter' into 'fields'. If 'filter' is None, +/// all fields are collected. fn find_fields_with_filter<'a>( nodes: &'a [ParsedNode], fields: &mut HashSet<&'a str>, - filter: &str, + filter: Option<&str>, ) { for node in nodes { match node { ParsedNode::Text(_) => {} ParsedNode::Replacement { key, filters } => { - if filters.iter().any(|f| f == filter) { + if filter.is_none() || filters.iter().any(|f| f == filter.unwrap()) { fields.insert(key); } }