Resolve tree/ links as directories in the cross-repository link checker

.github/workflows/cross-repo-links.yml

@@ -63,6 +63,9 @@ jobs:
           done < "$RUNNER_TEMP/repos.txt"
           ls "$org_dir"
 
+      - name: Test the link checker
+        run: python3 scripts/test_cross_repo_links.py
+
       # The exit status is captured rather than allowed to fail the step, so
       # that the issue is opened before the run goes red. A run that fails
       # without leaving a record is a run nobody reads.

scripts/check_cross_repo_links.py

@@ -56,7 +56,7 @@
 LINK = re.compile(r"\[[^\]]*\]\(([^)\s]+)\)")
 # https://github.com/<owner>/<repo>/(blob|tree)/<ref>/<path>
 GH_REF = re.compile(
-    r"https?://github\.com/([^/]+)/([^/]+)/(?:blob|tree)/([^/]+)/(.+?)(?:#.*)?$"
+    r"https?://github\.com/([^/]+)/([^/]+)/(blob|tree)/([^/]+)/(.+?)/?(?:[?#].*)?$"
 )
 SHA = re.compile(r"[0-9a-f]{7,40}")
 
@@ -107,6 +107,31 @@ def tree_files(repo: Path, sha: str) -> set[str]:
     return {line for line in out.split("\n") if line}
 
 
+def tree_dirs(paths: set[str]) -> set[str]:
+    """Every directory that contains at least one tracked file."""
+    dirs: set[str] = set()
+    for path in paths:
+        parts = path.split("/")[:-1]
+        for i in range(1, len(parts) + 1):
+            dirs.add("/".join(parts[:i]))
+    return dirs
+
+
+def target_exists(kind: str, path: str, files: set[str], dirs: set[str]) -> bool:
+    """A `tree/` link names a directory, a `blob/` link names a file.
+
+    GitHub serves `tree/<ref>/<dir>` as a directory listing, so checking it
+    against the file list alone reports every directory link as broken.
+    Relative links may point at either.
+    """
+    path = path.rstrip("/")
+    if kind == "github-tree":
+        return path in dirs
+    if kind == "github-blob":
+        return path in files
+    return path in files or path in dirs
+
+
 def grep_links(repo: Path, sha: str) -> list[tuple[str, int, str]]:
     """Every line of every tracked Markdown file at `sha` that holds a link."""
     out = git(repo, "grep", "-n", "-I", "-E", r"\]\(", sha, "--", "*.md")
@@ -141,13 +166,15 @@ def scan(repos: dict[str, Path]) -> tuple[list[dict], list[dict], int]:
     heads: dict[str, str] = {}
     branches: dict[str, str] = {}
     files: dict[str, set[str]] = {}
+    dirs: dict[str, set[str]] = {}
     for name, repo in repos.items():
         head = remote_head(repo)
         if head is None:
             print(f"warning: {name}: cannot fetch the default branch", file=sys.stderr)
             continue
         branches[name], heads[name] = head
         files[name] = tree_files(repo, head[1])
+        dirs[name] = tree_dirs(files[name])
 
     broken: list[dict] = []
     unverifiable: list[dict] = []
@@ -182,7 +209,7 @@ def scan(repos: dict[str, Path]) -> tuple[list[dict], list[dict], int]:
                 }
                 if note:
                     unverifiable.append({**record, "reason": note})
-                elif target_path not in files[target_repo]:
+                elif not target_exists(kind, target_path, files[target_repo], dirs[target_repo]):
                     broken.append(record)
     return broken, unverifiable, pinned
 
@@ -193,13 +220,14 @@ def classify(
     """Return (target_repo, target_path, kind, unverifiable_reason)."""
     m = GH_REF.match(url)
     if m:
-        _, target_repo, ref, path = m.groups()
+        _, target_repo, mode, ref, path = m.groups()
         if SHA.fullmatch(ref):
             return target_repo, path, "pinned", "commit sha"
+        kind = f"github-{mode}"
         branch = branches.get(target_repo)
         if branch is not None and ref != branch:
-            return target_repo, path, "github-blob", f"ref {ref} is not the default branch"
-        return target_repo, path, "github-blob", ""
+            return target_repo, path, kind, f"ref {ref} is not the default branch"
+        return target_repo, path, kind, ""
 
     if url.startswith(("http", "#", "mailto")):
         return None

scripts/test_cross_repo_links.py

@@ -0,0 +1,63 @@
+"""Regression tests for the cross-repository link checker.
+
+Pins the distinction behind issue #28: a `tree/` link names a directory and a
+`blob/` link names a file, so each must be resolved against the right set.
+
+Run with `python3 scripts/test_cross_repo_links.py`.
+"""
+
+from __future__ import annotations
+
+import importlib.util
+import unittest
+from pathlib import Path
+
+_spec = importlib.util.spec_from_file_location(
+    "check_cross_repo_links", Path(__file__).with_name("check_cross_repo_links.py")
+)
+links = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(links)
+
+FILES = {"compat/bundles/a.json", "docs/guide.md", "README.md"}
+DIRS = links.tree_dirs(FILES)
+URL = "https://github.com/QuantStrategyLab/Sibling/{mode}/main/{path}"
+
+
+def check(mode: str, path: str) -> bool:
+    hit = links.classify(URL.format(mode=mode, path=path), "Here", "x.md", {}, {"Sibling": "main"})
+    assert hit is not None
+    _, target_path, kind, note = hit
+    assert note == ""
+    return links.target_exists(kind, target_path, FILES, DIRS)
+
+
+class TreeVersusBlob(unittest.TestCase):
+    def test_tree_link_to_existing_directory_resolves(self):
+        self.assertTrue(check("tree", "compat/bundles"))
+        self.assertTrue(check("tree", "compat/bundles/"))
+        self.assertTrue(check("tree", "compat"))
+
+    def test_tree_link_to_missing_directory_is_broken(self):
+        self.assertFalse(check("tree", "compat/missing"))
+
+    def test_tree_link_to_a_file_is_broken(self):
+        self.assertFalse(check("tree", "docs/guide.md"))
+
+    def test_blob_link_to_existing_file_resolves(self):
+        self.assertTrue(check("blob", "docs/guide.md"))
+        self.assertTrue(check("blob", "docs/guide.md#section"))
+
+    def test_blob_link_to_a_directory_is_broken(self):
+        self.assertFalse(check("blob", "compat/bundles"))
+
+    def test_blob_link_to_missing_file_is_broken(self):
+        self.assertFalse(check("blob", "docs/missing.md"))
+
+    def test_relative_link_accepts_file_or_directory(self):
+        self.assertTrue(links.target_exists("relative", "compat/bundles", FILES, DIRS))
+        self.assertTrue(links.target_exists("relative", "README.md", FILES, DIRS))
+        self.assertFalse(links.target_exists("relative", "nope", FILES, DIRS))
+
+
+if __name__ == "__main__":
+    unittest.main()