test: a conftest cannot rebind the order-killer list (#924)

CHANGELOG.md

@@ -1268,6 +1268,19 @@ true until the next version shipped.
   now re-records. It names the two cases that remain: a declaration that no longer
   resolves, and the implicit base projection, which is not readable by name at all.
 
+- A `conftest.py` can no longer switch off the order-collapse guard by rebinding
+  the module-level name it used to read (#924).
+
+  The scan looked up `_ORDER_KILLERS` on each call. A conftest is imported before
+  collection, so `pgc_vacuity._ORDER_KILLERS = ()` turned the refusal off for every
+  test in that directory, with no reason recorded. Two lines, less to type than
+  the honest form. Measured: the same collapse test was uncollectable with no extra
+  file, and reported `1 passed` with only that rebind.
+
+  The killer names are bound at definition time, in a default argument, the same
+  way `query_error` already binds its prefix. There is no module-level name left
+  to rebind. `_RECORDERS` is a registry the layer writes, not rule data, and is
+  unchanged.
 - The pytest harness no longer reports results against a library another process
   installed (#956).
 

test/pytest/TESTS.md

@@ -1114,6 +1114,7 @@ tests port that pair and its premise check.
 | `test_ordered_rows_fails_on_the_wrong_order` | the oracle detects order | proves it can fail, not merely that it permits |
 | `test_layer_refuses_sorting_the_input_to_an_ordered_claim` | `sorted()` feeding `ordered_rows` is uncollectable, found by AST | `ordered_rows(sorted(got), sorted(want))` cannot fail on order |
 | `test_layer_refuses_a_name_bound_to_a_sorted_call` | `g = sorted(got)` one line above the claim is the same collapse | the inline spelling was the only one caught, so the guard was blind to the version least likely to be noticed |
+| `test_a_conftest_cannot_switch_off_the_order_collapse_guard` | a conftest rebinding `_ORDER_KILLERS` cannot silence the scan | two lines of conftest is less to type than the honest form, and that is the hatch the layer forbids |
 | `test_layer_refuses_a_list_sorted_in_place` | `got.sort()` kills the order and leaves the name spelled the same | nothing at the call site says anything happened |
 | `test_layer_allows_a_name_sorted_after_the_claim` | **control** | a name sorted AFTER the claim did not affect it; refusing that would be a false red |
 | `test_the_order_killer_scan_is_one_function_deep` | **pinned limit** | a sort behind a helper is not caught, and this arm reddens if that documented limit ever moves |

test/pytest/pgc_vacuity.py

@@ -1171,85 +1171,106 @@ def pytest_collection_finish(session):
 # sorted() to reach for.
 #
 # Parsed, not grepped, for the same reason as the except scan below.
-_ORDER_KILLERS = ("sorted", "set", "frozenset")
-
-
-def _order_killed_names(fn):
-    """Names bound to an order-killing value earlier in one function body.
-
-    -> {name: (lineno, how)}
+# THE KILLER LIST IS BOUND AT DEFINITION TIME, in a default argument, and that
+# is the whole mechanism rather than a style choice (#924).
+#
+# `_ORDER_KILLERS` used to live as a module-level name. A conftest imported
+# before collection rebound it to () and the order-collapse refusal stopped
+# firing for every test in that directory, with no reason recorded. Two lines,
+# less to type than the honest form, which is the hatch the layer's own
+# false-positive budget forbids. Measured on main: the same collapse test was
+# uncollectable with no extra file, and reported `1 passed` with only
+#
+#     import pgc_vacuity
+#     pgc_vacuity._ORDER_KILLERS = ()
+#
+# The layer already closed this shape for QUERY_ERROR. The same default-argument
+# bind is used here: the tuple is evaluated once, when the factory is defined,
+# and is not read from the module namespace afterwards. There is no module-level
+# name left to rebind.
+def _order_guard(_killers=("sorted", "set", "frozenset")):
+    def _order_killed_names(fn):
+        """Names bound to an order-killing value earlier in one function body.
+
+        -> {name: (lineno, how)}
+
+        The inline spelling is only the shortest way to write the collapse. These two
+        are the same defect and read as more careful code, which is worse:
+
+            g = sorted(got)                 # bound to an order-killing call
+            expect.ordered_rows(g, want)
+
+            got.sort()                      # killed in place
+            expect.ordered_rows(got, want)
+
+        WHAT THIS DOES NOT SEE, stated because a guard's blind spots are part of its
+        meaning: it is one function deep, so a helper that sorts and returns is invisible;
+        it does not follow aliases (`h = g`), attributes (`self.rows.sort()`), branches,
+        or a name re-bound to something honest after being killed. It is a floor, not a
+        proof of order-sensitivity. The suite's own removal proofs are what establish
+        that an ordered claim can actually fail on order.
+        """
+        killed = {}
+        for node in ast.walk(fn):
+            # X = sorted(...) / set(...) / frozenset(...)
+            if isinstance(node, ast.Assign) and isinstance(node.value, ast.Call):
+                f = node.value.func
+                if isinstance(f, ast.Name) and f.id in _killers:
+                    for t in node.targets:
+                        if isinstance(t, ast.Name):
+                            killed.setdefault(t.id, (node.lineno, f"{f.id}()"))
+            # X.sort() -- in place, and the name keeps its spelling at the call site
+            elif isinstance(node, ast.Expr) and isinstance(node.value, ast.Call):
+                f = node.value.func
+                if (isinstance(f, ast.Attribute) and f.attr == "sort"
+                        and isinstance(f.value, ast.Name)):
+                    killed.setdefault(f.value.id, (node.lineno, ".sort()"))
+        return killed
+
+
+    def _sorted_ordered_sites(path):
+        try:
+            tree = ast.parse(pathlib.Path(path).read_text())
+        except (OSError, SyntaxError):
+            return []
+        out = []
+        name = pathlib.Path(path).name
+        # Per function, because a killed name means nothing outside the body that
+        # killed it, and a module-level walk would carry one test's `g` into the next.
+        fns = [n for n in ast.walk(tree)
+               if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))]
+        for fn in fns:
+            killed = _order_killed_names(fn)
+            for node in ast.walk(fn):
+                if not isinstance(node, ast.Call):
+                    continue
+                f = node.func
+                if not (isinstance(f, ast.Attribute) and f.attr in ("ordered_rows",
+                                                                    "ordering_observable")):
+                    continue
+                for arg in node.args:
+                    if (isinstance(arg, ast.Call) and isinstance(arg.func, ast.Name)
+                            and arg.func.id in _killers):
+                        out.append(
+                            f"{name}:{node.lineno} {arg.func.id}() feeds an ordered claim"
+                        )
+                    elif isinstance(arg, ast.Name) and arg.id in killed:
+                        where, how = killed[arg.id]
+                        # Only a kill that already happened. A name sorted AFTER the
+                        # claim was made did not affect it, and flagging that would be
+                        # a false red -- the thing this whole layer exists to refuse.
+                        if where < node.lineno:
+                            out.append(
+                                f"{name}:{node.lineno} {arg.id} was order-killed by "
+                                f"{how} at line {where} and feeds an ordered claim"
+                            )
+        return out
 
-    The inline spelling is only the shortest way to write the collapse. These two
-    are the same defect and read as more careful code, which is worse:
 
-        g = sorted(got)                 # bound to an order-killing call
-        expect.ordered_rows(g, want)
+    return _order_killed_names, _sorted_ordered_sites
 
-        got.sort()                      # killed in place
-        expect.ordered_rows(got, want)
 
-    WHAT THIS DOES NOT SEE, stated because a guard's blind spots are part of its
-    meaning: it is one function deep, so a helper that sorts and returns is invisible;
-    it does not follow aliases (`h = g`), attributes (`self.rows.sort()`), branches,
-    or a name re-bound to something honest after being killed. It is a floor, not a
-    proof of order-sensitivity. The suite's own removal proofs are what establish
-    that an ordered claim can actually fail on order.
-    """
-    killed = {}
-    for node in ast.walk(fn):
-        # X = sorted(...) / set(...) / frozenset(...)
-        if isinstance(node, ast.Assign) and isinstance(node.value, ast.Call):
-            f = node.value.func
-            if isinstance(f, ast.Name) and f.id in _ORDER_KILLERS:
-                for t in node.targets:
-                    if isinstance(t, ast.Name):
-                        killed.setdefault(t.id, (node.lineno, f"{f.id}()"))
-        # X.sort() -- in place, and the name keeps its spelling at the call site
-        elif isinstance(node, ast.Expr) and isinstance(node.value, ast.Call):
-            f = node.value.func
-            if (isinstance(f, ast.Attribute) and f.attr == "sort"
-                    and isinstance(f.value, ast.Name)):
-                killed.setdefault(f.value.id, (node.lineno, ".sort()"))
-    return killed
-
-
-def _sorted_ordered_sites(path):
-    try:
-        tree = ast.parse(pathlib.Path(path).read_text())
-    except (OSError, SyntaxError):
-        return []
-    out = []
-    name = pathlib.Path(path).name
-    # Per function, because a killed name means nothing outside the body that
-    # killed it, and a module-level walk would carry one test's `g` into the next.
-    fns = [n for n in ast.walk(tree)
-           if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))]
-    for fn in fns:
-        killed = _order_killed_names(fn)
-        for node in ast.walk(fn):
-            if not isinstance(node, ast.Call):
-                continue
-            f = node.func
-            if not (isinstance(f, ast.Attribute) and f.attr in ("ordered_rows",
-                                                                "ordering_observable")):
-                continue
-            for arg in node.args:
-                if (isinstance(arg, ast.Call) and isinstance(arg.func, ast.Name)
-                        and arg.func.id in _ORDER_KILLERS):
-                    out.append(
-                        f"{name}:{node.lineno} {arg.func.id}() feeds an ordered claim"
-                    )
-                elif isinstance(arg, ast.Name) and arg.id in killed:
-                    where, how = killed[arg.id]
-                    # Only a kill that already happened. A name sorted AFTER the
-                    # claim was made did not affect it, and flagging that would be
-                    # a false red -- the thing this whole layer exists to refuse.
-                    if where < node.lineno:
-                        out.append(
-                            f"{name}:{node.lineno} {arg.id} was order-killed by "
-                            f"{how} at line {where} and feeds an ordered claim"
-                        )
-    return out
+_order_killed_names, _sorted_ordered_sites = _order_guard()
 
 
 def _broad_except_sites(path):

test/pytest/test_ordered.py

@@ -137,6 +137,43 @@ def test_sorted_afterwards(expect):
                     passed=1, failed=0)
 
 
+def test_a_conftest_cannot_switch_off_the_order_collapse_guard(pytester, expect):
+    """#924. The scan reads `_ORDER_KILLERS` from the module, and a conftest
+    is imported before collection, so two lines switch the guard off.
+
+    Measured on main: the same collapse test is uncollectable with no extra
+    file, and reports `1 passed` when the only extra file is
+
+        import pgc_vacuity
+        pgc_vacuity._ORDER_KILLERS = ()
+
+    That is less to type than the honest form, and the run records no reason.
+    The layer already closed this shape for QUERY_ERROR by binding the prefix
+    at definition time; the killer list was still a module-level name.
+    """
+    pytester.makepyfile(
+        """
+        def test_order_collapsed(expect):
+            got = ["b", "a"]
+            g = sorted(got)
+            expect.ordered_rows(g, ["a", "b"], "rows in order")
+        """
+    )
+    plain = pytester.runpytest("-p", "pgc_vacuity")
+    expect.run_failed(plain, "premise: the collapse is refused when nobody rebinds")
+    plain.stderr.fnmatch_lines(["*order-killed*"])
+
+    pytester.makeconftest(
+        """
+        import pgc_vacuity
+        pgc_vacuity._ORDER_KILLERS = ()
+        """
+    )
+    hatched = pytester.runpytest("-p", "pgc_vacuity")
+    expect.run_failed(hatched, "and it is still refused after a conftest rebinds the name")
+    hatched.stderr.fnmatch_lines(["*order-killed*"])
+
+
 def test_the_order_killer_scan_is_one_function_deep(pytester, expect):
     """A named limit, pinned so it cannot quietly become a claim of completeness.