Coverage presentation nits - #452
Open
System625 wants to merge 3 commits into
Open
Conversation
The report listed 10 judgments that are test fixtures rather than part of the model: `is_zero` and friends from `formality-core/tests/coverage.rs`, the scraper's own inline fixtures, and `move_place` from `formality-mdbook`'s `#[cfg(test)] mod`. Skip a source file when it lives under a `tests/` directory or carries an inner `#![cfg(test)]`, and skip `judgment_fn!` invocations that sit inside an inline `#[cfg(test)] mod`. The mdbook preprocessor builds its own index rather than calling `scrape_dir`, so it applies the same file-level filter.
The judgment summary page opened with a one-line signature rebuilt from the
scraped parameters, which collapsed the parameter list onto one line and
dropped the doc comment (`borrow_check_block(env: TypeckEnv, assumptions:
Wcs, ...,) => FlowState`). Replace it with an extract of the real source,
dedented, with the rules elided:
judgment_fn! {
/// Prove that any loans issued in this basic block are respected.
fn borrow_check_block(
env: TypeckEnv,
...
) => FlowState {
/* rules omitted */
}
}
followed by a link to the judgment on GitHub. Comments and visibility
inside the macro come along, since the text is sliced from the source
rather than rebuilt.
That made a pre-existing scraper bug glaring, so it is fixed here too: the
scraper read doc comments as code, so `judgment_fn!`'s own doc example was
reported as a judgment named `fmt`, and `prove_outlives` was reported as
`main` (the `fn main()` in its doc example matched the signature regex
first) which meant its recorded coverage matched no page at all. Line
comments are now masked out before scanning; offsets are preserved so
matches still slice out of the original source. The report drops `fmt` and
gains `prove_outlives` under its real name.
Nothing on the report said what the green and orange numbers, the ✗, or the N/A meant. Add one standing page that explains the index table, the two kinds of coverage, the remaining cell states, and where a number leads, and link it from the footer of every generated page: the index, each judgment page, each per-cell detail page, and both by-test pages. In the book it becomes a chapter of its own, just before the coverage index; the CLI writes it next to the other markdown.
Collaborator
|
Thanks for contributing to formality! :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Picks up three of the presentation items from the office-hours notes. One commit each.
1. Judgments defined by tests are gone
10 of the 87 judgments in the report were test fixtures:
is_zero,is_oneandall_evenfromformality-core/tests/coverage.rs,sum_allandtransitive_reachablefrom the#![cfg(test)]modules underformality-core/src/judgment/, the scraper's own inline fixtures (prove_thing,only_one,mixed), andmove_placefromformality-mdbook's#[cfg(test)] mod.A file is now skipped when it lives under a
tests/directory or carries an inner#![cfg(test)], and ajudgment_fn!inside an inline#[cfg(test)] modis skipped on its own. The mdbook preprocessor builds its own index rather than callingscrape_dir, so it applies the same file-level filter.2. A judgment's page opens with its real source
The page used to open with a signature rebuilt from the scraped parameters, which collapsed them onto one line and left a stray comma:
It now shows the real source, dedented, with the rules elided, followed by a link to the judgment on GitHub:
Because the text is sliced out of the source rather than rebuilt, doc comments and visibility come along, which is what you asked for.
A scraper bug fixed in the same commit. The scraper read doc comments as code, which this change made impossible to ignore:
judgment_fn!'s own doc example was reported as a judgment namedfmt, andprove_outliveswas reported asmain, because thefn main() {in its doc example matched the signature regex before the realpub fn prove_outlives. That second one meantprove_outlives's recorded coverage matched no page at all. Line comments are now masked out before scanning (offsets preserved, so matches still slice out of the real source). The report dropsfmtand gainsprove_outlivesunder its real name.the prove_outlives page, which did not exist before
This is the first of the two scraper gaps I described a thread ago; the
overlap_checkone (a hand-builtProofTree, not ajudgment_fn!at all) is untouched here.3. "How to read this page"
One standing page covering the index table, positive vs negative coverage, the
✗/N/A/ "no applicable rule observed" cells, and where a number leads. It is linked from the footer of every generated page: the index, each judgment page, each per-cell detail page, and both by-test pages. In the book it is a chapter just before the coverage index.the explainer page
the footer link on a cell page
Heads-up for the CI coverage diff
coverage-summary.jsonis keyed by judgment, so the first diff after this lands will report the 10 test judgments andfmtas removed andprove_outlivesas added. That is this PR, not a regression; it settles from the next build on.Not in scope
The remaining items from the office notes (proof-tree default expansion, the issue-tracking bot, the CI-diff follow-ups) are untouched.
How does it work, what questions do you have?
The source-extract change is safe because the text is sliced out of the real source, so if the slice were wrong the rendered page would be visibly wrong, and the doc-comment mask keeps byte offsets, so a match found in the mask still points at the same place in the original.
AI disclosure