diff --git a/src/docproof/verifiers/paths.py b/src/docproof/verifiers/paths.py index df4a665..5bb98bd 100644 --- a/src/docproof/verifiers/paths.py +++ b/src/docproof/verifiers/paths.py @@ -34,7 +34,7 @@ import re from collections.abc import Iterable, Iterator -from pathlib import PurePosixPath +from pathlib import Path, PurePosixPath from .. import tree from ..docs import Span, spans @@ -242,9 +242,60 @@ def looks_like_a_repo_path(text: str) -> bool: # broken internal links. It is not this, and pretending otherwise in a comment is how a file # ends up describing a tool nobody wrote. REF_DEFINITION = re.compile(r"(?m)^\[`?([^\]`]+)`?\]:\s*\S+") + +# The SAME reasoning as REF_DEFINITION, applied to the form people actually write. +# +# see [`docs/STATE-MD-LIFECYCLE.md`](reference/state-md.md) for the field reference +# +# The label is a display string that went stale; the target resolves. **Nobody following that +# link lands anywhere wrong.** Skipping `[label]: target` while judging `[label](target)` was +# an inconsistency rather than a position - the reference form is the rare one. +# +# Measured across the 134 cloned repositories before this was written: +# 137,937 inline links, 7,426 with a path-shaped label +# 5,529 where the label is not in the tree AND the target resolves +# ...but 4,838 of those resolve to a URL, and almost all are org/repo slugs used as a +# label - `shadcn/ui`, `encode/broadcaster` - which were never repository paths and +# could never have been findings +# 687 resolve to a file on disk, which is the real class +# 4 of the 217 corpus findings sit in it, none of them ever filed +# +# **A URL target earns no skip, and that is the whole safety of this rule.** This cannot +# fetch, and when a repository deletes a file the blob URL pointing at it dies too, so +# trusting a URL would convert a real broken reference into silence. Only a target that +# resolves on disk is evidence that the reader arrives somewhere. +# +# Outside witness, which no other skip family here has: triaging `open-gsd/gsd-core#3620` a +# maintainer listed his own out-of-scope items and named this shape unprompted - "the link +# whose label carries a pre-Diataxis name but whose target resolves". The exclusion had +# already been made by hand in that filing and he verified it independently. +INLINE_LINK = re.compile(r"\[`?([^\]`]+)`?\]\(([^)]*)\)") + URLISH = re.compile(r"(https?://|github\.com|gitlab\.com|\bgit@|\.git\b|\]\(|\bpip install\b)") +def inline_link_labels(text: str, document: Path, root: Path) -> set[str]: + """Labels of inline links whose target resolves to a file on disk. + + Returns labels only. The target is not judged here: a link target is bare text rather + than a backticked span, so it was never extracted as a claim in the first place. Judging + internal link targets is a real feature and this is not it. + """ + resolved: set[str] = set() + for match in INLINE_LINK.finditer(text): + label, target = match.group(1).strip(), match.group(2).strip().split(" ")[0] + if not label or not target: + continue + if target.startswith(("http://", "https://", "mailto:", "#")): + continue # unverifiable from here, and a dead file makes a dead blob URL + target = target.split("#")[0] + if not target: + continue + if (document.parent / target).exists() or (root / target.lstrip("/")).exists(): + resolved.add(label) + return resolved + + def command_span(span: Span) -> bool: """Whether a fenced block is a transcript whose non-prompt lines are output.""" return ( @@ -323,6 +374,7 @@ def extract(self, project: Project, documents: Iterable[Document]) -> Iterator[F for document in documents: lines = document.text.split("\n") ref_labels = {m.group(1).strip() for m in REF_DEFINITION.finditer(document.text)} + link_labels = inline_link_labels(document.text, document.path, project.root) for span in spans(document.text): # A directory diagram is read as a diagram: its leaves are only # meaningful once indentation has been turned back into a full path. @@ -387,6 +439,34 @@ def extract(self, project: Project, documents: Iterable[Document]) -> Iterator[F if token in ref_labels: continue key = (project.relative(document.path), token) + # The same thing in the form people actually write. VISIBLE, unlike the + # reference-label drop above: this rule decides by itself that a path is + # a display string, and over-firing has to show as a named skip rather + # than as a report that quietly got cleaner. + # + # **Only when the label does not resolve**, and that condition was added + # after watching it over-fire. Skipping every label whose target works + # turned 85 gsd-core claims into skips to remove 3 findings: the other 82 + # were labels that DO name a real file, which the tool was checking + # correctly and would have passed. A skip that swallows a passing check + # buys nothing and costs coverage, so a label that resolves is left alone + # to be judged and to hold. The count shrinking is what showed this, which + # is the entire reason the skip is printed rather than dropped. + if token in link_labels and key not in seen and not (project.root / token).exists(): + seen.add(key) + yield self.skip( + Claim( + kind="path", + subject=token, + doc=document.path, + line=span.line, + span=source_line.strip()[:200], + ), + "this is the label of a link whose target resolves, so it is a " + "display name rather than a claim that the path is there, and " + "nobody following the link lands anywhere wrong", + ) + continue if key in seen: continue seen.add(key) diff --git a/tests/test_link_label.py b/tests/test_link_label.py new file mode 100644 index 0000000..e7f1a7a --- /dev/null +++ b/tests/test_link_label.py @@ -0,0 +1,148 @@ +"""A stale path that is only the LABEL of a link whose target works. + + see [`docs/STATE-MD-LIFECYCLE.md`](reference/state-md.md) for the field reference + +The label is a display string that went stale. The target resolves. Nobody following that +link lands anywhere wrong, so calling it drift argues with a document that is doing its job. + +docproof already skipped the reference form, `[label]: target`, on exactly this reasoning. +Judging the inline form while skipping the reference form was an inconsistency rather than a +position, and the inline form is the common one: 137,937 of them across the 134 cloned +repositories against a few dozen reference definitions. + +**Measured before it was built**, and the measurement is why the rule is narrow: + + 137,937 inline links, 7,426 with a path-shaped label + 5,529 where the label is not in the tree and the target resolves + 4,838 of those resolve to a URL - and almost all are org/repo slugs used as a label + (`shadcn/ui`, `encode/broadcaster`), which were never repository paths + 687 resolve to a file on disk, which is the real class + 4 of the 217 corpus findings sit in it, and none was ever filed + +The 4 were excluded by hand at filing time with the reasoning written into the issue, and a +gsd-core maintainer verified that exclusion independently while triaging #3620. +""" + +from __future__ import annotations + +from collections.abc import Callable +from pathlib import Path + +PYPROJECT = """\ +[project] +name = "linklabel" +version = "0.1.0" +""" + + +def test_a_label_whose_target_resolves_is_skipped_and_the_reason_is_given( + make_repo: Callable[..., Path], capsys +) -> None: + """The shape itself, end to end. + + Asserted as a SKIP rather than silence: this rule decides by itself that a path is a + display string, so over-firing must show as a named skip and not as a report that + quietly got cleaner. + """ + from docproof.cli import main + + repo = make_repo( + {"pyproject.toml": PYPROJECT, "reference/state-md.md": "# State\n"}, + documented_before={ + "GUIDE.md": "See [`docs/STATE-MD-LIFECYCLE.md`](reference/state-md.md) for detail.\n" + }, + deleted={"docs/STATE-MD-LIFECYCLE.md": "# old\n"}, + ) + main([str(repo), "--show-skips"]) + out = capsys.readouterr().out + + assert "nobody following the link lands anywhere wrong" in out + + +def test_a_url_target_earns_no_skip(make_repo: Callable[..., Path], capsys) -> None: + """The whole safety of the rule, and the reason it is not the larger number. + + docproof cannot fetch, and when a repository deletes a file the blob URL pointing at it + dies with it. Trusting a URL would turn a real broken reference into silence, which is + the failure this tool exists to remove. + """ + from docproof.cli import main + + repo = make_repo( + {"pyproject.toml": PYPROJECT}, + documented_before={ + "GUIDE.md": "See [`src/gone.py`](https://example.invalid/blob/main/src/gone.py).\n" + }, + deleted={"src/gone.py": "x = 1\n"}, + ) + main([str(repo)]) + out = capsys.readouterr().out + + assert "src/gone.py" in out, "a URL target must not silence a deleted path" + + +def test_a_label_whose_target_is_also_missing_is_still_reported( + make_repo: Callable[..., Path], capsys +) -> None: + """A broken link is not a display name. If the target does not resolve either, the + reader lands nowhere and the finding stands.""" + from docproof.cli import main + + repo = make_repo( + {"pyproject.toml": PYPROJECT}, + documented_before={"GUIDE.md": "See [`src/gone.py`](also/missing.md) for detail.\n"}, + deleted={"src/gone.py": "x = 1\n"}, + ) + main([str(repo)]) + out = capsys.readouterr().out + + assert "src/gone.py" in out + + +def test_a_bare_mention_is_untouched(make_repo: Callable[..., Path], capsys) -> None: + """The rule is about links. A path named in prose is still a claim about the tree, and + this is the case that would break if the label matching ever went token-wide.""" + from docproof.cli import main + + repo = make_repo( + {"pyproject.toml": PYPROJECT, "reference/state-md.md": "# State\n"}, + documented_before={"GUIDE.md": "The entry point is `src/gone.py` today.\n"}, + deleted={"src/gone.py": "x = 1\n"}, + ) + main([str(repo)]) + out = capsys.readouterr().out + + assert "src/gone.py" in out + + +def test_the_target_may_resolve_from_the_repository_root(make_repo: Callable[..., Path], capsys) -> None: + """Real documents write both. `docs/x.md` from a doc in `docs/` resolves against the + root, not against the document's own directory, and a rule that only tried one of the + two would fire on half the class and look like it worked.""" + from docproof.cli import main + + repo = make_repo( + {"pyproject.toml": PYPROJECT, "reference/state-md.md": "# State\n"}, + documented_before={"docs/GUIDE.md": "See [`docs/OLD.md`](reference/state-md.md) for detail.\n"}, + deleted={"docs/OLD.md": "# old\n"}, + ) + main([str(repo), "--show-skips"]) + out = capsys.readouterr().out + + assert "nobody following the link lands anywhere wrong" in out + + +def test_an_anchor_only_link_earns_no_skip(make_repo: Callable[..., Path], capsys) -> None: + """`[label](#section)` goes nowhere near the tree, so it is no evidence at all that a + path exists. Left explicit because treating it as 'resolves' is the easy mistake.""" + from docproof.cli import main + + repo = make_repo( + {"pyproject.toml": PYPROJECT}, + documented_before={"GUIDE.md": "See [`src/gone.py`](#details) below.\n"}, + deleted={"src/gone.py": "x = 1\n"}, + ) + main([str(repo)]) + out = capsys.readouterr().out + + assert "src/gone.py" in out