From 2cde2874402ed1909238b78ea49cea43fc4b3a13 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sun, 27 Sep 2026 06:20:14 -0400 Subject: [PATCH] Hold every verdict write to a reviewed list of user actions FR-CULL-13 says evidence never writes a rating, flag, label or trash membership, and nothing enforced it. tools/traceability/src/verdicts.rs parses the shipped code with syn and enumerates every write: calls to the catalog setters and trash recorders, SQL that assigns those columns, sidecar Amendment::Judgement, and fields named rating/flag/label. Each site must be in ALLOWED with a reason, as Input (inside a Slint on_* closure, checked structurally), Relay (its callers are checked in turn), Carried (a verdict made elsewhere: sidecar and XMP pulls, sync merge, catalog mirrored to file, duplicates consolidation) or NotAVerdict. Unlisted sites and stale entries both fail `cargo test -p traceability`; `traces verdicts` prints the list. syn and proc-macro2 were already in the lockfile as proc-macro dependencies; this adds the edges, no new crate and no version change. --- Cargo.lock | 2 + Cargo.toml | 6 + tools/traceability/Cargo.toml | 2 + tools/traceability/src/lib.rs | 1 + tools/traceability/src/main.rs | 36 + tools/traceability/src/verdicts.rs | 1847 ++++++++++++++++++++++++++++ 6 files changed, 1894 insertions(+) create mode 100644 tools/traceability/src/verdicts.rs diff --git a/Cargo.lock b/Cargo.lock index 74a3d0b..5da0087 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7112,9 +7112,11 @@ name = "traceability" version = "0.18.1" dependencies = [ "anyhow", + "proc-macro2", "pulldown-cmark", "serde", "serde_json", + "syn 2.0.119", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 626b17c..fb29a6e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -134,6 +134,12 @@ serde_json = "1" # Slint's Markdown parser, so this adds a dependency edge and no crate; only # the HTML writer is needed, not the command-line front end. pulldown-cmark = { version = "0.13", default-features = false, features = ["html"] } +# The verdict-writer check (tools/traceability, FR-CULL-13) reads Rust as Rust: +# a text scan cannot tell a call from a comment, a test module from shipped +# code, or which callback closure a call sits in. Both already in the tree as +# every proc macro's parser; `span-locations` gives a problem its line. +syn = { version = "2", default-features = false, features = ["full", "parsing", "visit", "printing"] } +proc-macro2 = { version = "1", default-features = false, features = ["span-locations"] } base64 = "0.23" # Display-server clients, for FR-DSP-8's per-display profile acquisition. diff --git a/tools/traceability/Cargo.toml b/tools/traceability/Cargo.toml index b856ccd..a279323 100644 --- a/tools/traceability/Cargo.toml +++ b/tools/traceability/Cargo.toml @@ -19,3 +19,5 @@ anyhow.workspace = true serde = { workspace = true } serde_json.workspace = true pulldown-cmark.workspace = true +syn.workspace = true +proc-macro2.workspace = true diff --git a/tools/traceability/src/lib.rs b/tools/traceability/src/lib.rs index 155ad7a..1dfdd59 100644 --- a/tools/traceability/src/lib.rs +++ b/tools/traceability/src/lib.rs @@ -42,6 +42,7 @@ pub mod chord; pub mod gestures; pub mod keymap; pub mod manual; +pub mod verdicts; /// Requirement ID prefixes that participate in coverage. /// diff --git a/tools/traceability/src/main.rs b/tools/traceability/src/main.rs index 2bb99f7..ebf0ba9 100644 --- a/tools/traceability/src/main.rs +++ b/tools/traceability/src/main.rs @@ -8,6 +8,7 @@ //! traces gestures-check # gate: non-zero exit if either has drifted //! traces manual # write docs/manual/index.html from its README //! traces manual-check # gate: non-zero exit if the page has drifted +//! traces verdicts # list every verdict write; non-zero exit on an unlisted one //! ``` //! //! The gesture half scans a different tag out of the same files — see @@ -79,6 +80,9 @@ fn main() -> Result<()> { if mode.starts_with("manual") { return run_manual(&base, mode == "manual-check"); } + if mode == "verdicts" { + return run_verdicts(&base); + } let req_path = base.join("docs/dev/requirements.md"); let markdown = std::fs::read_to_string(&req_path) @@ -248,6 +252,38 @@ fn run_gestures(base: &Path, check: bool) -> Result<()> { Ok(()) } +/// List every verdict write with the reason it is allowed, and fail on any +/// that has none — the same check `cargo test -p traceability` runs, in a +/// form a reviewer can read ([`traceability::verdicts`]). +fn run_verdicts(base: &Path) -> Result<()> { + let files = verdicts::sources(base); + let report = verdicts::check(&files, verdicts::ALLOWED); + println!("files scanned {}", files.len()); + println!("writes found {}", report.sites.len()); + for s in &report.sites { + let kind = verdicts::ALLOWED + .iter() + .find(|a| a.file == s.file && a.within == s.within && a.writes == s.writes) + .map(|a| format!("{:?}", a.kind)) + .unwrap_or_else(|| "UNLISTED".into()); + println!( + " {kind:<11} {}:{} {} {} {}", + s.file, s.line, s.within, s.writes, s.detail + ); + } + if !report.problems.is_empty() { + for p in &report.problems { + println!(" {p}"); + } + bail!( + "{} verdict write problem(s) (FR-CULL-13)", + report.problems.len() + ); + } + println!("\nverdict gate: PASS"); + Ok(()) +} + /// Render the manual to its page, or check the committed page is that render. fn run_manual(base: &Path, check: bool) -> Result<()> { let source = std::fs::read_to_string(base.join(MANUAL_SOURCE)) diff --git a/tools/traceability/src/verdicts.rs b/tools/traceability/src/verdicts.rs new file mode 100644 index 0000000..5e76d4d --- /dev/null +++ b/tools/traceability/src/verdicts.rs @@ -0,0 +1,1847 @@ +//! TRACES: FR-CULL-13 | R7 +//! Every write of a verdict is a user's, and this is the list of them. +//! +//! # The rule +//! +//! FR-CULL-13: everything the app computes about a frame — clipping, focus, +//! burst membership, face and eye state, a duplicate's twin — is *evidence*, +//! and evidence never writes a rating, a flag, a colour label or trash +//! membership. The classifier does not hold the pen. R7 says the same thing +//! from the photographer's side. +//! +//! A rule like that is kept by nobody in particular: the well-meaning +//! shortcut ("reject the frames where every eye is shut") is one line in a +//! worker, and it compiles. So this module enumerates every place in the +//! shipped code that writes a verdict, and holds that enumeration to +//! [`ALLOWED`] — a list written by hand, one entry per call site, each with a +//! reason. A new writer anywhere fails the check until somebody adds it here +//! and says why, and the review of that line is the review the rule needs. +//! +//! # What counts as a write +//! +//! Scanned with `syn` over `core`, `ui`, `apps` and `platform`, test modules, +//! `tests/`, `examples/` and `benches/` left out: +//! +//! - a call to one of [`PRIMITIVES`] — `dr_catalog::rating`'s setters and the +//! trash recorders; +//! - SQL, in any string literal or macro, that assigns `versions.rating`, +//! `flag` or `label`, or `images.trashed_at` — whether by `UPDATE`, by an +//! upsert, or by an `INSERT` that gives them anything but `0` or `NULL` +//! (minting an unjudged row is the absence of a verdict, not one); +//! - building a sidecar `Amendment::Judgement`, which is how a verdict +//! reaches DarkRoom's own sidecar; +//! - assigning a field named `rating`, `flag` or `label` — the sidecar's +//! `Version` and the XMP record carry verdicts in fields of those names. +//! +//! # How an entry says what reaches it +//! +//! Each entry names the function (and the Slint callback closure, where +//! there is one, as `|on_name|`) that holds the write, and one of four +//! [`Kind`]s: +//! +//! - [`Kind::Input`] — the site sits inside a closure handed to an `on_*` +//! callback: a key, a click, a tap. Checked structurally: an `Input` entry +//! whose site is not inside such a closure is a failure, whatever its +//! reason says — so a timer or a worker cannot be listed as a press. +//! - [`Kind::Relay`] — the function writes on its caller's behalf. Its own +//! name becomes a writer, and *its* callers are enumerated and held to this +//! list in turn, until every chain ends in an `Input` or a `Carried`. +//! - [`Kind::Carried`] — the value was a verdict before it got here: a +//! sidecar or XMP written by the photographer on another device or in +//! another application, a catalog merge, the catalog's own state mirrored +//! out to a file, or consolidation, which moves verdicts the copies already +//! held onto the one that stays. Carried is the exemption, so its reason +//! has to say where the verdict was made. +//! - [`Kind::NotAVerdict`] — a field named `rating`, `flag` or `label` that +//! is not one: a filter's state, a grid cell's copy of the catalog, a clone +//! made to compare two edits. +//! +//! Which of `Relay` and `Carried` an entry is follows from where the value +//! comes from. A function that takes the verdict as an argument is a +//! `Relay` — the value is its caller's, so the caller is what has to be +//! justified. A function that reads the verdict out of a file, a remote +//! catalog or the copies being merged is `Carried`, and says so. +//! +//! Stale entries fail too: an entry nothing matches is a writer somebody +//! removed or renamed, and a list that only grows stops describing the code. +//! +//! # Limits +//! +//! Name-based, not type-based. Calls are matched by name and the module +//! they are reached through, following `use … as …` renames and `pub use` +//! re-exports; a method relay matches every `.name(` call, which is why the +//! peaking overlay's `set_colour` is listed as not a verdict. A struct +//! literal that fills a `rating` field is not seen (wgpu names every buffer +//! with a `label:` and the noise would bury the signal), and a writer reached +//! through a trait object is seen only where its name is spelled. A new +//! writer that is none of the shapes above should be added to [`PRIMITIVES`]. + +use std::collections::{BTreeMap, BTreeSet}; +use std::path::Path; + +use proc_macro2::{Delimiter, TokenStream, TokenTree}; +use syn::visit::{self, Visit}; + +/// Directories scanned: the shipped code. `tools` is left out for the reason +/// the gesture scan leaves it out — this module's own fixtures are writers. +pub const ROOTS: &[&str] = &["core", "ui", "apps", "platform"]; + +/// Path components that mark code which never ships. +const NOT_SHIPPED: &[&str] = &["tests", "examples", "benches", "target"]; + +/// Why a write is allowed. See the module documentation. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Kind { + /// Inside a Slint `on_*` callback: a key, a click, a tap. + Input, + /// Writes for its caller; the caller is checked in its place. + Relay, + /// A verdict made elsewhere, moved or mirrored, never made here. + Carried, + /// A field that shares a verdict's name and holds something else: a + /// filter, a display model's copy, a scratch clone. + NotAVerdict, +} + +/// One allowed write site. +#[derive(Debug, Clone, Copy)] +pub struct Allowed { + /// Repository-relative path of the file. + pub file: &'static str, + /// The enclosing function, then any `on_*` callback closure written as + /// `|on_name|`, joined with ` > `; a method is `Type::method`. + pub within: &'static str, + /// What it writes: a writer as `module::name`, `sql versions`, + /// `sql images`, `Amendment::Judgement` or `field .rating`. + pub writes: &'static str, + pub kind: Kind, + /// The justification a reviewer reads. Not checked, but required. + pub why: &'static str, +} + +/// A function whose call is a write. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub struct Writer { + /// The module a qualified call names it through — the file stem, or the + /// type for a method. + pub module: String, + pub name: String, + /// Where it is defined. A bare call matches only here or where the file + /// imports it. + pub file: String, + /// A method: matched on `.name(` anywhere, or `Self::name(` in its own + /// file. Anywhere, because a method's callers are in other files; so a + /// method is a `Relay` only when its name is its own. + pub method: bool, +} + +impl Writer { + fn display(&self) -> String { + format!("{}::{}", self.module, self.name) + } +} + +/// The primitive verdict writers: `(file, module, fn)`. +pub const PRIMITIVES: &[(&str, &str, &str)] = &[ + ("core/dr-catalog/src/rating.rs", "rating", "set_rating"), + ("core/dr-catalog/src/rating.rs", "rating", "set_flag"), + ("core/dr-catalog/src/rating.rs", "rating", "set_label"), + ("core/dr-catalog/src/rating.rs", "rating", "set_rating_many"), + ("core/dr-catalog/src/rating.rs", "rating", "set_flag_many"), + ("core/dr-catalog/src/rating.rs", "rating", "set_label_many"), + ("core/dr-catalog/src/trash.rs", "trash", "record_trashed"), + ( + "core/dr-catalog/src/trash.rs", + "trash", + "record_trashed_within", + ), + ("core/dr-catalog/src/trash.rs", "trash", "record_restored"), +]; + +/// The columns that hold a verdict, by table. +const VERDICT_COLUMNS: &[(&str, &[&str])] = &[ + ("versions", &["rating", "flag", "label"]), + ("images", &["trashed_at"]), +]; + +/// Field names that carry a verdict in the sidecar and XMP records. +const VERDICT_FIELDS: &[&str] = &["rating", "flag", "label"]; + +/// The sidecar amendment that carries a judgement, as `(type, variant)`. +const JUDGEMENT_AMENDMENT: (&str, &str) = ("Amendment", "Judgement"); + +/// One write found in the source. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub struct Site { + pub file: String, + pub line: usize, + pub within: String, + pub writes: String, + /// For SQL, the verdict columns it assigns. + pub detail: String, +} + +/// What a check found. +#[derive(Debug, Default)] +pub struct Report { + pub sites: Vec, + pub problems: Vec, +} + +/// Read the shipped sources under `base`, as `(relative path, text)`. +pub fn sources(base: &Path) -> Vec<(String, String)> { + crate::collect_sources(base, ROOTS) + .into_iter() + .filter(|p| p.extension().is_some_and(|e| e == "rs")) + .filter_map(|p| { + let rel = p + .strip_prefix(base) + .ok()? + .to_string_lossy() + .replace('\\', "/"); + if rel.split('/').any(|c| NOT_SHIPPED.contains(&c)) { + return None; + } + Some((rel, std::fs::read_to_string(&p).ok()?)) + }) + .collect() +} + +/// Enumerate every verdict write in `files` and hold it to `allowed`. +pub fn check(files: &[(String, String)], allowed: &[Allowed]) -> Report { + check_with(files, PRIMITIVES, allowed) +} + +/// [`check`] with the primitive writers named, for the fixtures. +pub fn check_with( + files: &[(String, String)], + primitives: &[(&str, &str, &str)], + allowed: &[Allowed], +) -> Report { + let mut report = Report::default(); + if files.is_empty() { + report + .problems + .push("no source files scanned — misconfigured, not zero writers".into()); + return report; + } + + let mut parsed = Vec::new(); + for (rel, text) in files { + match syn::parse_file(text) { + Ok(ast) => parsed.push((rel.as_str(), ast)), + // Loud: a file this cannot read is a file whose writers it + // cannot see. + Err(e) => report.problems.push(format!("{rel}: does not parse ({e})")), + } + } + + let mut writers: BTreeSet = BTreeSet::new(); + for (file, module, name) in primitives { + let defined = parsed + .iter() + .find(|(rel, _)| rel == file) + .is_some_and(|(_, ast)| defines_fn(ast, name)); + if !defined { + report.problems.push(format!( + "{file} no longer defines `{name}` — update PRIMITIVES, or every \ + caller of its replacement goes unchecked" + )); + } + writers.insert(Writer { + module: (*module).into(), + name: (*name).into(), + file: (*file).into(), + method: false, + }); + } + + let by_key: BTreeMap<(&str, &str, &str), &Allowed> = allowed + .iter() + .map(|a| ((a.file, a.within, a.writes), a)) + .collect(); + + // To a fixed point: every relay's callers are sites in turn. + let mut sites: BTreeSet = BTreeSet::new(); + loop { + let before = writers.len(); + sites.clear(); + for (rel, ast) in &parsed { + let mut scan = Scan::new(rel, ast, &writers); + scan.visit_file(ast); + sites.extend(scan.sites); + } + for site in &sites { + let key = ( + site.file.as_str(), + site.within.as_str(), + site.writes.as_str(), + ); + if let Some(a) = by_key.get(&key) { + if a.kind == Kind::Relay { + if let Some(w) = relay_writer(site) { + writers.insert(w); + } + } + } + } + // A `pub use` that re-exports a writer is the writer under the + // re-exporting module's name: `crate::library_ui::x` is + // `ratings_keywords::x` once `library_ui` says so. + for (rel, ast) in &parsed { + for item in &ast.items { + let syn::Item::Use(u) = item else { continue }; + if matches!(u.vis, syn::Visibility::Inherited) { + continue; + } + let mut uses = Uses::default(); + collect_use(&u.tree, &mut Vec::new(), &mut uses); + for (module, name, local) in uses.names { + let hit = writers + .iter() + .any(|w| !w.method && w.module == module && w.name == name); + if hit { + writers.insert(Writer { + module: module_of(rel), + name: local, + file: (*rel).to_string(), + method: false, + }); + } + } + } + } + if writers.len() == before { + break; + } + } + + let mut matched: BTreeSet<(&str, &str, &str)> = BTreeSet::new(); + for site in &sites { + let key = ( + site.file.as_str(), + site.within.as_str(), + site.writes.as_str(), + ); + match by_key.get(&key) { + None => report.problems.push(format!( + "{}:{}: `{}` writes a verdict ({}{}) and is not in verdicts::ALLOWED. \ + If a person's key, click or tap is what reaches it, add it with the \ + reason; if a signal is, it must not write (FR-CULL-13)", + site.file, + site.line, + site.within, + site.writes, + if site.detail.is_empty() { + String::new() + } else { + format!(": {}", site.detail) + }, + )), + Some(a) => { + matched.insert((a.file, a.within, a.writes)); + if a.kind == Kind::Input && !site.within.split(" > ").any(|s| s.starts_with("|on_")) + { + report.problems.push(format!( + "{}:{}: `{}` is allowed as Input but is not inside an `on_*` callback", + site.file, site.line, site.within + )); + } + if a.kind == Kind::Relay && relay_writer(site).is_none() { + report.problems.push(format!( + "{}:{}: `{}` is a Relay but not inside a named function", + site.file, site.line, site.within + )); + } + } + } + } + for a in allowed { + if !matched.contains(&(a.file, a.within, a.writes)) { + report.problems.push(format!( + "verdicts::ALLOWED names `{}` in {} writing {}, and no such write exists — \ + remove the entry", + a.within, a.file, a.writes + )); + } + } + + report.sites = sites.into_iter().collect(); + report +} + +/// The writer a relay site's enclosing function becomes. +fn relay_writer(site: &Site) -> Option { + let func = site + .within + .rsplit(" > ") + .find(|s| !s.starts_with('|') && !s.starts_with("const "))?; + let (module, name, method) = match func.rsplit_once("::") { + Some((ty, m)) => (ty.to_string(), m.to_string(), true), + None => (module_of(&site.file), func.to_string(), false), + }; + Some(Writer { + module, + name, + file: site.file.clone(), + method, + }) +} + +/// The module name a file is reached through. +fn module_of(file: &str) -> String { + let mut parts = file.rsplit('/'); + let stem = parts + .next() + .unwrap_or_default() + .trim_end_matches(".rs") + .to_string(); + if stem == "mod" || stem == "lib" || stem == "main" { + parts.next().unwrap_or_default().replace('-', "_") + } else { + stem + } +} + +fn defines_fn(ast: &syn::File, name: &str) -> bool { + ast.items + .iter() + .any(|i| matches!(i, syn::Item::Fn(f) if f.sig.ident == name)) +} + +/// Test code: `#[test]`, `#[cfg(test)]` and the like, but not +/// `#[cfg(not(test))]`. +fn is_test(attrs: &[syn::Attribute]) -> bool { + attrs.iter().any(|a| { + let path = a.path(); + if path.segments.last().is_some_and(|s| s.ident == "test") { + return true; + } + if path.is_ident("cfg") { + let text = a.meta.to_token_stream_string(); + return text.contains("test") && !text.contains("not"); + } + false + }) +} + +trait MetaText { + fn to_token_stream_string(&self) -> String; +} + +impl MetaText for syn::Meta { + fn to_token_stream_string(&self) -> String { + match self { + syn::Meta::List(l) => l.tokens.to_string(), + _ => String::new(), + } + } +} + +/// What a file's `use` declarations bring into scope, wherever they sit. +/// Scoping is ignored: an import inside one function counts for the file, +/// which can only make the check see more. +#[derive(Default)] +struct Uses { + /// `(module, name, local name)`: `use a::rating::set_flag as sf` is + /// `("rating", "set_flag", "sf")`. + names: BTreeSet<(String, String, String)>, + /// A module imported under another name: `use crate::duplicates as dups` + /// maps `dups` to `duplicates`. + renamed: BTreeMap, + /// Modules glob-imported. + globs: BTreeSet, +} + +impl<'ast> Visit<'ast> for Uses { + fn visit_item_use(&mut self, u: &'ast syn::ItemUse) { + let mut into = Uses::default(); + collect_use(&u.tree, &mut Vec::new(), &mut into); + self.names.extend(into.names); + self.renamed.extend(into.renamed); + self.globs.extend(into.globs); + } +} + +struct Scan<'a> { + file: &'a str, + writers: &'a BTreeSet, + imports: Uses, + ctx: Vec, + impl_ty: Vec, + sites: Vec, +} + +impl<'a> Scan<'a> { + fn new(file: &'a str, ast: &syn::File, writers: &'a BTreeSet) -> Self { + let mut imports = Uses::default(); + imports.visit_file(ast); + Self { + file, + writers, + imports, + ctx: Vec::new(), + impl_ty: Vec::new(), + sites: Vec::new(), + } + } + + fn within(&self) -> String { + self.ctx.join(" > ") + } + + fn push(&mut self, line: usize, writes: String, detail: String) { + self.sites.push(Site { + file: self.file.to_string(), + line, + within: self.within(), + writes, + detail, + }); + } + + /// The writer a path names, if any. `segments` are the path's idents. + fn path_writer(&self, segments: &[String]) -> Option<&'a Writer> { + let name = segments.last()?; + self.writers.iter().find(|w| { + // A bare name may be a writer imported under another name. + if segments.len() == 1 && &w.name != name { + return self.imports.names.contains(&( + w.module.clone(), + w.name.clone(), + name.clone(), + )); + } + if &w.name != name { + return false; + } + if segments.len() >= 2 { + let q = &segments[segments.len() - 2]; + let q = self.imports.renamed.get(q).unwrap_or(q); + return q == &w.module || (w.method && q == "Self" && w.file == self.file); + } + !w.method + && (w.file == self.file + || self.imports.globs.contains(&w.module) + || self.imports.names.contains(&( + w.module.clone(), + w.name.clone(), + name.clone(), + ))) + }) + } + + fn check_sql(&mut self, text: &str, line: usize) { + for (table, columns) in sql_writes(text) { + self.push(line, format!("sql {table}"), columns.join(", ")); + } + } + + /// Macro bodies are tokens, not expressions: look for SQL in their + /// literals and writer names followed by an argument list. + fn scan_tokens(&mut self, stream: TokenStream) { + let tokens: Vec = stream.into_iter().collect(); + for (i, t) in tokens.iter().enumerate() { + match t { + TokenTree::Group(g) => self.scan_tokens(g.stream()), + TokenTree::Literal(l) => { + if let Ok(s) = syn::parse_str::(&l.to_string()) { + self.check_sql(&s.value(), l.span().start().line); + } + } + TokenTree::Ident(id) => { + let called = matches!(tokens.get(i + 1), + Some(TokenTree::Group(g)) if g.delimiter() == Delimiter::Parenthesis); + if !called { + continue; + } + let mut segments = vec![id.to_string()]; + let mut j = i; + while j >= 3 { + match (&tokens[j - 1], &tokens[j - 2], &tokens[j - 3]) { + (TokenTree::Punct(a), TokenTree::Punct(b), TokenTree::Ident(q)) + if a.as_char() == ':' && b.as_char() == ':' => + { + segments.insert(0, q.to_string()); + j -= 3; + } + _ => break, + } + } + let method = j >= 1 + && matches!(&tokens[j - 1], TokenTree::Punct(p) if p.as_char() == '.'); + let hit = if method { + self.writers + .iter() + .find(|w| w.method && w.name == segments[0]) + } else { + self.path_writer(&segments) + }; + if let Some(w) = hit { + self.push(id.span().start().line, w.display(), String::new()); + } + } + TokenTree::Punct(_) => {} + } + } + } +} + +fn collect_use(tree: &syn::UseTree, prefix: &mut Vec, out: &mut Uses) { + match tree { + syn::UseTree::Path(p) => { + prefix.push(p.ident.to_string()); + collect_use(&p.tree, prefix, out); + prefix.pop(); + } + syn::UseTree::Name(n) => { + if let Some(m) = prefix.last() { + let name = n.ident.to_string(); + out.names.insert((m.clone(), name.clone(), name)); + } + } + syn::UseTree::Rename(r) => { + let (name, local) = (r.ident.to_string(), r.rename.to_string()); + // `use crate::duplicates::{self as dups}` renames the module the + // path ends in. + if name == "self" { + if let Some(m) = prefix.last() { + out.renamed.insert(local, m.clone()); + } + return; + } + if let Some(m) = prefix.last() { + out.names.insert((m.clone(), name.clone(), local.clone())); + } + out.renamed.insert(local, name); + } + syn::UseTree::Glob(_) => { + if let Some(m) = prefix.last() { + out.globs.insert(m.clone()); + } + } + syn::UseTree::Group(g) => { + for t in &g.items { + collect_use(t, prefix, out); + } + } + } +} + +impl<'ast> Visit<'ast> for Scan<'_> { + fn visit_item_mod(&mut self, m: &'ast syn::ItemMod) { + if !is_test(&m.attrs) { + visit::visit_item_mod(self, m); + } + } + + fn visit_item_fn(&mut self, f: &'ast syn::ItemFn) { + if is_test(&f.attrs) { + return; + } + self.ctx.push(f.sig.ident.to_string()); + visit::visit_item_fn(self, f); + self.ctx.pop(); + } + + fn visit_item_impl(&mut self, i: &'ast syn::ItemImpl) { + if is_test(&i.attrs) { + return; + } + let ty = match &*i.self_ty { + syn::Type::Path(p) => p + .path + .segments + .last() + .map(|s| s.ident.to_string()) + .unwrap_or_default(), + _ => String::new(), + }; + self.impl_ty.push(ty); + visit::visit_item_impl(self, i); + self.impl_ty.pop(); + } + + fn visit_impl_item_fn(&mut self, f: &'ast syn::ImplItemFn) { + if is_test(&f.attrs) { + return; + } + let ty = self.impl_ty.last().cloned().unwrap_or_default(); + self.ctx.push(format!("{ty}::{}", f.sig.ident)); + visit::visit_impl_item_fn(self, f); + self.ctx.pop(); + } + + fn visit_trait_item_fn(&mut self, f: &'ast syn::TraitItemFn) { + self.ctx.push(f.sig.ident.to_string()); + visit::visit_trait_item_fn(self, f); + self.ctx.pop(); + } + + fn visit_item_const(&mut self, c: &'ast syn::ItemConst) { + self.ctx.push(format!("const {}", c.ident)); + visit::visit_item_const(self, c); + self.ctx.pop(); + } + + fn visit_item_static(&mut self, s: &'ast syn::ItemStatic) { + self.ctx.push(format!("const {}", s.ident)); + visit::visit_item_static(self, s); + self.ctx.pop(); + } + + fn visit_expr_method_call(&mut self, m: &'ast syn::ExprMethodCall) { + let name = m.method.to_string(); + if let Some(w) = self.writers.iter().find(|w| w.method && w.name == name) { + self.push(m.method.span().start().line, w.display(), String::new()); + } + // A closure handed to `on_*` is a Slint callback: what runs inside it + // runs because a person did something. + if name.starts_with("on_") { + self.visit_expr(&m.receiver); + for arg in &m.args { + if matches!(arg, syn::Expr::Closure(_)) { + self.ctx.push(format!("|{name}|")); + self.visit_expr(arg); + self.ctx.pop(); + } else { + self.visit_expr(arg); + } + } + return; + } + visit::visit_expr_method_call(self, m); + } + + fn visit_expr_path(&mut self, p: &'ast syn::ExprPath) { + let segments: Vec = p + .path + .segments + .iter() + .map(|s| s.ident.to_string()) + .collect(); + if let Some(w) = self.path_writer(&segments) { + let line = p + .path + .segments + .last() + .map(|s| s.ident.span().start().line) + .unwrap_or(0); + self.push(line, w.display(), String::new()); + } + visit::visit_expr_path(self, p); + } + + fn visit_expr_struct(&mut self, s: &'ast syn::ExprStruct) { + let segs: Vec = s + .path + .segments + .iter() + .map(|s| s.ident.to_string()) + .collect(); + let (ty, variant) = JUDGEMENT_AMENDMENT; + if segs.len() >= 2 && segs[segs.len() - 2] == ty && segs[segs.len() - 1] == variant { + let line = s.path.segments[0].ident.span().start().line; + self.push(line, format!("{ty}::{variant}"), String::new()); + } + visit::visit_expr_struct(self, s); + } + + fn visit_expr_assign(&mut self, a: &'ast syn::ExprAssign) { + if let syn::Expr::Field(f) = &*a.left { + if let syn::Member::Named(n) = &f.member { + if VERDICT_FIELDS.iter().any(|v| n == v) { + self.push(n.span().start().line, format!("field .{n}"), String::new()); + } + } + } + visit::visit_expr_assign(self, a); + } + + fn visit_lit_str(&mut self, s: &'ast syn::LitStr) { + self.check_sql(&s.value(), s.span().start().line); + } + + fn visit_macro(&mut self, m: &'ast syn::Macro) { + self.scan_tokens(m.tokens.clone()); + } +} + +// ---- SQL ------------------------------------------------------------------ + +/// Split SQL into the tokens the statement reader needs: words (with +/// `{db}` and `?2` kept whole), `.`, `(`, `)`, `,`, `=`, `;`, and comparison +/// operators as one token so `>=` is never read as an assignment. +fn sql_tokens(sql: &str) -> Vec { + let mut out = Vec::new(); + let mut chars = sql.chars().peekable(); + // Line comments first: `--` to the end of the line says nothing. + while let Some(c) = chars.next() { + if c == '-' && chars.peek() == Some(&'-') { + for c in chars.by_ref() { + if c == '\n' { + break; + } + } + } else if c.is_alphanumeric() || "_{}?$:".contains(c) { + let mut w = c.to_lowercase().to_string(); + while let Some(&n) = chars.peek() { + if n.is_alphanumeric() || "_{}?$:".contains(n) { + w.extend(n.to_lowercase()); + chars.next(); + } else { + break; + } + } + out.push(w); + } else if "<>!".contains(c) { + let mut op = c.to_string(); + while let Some(&n) = chars.peek() { + if "<>!=".contains(n) { + op.push(n); + chars.next(); + } else { + break; + } + } + out.push(op); + } else if ".(),=;".contains(c) { + out.push(c.to_string()); + } else if c == '\'' { + // A quoted value is one opaque word. + for c in chars.by_ref() { + if c == '\'' { + break; + } + } + out.push("'".into()); + } + } + out +} + +fn is_word(t: &str) -> bool { + t.chars() + .next() + .is_some_and(|c| c.is_alphanumeric() || "_{?$:".contains(c)) +} + +fn watched(table: &str) -> Option<&'static [&'static str]> { + VERDICT_COLUMNS + .iter() + .find(|(t, _)| *t == table) + .map(|(_, c)| *c) +} + +/// The verdict columns each statement in `sql` assigns, by table. +pub fn sql_writes(sql: &str) -> Vec<(String, Vec)> { + let lower = sql.to_lowercase(); + if !(lower.contains("update") || lower.contains("insert") || lower.contains("replace")) { + return Vec::new(); + } + let toks = sql_tokens(sql); + let mut out: Vec<(String, Vec)> = Vec::new(); + let mut insert_table = String::new(); + let mut record = |table: &str, cols: Vec| { + if cols.is_empty() { + return; + } + match out.iter_mut().find(|(t, _)| t == table) { + Some((_, have)) => { + for c in cols { + if !have.contains(&c) { + have.push(c); + } + } + } + None => out.push((table.to_string(), cols)), + } + }; + + let mut i = 0; + while i < toks.len() { + let t = toks[i].as_str(); + if t == "into" { + // `INSERT [OR …] INTO [schema.]table (cols) VALUES (vals)`. + let mut j = i + 1; + let mut table = String::new(); + while j < toks.len() && (is_word(&toks[j]) || toks[j] == ".") { + if is_word(&toks[j]) { + table = toks[j].clone(); + } + j += 1; + if toks.get(j).map(String::as_str) != Some(".") { + break; + } + } + insert_table = table.clone(); + if let (Some(cols_of), Some("(")) = (watched(&table), toks.get(j).map(String::as_str)) { + let (cols, next) = list(&toks, j); + let vals = if toks.get(next).map(String::as_str) == Some("values") + && toks.get(next + 1).map(String::as_str) == Some("(") + { + list(&toks, next + 1).0 + } else { + Vec::new() + }; + let wrote: Vec = cols + .iter() + .enumerate() + .filter(|(_, c)| cols_of.contains(&c.as_str())) + .filter(|(k, _)| { + // Minting an unjudged row is not a verdict. + !matches!(vals.get(*k).map(String::as_str), Some("0") | Some("null")) + }) + .map(|(_, c)| c.clone()) + .collect(); + record(&table, wrote); + } else if let Some(cols_of) = watched(&table) { + // No column list: every column, verdicts included. + record(&table, cols_of.iter().map(|c| c.to_string()).collect()); + } + i = j; + continue; + } + if t == "update" { + let mut j = i + 1; + let mut table = String::new(); + while j < toks.len() && toks[j] != "set" && j < i + 8 { + if is_word(&toks[j]) + && !["or", "ignore", "replace", "abort", "fail", "rollback"] + .contains(&toks[j].as_str()) + { + table = toks[j].clone(); + } + j += 1; + } + if toks.get(j).map(String::as_str) != Some("set") { + i += 1; + continue; + } + if table.is_empty() { + // `ON CONFLICT … DO UPDATE SET`: the insert's own table. + table = insert_table.clone(); + } + let (cols, next) = set_clause(&toks, j + 1); + if let Some(cols_of) = watched(&table) { + record( + &table, + cols.into_iter() + .filter(|c| cols_of.contains(&c.as_str())) + .collect(), + ); + } + i = next; + continue; + } + i += 1; + } + out +} + +/// A parenthesised, comma-separated list starting at `open`: each element's +/// first token, and the index after the closing paren. +fn list(toks: &[String], open: usize) -> (Vec, usize) { + let mut out = Vec::new(); + let mut depth = 0; + let mut first = true; + let mut k = open; + while k < toks.len() { + match toks[k].as_str() { + "(" => { + depth += 1; + if depth == 1 { + first = true; + k += 1; + continue; + } + } + ")" => { + depth -= 1; + if depth == 0 { + return (out, k + 1); + } + } + "," if depth == 1 => { + first = true; + k += 1; + continue; + } + _ => {} + } + if first && depth == 1 { + out.push(toks[k].clone()); + first = false; + } else if depth == 1 && !first { + // A value longer than one token is never the literal 0. + if let Some(last) = out.last_mut() { + if !last.ends_with('…') { + last.push('…'); + } + } + } + k += 1; + } + (out, k) +} + +/// The columns a `SET` clause assigns, and the index where it ends. +fn set_clause(toks: &[String], start: usize) -> (Vec, usize) { + let mut cols = Vec::new(); + let mut depth = 0i32; + let mut case = 0i32; + let mut expect = true; + let mut k = start; + while k < toks.len() { + let t = toks[k].as_str(); + if depth == 0 + && case == 0 + && ["where", "from", "returning", ";", "on", "values"].contains(&t) + { + break; + } + match t { + "(" => depth += 1, + ")" => { + if depth == 0 { + break; + } + depth -= 1; + } + "case" => case += 1, + "end" => case -= 1, + "," if depth == 0 && case == 0 => { + expect = true; + k += 1; + continue; + } + _ => {} + } + if expect && depth == 0 && case == 0 && is_word(t) { + if toks.get(k + 1).map(String::as_str) == Some("=") { + cols.push(t.to_string()); + } + expect = false; + } + k += 1; + } + (cols, k) +} + +// ---- the list --------------------------------------------------------------- + +/// Every allowed verdict write in the shipped code. See the module +/// documentation for the kinds; each entry's `why` is the review. +/// +/// Grouped by where the verdict comes from: the catalog's own writers, then +/// a person's gestures, then the paths that carry a verdict made elsewhere, +/// then the fields that only share a name. +pub const ALLOWED: &[Allowed] = &[ + // ---- the catalog's writers: each takes the verdict as an argument ---- + allow( + Kind::Relay, + CATALOG_RATING, + "set_rating", + "sql versions", + "Writes the star it is handed; its callers are checked.", + ), + allow( + Kind::Relay, + CATALOG_RATING, + "set_flag", + "sql versions", + "Writes the flag it is handed; its callers are checked.", + ), + allow( + Kind::Relay, + CATALOG_RATING, + "set_label", + "sql versions", + "Writes the label it is handed; its callers are checked.", + ), + allow( + Kind::Relay, + CATALOG_RATING, + "set_rating_many", + "rating::set_rating", + "The one-transaction form of set_rating over a selection.", + ), + allow( + Kind::Relay, + CATALOG_RATING, + "set_flag_many", + "rating::set_flag", + "The one-transaction form of set_flag over a selection.", + ), + allow( + Kind::Relay, + CATALOG_RATING, + "set_label_many", + "rating::set_label", + "The one-transaction form of set_label over a selection.", + ), + allow( + Kind::Relay, + CATALOG_TRASH, + "record_trashed_within", + "sql images", + "Records the moves it is handed as trashed; its callers are checked.", + ), + allow( + Kind::Relay, + CATALOG_TRASH, + "record_trashed", + "trash::record_trashed_within", + "The own-transaction form of record_trashed_within.", + ), + allow( + Kind::Relay, + CATALOG_TRASH, + "record_restored", + "sql images", + "Takes the images it is handed out of the trash; its callers are checked.", + ), + // ---- the judgement dispatch: a person's key, click or tap ---- + allow( + Kind::Relay, + UI_JUDGE, + "apply_judgement", + "rating::set_rating_many", + "The judgement dispatch (R7): stars for the images its gesture names.", + ), + allow( + Kind::Relay, + UI_JUDGE, + "apply_judgement", + "rating::set_flag_many", + "The judgement dispatch (R7): pick or reject for the images its gesture names.", + ), + allow( + Kind::Relay, + UI_JUDGE, + "apply_label", + "rating::set_label_many", + "The label dispatch (R7): the colour its gesture names.", + ), + allow( + Kind::Input, + UI_JUDGE, + "wire_ratings_and_flags > |on_library_cell_rated|", + "ratings_keywords::apply_judgement", + "A star clicked or tapped on a grid cell, or develop's top-bar stars: that one photograph.", + ), + allow( + Kind::Input, + UI_JUDGE, + "wire_ratings_and_flags > |on_library_cell_flagged|", + "ratings_keywords::apply_judgement", + "Develop's Pick and Reject buttons and its P, X and U keys: the open photograph.", + ), + allow( + Kind::Input, + UI_JUDGE, + "wire_ratings_and_flags > |on_library_judged|", + "ratings_keywords::apply_judgement", + "A rating or flag key in the grid, or the selection bar's Flag: the selection, or \ + the frame under the pointer.", + ), + allow( + Kind::Input, + UI_JUDGE, + "wire_ratings_and_flags > |on_library_labelled|", + "ratings_keywords::apply_label", + "A label key in the grid (6 to 9 and the rest): the selection, toggled.", + ), + allow( + Kind::Input, + UI_JUDGE, + "wire_ratings_and_flags > |on_library_label_chosen|", + "ratings_keywords::apply_label", + "The selection bar's label picker: the selection, set outright.", + ), + allow( + Kind::Input, + UI_JUDGE, + "wire_ratings_and_flags > |on_library_cell_labelled|", + "ratings_keywords::apply_label", + "Develop's label keys and picker: the open photograph.", + ), + // ---- trash membership ---- + allow( + Kind::Relay, + CATALOG_DUPLICATES, + "consolidate", + "trash::record_trashed_within", + "Consolidation trashes the copies of a group in the transaction that merges \ + them; handed the copies, so its callers are checked.", + ), + allow( + Kind::Relay, + UI_TRASH, + "spawn_move", + "trash::record_trashed", + "The trash job records the moves that succeeded; its callers are checked.", + ), + allow( + Kind::Relay, + UI_TRASH, + "spawn_move", + "trash::record_restored", + "The restore job records the moves that succeeded; its callers are checked.", + ), + allow( + Kind::Relay, + UI_DUPLICATES, + "consolidate_group", + "duplicates::consolidate", + "Consolidates one reviewed group; its callers are checked.", + ), + allow( + Kind::Relay, + UI_COLL_TRASH, + "start_trash", + "trash::spawn_move", + "Starts the trash job for the images it is handed; its callers are checked.", + ), + allow( + Kind::Relay, + UI_COLL_TRASH, + "start_restore", + "trash::spawn_move", + "Starts the restore job for the images it is handed; its callers are checked.", + ), + allow( + Kind::Input, + UI_COLL_GRID, + "wire_drag > |on_library_drag_finished|", + "trash::start_trash", + "A drag of photographs dropped on the sidebar's Trash row.", + ), + allow( + Kind::Input, + UI_COLL_GRID, + "wire_trash > |on_trash_restore|", + "trash::start_restore", + "Restore, pressed in the trash view.", + ), + allow( + Kind::Input, + UI_COLL_GRID, + "wire_trash_from_grid > |on_library_cell_trashed|", + "trash::start_trash", + "The trash target on one cell's rating strip: that photograph.", + ), + allow( + Kind::Input, + UI_COLL_GRID, + "wire_trash_from_grid > |on_library_trash_selection|", + "trash::start_trash", + "Delete on the selection.", + ), + allow( + Kind::Relay, + UI_DUPLICATES, + "run_consolidate", + "duplicates::consolidate_group", + "The consolidation worker, over the plans it is handed; its callers are checked.", + ), + allow( + Kind::Relay, + UI_DUPLICATES, + "spawn_consolidate", + "duplicates::run_consolidate", + "Puts the consolidation worker on a thread; its callers are checked.", + ), + allow( + Kind::Relay, + UI_DUPLICATES_SCREEN, + "start_consolidate", + "duplicates::spawn_consolidate", + "Starts consolidating the reviewed groups; its callers are checked.", + ), + allow( + Kind::Input, + UI_DUPLICATES_SCREEN, + "wire > |on_confirm|", + "duplicates_ui::start_consolidate", + "Consolidate, pressed on the duplicates review after it has shown what each \ + group will keep and trash.", + ), + // ---- carried: a verdict made elsewhere ---- + allow( + Kind::Carried, + CATALOG_DUPLICATES, + "merge_within", + "sql versions", + "Consolidation: the survivor takes the highest rating any copy was given and a \ + flag or label only where the copies that carry one agree -- values read from \ + the copies inside the same transaction, never computed. A disagreement leaves \ + the survivor's own and is reported, so nothing is invented. Reached from the \ + Consolidate press, and its preview rolls back.", + ), + allow( + Kind::Carried, + SIDECAR, + "Version::merge", + "field .rating", + "Sync: two devices' ratings for one version, the higher revision winning; both \ + were given by the photographer, and a zero never erases a star.", + ), + allow( + Kind::Carried, + SIDECAR, + "Version::merge", + "field .flag", + "Sync: as the rating -- a flag set on another device.", + ), + allow( + Kind::Carried, + SIDECAR, + "Version::merge", + "field .label", + "Sync: as the rating -- a label set on another device.", + ), + allow( + Kind::Carried, + SIDECAR, + "Sidecar::parse", + "field .rating", + "Reading a sidecar file: the rating it records.", + ), + allow( + Kind::Carried, + SIDECAR, + "Sidecar::parse", + "field .flag", + "Reading a sidecar file: the flag it records.", + ), + allow( + Kind::Carried, + SIDECAR, + "Sidecar::parse", + "field .label", + "Reading a sidecar file: the label it records.", + ), + allow( + Kind::Relay, + XMP, + "Xmp::set_colour", + "field .label", + "Sets the label it is handed on an XMP record; its callers are checked.", + ), + allow( + Kind::Relay, + XMP, + "Xmp::set_values", + "field .rating", + "Sets a field from the values it is handed; its callers are checked.", + ), + allow( + Kind::Relay, + XMP, + "Xmp::set_values", + "field .label", + "Sets a field from the values it is handed; its callers are checked.", + ), + allow( + Kind::Carried, + UI_DUPLICATES, + "carry_edit", + "field .rating", + "Carrying a copy's edit onto the survivor keeps the survivor's own rating.", + ), + allow( + Kind::Carried, + UI_DUPLICATES, + "carry_edit", + "field .flag", + "Carrying a copy's edit onto the survivor keeps the survivor's own flag.", + ), + allow( + Kind::Carried, + UI_DUPLICATES, + "carry_edit", + "field .label", + "Carrying a copy's edit onto the survivor keeps the survivor's own label.", + ), + allow( + Kind::Relay, + UI_SCAN, + "apply_judgement", + "sql versions", + "Takes a sidecar's judgement into the catalog; handed the values, so its \ + callers are checked.", + ), + allow( + Kind::Carried, + UI_SIDECAR, + "amend", + "field .rating", + "Applies an Amendment::Judgement to the sidecar; every construction of one is \ + in this list.", + ), + allow( + Kind::Carried, + UI_SIDECAR, + "amend", + "field .flag", + "Applies an Amendment::Judgement to the sidecar; every construction of one is \ + in this list.", + ), + allow( + Kind::Carried, + UI_SIDECAR, + "amend", + "field .label", + "Applies an Amendment::Judgement to the sidecar; every construction of one is \ + in this list.", + ), + allow( + Kind::Carried, + UI_JUDGE, + "collect_sidecar_writes", + "Amendment::Judgement", + "Mirrors what the catalog holds out to the sidecar: rating, flag and label are \ + read from the versions row, after the write that put them there.", + ), + allow( + Kind::Carried, + UI_XMP, + "record_of", + "field .rating", + "Mirrors the catalog's rating and flag into the XMP record written beside \ + the original.", + ), + allow( + Kind::Relay, + UI_XMP, + "apply", + "sql versions", + "Writes the XMP record it is handed onto the catalog; its callers are checked.", + ), + allow( + Kind::Carried, + UI_XMP, + "carry_through", + "field .label", + "Keeps the label text the existing .xmp file holds when the catalog has none \ + of the five colours for it.", + ), + allow( + Kind::Carried, + XMP, + "reconcile", + "Xmp::set_values", + "Merges the catalog's record with the file's, field by field; each value it \ + sets is one side's, and a disagreement is recorded, not resolved.", + ), + allow( + Kind::Carried, + XMP_READ, + "parse", + "Xmp::set_values", + "Reading an .xmp file: the rating and label it records.", + ), + allow( + Kind::Carried, + UI_SCAN, + "pull_sidecars", + "scan::apply_judgement", + "The sidecar pull: the judgement a sidecar records, written there by the \ + photographer on another device.", + ), + allow( + Kind::Carried, + UI_XMP, + "record_of", + "Xmp::set_colour", + "Mirrors the catalog's label into the XMP record written beside the original.", + ), + allow( + Kind::Carried, + UI_XMP, + "take_in", + "xmp_sync::apply", + "The automatic XMP pull: another application's rating and label, the catalog \ + winning where both hold one and the disagreement kept for a person.", + ), + allow( + Kind::Carried, + UI_XMP, + "reload", + "xmp_sync::apply", + "The reload a person asks for in Settings: the file's values over the catalog's.", + ), + // ---- not a verdict ---- + allow( + Kind::NotAVerdict, + UI_DUPLICATES, + "strip", + "field .rating", + "Zeroes a clone so two copies' edits compare without their judgements; never \ + stored.", + ), + allow( + Kind::NotAVerdict, + UI_DUPLICATES, + "strip", + "field .flag", + "As the rating: a scratch clone.", + ), + allow( + Kind::NotAVerdict, + UI_DUPLICATES, + "strip", + "field .label", + "As the rating: a scratch clone.", + ), + allow( + Kind::NotAVerdict, + UI_DEVELOP, + "wire_peaking > |on_colour_picked|", + "Xmp::set_colour", + "Slint's Peaking.set_colour -- the focus-peaking overlay's colour, which only \ + shares the name.", + ), + allow( + Kind::NotAVerdict, + UI_DEVELOP, + "wire_peaking", + "Xmp::set_colour", + "Slint's Peaking.set_colour at start-up -- the overlay's colour.", + ), + allow( + Kind::NotAVerdict, + UI_JUDGE, + "sync_ratings", + "field .rating", + "The grid cell's copy of the catalog's stars, repainted after a write.", + ), + allow( + Kind::NotAVerdict, + UI_JUDGE, + "sync_ratings", + "field .flag", + "The grid cell's copy of the catalog's flag.", + ), + allow( + Kind::NotAVerdict, + UI_JUDGE, + "sync_ratings", + "field .label", + "The grid cell's copy of the catalog's label.", + ), + allow( + Kind::NotAVerdict, + UI_FILTER, + "wire_filter_ratings_and_people > |on_library_filter_unjudged_changed|", + "field .flag", + "The library filter's flag term.", + ), + allow( + Kind::NotAVerdict, + UI_FILTER, + "wire_filter_ratings_and_people > |on_library_filter_flag_changed|", + "field .flag", + "The library filter's flag term.", + ), + allow( + Kind::NotAVerdict, + UI_FILTER, + "wire_filter_ratings_and_people > |on_library_filter_label_changed|", + "field .label", + "The library filter's label term.", + ), +]; + +const CATALOG_RATING: &str = "core/dr-catalog/src/rating.rs"; +const CATALOG_TRASH: &str = "core/dr-catalog/src/trash.rs"; +const CATALOG_DUPLICATES: &str = "core/dr-catalog/src/duplicates.rs"; +const SIDECAR: &str = "core/dr-pipeline/src/sidecar.rs"; +const XMP: &str = "core/dr-xmp/src/lib.rs"; +const XMP_READ: &str = "core/dr-xmp/src/read.rs"; +const UI_DEVELOP: &str = "ui/dr-ui/src/develop_ui.rs"; +const UI_JUDGE: &str = "ui/dr-ui/src/library_ui/ratings_keywords.rs"; +const UI_FILTER: &str = "ui/dr-ui/src/library_ui/filter_bar.rs"; +const UI_DUPLICATES: &str = "ui/dr-ui/src/duplicates.rs"; +const UI_DUPLICATES_SCREEN: &str = "ui/dr-ui/src/duplicates_ui.rs"; +const UI_SCAN: &str = "ui/dr-ui/src/library/scan.rs"; +const UI_SIDECAR: &str = "ui/dr-ui/src/library/sidecar.rs"; +const UI_XMP: &str = "ui/dr-ui/src/xmp_sync.rs"; +const UI_TRASH: &str = "ui/dr-ui/src/trash.rs"; +const UI_COLL_TRASH: &str = "ui/dr-ui/src/collections_ui/trash.rs"; +const UI_COLL_GRID: &str = "ui/dr-ui/src/collections_ui/wiring_grid.rs"; + +const fn allow( + kind: Kind, + file: &'static str, + within: &'static str, + writes: &'static str, + why: &'static str, +) -> Allowed { + Allowed { + file, + within, + writes, + kind, + why, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn repo_root() -> std::path::PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("../..") + } + + // ---- fixtures ---------------------------------------------------------- + + const RATING: &str = "core/dr-catalog/src/rating.rs"; + const PRIMS: &[(&str, &str, &str)] = &[(RATING, "rating", "set_flag")]; + + /// A catalog with one writer, and a dispatch that calls it from a key. + fn tree(extra: &[(&str, &str)]) -> Vec<(String, String)> { + let mut files = vec![ + ( + RATING.to_string(), + r#" + pub fn set_flag(conn: &Connection, image: ImageId, flag: u8) { + conn.execute("UPDATE versions SET flag = ?2 WHERE id = ?1", (image, flag)); + } + #[cfg(test)] + mod tests { + fn helper() { super::set_flag(c, i, 2); } + } + "# + .to_string(), + ), + ( + "ui/judge.rs".to_string(), + r#" + fn apply_judgement(ids: &[ImageId], flag: u8) { + for id in ids { dr_catalog::rating::set_flag(conn, *id, flag); } + } + pub fn wire(window: &AppWindow) { + window.global::().on_library_judged(move |f| { + apply_judgement(&chosen, f as u8); + }); + } + "# + .to_string(), + ), + ]; + files.extend(extra.iter().map(|(p, t)| (p.to_string(), t.to_string()))); + files + } + + const LIST: &[Allowed] = &[ + allow( + Kind::Relay, + RATING, + "set_flag", + "sql versions", + "writes what it is handed", + ), + allow( + Kind::Relay, + "ui/judge.rs", + "apply_judgement", + "rating::set_flag", + "dispatch", + ), + allow( + Kind::Input, + "ui/judge.rs", + "wire > |on_library_judged|", + "judge::apply_judgement", + "a key", + ), + ]; + + fn problems(extra: &[(&str, &str)], list: &[Allowed]) -> Vec { + check_with(&tree(extra), PRIMS, list).problems + } + + #[test] + fn the_fixture_passes_as_listed() { + let report = check_with(&tree(&[]), PRIMS, LIST); + assert_eq!(report.problems, Vec::::new()); + // The test module's call is not a site. + assert_eq!(report.sites.len(), 3, "{:#?}", report.sites); + } + + /// The failure the clause exists for: a signal that writes. + #[test] + fn an_evidence_producer_that_writes_a_flag_fails() { + let eyes = r#" + pub fn record_eyes(conn: &Connection, image: ImageId, shut: bool) { + if shut { dr_catalog::rating::set_flag(conn, image, 2); } + } + "#; + let p = problems(&[("core/dr-face/src/eyes.rs", eyes)], LIST); + assert_eq!(p.len(), 1, "{p:#?}"); + assert!( + p[0].starts_with("core/dr-face/src/eyes.rs:3: `record_eyes`"), + "{}", + p[0] + ); + } + + /// Going round the catalog function does not go round the check. + #[test] + fn sql_that_writes_a_verdict_is_a_writer_wherever_it_is() { + let focus = r#" + fn peaking_pass(conn: &Connection, db: &str) { + conn.execute(&format!("UPDATE {db}.versions SET label = 1 WHERE id = ?1"), []); + } + "#; + let p = problems(&[("ui/peaking.rs", focus)], LIST); + assert_eq!(p.len(), 1, "{p:#?}"); + assert!( + p[0].contains("`peaking_pass`") && p[0].contains("label"), + "{}", + p[0] + ); + } + + /// A relay's callers are checked: the dispatch called from a burst pass + /// is as much a write as the setter called there. + #[test] + fn a_relay_called_from_anywhere_but_its_listed_callers_fails() { + let bursts = r#" + fn choose_representative(ids: &[ImageId]) { + super::judge::apply_judgement(&ids[1..], 2); + } + "#; + let p = problems(&[("ui/bursts.rs", bursts)], LIST); + assert_eq!(p.len(), 1, "{p:#?}"); + assert!(p[0].contains("`choose_representative`"), "{}", p[0]); + } + + #[test] + fn a_renamed_import_is_followed() { + let dup = r#" + use dr_catalog::rating::{self as r}; + use dr_catalog::rating::set_flag as reject; + fn by_module() { r::set_flag(c, i, 2); } + fn by_name() { reject(c, i); } + "#; + let p = problems(&[("ui/dups.rs", dup)], LIST); + assert_eq!(p.len(), 2, "{p:#?}"); + } + + #[test] + fn an_input_entry_must_sit_in_a_callback() { + let mut list = LIST.to_vec(); + list.push(allow( + Kind::Input, + "ui/auto.rs", + "on_timer_elapsed_but_not_a_closure", + "judge::apply_judgement", + "a timeout is not a press", + )); + let auto = r#" + fn on_timer_elapsed_but_not_a_closure() { apply_judgement(&ids, 2); } + use crate::judge::apply_judgement; + "#; + let p = problems(&[("ui/auto.rs", auto)], &list); + assert_eq!(p.len(), 1, "{p:#?}"); + assert!(p[0].contains("not inside an `on_*` callback"), "{}", p[0]); + } + + #[test] + fn a_stale_entry_fails() { + let mut list = LIST.to_vec(); + list.push(allow( + Kind::Input, + "ui/gone.rs", + "wire > |on_gone|", + "rating::set_flag", + ".", + )); + let p = problems(&[], &list); + assert_eq!(p.len(), 1, "{p:#?}"); + assert!(p[0].contains("no such write exists"), "{}", p[0]); + } + + #[test] + fn a_field_named_as_a_verdict_is_a_site() { + let sidecar = "fn stamp(v: &mut Version) { v.rating = 5; }"; + let p = problems(&[("core/dr-pipeline/src/x.rs", sidecar)], LIST); + assert_eq!(p.len(), 1, "{p:#?}"); + assert!(p[0].contains("field .rating"), "{}", p[0]); + } + + #[test] + fn the_sql_reader_finds_assignments_and_only_those() { + let w = |sql: &str| sql_writes(sql); + let cols = + |t: &str, c: &[&str]| vec![(t.to_string(), c.iter().map(|s| s.to_string()).collect())]; + assert_eq!( + w("UPDATE versions SET rating = ?2 WHERE id = ?1"), + cols("versions", &["rating"]) + ); + assert_eq!( + w( + "UPDATE versions SET rating = CASE WHEN ?2 > 0 THEN ?2 ELSE rating END, + flag = CASE WHEN flag = ?3 THEN 0 ELSE flag END WHERE id = ?1 AND rating >= 1" + ), + cols("versions", &["rating", "flag"]) + ); + assert_eq!( + w("update main.versions set label=1"), + cols("versions", &["label"]) + ); + // Minting an unjudged row is not a verdict; a judged one is. + assert!( + w("INSERT INTO versions(image_id, uuid, rating, flag) VALUES (?1, ?2, 0, 0)") + .is_empty() + ); + assert_eq!( + w("INSERT INTO versions(image_id, rating, flag) VALUES (?1, 0, ?2)"), + cols("versions", &["flag"]) + ); + assert_eq!( + w("INSERT INTO versions(image_id, label) SELECT id, 3 FROM images"), + cols("versions", &["label"]) + ); + assert_eq!( + w("INSERT INTO versions SELECT * FROM other.versions"), + cols("versions", &["rating", "flag", "label"]) + ); + assert_eq!( + w("INSERT INTO versions(id, uuid) VALUES (?1, ?2) + ON CONFLICT(uuid) DO UPDATE SET flag = excluded.flag"), + cols("versions", &["flag"]) + ); + assert_eq!( + w("UPDATE images SET trashed_at = ?3 WHERE id = ?1"), + cols("images", &["trashed_at"]) + ); + // Reads, other tables and other columns are not writes. + assert!(w("SELECT rating, flag FROM versions WHERE flag = 2").is_empty()); + assert!(w("UPDATE versions SET uuid = ?2 WHERE rating = 5").is_empty()); + assert!(w("UPDATE images SET source_ref = ?2 WHERE trashed_at = 1").is_empty()); + assert!(w("INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'lib')").is_empty()); + assert!(w("-- UPDATE versions SET flag = 2\nSELECT 1").is_empty()); + } + + /// The gate: the real tree, against the real list. + #[test] + fn no_verdict_is_written_but_by_a_person() { + let files = sources(&repo_root()); + assert!( + files.len() > 100, + "read {} files — the wrong tree", + files.len() + ); + let report = check(&files, ALLOWED); + assert!( + report.problems.is_empty(), + "FR-CULL-13:\n {}", + report.problems.join("\n ") + ); + } +}