Assert a floor on what a diff-scoped prose run actually scanned

scripts/prose_lint.py

@@ -86,6 +86,48 @@ def changed_lines(base: str) -> dict[str, set[int]] | None:
     return out
 
 
+def asked_about(key: str, paths: list[str]) -> bool:
+    """Whether a repository-relative diff key falls under one of the requested paths.
+
+    The floor below compares the diff's file list against what the run matched, and a caller who
+    narrowed the scan on purpose must not be told the narrowing is a defect. Anything the request
+    did not cover is not a file this run failed to read.
+    """
+    for raw in paths:
+        # `Path` drops a trailing separator, so the test appends one rather than stripping it.
+        # Comparing the bare prefix would let `catalog` claim `catalogue/x.md`.
+        r = rel(Path(raw))
+        if r in ('', '.'):
+            return True
+        if key == r or key.startswith(r + '/'):
+            return True
+    return False
+
+
+def unread_diff_files(scope: dict[str, set[int]], paths: list[str],
+                      excludes: tuple[str, ...]) -> list[str]:
+    """Files the diff names that this run was asked about and could have read, in sorted order.
+
+    Keys are resolved against the repository top level rather than the working directory, because
+    `git diff` reports repository-relative paths while discovery keys off the directory the run
+    started in. Reading them against the working directory would make this list empty from a
+    subdirectory, which is the one place it most needs to be full.
+    """
+    root = Path(repo_root(Path('.')) or '.')
+    out: list[str] = []
+    for key in sorted(scope):
+        if not asked_about(key, paths):
+            continue
+        if any(x in key for x in excludes):
+            continue
+        if not GENERATED_TREES.isdisjoint(Path(key).parts):
+            continue
+        target = root / key
+        if target.is_file() and is_text(target):
+            out.append(key)
+    return out
+
+
 def repo_prefix(root: Path) -> str:
     """Where `root` sits inside its repository, as a posix prefix, or '' when git cannot say.
 
@@ -1021,7 +1063,27 @@ def main(argv: list[str] | None = None) -> int:
               'checkout carries its history.', file=sys.stderr)
         return 2
     if scope is not None:
-        files = [f for f in files if rel(f) in scope]
+        matched = [f for f in files if rel(f) in scope]
+        # The floor every verdict below rests on, asserted rather than guarded.
+        # Each route to a false clean so far was closed after a reviewer saw it.
+        # The next is closed that way or not at all, which is what a floor covers.
+        # A run that resolves a non-empty diff and matches none of its files failed to scope.
+        # Zero alone is not the test.
+        # A change touching only files the rules do not read matches nothing and is clean.
+        # An image or a lock file is that case.
+        # So the comparison is against the diff's own list of files this run could have read.
+        if scope and not matched:
+            unread = unread_diff_files(scope, a.paths or ['.'], tuple(a.exclude))
+            if unread:
+                shown = ', '.join(unread[:5]) + (' and more' if len(unread) > 5 else '')
+                print(f'error: the diff against {a.diff!r} names {len(unread)} readable file(s) '
+                      f'this run was asked about, and the scan matched none of them: {shown}. '
+                      'Refusing to report a clean run, since a gate that read nothing is '
+                      'indistinguishable from a gate with nothing to read. Check that the run '
+                      'starts at the repository top level and that the requested paths cover '
+                      'the change.', file=sys.stderr)
+                return 2
+        files = matched
 
     total = 0
     bykind: dict[str, int] = {}

scripts/test_prose_lint.py

@@ -1540,6 +1540,134 @@ def test_an_unreadable_file_is_skipped_rather_than_raising(self) -> None:
         self.assertEqual([], prose_lint.check_file(self.tmp / 'absent.md', {'dupword'}))
 
 
+class TestDiffScopeFloor(unittest.TestCase):
+    """The assertion every `--diff` verdict rests on, that the run matched something it was given.
+
+    Four routes to the same false clean are on record, each closed by a guard written after a
+    reviewer noticed it: an unresolvable base widening to a whole-tree scan, a multi-line `paths`
+    input read only to its first newline, a diff taken in one repository while scanning another,
+    and a path under no repository at all. Per-route guards are the wrong shape for a fifth,
+    because the fifth is found by a reviewer or not at all. This asserts the floor instead.
+
+    Zero is not the test on its own. A change touching only files the rules do not read matches
+    nothing and is honestly clean, so the comparison is against the diff's own list of files this
+    run could have read.
+    """
+
+    def setUp(self) -> None:
+        self.tmp = Path(self.enterContext(tempfile.TemporaryDirectory()))
+        self.enterContext(contextlib.redirect_stdout(io.StringIO()))
+        self.err = self.enterContext(contextlib.redirect_stderr(io.StringIO()))
+
+    def test_a_diff_naming_readable_files_that_match_nothing_is_refused(self) -> None:
+        """The floor itself: a non-empty diff, a readable file in it, and nothing scanned."""
+        named = self.tmp / 'named.md'
+        named.write_text('A clean line.\n', encoding='utf-8')
+        elsewhere = self.tmp / 'elsewhere.md'
+        elsewhere.write_text('A clean line.\n', encoding='utf-8')
+        with mock.patch.object(prose_lint, 'repo_root', return_value=str(self.tmp)), \
+                mock.patch.object(prose_lint, 'discover', return_value=[elsewhere]), \
+                mock.patch.object(prose_lint, 'changed_lines', return_value={'named.md': {1}}):
+            self.assertEqual(2, prose_lint.main(['--check', 'dupword', '--diff', 'HEAD']))
+        self.assertIn('named.md', self.err.getvalue())
+
+    def test_a_diff_of_only_unreadable_files_is_a_clean_run(self) -> None:
+        """The honest limit, and the reason zero alone cannot be the test.
+
+        An image or a lock file is a real change the rules do not read, so it matches nothing and
+        the run is clean rather than broken. Refusing here would make the gate cry wolf on a
+        commit that adds a logo, which is how a safety check stops being read.
+        """
+        blob = self.tmp / 'logo.png'
+        blob.write_bytes(b'\x89PNG\r\n\x1a\n\x00\x00\x00binary')
+        with mock.patch.object(prose_lint, 'repo_root', return_value=str(self.tmp)), \
+                mock.patch.object(prose_lint, 'discover', return_value=[]), \
+                mock.patch.object(prose_lint, 'changed_lines', return_value={'logo.png': {1}}):
+            self.assertEqual(0, prose_lint.main(['--check', 'dupword', '--diff', 'HEAD']))
+
+    def test_a_diff_naming_a_file_that_no_longer_exists_is_a_clean_run(self) -> None:
+        """A deletion names a path with nothing behind it, which this run cannot have read."""
+        with mock.patch.object(prose_lint, 'repo_root', return_value=str(self.tmp)), \
+                mock.patch.object(prose_lint, 'discover', return_value=[]), \
+                mock.patch.object(prose_lint, 'changed_lines', return_value={'gone.md': {1}}):
+            self.assertEqual(0, prose_lint.main(['--check', 'dupword', '--diff', 'HEAD']))
+
+    def test_an_empty_diff_is_not_a_scoping_failure(self) -> None:
+        """A branch level with its base names no files, which is a result rather than a fault."""
+        with mock.patch.object(prose_lint, 'repo_root', return_value=str(self.tmp)), \
+                mock.patch.object(prose_lint, 'discover', return_value=[]), \
+                mock.patch.object(prose_lint, 'changed_lines', return_value={}):
+            self.assertEqual(0, prose_lint.main(['--check', 'dupword', '--diff', 'HEAD']))
+
+    def test_a_run_that_matched_a_file_asserts_nothing_further(self) -> None:
+        """The floor is a floor. One match clears it, and the findings decide the exit code."""
+        bait = self.tmp / 'bait.md'
+        bait.write_text(f'{DUP} thing\n', encoding='utf-8')
+        with mock.patch.object(prose_lint, 'repo_root', return_value=str(self.tmp)), \
+                mock.patch.object(prose_lint, 'discover', return_value=[bait]), \
+                mock.patch.object(prose_lint, 'changed_lines',
+                                  return_value={prose_lint.rel(bait): {1}}):
+            self.assertEqual(1, prose_lint.main(['--check', 'dupword', '--diff', 'HEAD']))
+
+    def test_a_deliberately_narrowed_scan_is_not_told_the_narrowing_is_a_defect(self) -> None:
+        """Asking about one subtree while the change sits in another is a request, not a failure."""
+        (self.tmp / 'scripts').mkdir()
+        changed = self.tmp / 'scripts' / 'tool.py'
+        changed.write_text('# A clean comment.\n', encoding='utf-8')
+        with mock.patch.object(prose_lint, 'repo_root', return_value=str(self.tmp)), \
+                mock.patch.object(prose_lint, 'discover', return_value=[]), \
+                mock.patch.object(prose_lint, 'changed_lines',
+                                  return_value={'scripts/tool.py': {1}}):
+            self.assertEqual(0, prose_lint.main(['--check', 'dupword', '--diff', 'HEAD',
+                                                 'catalog']))
+
+    def test_an_excluded_file_is_not_counted_as_one_the_run_should_have_read(self) -> None:
+        """`--exclude` removes a file from the scan, so it cannot also be evidence of a failure."""
+        skipped = self.tmp / 'vendor.md'
+        skipped.write_text('A clean line.\n', encoding='utf-8')
+        with mock.patch.object(prose_lint, 'repo_root', return_value=str(self.tmp)), \
+                mock.patch.object(prose_lint, 'discover', return_value=[]), \
+                mock.patch.object(prose_lint, 'changed_lines', return_value={'vendor.md': {1}}):
+            self.assertEqual(0, prose_lint.main(['--check', 'dupword', '--diff', 'HEAD',
+                                                 '--exclude', 'vendor']))
+
+    def test_a_generated_tree_is_not_counted_as_one_the_run_should_have_read(self) -> None:
+        """Discovery drops `reports/`, so a change confined to it matches nothing by design."""
+        (self.tmp / 'reports').mkdir()
+        generated = self.tmp / 'reports' / 'divergences.md'
+        generated.write_text('A clean line.\n', encoding='utf-8')
+        with mock.patch.object(prose_lint, 'repo_root', return_value=str(self.tmp)), \
+                mock.patch.object(prose_lint, 'discover', return_value=[]), \
+                mock.patch.object(prose_lint, 'changed_lines',
+                                  return_value={'reports/divergences.md': {1}}):
+            self.assertEqual(0, prose_lint.main(['--check', 'dupword', '--diff', 'HEAD']))
+
+    def test_diff_keys_resolve_against_the_repository_rather_than_the_working_directory(
+            self) -> None:
+        """The fifth route, which no per-route guard covers and this one found.
+
+        Run from a subdirectory, `git diff` reports repository-relative keys while discovery keys
+        off the directory the run started in, so the intersection is empty and the run reports
+        clean. Reading the diff's paths against the working directory would leave this list empty
+        in exactly that case, making the floor silent where it is most needed.
+        """
+        (self.tmp / 'scripts').mkdir()
+        nested = self.tmp / 'scripts' / 'tool.py'
+        nested.write_text('# A clean comment.\n', encoding='utf-8')
+        with mock.patch.object(prose_lint, 'repo_root', return_value=str(self.tmp)):
+            found = prose_lint.unread_diff_files({'scripts/tool.py': {1}}, ['.'], ())
+        self.assertEqual(['scripts/tool.py'], found)
+
+    def test_asked_about_reads_a_prefix_as_a_directory_boundary(self) -> None:
+        """`catalog` must not claim `catalogue/x.md`, which shares its first seven characters."""
+        self.assertTrue(prose_lint.asked_about('catalog/x.md', ['catalog']))
+        self.assertTrue(prose_lint.asked_about('catalog/x.md', ['catalog/']))
+        self.assertTrue(prose_lint.asked_about('README.md', ['README.md']))
+        self.assertTrue(prose_lint.asked_about('anything/at/all.md', ['.']))
+        self.assertFalse(prose_lint.asked_about('catalogue/x.md', ['catalog']))
+        self.assertFalse(prose_lint.asked_about('docs/x.md', ['catalog']))
+
+
 class TestHarness(unittest.TestCase):
     def test_this_module_collects_a_plausible_number_of_cases(self) -> None:
         """A module whose cases fail to load still reports OK, which is a pass proving nothing."""