Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion scripts/check_pr_perimeter.py
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,24 @@ def _extract_line_candidates(text: str) -> list[tuple[int, str]]:
continue
candidates.append((idx, line))
continue
if _has_exclusivity(low) and any(w in low for w in STRONG_SCOPE_WORDS):
# #15833/#15846: the scope-word test goes through `_has_strong_scope`,
# NOT a plain substring scan. The whole-word guard already exists and is
# already used by the two other call sites (l.~1168, l.~1257); this one
# was never migrated, so it kept matching scope words INSIDE longer
# words. Two measured misfires, both on a line whose subject is not the
# PR perimeter at all:
# #15833 l.45 "`--dist loadscope`, jamais `load` [...] il est sur
# **uniquement** grace a ce groupement" -- 'scope' inside
# "loadscope". #12718 added the `(?<![-\w])scope(?![-\w])`
# lookbehind to `_has_strong_scope` for exactly this shape.
# #15846 l.44 "cette ligne ne peut pas **changer** le comportement de
# build" (marker "seulement" earlier on the line) -- 'change'
# inside "changer". #11800 added the `\b` boundary to
# `_has_strong_scope` for exactly this shape ("inchanges").
# Both guards were built, tested, and then bypassed here. Same failure
# family as the "read-only" (#11654) and "pas seulement" (#12547)
# misfires above: the marker is present, its force is not.
if _has_exclusivity(low) and _has_strong_scope(low):
if _markers_all_quoted(line):
continue # quoting an exclusivity claim, not making one
candidates.append((idx, line))
Expand Down
51 changes: 51 additions & 0 deletions scripts/tests/test_check_pr_perimeter.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,57 @@ def test_extract_skips_read_only_compound():
"permissions read-only inchangées")


def test_extract_skips_scope_word_inside_a_longer_word():
r"""A strong scope word must be a WHOLE word -- at the extraction site too.

`_has_strong_scope` has carried that guard for a while: #11800 added the
`\b` boundary so "inchanges" stops supplying 'change', and #12718 added
the `(?<![-\w])scope(?![-\w])` lookbehind so "out-of-scope" stops
supplying 'scope'. Two of its three call sites went through it;
`_extract_line_candidates` kept a plain substring scan, so the guard was
built, tested, and then bypassed on the path that feeds the report.

Both lines below are measured, taken verbatim from PRs whose real
perimeter is a single file and whose bodies assert nothing false about
it -- the misfire turned the required `Always-on guards` red on two lanes
at once:

* #15833 l.45 -- 'scope' inside "loadscope", marker "uniquement".
* #15846 l.44 -- 'change' inside "changer", marker "seulement".

Same failure family as the "read-only" compound above (#11654) and "pas
seulement" (#12547): the marker is present, its force is not.
"""
loadscope = (
"1. **`--dist loadscope`, jamais `load`.** `loadscope` groupe par "
"module, donc les tests d'un module ne tournent jamais concurremment "
"entre eux : il est sur **uniquement** grace a ce groupement."
)
changer = (
"Le workflow de `main` est `c631a7a9` : `schedule` + `dispatch` "
"seulement, aucune jambe PR. Cette ligne ne peut pas changer le "
"comportement de build."
)
assert extract_perimeter_assertions(loadscope) == []
assert extract_perimeter_assertions(changer) == []
assert not _has_strong_scope(loadscope.lower())
assert not _has_strong_scope(changer.lower())


def test_whole_word_scope_still_extracts_real_declarations():
"""Positive control for the guard above: it must not silence the real
thing. Each line carries a STANDALONE scope word and stays a perimeter
assertion -- first among them the founding #11227 sentence, which is what
this organ exists to catch."""
for line in (
"**Perimetre** : 2 fichiers twins uniquement, aucune autre modification.",
"Cette PR touche uniquement ces fichiers, aucune autre modification.",
"Scope: only the workflow file, nothing else.",
"lake 70 fichiers uniquement, scope = perimetre PR",
):
assert extract_perimeter_assertions(line), line


def test_only_standalone_still_flags():
"""The control positive side: a standalone "only" with a scope word stays
a live exclusivity assertion -- the fix must not kill the English arm."""
Expand Down
Loading