diff --git a/.agents/skills/comment-and-doc-style/SKILL.md b/.agents/skills/comment-and-doc-style/SKILL.md index 5f29c390e..aca382d08 100644 --- a/.agents/skills/comment-and-doc-style/SKILL.md +++ b/.agents/skills/comment-and-doc-style/SKILL.md @@ -128,10 +128,10 @@ restricted. Each structural rule a pattern can reach lands as a `prose_lint.py` incrementally, and this section names each check as it ships. - **Short sentences: at most 25 words in one sentence**, ASD-STE100's descriptive cap, checked by - the `sentence-length` rule in `prose_lint.py`. The check is opt-in like `sentence-split`, - because the existing corpus predates the cap and a default gate would fail whole files nobody - is editing. Write new prose under the cap, and scope a run to a change with - `--check sentence-length --diff `. + the `sentence-length` rule in `prose_lint.py`. The rule reads Markdown prose, code comments, and + docstrings. The check is opt-in like `sentence-split`, because the existing corpus predates the + cap and a default gate would fail whole files nobody is editing. Write new prose under the cap, + and scope a run to a change with `--check sentence-length --diff `. - **One instruction per sentence.** A procedure step states one action, and a second action is a second step. No pattern reaches this, so it is authoring discipline with no check. - **Active voice, imperative mood for procedure steps.** Write "run the gate", never "the gate diff --git a/.claude-plugin/fleet-skills/.source-digests/comment-and-doc-style b/.claude-plugin/fleet-skills/.source-digests/comment-and-doc-style index 93806e89d..edb797b81 100644 --- a/.claude-plugin/fleet-skills/.source-digests/comment-and-doc-style +++ b/.claude-plugin/fleet-skills/.source-digests/comment-and-doc-style @@ -1 +1 @@ -bf9626afca871416 +5104f29713e7cc6c diff --git a/.claude-plugin/fleet-skills/skills/comment-and-doc-style/SKILL.md b/.claude-plugin/fleet-skills/skills/comment-and-doc-style/SKILL.md index 5f29c390e..aca382d08 100644 --- a/.claude-plugin/fleet-skills/skills/comment-and-doc-style/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/comment-and-doc-style/SKILL.md @@ -128,10 +128,10 @@ restricted. Each structural rule a pattern can reach lands as a `prose_lint.py` incrementally, and this section names each check as it ships. - **Short sentences: at most 25 words in one sentence**, ASD-STE100's descriptive cap, checked by - the `sentence-length` rule in `prose_lint.py`. The check is opt-in like `sentence-split`, - because the existing corpus predates the cap and a default gate would fail whole files nobody - is editing. Write new prose under the cap, and scope a run to a change with - `--check sentence-length --diff `. + the `sentence-length` rule in `prose_lint.py`. The rule reads Markdown prose, code comments, and + docstrings. The check is opt-in like `sentence-split`, because the existing corpus predates the + cap and a default gate would fail whole files nobody is editing. Write new prose under the cap, + and scope a run to a change with `--check sentence-length --diff `. - **One instruction per sentence.** A procedure step states one action, and a second action is a second step. No pattern reaches this, so it is authoring discipline with no check. - **Active voice, imperative mood for procedure steps.** Write "run the gate", never "the gate diff --git a/.github/actions/prose-gate/prose_lint.py b/.github/actions/prose-gate/prose_lint.py index ae4d23351..6a8ad4472 100755 --- a/.github/actions/prose-gate/prose_lint.py +++ b/.github/actions/prose-gate/prose_lint.py @@ -12,7 +12,7 @@ comment-added No comment line opening its own line added or edited, unless the change says so. dupword No duplicated consecutive word. sentence-split A sentence must not wrap across lines (one sentence per line). - sentence-length A Markdown prose sentence must not exceed the word cap. + sentence-length A Markdown, comment, or docstring sentence must not exceed the word cap. spelling No British spelling, the repo-wide convention being US English. home-path No absolute home path naming a real account, per the representative-data rule. dead-path No mention of a path git once tracked and the tree no longer holds. @@ -1801,7 +1801,7 @@ def quote(m: re.Match[str]) -> str: return '"' * (len(inner) + 1) + inner[-1] return '"' * len(m.group()) - text = re.sub(r"`[^`]*`", code, text) + text = re.sub(r"(? str: return "".join(out) +def over_cap_findings( + block: list[tuple[int, str]], allowed: set[int] | None = None +) -> list[tuple[int, str, str]]: + """Over-cap sentences in one block of `(line number, text)` pairs, each judged whole. + + The pairs are joined before the split, so a sentence wrapped across lines is counted once. + Both the Markdown reader and the comment reader hand their blocks here. + """ + if not block: + return [] + raw = "" + starts: list[int] = [] + for _, span in block: + if raw: + raw += " " + starts.append(len(raw)) + raw += span + numbers = [n for n, _ in block] + lengths = [len(span) for _, span in block] + text = mask_code_and_quotes(raw, starts, lengths) + out: list[tuple[int, str, str]] = [] + pos = 0 + for m in [*SENTENCE_BREAK.finditer(text), None]: + end = m.start() if m else len(text) + words = len(text[pos:end].split()) + if words > SENTENCE_WORD_CAP: + first = bisect_right(starts, pos) - 1 + last = bisect_right(starts, max(end - 1, pos)) - 1 + rows = numbers[first : last + 1] + hit = rows[0] if allowed is None else next((n for n in rows if n in allowed), 0) + if hit: + msg = ( + f"{words} words in one sentence -> " + f"sentences of {SENTENCE_WORD_CAP} words or fewer" + ) + out.append((hit, "sentence-length", msg)) + if m: + pos = m.end() + return out + + def sentence_length_findings( lines: list[str], allowed: set[int] | None = None ) -> list[tuple[int, str, str]]: @@ -1828,37 +1869,8 @@ def sentence_length_findings( def flush() -> None: nonlocal block_is_item block_is_item = False - if not block: - return - raw = "" - starts: list[int] = [] - for _, span in block: - if raw: - raw += " " - starts.append(len(raw)) - raw += span - numbers = [n for n, _ in block] - lengths = [len(span) for _, span in block] + out.extend(over_cap_findings(block, allowed)) block.clear() - text = mask_code_and_quotes(raw, starts, lengths) - pos = 0 - for m in [*SENTENCE_BREAK.finditer(text), None]: - end = m.start() if m else len(text) - piece = text[pos:end] - words = len(piece.split()) - if words > SENTENCE_WORD_CAP: - first = bisect_right(starts, pos) - 1 - last = bisect_right(starts, max(end - 1, pos)) - 1 - rows = numbers[first : last + 1] - hit = rows[0] if allowed is None else next((n for n in rows if n in allowed), 0) - if hit: - msg = ( - f"{words} words in one sentence -> " - f"sentences of {SENTENCE_WORD_CAP} words or fewer" - ) - out.append((hit, "sentence-length", msg)) - if m: - pos = m.end() start = front_matter_end(lines) for i, line in enumerate(lines[:start], 1): @@ -2256,6 +2268,34 @@ def flush() -> None: return out +def prose_comment_candidates( + path: Path, raw: str, lines: list[str] +) -> tuple[list[Comment], list[Comment], set[int]]: + """The comments a prose rule reads, the originals beside them, and the config-run lines. + + The bodies have a trailing checker directive cut off, and fenced lines are dropped. + """ + comments = python_comments(raw) if path.suffix.lower() == ".py" else None + if comments is None: + comments = extracted_comments(path, lines) + skip = fenced_lines(lines) + kept = [c for c in comments if c[0] not in skip] + trimmed = [c._replace(body=without_trailing_directive(path, c.body)) for c in kept] + return trimmed, kept, config_run_lines(trimmed) + + +def is_non_prose_comment(n: int, body: str, configured: set[int]) -> bool: + """Whether a comment body is not a sentence, so no prose rule judges it.""" + return bool( + not body + or n in configured + or NOT_PROSE.search(body) + or BARE_URI.match(body.strip()) + or KEY_ONLY.match(body) + or (SNIPPET.match(body) and not SNIPPET_END.search(body)) + ) + + def comment_wrap_findings(path: Path, raw: str, lines: list[str]) -> list[tuple[int, str, str]]: """Comment lines whose sentence wraps into the next, or that carry two sentences. @@ -2266,27 +2306,14 @@ def comment_wrap_findings(path: Path, raw: str, lines: list[str]) -> list[tuple[ not end a sentence either, since a directive named mid-sentence leaves a fragment the sentence goes on past. """ - comments = python_comments(raw) if path.suffix.lower() == ".py" else None - if comments is None: - comments = extracted_comments(path, lines) - skip = fenced_lines(lines) - kept = [c for c in comments if c[0] not in skip] - comments = [c._replace(body=without_trailing_directive(path, c.body)) for c in kept] - configured = config_run_lines(comments) + comments, kept, configured = prose_comment_candidates(path, raw, lines) out: list[tuple[int, str, str]] = [] prev_body = "" prev_whole = "" prev_no = 0 for (n, body, leading, _raw), whole in zip(comments, kept, strict=True): - if ( - not body - or n in configured - or NOT_PROSE.search(body) - or BARE_URI.match(body.strip()) - or KEY_ONLY.match(body) - or (SNIPPET.match(body) and not SNIPPET_END.search(body)) - ): + if is_non_prose_comment(n, body, configured): prev_body = "" continue # An unpunctuated Markdown HTML comment is a structural marker, not commentary. @@ -2632,7 +2659,127 @@ def issue_ref_findings( return out -PARSED_RULES = frozenset({"comment-wrap", "comment-case", "comment-added", "issue-ref"}) +PARSED_RULES = frozenset( + {"comment-wrap", "comment-case", "comment-added", "issue-ref", "sentence-length"} +) + + +DOCSTRING_OPEN = re.compile(r"^\s*[rRuU]?(?:\"\"\"|\'\'\'|\"|\')") +DOCSTRING_CLOSE = re.compile(r"(?:\"\"\"|\'\'\'|\"|\')\s*$") +DOCTEST_PROMPT = re.compile(r"^\s*>>>(?:\s|$)") +DOCSTRING_CLOSED = re.compile(r"(?:\"\"\"|\'\'\'|\"|\')\s*$") +FIELD_LINE = re.compile(r"^\s*(?::\w+[^:]*:|\w+(?:\s*\([^)]*\))?:)(?:\s|$)") + + +def comment_sentence_findings( + path: Path, raw: str, lines: list[str], allowed: set[int] | None = None +) -> list[tuple[int, str, str]]: + """Over-cap sentences in the comments of a source file, each judged whole. + + Comment lines on consecutive source lines are one block, so a wrapped sentence counts once. + A line that is not prose, a trailing comment, or a gap ends the block. + The prose filters are shared with `comment_wrap_findings`, so both rules agree on what a comment is. + """ + comments, _kept, configured = prose_comment_candidates(path, raw, lines) + out: list[tuple[int, str, str]] = [] + block: list[tuple[int, str]] = [] + prev_no = 0 + + def flush() -> None: + out.extend(over_cap_findings(block, allowed)) + block.clear() + + for n, body, leading, _raw in comments: + if is_non_prose_comment(n, body, configured): + flush() + continue + if ( + not leading + or n != prev_no + 1 + or LIST_MARKER.match(body) + or FIELD_LINE.match(body) + or body[:1].isupper() + ): + flush() + block.append((n, ENUM_PREFIX.sub("", body).strip())) + prev_no = n + if not leading or body.rstrip().endswith(":"): + flush() + flush() + return out + + +def docstring_sentence_findings( + raw: str, lines: list[str], allowed: set[int] | None = None +) -> list[tuple[int, str, str]]: + """Over-cap sentences in Python docstrings, each judged whole. + + A blank docstring line ends a paragraph, as a blank line does in Markdown. + A doctest and the output under it, and an indented code block, are code and are not counted. + """ + spans = python_docstring_spans(raw, lines) + out: list[tuple[int, str, str]] = [] + numbers = sorted(spans) + groups: list[list[int]] = [] + for n in numbers: + prior = "".join(spans[groups[-1][-1]]) if groups else "" + joined = bool(groups) and n == groups[-1][-1] + 1 + here = "".join(spans[n]) + if ( + joined + and DOCSTRING_CLOSED.search(prior) + and DOCSTRING_OPEN.match(here) + and here[:1].strip() + ): + joined = False + if joined: + groups[-1].append(n) + else: + groups.append([n]) + for group in groups: + texts = ["".join(spans[n]).expandtabs(4) for n in group] + texts[0] = DOCSTRING_OPEN.sub("", texts[0], count=1) + texts[-1] = DOCSTRING_CLOSE.sub("", texts[-1], count=1) + rest = [t for t in texts[1:] if t.strip()] + base = min((len(t) - len(t.lstrip()) for t in rest), default=0) + if len(texts) > 1 and texts[-1] and not texts[-1].strip(): + base = min(base, len(texts[-1])) if rest else len(texts[-1]) + block: list[tuple[int, str]] = [] + in_doctest = in_code = False + prev_blank = True + prev_colon = False + for idx, (n, text) in enumerate(zip(group, texts, strict=True)): + indent = len(text) - len(text.lstrip()) if idx else base + if not text.strip(): + out.extend(over_cap_findings(block, allowed)) + block.clear() + in_doctest = in_code = False + prev_blank = True + prev_colon = False + continue + if DOCTEST_PROMPT.match(text): + out.extend(over_cap_findings(block, allowed)) + block.clear() + in_doctest = True + elif in_doctest or (in_code and indent >= base + 4): + pass + elif (prev_blank or prev_colon) and indent >= base + 4: + out.extend(over_cap_findings(block, allowed)) + block.clear() + in_code = True + else: + in_code = False + if LIST_MARKER.match(text) or FIELD_LINE.match(text): + out.extend(over_cap_findings(block, allowed)) + block.clear() + block.append((n, text.strip())) + if text.rstrip().endswith(":"): + out.extend(over_cap_findings(block, allowed)) + block.clear() + prev_blank = False + prev_colon = text.rstrip().endswith(":") + out.extend(over_cap_findings(block, allowed)) + return out def check_file( @@ -2696,8 +2843,13 @@ def check_file( if {"spelling", "dupword"} & rules and path.suffix != ".md": for ln, text, _leading, _raw in extracted_comments(path, lines): comments.setdefault(ln, []).append(text) - if "sentence-length" in rules and path.suffix == ".md": - out.extend(sentence_length_findings(lines, allowed)) + if "sentence-length" in rules: + if path.suffix == ".md": + out.extend(sentence_length_findings(lines, allowed)) + elif path.suffix.lower() != ".md": + out.extend(comment_sentence_findings(path, raw, lines, allowed)) + if path.suffix.lower() == ".py": + out.extend(docstring_sentence_findings(raw, lines, allowed)) in_fence = False prev_txt = "" prev_no = 0 diff --git a/.github/skills/comment-and-doc-style/SKILL.md b/.github/skills/comment-and-doc-style/SKILL.md index 5f29c390e..aca382d08 100644 --- a/.github/skills/comment-and-doc-style/SKILL.md +++ b/.github/skills/comment-and-doc-style/SKILL.md @@ -128,10 +128,10 @@ restricted. Each structural rule a pattern can reach lands as a `prose_lint.py` incrementally, and this section names each check as it ships. - **Short sentences: at most 25 words in one sentence**, ASD-STE100's descriptive cap, checked by - the `sentence-length` rule in `prose_lint.py`. The check is opt-in like `sentence-split`, - because the existing corpus predates the cap and a default gate would fail whole files nobody - is editing. Write new prose under the cap, and scope a run to a change with - `--check sentence-length --diff `. + the `sentence-length` rule in `prose_lint.py`. The rule reads Markdown prose, code comments, and + docstrings. The check is opt-in like `sentence-split`, because the existing corpus predates the + cap and a default gate would fail whole files nobody is editing. Write new prose under the cap, + and scope a run to a change with `--check sentence-length --diff `. - **One instruction per sentence.** A procedure step states one action, and a second action is a second step. No pattern reaches this, so it is authoring discipline with no check. - **Active voice, imperative mood for procedure steps.** Write "run the gate", never "the gate diff --git a/scripts/README.md b/scripts/README.md index e552bff8e..ee5808c76 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -182,7 +182,7 @@ python3 scripts/pr_review.py attest 452 --repo ptr727/ProjectTemplate --checkout **Do not infer one field's spelling from another's.** Several states print upper-case, `merge=` reproduces GitHub's own enum verbatim, and a field can arrive behind a prefix, so a pattern built by analogy with a neighboring field silently never fires. `review_on_head` is the one that catches a watcher, rendering `yes` against `NO`, so a pattern written for `YES` waits out its whole bound over a review that landed. A watcher that cannot fire reads exactly like a review that has not landed, so run the command once, read a real line, and write the pattern against that. Anchor the pattern on what is unique to the field being read, meaning that field's own name with its value, and for a pair nested in a parenthesized group the group's name ahead of it, since values repeat across fields and the groups carry the same inner names as each other. A watcher testing for a bare negative matches a line a covered review wrote, so it reports no review over a review that landed and waits out its whole bound saying so. That one matches on the wrong evidence rather than failing to match, which is worse than the casing trap above, since its answer is the digest's own inverted rather than absent. Give the watcher a branch that exits non-zero where the digest cannot be read at all, or a pattern that never fires and a command that never ran are equally silent. -`wait` exits `30` when the review is still pending at the timeout, which is pending rather than failed. On an attested held head it exits `30` with `status=CHECKS_PENDING` at the timeout. That is a required check still settling, one running long included, or any check while no required one has posted. It is also no check posted on a `BLOCKED`, `BEHIND`, or `DRAFT` merge, or the merge still `UNKNOWN`. A merge reading `CLEAN`, `UNSTABLE`, or `HAS_HOOKS` ends the poll ahead of all of these. On a `BLOCKED` merge three shapes of required check end the poll at once even while another required check still runs, and the exit is then `44` unless a review verdict outranks it. A failed check ends it on the first read. A status expected and never posted, and a check queued with no runner, end it only once each is past `--check-grace`. A required check merely running long ends nothing. A round requested while that poll runs ends it with `status=PENDING`, so the next `wait` polls for that round. A quota reading outranks that line with its own `46` or `47`, and an answer outside a review with `40`. `--min-rounds N` counts what `status` prints as `rounds=`, every round the reviewer posted on the pull request. That count spans every head and includes refusals, and the wait holds out for a round past the Nth. N is therefore the `rounds=` a `status` read before a re-request. One above it waits for the round after the one the re-request was for, so the round that lands leaves the wait at `30`. Where that round lands during the wait, the `status=PENDING` line names the position it still waits for. A round already there when `wait` began leaves the bare line, since nothing then tells it apart from a correct count. `wait`'s failure mode is a wrong answer rather than a crash, so the cases feed crafted GraphQL payloads: a review attributed to the wrong login, a review counted against a stale head, an unknown author's open thread left uncounted, and a wait that returns success while nothing landed. One case reads the reviewer login out of the runbook rather than restating it, since GraphQL drops the `[bot]` suffix REST carries, and another holds the script to exactly the four mutation documents named `addComment`, `requestReviews`, `addPullRequestReviewThreadReply`, and `resolveReviewThread`, so a fifth arriving is a write nobody reviewed as one. +`wait` exits `30` when the review is still pending at the timeout, which is pending rather than failed. On an attested held head it exits `30` with `status=CHECKS_PENDING` at the timeout. That is a required check still settling, one running long included, or any check while no required one has posted. It is also a check the base branch's rules require that the head has not registered yet, on a `BLOCKED` merge whose rollup was read whole. It is also no check posted on a `BLOCKED`, `BEHIND`, or `DRAFT` merge, or the merge still `UNKNOWN`. A merge reading `CLEAN`, `UNSTABLE`, or `HAS_HOOKS` ends the poll ahead of all of these. On a `BLOCKED` merge three shapes of required check end the poll at once even while another required check still runs, and the exit is then `44` unless a review verdict outranks it. A failed check ends it on the first read. A status expected and never posted, and a check queued with no runner, end it only once each is past `--check-grace`. A required check merely running long ends nothing. A round requested while that poll runs ends it with `status=PENDING`, so the next `wait` polls for that round. A quota reading outranks that line with its own `46` or `47`, and an answer outside a review with `40`. `--min-rounds N` counts what `status` prints as `rounds=`, every round the reviewer posted on the pull request. That count spans every head and includes refusals, and the wait holds out for a round past the Nth. N is therefore the `rounds=` a `status` read before a re-request. One above it waits for the round after the one the re-request was for, so the round that lands leaves the wait at `30`. Where that round lands during the wait, the `status=PENDING` line names the position it still waits for. A round already there when `wait` began leaves the bare line, since nothing then tells it apart from a correct count. `wait`'s failure mode is a wrong answer rather than a crash, so the cases feed crafted GraphQL payloads: a review attributed to the wrong login, a review counted against a stale head, an unknown author's open thread left uncounted, and a wait that returns success while nothing landed. One case reads the reviewer login out of the runbook rather than restating it, since GraphQL drops the `[bot]` suffix REST carries, and another holds the script to exactly the four mutation documents named `addComment`, `requestReviews`, `addPullRequestReviewThreadReply`, and `resolveReviewThread`, so a fifth arriving is a write nobody reviewed as one. `wait` exits `64` when the target is out of scope, the same refusal `comment` and `reply` raise, checked before either the auto-request or the backoff loop reaches GitHub. The `reply` paragraph below states what that refusal covers. diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 5e2b01576..49155575a 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -268,7 +268,8 @@ no wait clears: queued with nothing acting on it, expected and never posted, running far past what the job costs, or failed, though on a held head a required check running long is still polled, as 30 states. A check merely still running normally is not this and - exits 0, or on a held head is polled while it can still move the merge. Neither is a + exits 0, or on a held head is polled while it can still move the merge, which includes + a context the base branch's rules require and the head has not yet registered. Neither is a stuck check its rollup node reads as not required, since the rollup carries checks no ruleset requires, nor any stuck check on a merge that is not BLOCKED. The digest reports the check in every case, so a shape outside 44 is still named rather than lost. @@ -340,6 +341,7 @@ import tarfile import time import unicodedata +import urllib.parse from collections.abc import Callable, Iterable, Iterator from datetime import UTC, datetime from pathlib import Path @@ -780,6 +782,7 @@ def strip_fences( # The REST compare endpoint's own ceiling on the file list it returns, which it does not flag. COMPARE_FILE_CAP = 300 READ_RETRY_DELAYS = (2, 5) +ATTEST_HEAD_DELAYS = (3, 5, 10, 15, 15, 12) # What a git ref may not hold before it is interpolated into a REST path. # Refused rather than allow-listed, git permitting more than a branch name usually carries. # An allow-list of the usual ASCII refused `release/1.0+build` and any non-ASCII name alike. @@ -958,6 +961,10 @@ def strip_fences( query($o:String!,$r:String!,$n:Int!){ repository(owner:$o,name:$r){ pullRequest(number:$n){ headRefOid baseRefName } }} """ +Q_ATTEST_BRANCH = """ +query($o:String!,$r:String!,$n:Int!){ + repository(owner:$o,name:$r){ pullRequest(number:$n){ state headRefName headRepository{ url } } }} +""" ATTESTATION = re.compile( r"^$", re.MULTILINE, @@ -3646,7 +3653,12 @@ def checks_settling(nodes: list[dict], now: datetime, grace: float, stall: float def held_checks_open( - pr: dict, nodes: list[dict], now: datetime, grace: float, stall: float + pr: dict, + nodes: list[dict], + now: datetime, + grace: float, + stall: float, + required: frozenset[str] = frozenset(), ) -> bool: """True where a covered head's checks can still move its merge, so a held wait polls on. @@ -3661,6 +3673,12 @@ def held_checks_open( reading UNKNOWN holds it whatever the rollup carries, since GitHub has not yet decided the word the exit code is chosen by. A pull request no longer open has no merge left to move, and GitHub reads UNKNOWN on it for good. + + A BLOCKED merge also holds it, where the rollup was read whole, while a context the base + branch's rules require is absent from it by exact name, which `required` carries. The rollup + only knows the checks that have registered, so a push whose other workflows have not started + reads five of five passed and BLOCKED. An empty `required` is a rules read that found nothing or did not happen, and it + holds nothing. """ if pr.get("state", "OPEN") != "OPEN": return False @@ -3679,9 +3697,58 @@ def held_checks_open( return True if settling and not any(n.get("required") for n in nodes): return True + if ( + merge == "BLOCKED" + and not checks_unreadable(pr) + and not checks_truncated(pr) + and required - {n["name"] for n in nodes if not n.get("unreadable")} + ): + return True return not nodes and merge in ("BLOCKED", "BEHIND", "DRAFT") and not checks_unreadable(pr) +def required_for(pr: dict, owner: str, repo: str) -> frozenset[str]: + """The required contexts a BLOCKED merge can be short of, empty wherever none can matter.""" + if pr.get("mergeStateStatus") != "BLOCKED": + return frozenset() + return required_contexts(owner, repo, pr.get("baseRefName") or "") or frozenset() + + +@functools.cache +def required_contexts(owner: str, repo: str, base: str) -> frozenset[str] | None: + """Every status check context the base branch's rules require, or None where it was unread. + + Read from the rules rather than the rollup, since the rollup lists only the checks that have + registered on the head, which is exactly the set a push in flight is short of. None rather than + an empty set wherever the read did not happen, so an unreadable rules endpoint never reads as + nothing being required. The read is one page of a hundred rules, and a full page is taken as + possibly cut short and returned as None. Memoized for the run, which also prints the note once. + """ + safe = base and not UNSAFE_REF.search(base) and not DOT_SEGMENT.search(base) + read = ( + reread_json( + f"repos/{owner}/{repo}/rules/branches/{urllib.parse.quote(base, safe='')}?per_page=100", + '{n: length, c: [.[] | select(.type=="required_status_checks")' + " | .parameters.required_status_checks[].context]}", + ) + if safe + else None + ) + answer = read if isinstance(read, dict) else {} + contexts = answer.get("c") + if ( + not isinstance(contexts, list) + or not all(isinstance(c, str) and c for c in contexts) + or answer.get("n", 100) >= 100 + ): + print( + f"note: the rules for {base} were unread, so which checks they require is unknown and " + "this wait cannot hold for one that has not registered yet." + ) + return None + return frozenset(contexts) + + def checks_truncated(pr: dict) -> bool: """True where the head's rollup carries more contexts than the query asked for. @@ -4858,11 +4925,17 @@ def attest(owner: str, repo: str, num: int, checkout: str) -> int: f"status=CHECKOUT_NOT_READ nothing was written: {checkout} is not a readable checkout" ) return 67 + registered = True + if local.stdout.strip() != head: + head, registered = await_registered_head( + owner, repo, num, checkout, local.stdout.strip(), head + ) if local.stdout.strip() != head: print( f"status=CHECKOUT_NOT_THE_HEAD nothing was written: the checkout is at " f"{local.stdout.strip()[:8]} and the pull request's head is {head[:8]}, so push or " "fetch until they agree" + + ("" if registered else ". The push landed and GitHub had not registered it yet") ) return 67 if dirty.stdout.strip(): @@ -4925,6 +4998,40 @@ def attest(owner: str, repo: str, num: int, checkout: str) -> int: return comment_on_pr(owner, repo, num, body) +def await_registered_head( + owner: str, repo: str, num: int, checkout: str, local: str, head: str +) -> tuple[str, bool]: + """The pull request's head after waiting for GitHub to register a push, and whether it did. + + A push moves the remote branch at once and the pull request's head a moment later, so an + attest straight after a push read the old head and refused. The wait applies only where the + remote branch tip is the checkout's own commit, which is what makes the head merely late. + Any other tip, or one that cannot be read, returns at once with the head as read. + """ + meta = gql(Q_ATTEST_BRANCH, owner, repo, num) or {} + ref, source = meta.get("headRefName") or "", (meta.get("headRepository") or {}).get("url") or "" + if ( + meta.get("state") != "OPEN" + or not ref + or not source + or UNSAFE_REF.search(ref) + or DOT_SEGMENT.search(ref) + ): + return head, True + tip = _git(checkout, "ls-remote", source, f"refs/heads/{ref}") + if tip.returncode != 0 or tip.stdout.split()[:1] != [local]: + return head, True + first = head + for delay in ATTEST_HEAD_DELAYS: + time.sleep(delay) + head = (gql(Q_ATTEST_TARGET, owner, repo, num) or {}).get("headRefOid") or head + if head == local: + return head, True + if head != first: + return head, True + return head, False + + def receipt_reading(checkout: str, base: str) -> tuple[bool, str, str]: """Whether a recorded pass covers the checkout's content, its findings, and what was read. @@ -5686,6 +5793,7 @@ def still_held(polled: dict) -> bool: datetime.now(UTC), a.check_grace, a.check_stall, + required_for(polled, owner, repo), ) ) @@ -5774,11 +5882,19 @@ def held_read() -> dict: # Only once the review itself is sound does a stuck required check decide the code. if verdict: return verdict - if covered and held_checks_open(final, checks, now, a.check_grace, a.check_stall): + if covered and held_checks_open( + final, + checks, + now, + a.check_grace, + a.check_stall, + required_for(final, owner, repo), + ): print( "status=CHECKS_PENDING an attested local pass covers this head, and by the " - "timeout a check had not concluded, none had posted, or the merge was still " - "UNKNOWN, so the merge is not ready yet: wait again, or read the checks above" + "timeout a check had not concluded, none had posted, a check the base branch " + "requires had not registered, or the merge was still UNKNOWN, so the merge is " + "not ready yet: wait again, or read the checks above" ) return 30 # The review loop closing is not the merge gate, and 0 alone was saying it was. diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index 16140fad4..da7af79df 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -504,6 +504,8 @@ def setUp(self) -> None: self.addCleanup(pr_review.tree_changed_at.cache_clear) pr_review.run_cause.cache_clear() self.addCleanup(pr_review.run_cause.cache_clear) + pr_review.required_contexts.cache_clear() + self.addCleanup(pr_review.required_contexts.cache_clear) self.enterContext( mock.patch.object( pr_review, @@ -7916,6 +7918,55 @@ def test_an_unattested_held_head_is_not_polled_for_its_checks(self) -> None: self.assertEqual(49, self.cli(["wait", "7", "--timeout", "1"])) slept.assert_not_called() + def rules(self, *contexts: str) -> mock.MagicMock: + """Patch `gh_rest` to answer the base branch's rules read with these required contexts.""" + out = json.dumps({"n": 2, "c": list(contexts)}) + return self.enterContext( + mock.patch.object( + pr_review, "gh_rest", return_value=subprocess.CompletedProcess([], 0, out, "") + ) + ) + + def test_a_required_check_not_yet_registered_holds_an_attested_head(self) -> None: + """Five of five passed and BLOCKED is a push whose other workflows have not registered.""" + self.rules("Check pull request workflow status job", "Later workflow") + short = self.into( + payload([review(oid=OLD)], merge="BLOCKED", checks=[check()]), attest=True + ) + full = self.into( + payload([review(oid=OLD)], checks=[check(), check(name="Later workflow")]), attest=True + ) + self.answer(short, short, full) + self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)]) + with mock.patch.object(pr_review.time, "sleep") as slept: + self.assertEqual(0, self.cli(["wait", "7"])) + slept.assert_called_once() + self.out.seek(0) + self.out.truncate() + self.answer(short) + self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)]) + with mock.patch.object(pr_review.time, "sleep"): + self.assertEqual(30, self.cli(["wait", "7", "--timeout", "0"])) + self.assertIn("status=CHECKS_PENDING", self.out.getvalue()) + + def test_every_required_check_registered_does_not_hold_a_blocked_head(self) -> None: + self.rules("Check pull request workflow status job") + pr = self.into(payload([review(oid=OLD)], merge="BLOCKED", checks=[check()]), attest=True) + self.answer(pr) + self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)]) + with mock.patch.object(pr_review.time, "sleep") as slept: + self.assertNotEqual(30, self.cli(["wait", "7", "--timeout", "0"])) + slept.assert_not_called() + + def test_an_unread_rules_read_keeps_the_old_result_and_says_so(self) -> None: + """The suite's default `gh_rest` fails, which must never read as nothing being required.""" + pr = self.into(payload([review(oid=OLD)], merge="BLOCKED", checks=[check()]), attest=True) + self.answer(pr) + self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)]) + with mock.patch.object(pr_review.time, "sleep"): + self.assertNotEqual(30, self.cli(["wait", "7", "--timeout", "0"])) + self.assertIn("rules for develop were unread", self.out.getvalue()) + def test_a_check_nothing_requires_does_not_hold_an_attested_head(self) -> None: """A merge reading CLEAN, UNSTABLE, or HAS_HOOKS has no required check outstanding.""" running = check(status="IN_PROGRESS", conclusion="", started=real_ago(60)) @@ -10171,6 +10222,67 @@ def test_each_write_command_refuses_before_reaching_either_transport(self) -> No gh_graphql.assert_not_called() +class TestRequiredContexts(unittest.TestCase): + """The rules read names what is required, and the hold arm compares it with the rollup.""" + + def setUp(self) -> None: + pr_review.required_contexts.cache_clear() + self.addCleanup(pr_review.required_contexts.cache_clear) + self.enterContext(contextlib.redirect_stdout(io.StringIO())) + + def read(self, out: str, code: int = 0) -> frozenset[str] | None: + proc = subprocess.CompletedProcess([], code, out, "") + with mock.patch.object(pr_review, "gh_rest", return_value=proc): + return pr_review.required_contexts("o", "r", "develop") + + def test_the_contexts_are_read_from_the_rules(self) -> None: + self.assertEqual(frozenset({"a", "b"}), self.read('{"n": 3, "c": ["a", "b"]}')) + + def test_a_slash_in_the_base_is_one_path_segment(self) -> None: + proc = subprocess.CompletedProcess([], 0, '{"n": 1, "c": []}', "") + with mock.patch.object(pr_review, "gh_rest", return_value=proc) as rest: + pr_review.required_contexts("o", "r", "release/1.0") + self.assertIn("/rules/branches/release%2F1.0?", rest.call_args.args[0]) + + def test_a_read_that_did_not_happen_is_none_not_empty(self) -> None: + for out, code in (("", 1), ("not json", 0), ('{"n": 1}', 0), ('{"n": 100, "c": []}', 0)): + with self.subTest(out=out, code=code): + pr_review.required_contexts.cache_clear() + self.assertIsNone(self.read(out, code)) + + def test_blocked_with_a_required_context_absent_holds(self) -> None: + pr = payload([], merge="BLOCKED", checks=[check()]) + nodes = pr_review.check_nodes(pr) + now = datetime.now(UTC) + want = frozenset({"Check pull request workflow status job", "Later"}) + self.assertTrue(pr_review.held_checks_open(pr, nodes, now, 600, 3600, want)) + self.assertFalse(pr_review.held_checks_open(pr, nodes, now, 600, 3600, want - {"Later"})) + self.assertFalse(pr_review.held_checks_open(pr, nodes, now, 600, 3600)) + clean = {**pr, "mergeStateStatus": "CLEAN"} + self.assertFalse(pr_review.held_checks_open(clean, nodes, now, 600, 3600, want)) + + def test_a_rollup_not_read_whole_never_holds_on_an_absent_context(self) -> None: + now = datetime.now(UTC) + want = frozenset({"Later"}) + other = payload([], merge="BLOCKED", checks=[check()], rollup_oid="f" * 40) + self.assertFalse( + pr_review.held_checks_open(other, pr_review.check_nodes(other), now, 600, 3600, want) + ) + cut = payload([], merge="BLOCKED", checks=[check()]) + cut["commits"]["nodes"][0]["commit"]["statusCheckRollup"]["contexts"]["pageInfo"] = { + "hasNextPage": True + } + self.assertFalse( + pr_review.held_checks_open(cut, pr_review.check_nodes(cut), now, 600, 3600, want) + ) + + def test_the_rules_are_read_only_for_a_blocked_merge(self) -> None: + with mock.patch.object(pr_review, "gh_rest") as rest: + clean = payload([], merge="CLEAN", checks=[check()]) + self.assertEqual(frozenset(), pr_review.required_for(clean, "o", "r")) + rest.assert_not_called() + + class TestAttest(unittest.TestCase): """A local pass is published only for the content the pull request's head carries.""" @@ -10213,7 +10325,8 @@ def post(_o: str, _r: str, _n: int, body: str) -> int: return 0 self.enterContext(mock.patch.object(pr_review, "comment_on_pr", side_effect=post)) - self.enterContext(contextlib.redirect_stdout(io.StringIO())) + self.out = io.StringIO() + self.enterContext(contextlib.redirect_stdout(self.out)) def run_attest(self, head: str, check_exit: int = 0, base: str | None = None) -> int: """Run `attest` with a stub `local_review.py` that records how it was called.""" @@ -10251,6 +10364,78 @@ def test_a_covered_head_is_attested_by_its_full_commit(self) -> None: for cwd, _ in calls: self.assertEqual(os.path.realpath(self.dir), os.path.realpath(cwd)) + def lagging(self, tip: str, heads: list[str]) -> int: + """Run `attest` while the pull request's head reads each of `heads` in turn.""" + replies = iter(heads) + target = {"headRefOid": heads[0], "baseRefName": "develop"} + + def gql(query: str, *_: object) -> dict: + if query == pr_review.Q_ATTEST_BRANCH: + return { + "state": "OPEN", + "headRefName": "feature", + "headRepository": {"url": "https://x.test/o/r"}, + } + return {**target, "headRefOid": next(replies, heads[-1])} + + self.slept: list[float] = [] + remote = subprocess.CompletedProcess([], 0, f"{tip}\trefs/heads/feature\n", "") + original = pr_review._git + + def git(checkout: str, *args: str) -> subprocess.CompletedProcess: + return remote if args[0] == "ls-remote" else original(checkout, *args) + + merge_base = subprocess.CompletedProcess([], 0, self.head + "\n", "") + stub = Path(self.dir).parent / "stub-lag.py" + stub.write_text( + "import json, sys\n" + "print(json.dumps({'covered': True, 'receiptProblems': [], 'findings': {'a': 0}}))\n" + ) + self.addCleanup(stub.unlink) + with ( + mock.patch.object(pr_review, "gql", side_effect=gql), + mock.patch.object(pr_review, "_git", side_effect=git), + mock.patch.object(pr_review, "gh_rest", return_value=merge_base), + mock.patch.object(pr_review, "LOCAL_REVIEW", stub), + mock.patch.object(pr_review.time, "sleep", side_effect=self.slept.append), + ): + return pr_review.attest("o", "r", 7, self.dir) + + def test_a_head_that_catches_up_within_the_bound_is_attested(self) -> None: + old = "d" * 40 + self.assertEqual(0, self.lagging(self.head, [old, old, self.head])) + self.assertEqual(2, len(self.slept)) + self.assertEqual(1, len(self.posted)) + + def test_a_head_that_never_catches_up_is_refused_after_the_bound(self) -> None: + old = "d" * 40 + self.assertEqual(67, self.lagging(self.head, [old])) + self.assertLessEqual(sum(self.slept), 60) + self.assertGreater(sum(self.slept), 0) + self.assertIn("had not registered it yet", self.out.getvalue()) + self.assertEqual([], self.posted) + + def test_a_closed_pull_request_is_refused_without_waiting(self) -> None: + old = "d" * 40 + replies = {"state": "MERGED", "headRefName": "feature", "headRepository": {"url": "u"}} + with mock.patch.object(pr_review, "gql", return_value=replies): + head, registered = pr_review.await_registered_head( + "o", "r", 7, self.dir, self.head, old + ) + self.assertEqual((old, True), (head, registered)) + + def test_a_head_that_moves_to_a_third_commit_is_not_called_unregistered(self) -> None: + old, other = "d" * 40, "c" * 40 + self.assertEqual(67, self.lagging(self.head, [old, other])) + self.assertEqual(1, len(self.slept)) + self.assertNotIn("had not registered", self.out.getvalue()) + + def test_a_remote_tip_that_is_not_the_checkout_is_refused_at_once(self) -> None: + old = "d" * 40 + self.assertEqual(67, self.lagging("e" * 40, [old])) + self.assertEqual([], self.slept) + self.assertNotIn("had not registered", self.out.getvalue()) + def test_a_checkout_measuring_another_merge_base_is_refused(self) -> None: self.assertEqual(67, self.run_attest(self.head, base="e" * 40)) self.assertEqual([], self.posted) diff --git a/tests/test_prose_lint.py b/tests/test_prose_lint.py index 4f1b4f357..c6dbee4b9 100755 --- a/tests/test_prose_lint.py +++ b/tests/test_prose_lint.py @@ -2483,10 +2483,10 @@ def test_a_sentence_starting_mid_line_is_reported_on_that_line(self) -> None: tail = f"Done. {' '.join(['word'] * 20)}\n{' '.join(['word'] * 10)}.\n" self.assertEqual([(2, "sentence-length")], self.found("Intro line.\n" + tail)) - def test_the_rule_is_markdown_only(self) -> None: - """A source file's long lines are code, which no sentence rule judges.""" - over = self.sentence_of(prose_lint.SENTENCE_WORD_CAP + 1) - self.assertEqual([], self.kinds(f"# {over}\n", {"sentence-length"}, name="bait.py")) + def test_a_source_line_that_is_not_a_comment_is_not_judged(self) -> None: + """A source file's long code lines are code, which no sentence rule judges.""" + over = " ".join(["word"] * (prose_lint.SENTENCE_WORD_CAP + 1)) + self.assertEqual([], self.kinds(f'x = "{over}."\n', {"sentence-length"}, name="bait.py")) def test_the_rule_is_opt_in(self) -> None: """Adoption is incremental, so the default set stays unchanged until a promotion.""" @@ -2494,6 +2494,180 @@ def test_the_rule_is_opt_in(self) -> None: self.assertIn("sentence-length", prose_lint.RULES) +class TestCommentSentenceLength(BaitCase): + """The word cap reaches comments and docstrings, where the prose of a source file lives.""" + + def words(self, count: int) -> str: + return " ".join(["word"] * count) + + def over(self) -> str: + return self.words(prose_lint.SENTENCE_WORD_CAP + 1) + "." + + def at_cap(self) -> str: + return self.words(prose_lint.SENTENCE_WORD_CAP) + "." + + def found(self, text: str, name: str, allowed: set[int] | None = None) -> list[tuple[int, str]]: + path = self.tmp / name + path.write_text(text, encoding="utf-8") + found = prose_lint.check_file(path, {"sentence-length"}, allowed=allowed) + return [(n, kind) for n, kind, _ in found] + + def test_an_over_cap_shell_comment_is_flagged(self) -> None: + self.assertEqual( + [(2, "sentence-length")], self.found(f"set -e\n# {self.over()}\nrun\n", "bait.sh") + ) + + def test_an_at_cap_shell_comment_passes(self) -> None: + self.assertEqual([], self.found(f"set -e\n# {self.at_cap()}\nrun\n", "bait.sh")) + + def test_an_over_cap_yaml_comment_is_flagged(self) -> None: + self.assertEqual( + [(1, "sentence-length")], self.found(f"# {self.over()}\nname: value\n", "bait.yml") + ) + + def test_a_wrapped_sentence_is_counted_whole_and_reported_once(self) -> None: + half = self.words(prose_lint.SENTENCE_WORD_CAP // 2 + 1) + text = f"x=1\n# {half}\n# {half}.\ny=2\n" + self.assertEqual([(2, "sentence-length")], self.found(text, "bait.sh")) + + def test_a_gap_ends_the_block(self) -> None: + half = self.words(prose_lint.SENTENCE_WORD_CAP // 2 + 1) + text = f"# {half}\nx=1\n# {half}.\n" + self.assertEqual([], self.found(text, "bait.sh")) + + def test_a_code_span_in_a_comment_is_one_word(self) -> None: + span = "`" + self.words(prose_lint.SENTENCE_WORD_CAP) + "`" + self.assertEqual([], self.found(f"# The value {span} is read.\n", "bait.sh")) + + def test_the_diff_scope_drops_an_unchanged_sentence_and_keeps_a_changed_one(self) -> None: + half = self.words(prose_lint.SENTENCE_WORD_CAP // 2 + 1) + text = f"# {self.over()}\nx=1\n# {half}\n# {half}.\n" + self.assertEqual([], self.found(text, "bait.sh", allowed={2})) + self.assertEqual([(4, "sentence-length")], self.found(text, "bait.sh", allowed={4})) + + def test_an_over_cap_docstring_sentence_is_flagged(self) -> None: + text = f'def f() -> None:\n """{self.over()}"""\n' + self.assertEqual([(2, "sentence-length")], self.found(text, "bait.py")) + + def test_a_multiline_docstring_sentence_is_counted_whole(self) -> None: + half = self.words(prose_lint.SENTENCE_WORD_CAP // 2 + 1) + text = f'def f() -> None:\n """Summary.\n\n {half}\n {half}.\n """\n' + self.assertEqual([(4, "sentence-length")], self.found(text, "bait.py")) + + def test_a_blank_line_ends_a_docstring_paragraph(self) -> None: + half = self.words(prose_lint.SENTENCE_WORD_CAP // 2 + 1) + text = f'def f() -> None:\n """Summary.\n\n {half}\n\n {half}.\n """\n' + self.assertEqual([], self.found(text, "bait.py")) + + def test_a_doctest_line_is_not_counted(self) -> None: + text = ( + 'def f() -> None:\n """Summary.\n\n' + f" >>> print({self.words(prose_lint.SENTENCE_WORD_CAP + 1)})\n" + ' """\n' + ) + self.assertEqual([], self.found(text, "bait.py")) + + def test_an_indented_code_block_is_not_counted(self) -> None: + text = f'def f() -> None:\n """Summary.\n\n {self.over()}\n """\n' + self.assertEqual([], self.found(text, "bait.py")) + + def test_a_colon_line_opens_an_indented_command_block_that_is_not_counted(self) -> None: + run = " ".join(["tool list item"] * 5) + text = f'def f() -> None:\n """Usage:\n {run}\n {run}\n """\n' + self.assertEqual([], self.found(text, "bait.py")) + + def test_unpunctuated_docstring_bullets_are_separate_sentences(self) -> None: + item = self.words(9) + text = f'def f() -> None:\n """Summary.\n\n - {item}\n - {item}\n - {item}\n """\n' + self.assertEqual([], self.found(text, "bait.py")) + + def test_unpunctuated_comment_bullets_are_separate_sentences(self) -> None: + item = self.words(9) + text = f"# Steps:\n# - {item}\n# - {item}\n# - {item}\n" + self.assertEqual([], self.found(text, "bait.sh")) + + def test_adjacent_docstrings_are_judged_apart(self) -> None: + half = self.words(13) + text = f'class C:\n """{half}"""\n def f(self): """{half}"""\n' + self.assertEqual([], self.found(text, "bait.py")) + + def test_a_tab_indented_docstring_code_block_is_not_counted(self) -> None: + text = f'def f() -> None:\n\t"""Summary.\n\n\t\t{self.over()}\n\t"""\n' + self.assertEqual([], self.found(text, "bait.py")) + + def test_an_ellipsis_opening_prose_is_not_a_doctest(self) -> None: + text = f'def f() -> None:\n """Summary.\n\n ... {self.over()}\n """\n' + self.assertEqual([(4, "sentence-length")], self.found(text, "bait.py")) + + def test_doctest_output_and_continuation_lines_are_not_counted(self) -> None: + long = self.words(prose_lint.SENTENCE_WORD_CAP + 1) + text = ( + 'def f() -> None:\n """Summary.\n\n >>> x = (\n ... 1)\n' + f' {long}\n """\n' + ) + self.assertEqual([], self.found(text, "bait.py")) + + def test_prose_resumes_after_a_docstring_code_block(self) -> None: + text = ( + 'def f() -> None:\n """Summary.\n\n code line\n\n' + f' {self.over()}\n """\n' + ) + self.assertEqual([(6, "sentence-length")], self.found(text, "bait.py")) + + def test_the_diff_scope_reaches_a_docstring_continuation_line(self) -> None: + half = self.words(prose_lint.SENTENCE_WORD_CAP // 2 + 1) + text = f'def f() -> None:\n """Summary.\n\n {half}\n {half}.\n """\n' + self.assertEqual([(5, "sentence-length")], self.found(text, "bait.py", allowed={5})) + + def test_a_trailing_comment_ends_the_block(self) -> None: + half = self.words(prose_lint.SENTENCE_WORD_CAP // 2 + 1) + self.assertEqual([], self.found(f"x=1 # {half}\n# {half}.\n", "bait.sh")) + + def test_a_non_prose_comment_ends_the_block(self) -> None: + half = self.words(prose_lint.SENTENCE_WORD_CAP // 2 + 1) + text = f"# {half}\n# https://example.com/a\n# {half}.\n" + self.assertEqual([], self.found(text, "bait.sh")) + + def test_a_single_quoted_and_a_raw_docstring_are_read(self) -> None: + single = chr(39) * 3 + for opener in ("r" + single, single, 'r"""'): + with self.subTest(opener=opener): + text = f"def f() -> None:\n {opener}{self.over()}{opener[-3:]}\n" + self.assertEqual([(2, "sentence-length")], self.found(text, "bait.py")) + + def test_a_double_backtick_span_is_one_word(self) -> None: + span = "``" + self.words(prose_lint.SENTENCE_WORD_CAP) + "``" + self.assertEqual([], self.found(f"# The value {span} is read.\n", "bait.sh")) + + def test_capitalized_unpunctuated_comment_lines_are_separate_sentences(self) -> None: + line = self.words(9) + text = "".join(f"# Step {line}\n" for _ in range(4)) + self.assertEqual([], self.found(text, "bait.sh")) + + def test_sphinx_field_lines_are_separate_sentences(self) -> None: + field = self.words(8) + fields = "".join(f" :param p{i}: {field}\n" for i in range(4)) + text = f'def f() -> None:\n """Summary.\n\n{fields} """\n' + self.assertEqual([], self.found(text, "bait.py")) + + def test_an_implicit_concatenation_in_a_docstring_code_block_is_one_docstring(self) -> None: + code = "".join(f" call_{i}(a, b, c, d, e, f, g, h)\n" for i in range(4)) + text = ( + 'def f() -> None:\n """Example::\n\n msg = ("first "\n' + f' "second")\n{code} """\n' + ) + self.assertEqual([], self.found(text, "bait.py")) + + def test_a_numbered_comment_marker_is_not_a_word(self) -> None: + text = f"# 1) {self.at_cap()}\n" + self.assertEqual([], self.found(text, "bait.sh")) + + def test_a_python_comment_is_judged_too(self) -> None: + self.assertEqual( + [(1, "sentence-length")], self.found(f"# {self.over()}\nx = 1\n", "bait.py") + ) + + class TestSpelling(BaitCase): """US English, read from Markdown prose and from the comments of every other syntax. @@ -2946,7 +3120,7 @@ def test_a_python_file_the_interpreter_cannot_parse_is_named_by_every_parser_rul """ for name, text in (("open.py", self.UNTOKENIZED), ("newer.py", self.UNPARSED)): bait = self.bait(name, text) - for rule in ("comment-wrap", "comment-case", "issue-ref"): + for rule in ("comment-wrap", "comment-case", "issue-ref", "sentence-length"): with self.subTest(file=name, rule=rule): err = self.scope_line([bait], ["--check", rule]) self.assertIn("1 .py file(s) Python", err)