From 04476753188e8e2aacc40e21bd4b7d08f16d30ab Mon Sep 17 00:00:00 2001 From: Michael Grier Date: Thu, 27 Aug 2026 12:12:44 -0400 Subject: [PATCH] fix(tools): exclude a preceding slash from the glued-doc-comment guard Cherry-picked from 2d848946 onto a branch that had already removed the end-of-line anchor by another route (d536a437, then b0a29eff dropped the `^.*` wrapper it left behind), so the two sides were parallel fixes of the same guard and each carried a piece the other lacked. The pattern that lands is `[^\s/]///`: HEAD's unanchored form, which is the cheap one -- `^.*` matches to end of line and then backtracks hunting for the marker, on every line of every file -- plus the cherry-pick's exclusion of a preceding slash. That exclusion is the whole substance of the pick here. `\S///` flags a `////` banner comment, because the banner's third slash is itself a non-space character; this repository happens to contain no such banner, so the false positive was latent rather than firing. Verified in both directions rather than only on the reject side: the glued line form (`let x = 1;/// The next thing`), the bare glued marker, and a sentence welded to a closing fence are each flagged with the right line number, while a `////` banner -- planted as the control -- leaves the run clean. All tracked files still pass. The commit's PLANS.md hunk is dropped. It predates this branch's root PLANS.md and would have replaced a thirteen-crate tracker list with a two-crate one and declared no checklists active over a table of eight; the link repair it was made for is already present here. It applied without conflict, which is why it had to be caught by eye rather than by git. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- DESIGN-NOTES.md | 15 +++++++++++---- tools/check-encoding.ps1 | 24 ++++++++++++++---------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/DESIGN-NOTES.md b/DESIGN-NOTES.md index 58e196b1d..185cb6f09 100644 --- a/DESIGN-NOTES.md +++ b/DESIGN-NOTES.md @@ -710,10 +710,11 @@ the damage was real and the stated consequence was not -- worth separating, beca what would have driven the wrong fix (a lint setting that does nothing). [tools/check-encoding.ps1](tools/check-encoding.ps1) now rejects `///` immediately following a non-space -character at end of line, in `.rs` files. The pattern was checked for false positives across the whole -repository before being adopted: there are none, because a legitimate doc comment is always preceded by -whitespace or begins its line. The script's own description was widened from "encoding" to **text hygiene**, -since neither this rule nor the control-character rule is an encoding fault; they live here because this is the +character, in `.rs` files. The pattern was checked for false positives across the whole repository before being +adopted: there are none, because a legitimate doc comment is always preceded by whitespace or begins its line. +A preceding slash is excluded so a `////` banner comment is not flagged. The script's own description was +widened from "encoding" to **text hygiene**, since neither this rule nor the control-character rule is an +encoding fault; they live here because this is the check CI already runs over every tracked file. The first version of this guard was a **no-op**: it gated on a `$ext` variable that was not in scope inside the @@ -722,6 +723,12 @@ that a guard has been written, observed to pass, and believed. The rule that cau guard you have only seen pass is untested.** Plant the defect it is meant to catch and watch it fail, then remove the defect and watch it pass. Both directions, every time. +The second version was anchored at end of line (`\S///\s*$`), so it caught only the *bare* glued marker -- the +rarer form. The typical splice glues a whole doc line onto the code (`let x = 1;/// The next thing`), and that +passed. This is a narrower rule than "test the guard": the guard *was* tested, against the exact instance that +motivated it, and the instance was not representative. **A guard written from one observed defect must be +checked against the general shape of the defect, not only against the instance in hand.** + ## Restoring a file with an old timestamp silently disables the rebuild Verifying a fix by planting the defect back requires *building* both states. Restoring the fixed file with diff --git a/tools/check-encoding.ps1 b/tools/check-encoding.ps1 index 45a68a721..e7cfbdd54 100644 --- a/tools/check-encoding.ps1 +++ b/tools/check-encoding.ps1 @@ -154,17 +154,21 @@ foreach ($file in $files) { # leaving its own `# Errors` section truncated or empty where the moved # text had come from. # - # `\S///` is the whole condition: a doc marker touching a non-space - # character. The surrounding `^.*` and `.*$` the pattern used to carry are - # not just redundant once the anchors are gone, they are the expensive - # part -- `^.*` matches to end of line and then backtracks looking for the - # marker, on every line of every file. Measured over 1.47 MB of this - # repository's own Rust, dropping them cut a no-match scan (the case CI - # runs on every clean build) from 872 ms to 288 ms across 20 passes. The - # match index still identifies the line, because the marker and the - # character it is glued to are on it. + # A doc marker touching a non-space character is the whole condition. The + # `^.*` and `.*$` the pattern used to carry are not merely redundant once + # the anchors are gone, they are the expensive part -- `^.*` matches to + # end of line and then backtracks looking for the marker, on every line of + # every file. Measured over 1.47 MB of this repository's own Rust, + # dropping them cut a no-match scan (the case CI runs on every clean + # build) from 872 ms to 288 ms across 20 passes. The match index still + # identifies the line, because the marker and the character it is glued to + # are on it. + # + # A preceding slash is excluded -- `[^\s/]` rather than `\S` -- so a + # `////` banner comment is not flagged. `\S///` matched every one of them, + # because the third slash of the banner is itself a non-space character. if ([System.IO.Path]::GetExtension($file) -eq '.rs') { - $glued = [regex]::Match($text, '\S///') + $glued = [regex]::Match($text, '[^\s/]///') if ($glued.Success) { $prefix = $text.Substring(0, $glued.Index) $line = ($prefix -split "`n").Count