test(ui): page modules never read the router; the compiler holds it

With the Edit text group, the text layer and link following told what
the press is to their page, only the page view reads the router.

- The router's methods and `DocView.router` are `pub(in crate::canvas)`:
  outside `canvas` the compiler refuses a read or a write, aliases
  included, which the guard's text match could miss. `ROUTER_READERS`
  keeps only the page view's dispatcher (`page_tool_input`).
- Rule (f): no function in a page module is named with "grabs" or
  "owns", which decide for themselves whether a press is theirs.
- CLAUDE.md's "Page presses" rule says so: modules never read the
  router; they are told.

Each rule seen failing against a violation planted in `#[cfg(any())]`
code (a `router.keeps_drag()` read in `content_ui`, a `fn fill_grabs` in
`fill_sign`), and a real router read from `content_ui` no longer
compiles.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
alehelandClaude Opus 5.5 committed 2026-11-04 19:37:46 +01:00
1 parent b3509dc629
commit 0378eee687
4 files changed
+38 -15

No files matched your search

+1 -1
View File
@@ -23,7 +23,7 @@ PdfCraft is a clean-room, open-source, Rust-native PDF application targeting Ado
- **Layering.** Nothing below L7 depends on egui/winit/eframe/rfd (`plan/architecture.md` §3). `cos`/`filters`/`crypt`/`arlington` stay standalone.
- **Page input.** Page modules read `PagePointer` (`crates/ui-egui/src/page_pointer.rs`), never egui's raw pointer state (`hover_pos`, `interact_pos`, `latest_pos`, `press_origin`, button reads); `page_pointer::tests::page_modules_read_the_pointer_only_through_page_pointer` enforces it.
- **Page cursor.** Page modules claim the cursor for what is under the pointer (`view.cursor.claim(Layer::…, icon)`, `crates/ui-egui/src/canvas/page_cursor.rs`), never set it (`set_cursor_icon`, `cursor_icon =`, `on_hover_cursor`); `document_area` sets it once, from the claim on the highest layer the active tool reaches. `page_pointer::tests::page_modules_claim_the_cursor_and_never_set_it` enforces it.
- **Page presses.** A page module decides what of its own is under a point in one hit test, which returns a `Hit` (owner, layer, cursor, tip; `crates/ui-egui/src/canvas/page_router.rs`) and gives its cursor, its tip and its press. It acts on a press only when the press is its own or, for the active tool, nobody's: the page view tells it which (`PagePress`: its own, nobody's, or another page's), and it never tests the press point again or asks another module whether it wants it. Escape cancels any press the pages hold (`cancel_on_escape`), and that press is ignored until the button is up. `page_pointer::tests::page_modules_act_on_a_press_only_through_the_router` enforces it.
- **Page presses.** A page module decides what of its own is under a point in one hit test, which returns a `Hit` (owner, layer, cursor, tip; `crates/ui-egui/src/canvas/page_router.rs`) and gives its cursor, its tip and its press. It acts on a press only when the press is its own or, for the active tool, nobody's: the page view tells it which (`PagePress`: its own, nobody's, or another page's), and it never tests the press point again or asks another module whether it wants it. Modules never read the router; they are told. Escape cancels any press the pages hold (`cancel_on_escape`), and that press is ignored until the button is up. `page_pointer::tests::page_modules_act_on_a_press_only_through_the_router` enforces it.
- **Fidelity.** The PDF object graph is the model. Preserve unknown data. Saves are incremental unless a full rewrite is required. Never silently drop or repair data without recording it.
- **Never panic.** Every PDF, script, MCP call, settings file and keystroke is untrusted input, and none of it may crash the app or lose the user's work. This outranks feature work. Return `Result` (the crate's error enum) and propagate with `?`, or fall back leniently and record it. In non-test code:
- No `unwrap`/`expect`/`panic!`/`unreachable!`/`todo!` unless it is provably infallible, with a comment saying why.
+3 -2
View File
@@ -650,8 +650,9 @@ pub struct DocView {
pub(crate) signature_drag: crate::signature_drag::SignatureDrag,
/// What the page modules claim for the cursor this frame; `document_area` sets it once.
pub(crate) cursor: crate::page_cursor::PageCursor,
/// Who the press the pages hold belongs to.
pub(crate) router: crate::page_router::PageRouter,
/// Who the press the pages hold belongs to. Only the page view reads it: it tells each page
/// module what the press is to its page (`PagePress`).
pub(in crate::canvas) router: crate::page_router::PageRouter,
/// What the last right-click on a page went to, and on which page: the page's menu shows its
/// entries, or the page's own when it went to nothing that has a menu.
pub(crate) menu_for: Option<(usize, crate::page_router::Owner)>,
@@ -525,9 +525,10 @@ fn hit_test_calls(line: &str) -> Vec<String> {
/// The functions that call a hit test: (file, function, why). Hit tests are found by name (a
/// function named `hit` or `…_hit`, called by path, bare or as a method). The comments' point test
/// has a list of its own (`COMMENT_POINT_TESTERS`); the others by other names (added content's,
/// Edit text's paragraphs', Redact's text test, the text layer's links) are not listed yet, and
/// most are PR 4b's. `PageHits::hits` is the one: the hits
/// has a list of its own (`COMMENT_POINT_TESTERS`). Point tests by other names are not seen: what is
/// left of them paints (a picture's hover outline) or keeps a behaviour no hit stands for (added
/// text's click with the tools that don't pick it up, crop mode's controls). `PageHits::hits` is the
/// one: the hits
/// at the press point decide its owner, and those under the pointer give its cursor, its tip, hover
/// painting and a right-click.
const HIT_TESTERS: [(&str, &str, &str); 1] = [(
@@ -571,14 +572,24 @@ const COMMENT_POINT_TESTERS: [(&str, &str, &str); 3] = [
const ROUTER_READS: [&str; 4] = ["router.press_for(", "router.keeps_drag(", "PageRouter::press_for(", "PageRouter::keeps_drag("];
/// The functions that read who holds the press: (file, function, why). The page view asks once per
/// page and tells each module what the press is to its page (`PagePress`); an unused entry fails
/// the test.
/// page and tells each module what the press is to its page (`PagePress`): modules never read the
/// router, and outside `canvas` the compiler refuses them (its methods are `pub(in crate::canvas)`).
/// An unused entry fails the test.
const ROUTER_READERS: [(&str, &str, &str); 1] = [(
"canvas/page_input.rs",
"page_tool_input",
"the page view: what the press is to each page (`press_for`), and whether its drag is the text's (`keeps_drag`)",
)];
/// Words a page module's functions are not named with: a function that grabs or owns a press decides
/// for itself whether the press is its own, which the page view tells it (`PagePress`).
const PRESS_CLAIMING_WORDS: [&str; 2] = ["grabs", "owns"];
/// The function `line` declares, when its name has one of [`PRESS_CLAIMING_WORDS`] in it.
fn press_claiming_fn(line: &str) -> Vec<String> {
declares(line.trim_start()).filter(|name| PRESS_CLAIMING_WORDS.iter().any(|w| name.contains(w))).map(str::to_owned).into_iter().collect()
}
/// What ignores the press the pages hold until the button is up.
const PRESS_IGNORES: [&str; 1] = ["press_cancelled = true"];
@@ -668,6 +679,13 @@ fn page_modules_act_on_a_press_only_through_the_router() {
place that must ask? Add the function to PAGE_HITS_ASKERS in src/canvas/page_pointer/tests.rs, with why.",
"no longer asks the page view's hits: remove it from PAGE_HITS_ASKERS.",
));
// A module is told what the press is to its page; none decides it for itself.
problems.extend(page_module_matches(press_claiming_fn).into_iter().map(|(file, _, n, name)| {
format!(
"src/{file}:{n}: `fn {name}` decides whether a press is its module's. The page view tells each module what the \
press is to its page (`PagePress`): act on that, in a function named for what it does."
)
}));
problems.extend(misuses(
&PRESS_IGNORES,
&PRESS_IGNORERS,
@@ -726,6 +744,10 @@ fn the_page_guards_read_hit_tests_split_calls_and_aliases() {
assert!(hit_test_calls("let all = self.hits(view, p, false);").is_empty());
assert_eq!(declares("pub(crate) const fn crop_hit<'a>(view: &DocView) {"), Some("crop_hit"));
assert_eq!(declares("let hit = 1;"), None);
assert_eq!(press_claiming_fn(" pub(crate) fn fill_grabs(view: &DocView) -> bool {"), ["fill_grabs"]);
assert_eq!(press_claiming_fn("fn owns_press() {"), ["owns_press"]);
assert!(press_claiming_fn("fn holds(&self, page: usize) -> bool {").is_empty());
assert!(press_claiming_fn("let grabs = 1;").is_empty());
let source = "impl PageHits<'_> {\n fn at(&self) {\n self\n .hits(view, p, false)\n }\n}\nfn after() {\n // view.router.owner(0)\n let _ = view\n .router\n .owner(0);\n}\n";
assert_eq!(
code_lines(source),
+7 -7
View File
@@ -182,7 +182,7 @@ pub(crate) struct PageRouter {
impl PageRouter {
/// A new press on the pages: the last one's owner goes, and until a page claims it the press is
/// between the pages.
pub(crate) fn begin(&mut self) {
pub(in crate::canvas) fn begin(&mut self) {
self.capture = None;
self.began = Some(None);
}
@@ -192,7 +192,7 @@ impl PageRouter {
/// by nobody (the page's text, or a tool that acts anywhere on the page).
/// Only a press just begun ([`Self::begin`]) is claimed, by the first page that does: pages
/// that touch (the gaps hidden) can share a row of pixels, and a press there is one page's.
pub(crate) fn press(&mut self, page: usize, hits: impl IntoIterator<Item = Hit>, tool: QuickTool) {
pub(in crate::canvas) fn press(&mut self, page: usize, hits: impl IntoIterator<Item = Hit>, tool: QuickTool) {
if self.began != Some(None) {
return;
}
@@ -201,7 +201,7 @@ impl PageRouter {
}
/// What `page` has to do with the press the pages hold.
pub(crate) fn press_for(&self, page: usize) -> PagePress {
pub(in crate::canvas) fn press_for(&self, page: usize) -> PagePress {
match (self.began, &self.capture) {
(None, _) => PagePress::Free,
(Some(Some(p)), Some(capture)) if p == page => PagePress::Mine(capture.owner),
@@ -212,21 +212,21 @@ impl PageRouter {
/// Whether the press is held by someone who keeps its drag: then the page's text is not
/// selected by it.
pub(crate) fn keeps_drag(&self) -> bool {
pub(in crate::canvas) fn keeps_drag(&self) -> bool {
self.capture.as_ref().is_some_and(|c| !c.declines_drag)
}
/// The press turned into a drag: an owner that declines its drag lets it go, and from then on
/// the press is nobody's (a drag across a highlight selects the text under it; with the Link
/// tool, a drag from a link draws a new one). Its click stayed the owner's.
pub(crate) fn drag_started(&mut self) {
pub(in crate::canvas) fn drag_started(&mut self) {
if self.capture.as_ref().is_some_and(|c| c.declines_drag) {
self.capture = None;
}
}
/// The press is over once the button is up and its release has been seen.
pub(crate) fn settle(&mut self, any_down: bool, edges: bool) {
pub(in crate::canvas) fn settle(&mut self, any_down: bool, edges: bool) {
if !any_down && !edges {
self.capture = None;
self.began = None;
@@ -235,7 +235,7 @@ impl PageRouter {
/// Drops the press (Escape, automation's cancel): every page is the active tool's again
/// ([`PagePress::Free`]). Page input skips a press Escape cancelled until the button is up.
pub(crate) fn cancel(&mut self) {
pub(in crate::canvas) fn cancel(&mut self) {
self.capture = None;
self.began = None;
}