From 4d2b628b9ece60f9ccb0e3b9ec706f9028b93985 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 24 Oct 2026 15:34:57 +0200 Subject: [PATCH] refactor(scriptcheck): feature and outside ids as enums FeatureId and OutsideId replace the string ids the analyzer passed around. One table each gives the id, the need and the report order, so catalog::FEATURES and catalog::need() are gone. Feature.id and the engine JSON are unchanged; OUTSIDE is generated from the same table. Fixes #4013 Co-Authored-By: Claude Sonnet 5.5 --- crates/scriptcheck/src/analyze.rs | 118 ++++++++++++++-------------- crates/scriptcheck/src/catalog.rs | 125 +++++++++++++++--------------- crates/scriptcheck/src/lib.rs | 2 - crates/scriptcheck/src/report.rs | 60 ++++++++++---- crates/scriptcheck/src/tests.rs | 21 +++-- 5 files changed, 182 insertions(+), 144 deletions(-) diff --git a/crates/scriptcheck/src/analyze.rs b/crates/scriptcheck/src/analyze.rs index dffaae9bc..aa2119e43 100644 --- a/crates/scriptcheck/src/analyze.rs +++ b/crates/scriptcheck/src/analyze.rs @@ -23,10 +23,10 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::rc::Rc; -use crate::catalog; +use crate::catalog::{self, FeatureId}; use crate::directives; use crate::lex::{Lexer, T, Token, expr_after}; -use crate::report::{self, Acc}; +use crate::report::{self, Acc, OutsideId}; /// An `#include` line: what it names, its line, and the folders `#includepath` named at that point /// (one list, shared by every line it applies to). @@ -361,14 +361,14 @@ fn platform_name(n: &str) -> bool { } /// The feature of a window of type `value` (`"dialog"`, or a resource string that starts with it). -fn window_type(value: &str) -> &'static str { +fn window_type(value: &str) -> FeatureId { let word = value.trim_start().split(|c: char| c.is_whitespace() || c == '{').next().unwrap_or_default(); if word.eq_ignore_ascii_case("dialog") { - "scriptui.dialog" + FeatureId::ScriptuiDialog } else if word.eq_ignore_ascii_case("palette") { - "scriptui.palette" + FeatureId::ScriptuiPalette } else { - "scriptui.window" + FeatureId::ScriptuiWindow } } @@ -1101,13 +1101,13 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { self.acc.member(&c.path); } if c.path.strip_prefix("app.preferences").is_some_and(then) { - self.acc.feature("preferences", line); + self.acc.feature(FeatureId::Preferences, line); if c.change_call || ending.is_some_and(|t| t.is("=")) { - self.acc.outside("preferences", line, None); + self.acc.outside(OutsideId::Preferences, line, None); } } if ["app.name", "app.version"].iter().any(|p| c.path.strip_prefix(p).is_some_and(then)) { - self.acc.feature("hostCheck", line); + self.acc.feature(FeatureId::HostCheck, line); } } // A chain that an operand in a grouped value's parentheses starts with is complete: the @@ -1751,11 +1751,11 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { if t.is("(") { self.pending = Pending::WindowType(l); } else { - self.acc.feature("scriptui.window", l); + self.acc.feature(FeatureId::ScriptuiWindow, l); } } Pending::WindowType(l) => { - let feature = if t.kind == T::Str { window_type(&unquote(t.text, 64)) } else { "scriptui.window" }; + let feature = if t.kind == T::Str { window_type(&unquote(t.text, 64)) } else { FeatureId::ScriptuiWindow }; self.acc.feature(feature, l); } Pending::Menu(l) => { @@ -1776,7 +1776,7 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { T::Ident => self.name(t, line), T::Punct => self.punct(t, line), T::Str => self.string(t, line), - T::Xml => self.acc.feature("e4x", line), + T::Xml => self.acc.feature(FeatureId::E4x, line), _ => {} } } @@ -1790,15 +1790,15 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { } None => self.acc.menu_dynamic(), } - self.acc.feature("menuCommand", line); + self.acc.feature(FeatureId::MenuCommand, line); } fn open_mode(&mut self, mode: &str, line: Option) { if mode.eq_ignore_ascii_case("r") { - self.acc.feature("file.read", line); + self.acc.feature(FeatureId::FileRead, line); } else if let Some(m) = catalog::WRITE_MODES.iter().copied().find(|m| m.eq_ignore_ascii_case(mode)) { - self.acc.feature("file.write", line); - self.acc.outside("filesWritten", line, Some(m)); + self.acc.feature(FeatureId::FileWrite, line); + self.acc.outside(OutsideId::FilesWritten, line, Some(m)); } } @@ -1810,7 +1810,7 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { pending = std::mem::take(&mut self.pending); } match pending { - Pending::Window(l) | Pending::WindowType(l) => self.acc.feature("scriptui.window", l), + Pending::Window(l) | Pending::WindowType(l) => self.acc.feature(FeatureId::ScriptuiWindow, l), Pending::Menu(l) | Pending::MenuString(l, _) => self.menu(None, l), Pending::Open(_) | Pending::Name(..) | Pending::None => {} } @@ -1820,23 +1820,23 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { /// outside the document. fn platform(&mut self, n: &'s str, line: Option) { match n { - catalog::UNIT_VALUE => self.acc.feature("unitValue", line), - catalog::FILE | catalog::FOLDER => self.acc.feature("file.objects", line), + catalog::UNIT_VALUE => self.acc.feature(FeatureId::UnitValue, line), + catalog::FILE | catalog::FOLDER => self.acc.feature(FeatureId::FileObjects, line), catalog::BRIDGE_TALK => { - self.acc.feature("bridgeTalk", line); - self.acc.outside("otherApps", line, None); + self.acc.feature(FeatureId::BridgeTalk, line); + self.acc.outside(OutsideId::OtherApps, line, None); } catalog::SOCKET => { - self.acc.feature("socket", line); - self.acc.outside("network", line, None); + self.acc.feature(FeatureId::Socket, line); + self.acc.outside(OutsideId::Network, line, None); } catalog::EXTERNAL_OBJECT => { - self.acc.feature("externalObject", line); - self.acc.outside("nativeCode", line, None); + self.acc.feature(FeatureId::ExternalObject, line); + self.acc.outside(OutsideId::NativeCode, line, None); } catalog::WINDOW => self.pending = Pending::Window(line), - _ if catalog::XMP.contains(&n) => self.acc.feature("xmp", line), - _ if catalog::XML.contains(&n) => self.acc.feature("xml", line), + _ if catalog::XMP.contains(&n) => self.acc.feature(FeatureId::Xmp, line), + _ if catalog::XML.contains(&n) => self.acc.feature(FeatureId::Xml, line), _ => {} } } @@ -1850,26 +1850,26 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { let of = |name: &str| word(h1, name) && !member_mark(h2) && !own.contains(name); if of(catalog::DOLLAR) { if catalog::DOLLAR_ENV.contains(&n) { - self.acc.feature("dollar.env", line); - self.acc.outside("environment", line, None); + self.acc.feature(FeatureId::DollarEnv, line); + self.acc.outside(OutsideId::Environment, line, None); } else if n == catalog::DOLLAR_EVAL_FILE { - self.acc.feature("dollar.evalFile", line); - self.acc.outside("unseenCode", line, None); + self.acc.feature(FeatureId::DollarEvalFile, line); + self.acc.outside(OutsideId::UnseenCode, line, None); } else if n == catalog::LOCALIZE { - self.acc.feature("localize", line); + self.acc.feature(FeatureId::Localize, line); } else { - self.acc.feature("dollar", line); + self.acc.feature(FeatureId::Dollar, line); } } else if (of(catalog::FILE) && catalog::FILE_DIALOGS.contains(&n)) || (of(catalog::FOLDER) && catalog::FOLDER_DIALOGS.contains(&n)) { - self.acc.feature("file.dialog", line); + self.acc.feature(FeatureId::FileDialog, line); } else if of(catalog::FOLDER) && catalog::FOLDER_PLACES.contains(&n) { - self.acc.feature("file.places", line); + self.acc.feature(FeatureId::FilePlaces, line); } if n == catalog::ON_DRAW { - self.acc.feature("scriptui.drawing", line); + self.acc.feature(FeatureId::ScriptuiDrawing, line); } if n == catalog::REFLECT { - self.acc.feature("reflection", line); + self.acc.feature(FeatureId::Reflection, line); } return; } @@ -1923,8 +1923,8 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { } match n { // E4X: `default xml namespace = …` and `for each (…)`. - "namespace" if word(h0, "xml") && word(h1, "default") => self.acc.feature("e4x", line), - "each" if word(h0, "for") => self.acc.feature("e4x", line), + "namespace" if word(h0, "xml") && word(h1, "default") => self.acc.feature(FeatureId::E4x, line), + "each" if word(h0, "for") => self.acc.feature(FeatureId::E4x, line), _ => {} } } @@ -1937,11 +1937,11 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { "=" => { let operator = || h1.filter(|s| s.kind == T::Str).is_some_and(|s| catalog::OPERATORS.contains(&unquote(s.text, 4).as_str())); if is(h0, "]") && is(h2, "[") && word(h3, "prototype") && operator() { - self.acc.feature("operatorOverloading", line); + self.acc.feature(FeatureId::OperatorOverloading, line); } } // E4X: attributes (`x.@id`, `@*`), descendants (`x..item`) and qualified names (`ns::x`). - ".@" | ".." | "::" | "@" => self.acc.feature("e4x", line), + ".@" | ".." | "::" | "@" => self.acc.feature(FeatureId::E4x, line), _ => {} } } @@ -1961,30 +1961,30 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { } else if n == catalog::OPEN && file { self.pending = Pending::Open(line); } else if catalog::DRAWING.contains(&n) { - self.acc.feature("scriptui.drawing", line); + self.acc.feature(FeatureId::ScriptuiDrawing, line); } else if catalog::INSTANCE_DIALOGS.contains(&n) { - self.acc.feature("file.dialog", line); + self.acc.feature(FeatureId::FileDialog, line); } else if catalog::FOLDER_READS.contains(&n) || (file && catalog::FILE_READS.contains(&n)) { - self.acc.feature("file.read", line); + self.acc.feature(FeatureId::FileRead, line); } else if file && catalog::FILE_WRITES.contains(&n) { - self.acc.feature("file.write", line); - self.acc.outside("filesWritten", line, None); + self.acc.feature(FeatureId::FileWrite, line); + self.acc.outside(OutsideId::FilesWritten, line, None); } else if n == catalog::EXECUTE { - self.acc.feature("file.execute", line); - self.acc.outside("filesOpened", line, None); + self.acc.feature(FeatureId::FileExecute, line); + self.acc.outside(OutsideId::FilesOpened, line, None); } else if catalog::ACTIONS.contains(&n) { - self.acc.feature("actions", line); + self.acc.feature(FeatureId::Actions, line); } else if n == catalog::SELECT_TOOL { - self.acc.feature("selectTool", line); + self.acc.feature(FeatureId::SelectTool, line); } } else if !member_mark(h1) && !word(h1, "function") && !self.own_names.contains(n) { if catalog::NOTIFY.contains(&n) { - self.acc.feature("notify", line); + self.acc.feature(FeatureId::Notify, line); } else if n == catalog::LOCALIZE { - self.acc.feature("localize", line); + self.acc.feature(FeatureId::Localize, line); } else if matches!(n, "eval" | "Function") { - self.acc.feature("dynamicCode", line); - self.acc.outside("unseenCode", line, None); + self.acc.feature(FeatureId::DynamicCode, line); + self.acc.outside(OutsideId::UnseenCode, line, None); } } } @@ -1992,7 +1992,7 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { fn string(&mut self, t: &Sig<'s>, line: Option) { let [h0, h1, h2, _] = self.h; if t.text.contains(catalog::ACTION_TEXT) { - self.acc.feature("actions", line); + self.acc.feature(FeatureId::Actions, line); } let compares = h0.is_some_and(|s| s.kind == T::Punct && matches!(s.text, "==" | "===" | "!=" | "!==")); if compares && word(h1, "typename") && is(h2, ".") { @@ -2055,10 +2055,10 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { return; }; let feature = match name { - "target" => "directive.target", - "include" | "includepath" => "directive.include", - "targetengine" => "directive.targetengine", - _ => "directive.other", + "target" => FeatureId::DirectiveTarget, + "include" | "includepath" => FeatureId::DirectiveInclude, + "targetengine" => FeatureId::DirectiveTargetengine, + _ => FeatureId::DirectiveOther, }; let line = self.agg(t.line); self.acc.feature(feature, line); diff --git a/crates/scriptcheck/src/catalog.rs b/crates/scriptcheck/src/catalog.rs index 720e5c097..50ddee3a1 100644 --- a/crates/scriptcheck/src/catalog.rs +++ b/crates/scriptcheck/src/catalog.rs @@ -114,66 +114,69 @@ pub(crate) const CONSTRUCTORS: &[&str] = &[ "XMPDateTime", ]; -/// Every feature id a report can list, in the order the report lists them. -pub const FEATURES: &[&str] = &[ - "actions", - "bridgeTalk", - "directive.include", - "directive.other", - "directive.target", - "directive.targetengine", - "dollar", - "dollar.env", - "dollar.evalFile", - "dynamicCode", - "e4x", - "externalObject", - "file.dialog", - "file.execute", - "file.objects", - "file.places", - "file.read", - "file.write", - "hostCheck", - "localize", - "menuCommand", - "notify", - "operatorOverloading", - "preferences", - "reflection", - "scriptui.dialog", - "scriptui.drawing", - "scriptui.palette", - "scriptui.window", - "selectTool", - "socket", - "unitValue", - "xml", - "xmp", -]; - -/// The need feature `id` sets. -pub(crate) fn need(id: &str) -> Option { - Some(match id { - "directive.target" | "directive.include" | "directive.other" | "dollar" | "dynamicCode" | "notify" | "localize" | "unitValue" - | "preferences" | "hostCheck" => Need::Language, - "dollar.env" | "dollar.evalFile" | "file.objects" | "file.read" | "file.write" | "file.dialog" | "file.places" | "file.execute" => { - Need::Files +/// Declares [`FeatureId`] from its table: each platform feature a report can list, with its id and +/// the need it sets. The table is sorted by id, which is the order the report lists them in. +macro_rules! features { + ($($variant:ident $id:literal $need:ident,)*) => { + /// A platform feature a report can list ([`crate::Feature::id`] is its [`FeatureId::id`]). + #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] + pub(crate) enum FeatureId { + $($variant),* } - "menuCommand" => Need::MenuCommands, - "scriptui.dialog" | "scriptui.window" | "scriptui.drawing" => Need::Dialogs, - "directive.targetengine" - | "scriptui.palette" - | "bridgeTalk" - | "socket" - | "externalObject" - | "xmp" - | "xml" - | "e4x" - | "operatorOverloading" - | "reflection" - | "actions" - | "selectTool" => Need::HostOnly, - _ => return None, - }) + + impl FeatureId { + #[cfg(test)] + pub(crate) const ALL: &[FeatureId] = &[$(FeatureId::$variant),*]; + + pub(crate) fn id(self) -> &'static str { + match self { + $(FeatureId::$variant => $id),* + } + } + + /// The need the feature sets. + pub(crate) fn need(self) -> Need { + match self { + $(FeatureId::$variant => Need::$need),* + } + } + } + }; +} + +features! { + Actions "actions" HostOnly, + BridgeTalk "bridgeTalk" HostOnly, + DirectiveInclude "directive.include" Language, + DirectiveOther "directive.other" Language, + DirectiveTarget "directive.target" Language, + DirectiveTargetengine "directive.targetengine" HostOnly, + Dollar "dollar" Language, + DollarEnv "dollar.env" Files, + DollarEvalFile "dollar.evalFile" Files, + DynamicCode "dynamicCode" Language, + E4x "e4x" HostOnly, + ExternalObject "externalObject" HostOnly, + FileDialog "file.dialog" Files, + FileExecute "file.execute" Files, + FileObjects "file.objects" Files, + FilePlaces "file.places" Files, + FileRead "file.read" Files, + FileWrite "file.write" Files, + HostCheck "hostCheck" Language, + Localize "localize" Language, + MenuCommand "menuCommand" MenuCommands, + Notify "notify" Language, + OperatorOverloading "operatorOverloading" HostOnly, + Preferences "preferences" Language, + Reflection "reflection" HostOnly, + ScriptuiDialog "scriptui.dialog" Dialogs, + ScriptuiDrawing "scriptui.drawing" Dialogs, + ScriptuiPalette "scriptui.palette" HostOnly, + ScriptuiWindow "scriptui.window" Dialogs, + SelectTool "selectTool" HostOnly, + Socket "socket" HostOnly, + UnitValue "unitValue" Language, + Xml "xml" HostOnly, + Xmp "xmp" HostOnly, } diff --git a/crates/scriptcheck/src/lib.rs b/crates/scriptcheck/src/lib.rs index 818949f05..b9d9173fc 100644 --- a/crates/scriptcheck/src/lib.rs +++ b/crates/scriptcheck/src/lib.rs @@ -137,8 +137,6 @@ impl Need { } } -/// Every feature id a report can list, in the order the report lists them. -pub use catalog::FEATURES; /// Every id in [`Report::outside`], in the order the report lists them. pub use report::OUTSIDE; /// Every problem code a report can list. diff --git a/crates/scriptcheck/src/report.rs b/crates/scriptcheck/src/report.rs index e24e12114..975ef4118 100644 --- a/crates/scriptcheck/src/report.rs +++ b/crates/scriptcheck/src/report.rs @@ -3,7 +3,8 @@ use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; -use crate::{Kind, Limits, Need, catalog}; +use crate::catalog::FeatureId; +use crate::{Kind, Limits, Need}; pub(crate) const UNTERMINATED_STRING: &str = "unterminatedString"; pub(crate) const UNTERMINATED_COMMENT: &str = "unterminatedComment"; @@ -34,10 +35,40 @@ pub const PROBLEMS: &[&str] = &[ TOO_DEEP, ]; -/// Every id in [`Report::outside`], in the order the report lists them: files written, moved or -/// deleted; files opened in other apps; network connections; native code; messages to other apps; -/// environment variables; code the checker can't read; the reference app's preferences changed. -pub const OUTSIDE: &[&str] = &["filesWritten", "filesOpened", "network", "nativeCode", "otherApps", "environment", "unseenCode", "preferences"]; +/// Declares [`OutsideId`] and [`OUTSIDE`] from one table, in the order the report lists them. +macro_rules! outside { + ($($variant:ident $id:literal,)*) => { + /// Something the script does outside the document ([`Outside::id`] is its [`OutsideId::id`]). + #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] + pub(crate) enum OutsideId { + $($variant),* + } + + impl OutsideId { + pub(crate) fn id(self) -> &'static str { + match self { + $(OutsideId::$variant => $id),* + } + } + } + + /// Every id in [`Report::outside`], in the order the report lists them: files written, moved or + /// deleted; files opened in other apps; network connections; native code; messages to other apps; + /// environment variables; code the checker can't read; the reference app's preferences changed. + pub const OUTSIDE: &[&str] = &[$($id),*]; + }; +} + +outside! { + FilesWritten "filesWritten", + FilesOpened "filesOpened", + Network "network", + NativeCode "nativeCode", + OtherApps "otherApps", + Environment "environment", + UnseenCode "unseenCode", + Preferences "preferences", +} /// What [`crate::check`] found in a script. Lists of located things (targets, directives, /// includes, problems) give a line and, for an included file, its path (`file`); tallies (features, @@ -54,7 +85,7 @@ pub struct Report { pub targets: Vec, pub directives: Vec, pub includes: Vec, - /// The platform features used, by id ([`crate::FEATURES`]). + /// The platform features used, by id. pub features: Vec, /// The document-model member chains used, as written (`app.activeDocument.layers[].name`, with /// `[]` for an index and `()` for a call), a variable assigned from a chain replaced by that @@ -225,8 +256,8 @@ impl Tally { pub(crate) struct Acc<'l> { pub limits: &'l Limits, pub steps: usize, - features: BTreeMap<&'static str, Tally>, - outside: BTreeMap<&'static str, (Tally, Vec<&'static str>)>, + features: BTreeMap, + outside: BTreeMap)>, menu: BTreeMap, menu_dynamic: usize, members: BTreeMap, @@ -279,12 +310,12 @@ impl<'l> Acc<'l> { room } - pub fn feature(&mut self, id: &'static str, line: Option) { + pub fn feature(&mut self, id: FeatureId, line: Option) { let max = self.limits.lines; self.features.entry(id).or_default().add(line, max); } - pub fn outside(&mut self, id: &'static str, line: Option, mode: Option<&'static str>) { + pub fn outside(&mut self, id: OutsideId, line: Option, mode: Option<&'static str>) { let max = self.limits.lines; let (tally, modes) = self.outside.entry(id).or_default(); tally.add(line, max); @@ -385,11 +416,10 @@ impl<'l> Acc<'l> { let mut menu_strings: Vec = self.menu.into_iter().map(|(value, t)| MenuString { value, count: t.count, lines: t.lines }).collect(); menu_strings.sort_by(|a, b| b.count.cmp(&a.count).then_with(|| a.value.cmp(&b.value))); - let features: Vec = self.features.into_iter().map(|(id, t)| Feature { id, count: t.count, lines: t.lines }).collect(); - let mut outside: Vec = - self.outside.into_iter().map(|(id, (t, modes))| Outside { id, count: t.count, lines: t.lines, modes }).collect(); - outside.sort_by_key(|o| OUTSIDE.iter().position(|id| *id == o.id)); - let mut needs: BTreeSet = features.iter().filter_map(|f| catalog::need(f.id)).collect(); + let mut needs: BTreeSet = self.features.keys().map(|id| id.need()).collect(); + let features: Vec = self.features.into_iter().map(|(id, t)| Feature { id: id.id(), count: t.count, lines: t.lines }).collect(); + let outside: Vec = + self.outside.into_iter().map(|(id, (t, modes))| Outside { id: id.id(), count: t.count, lines: t.lines, modes }).collect(); if !members.is_empty() || !types.is_empty() { needs.insert(Need::Dom); } diff --git a/crates/scriptcheck/src/tests.rs b/crates/scriptcheck/src/tests.rs index 6934a9a9d..9f08b94ea 100644 --- a/crates/scriptcheck/src/tests.rs +++ b/crates/scriptcheck/src/tests.rs @@ -2,6 +2,7 @@ use proptest::prelude::*; +use crate::catalog::FeatureId; use crate::lex::{Lexer, T}; use crate::*; @@ -95,7 +96,7 @@ fn each_feature_is_found_where_it_is_and_nowhere_else() { assert_eq!(r.kind, Kind::Script, "{with}"); let f = feature(&r, id).unwrap_or_else(|| panic!("{id} not found in {with:?}: {:?}", r.features)); assert_eq!(f.lines, [1], "{with}"); - let need = catalog::need(id).unwrap_or_else(|| panic!("{id} sets no need")); + let need = FeatureId::ALL.iter().find(|f| f.id() == *id).map(|f| f.need()).unwrap_or_else(|| panic!("{id} sets no need")); assert!(r.needs.contains(&need), "{with}: {:?}", r.needs); let r = report(without); assert!(feature(&r, id).is_none(), "{id} found in {without:?}"); @@ -103,14 +104,20 @@ fn each_feature_is_found_where_it_is_and_nowhere_else() { } #[test] -fn every_feature_id_has_a_need_and_a_case() { - assert!(FEATURES.windows(2).all(|w| w[0] < w[1]), "FEATURES is sorted, as the report lists them"); - for id in FEATURES { - assert!(catalog::need(id).is_some(), "{id}"); - assert!(FEATURE_CASES.iter().any(|(case, ..)| case == id), "{id} has no case"); +fn features_are_listed_in_alphabetical_order_of_their_ids() { + let ids: Vec<&str> = FeatureId::ALL.iter().map(|f| f.id()).collect(); + assert!(ids.windows(2).all(|w| w[0] < w[1]), "{ids:?}"); + assert!(FeatureId::ALL.windows(2).all(|w| w[0] < w[1]), "the enum is declared in the order of its ids"); +} + +#[test] +fn every_feature_id_has_a_case() { + for f in FeatureId::ALL { + let id = f.id(); + assert!(FEATURE_CASES.iter().any(|(case, ..)| *case == id), "{id} has no case"); } for (id, ..) in FEATURE_CASES { - assert!(FEATURES.contains(id), "{id} isn't in FEATURES"); + assert!(FeatureId::ALL.iter().any(|f| f.id() == *id), "{id} isn't a FeatureId"); } let unique = |ids: &[&str]| ids.iter().collect::>().len() == ids.len(); assert!(unique(PROBLEMS) && unique(OUTSIDE));