From 65228f7828f1e54ba9393a2deed41bf8984a99db Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Thu, 13 Aug 2026 10:53:35 +0200 Subject: [PATCH] fix(gate-16): a prettier reformat is not a set of changed methods either MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `.github#395` taught gate-16 that layout is not a change, in PHP. The fleet is now adopting `@nextcloud/prettier-config`, which reformats `.js`/`.ts`/`.vue`/ `.css`/`.scss`, and the same defect is back on the frontend half: pipelinq#820 reports 468 changed methods against a `development` that reports none. #395 deliberately excluded JS trailing commas and JS re-wraps, for three real reasons — array elision, automatic semicolon insertion, and a re-wrap across a `//`. Those are the specification for the JS rules, not a reason to have none. Each is refused explicitly, with its own control. Measured: pipelinq#820 468 -> 11, shillinq#545 197 -> 1, scholiq#329 125 -> 2, openregister#2466 29 -> 1. PHP byte-identical over 3,054 real file pairs. --- hydra-gates/README.md | 20 +- .../scripts/lib/check_spec_coverage.py | 816 +++++++++++++++++- .../scripts/lib/test_check_spec_coverage.py | 262 +++++- .../lib/test_gate16_spec_coverage_scope.sh | 95 ++ .../app/src/views/ReflowedView.vue | 76 ++ .../app/src/views/ReflowedView.vue.prettier | 84 ++ .../views/ReflowedView.vue.prettier-changed | 84 ++ 7 files changed, 1383 insertions(+), 54 deletions(-) create mode 100644 hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue create mode 100644 hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue.prettier create mode 100644 hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue.prettier-changed diff --git a/hydra-gates/README.md b/hydra-gates/README.md index a5b9028a..5f346332 100644 --- a/hydra-gates/README.md +++ b/hydra-gates/README.md @@ -222,8 +222,8 @@ checkout. With a base they run at any file scope; with none they report A delta gate also has to decide what *counts* as a change, and a plain `git diff` answers "a line moved". gate-16 therefore compares each changed file against its own base version with layout normalised away on both sides: brace style (K&R vs -Allman), indentation and intra-line spacing, a PHP trailing comma, two spellings -of one string, and a PHP statement re-wrapped across lines. Adopting +Allman), indentation and intra-line spacing, a trailing comma, two spellings of +one string, and a statement re-wrapped across lines. Adopting `nextcloud/coding-standard` consequently reports nothing, while a changed value, a new parameter or an edited string still reports (`.github#395` — measured 1071 → 0 findings across the fleet's seven adoption PRs, with a positive control @@ -231,6 +231,22 @@ per rule). The narrowing intersects git's own answer, so it can only ever shrink the scope. `git diff -w` does **not** express this: a brace is a token that moved lines, not whitespace that changed width. +`.github#435` extended the same normalisation to **JS / TS / Vue**, for +`@nextcloud/prettier-config`, and added the one rule prettier makes unavoidable: +it re-prints parentheses from its own precedence table, in both directions +(`return (…)` appears, `(a && b) ? c : d` loses its pair). A parenthesis is +dropped only when the expression inside binds **strictly tighter** than both +neighbours, so associativity never enters the argument and `(a || b) && c` keeps +its pair. Four JS-specific hazards are **refused** rather than reasoned about, +each with its own control: an array **elision** (`[a, , b]` is three elements), +an **ASI**-sensitive line break (`return` + `x` is not `return x`), a re-wrap +across a **`//`** (which uncomments what followed, or comments out what follows), +and a line break inside a **template literal** (a character of the string). +Measured on the fleet's prettier PRs: pipelinq#820 **468 → 11**, shillinq#545 +**197 → 1**, scholiq#329 **125 → 2**, openregister#2466 **29 → 1**, with +`development` at PASS throughout and the PHP path byte-identical over 3,054 real +file pairs. + To get the old behaviour explicitly: ```bash diff --git a/hydra-gates/scripts/lib/check_spec_coverage.py b/hydra-gates/scripts/lib/check_spec_coverage.py index aaedaceb..76621dc6 100644 --- a/hydra-gates/scripts/lib/check_spec_coverage.py +++ b/hydra-gates/scripts/lib/check_spec_coverage.py @@ -20,11 +20,18 @@ when a neighbouring method changes. That added-line set is then narrowed to the lines whose CONTENT changed, not -merely their layout (``.github#395``): brace style, indentation, intra-line -spacing and a PHP trailing comma are normalised away on both sides before a -line counts as added. Only a line the normalisation still shows as different -puts a method in scope — and the narrowing is an intersection with git's own -answer, so it can never widen the scope. +merely their layout (``.github#395``, ``.github#435``): brace style, +indentation, intra-line spacing, a trailing comma, quote style, a statement +re-wrapped across lines and — in JS/TS/Vue — a parenthesis prettier re-printed +from its own precedence table are normalised away on both sides before a line +counts as added. Only a line the normalisation still shows as different puts a +method in scope — and the narrowing is an intersection with git's own answer, so +it can never widen the scope. + +Every rule is paired with a control proving a real change still travels through +it (``NormalisationTest`` and ``JsNormalisationTest``), because a normalisation +loose enough to swallow ``+`` -> ``-`` would pass every "reports nothing" arm +and quietly retire the gate. This runs against the CURRENT working directory's git repo (the script is repo-agnostic — it lives in hydra but operates on whatever app is checked out @@ -150,7 +157,7 @@ r"\([^)]*\)\s*\{", ) -# ---- cosmetic-reformat normalisation (.github#395) -------------------------- +# ---- cosmetic-reformat normalisation (.github#395, .github#435) ------------- # # A line whose only difference from its base version is LAYOUT is not a changed # method. `git diff -w` cannot express that: the brace of Nextcloud's K&R style @@ -161,15 +168,124 @@ # Each rule below is narrow enough that the two forms it equates are the same # program. Nothing here is a heuristic about intent. -# `foo() {` -> `foo()`. The brace still has to be SOMEWHERE — Allman puts it on -# its own line, which normalises to the empty string and is dropped — so no -# information is lost, only its position. +# `foo() {` -> `foo()`. PHP ONLY. The brace still has to be SOMEWHERE — Allman +# puts it on its own line, which normalises to the empty string and is dropped — +# so no information is lost, only its position, and php-cs-fixer moves it. +# +# JS/TS/Vue deliberately KEEP the brace (.github#435). prettier never moves an +# opening brace off its line, so the rule buys nothing there, and dropping the +# character actively CORRUPTS the re-wrap comparison below: a mustache split as +# `{{` + `x` + `}}` loses one `{` and stops matching its own +# single-line base; a multi-line `import {` loses the brace that opens the +# specifier list. A `}`-only line was already kept on both languages, so keeping +# `{` makes the two halves of a brace pair symmetric rather than adding a rule. _NORM_TRAILING_BRACE_RE = re.compile(r"\s*\{$") _NORM_WS_RE = re.compile(r"\s+") # A complete single- or double-quoted literal, escapes included. Whitespace # INSIDE one is content and is left exactly as it is: `'a b'` -> `'ab'` is a # change to what a user sees and must stay visible to this gate. _NORM_STRING_RE = re.compile(r"'(?:\\.|[^'\\])*'|\"(?:\\.|[^\"\\])*\"") +# A comma sitting immediately before its own closer — `[a,]`, `f(a,)`, `{k:v,}`. +# In both PHP (7.3+/8.0) and JS (ES2017+) that comma is punctuation, not an +# element, and prettier's `trailingComma: "all"` adds one to every construct it +# breaks over lines. It is dropped ONLY when the character before it is neither +# `[` nor `,` — `[,]` is a one-element array with a HOLE and `[a,,]` is a +# two-element one, and erasing either comma would change the value. +_TRAILING_COMMA_BEFORE_CLOSER_RE = re.compile(r"(?y` is not an arrow function at all. +_JS_NO_PRECEDING_BREAK_RE = re.compile(r"^(?:\+\+|--|=>)") + +# ---- redundant parentheses (.github#435) ------------------------------------ +# +# prettier does not edit the source text, it RE-PRINTS the syntax tree, so every +# parenthesis in its output is one its own precedence table says is needed and +# every parenthesis the author wrote that is not needed is gone. Both directions +# show up in one adoption diff: +# +# return a || b -> return (\n a\n || b\n ) (added: ASI) +# map[s] || (s || '-') -> map[s] || s || '-' (removed) +# x = (a && b) ? c : d -> x = a && b ? c : d (removed) +# x = await f() || {} -> x = (await f()) || {} (added) +# +# Equating those needs PRECEDENCE, which is the one thing #395 refused to reason +# about. It is reasoned about here, conservatively and in one place: a paren pair +# is dropped only when the expression inside it binds STRICTLY TIGHTER than both +# of its neighbours. Strictly, so associativity never enters the argument — +# `a - (b - c)` and `(a - b) - c` are both refused rather than one of them being +# proved. Every construct the table does not recognise is refused too. +# +# The residual risk of a wrong entry is bounded by the caller: the two sides must +# already be character-identical apart from parentheses before this runs, so the +# only edit it could hide is one that changes NOTHING BUT a parenthesis. That is +# a real bug class (a precedence bug), which is why the controls in +# `RedundantParenTest` are all of that exact shape. +_JS_IDENT_CHAR = re.compile(r"[A-Za-z0-9_$]") +# Binding power, higher binds tighter. Longest spelling first — the scanner is +# greedy, so `===` must be tried before `==` and `**` before `*`. +_JS_OPERATORS: tuple[tuple[str, int], ...] = ( + (">>>=", 2), ("**=", 2), ("<<=", 2), (">>=", 2), ("&&=", 2), ("||=", 2), + ("??=", 2), ("===", 9), ("!==", 9), (">>>", 11), + ("**", 14), ("==", 9), ("!=", 9), ("<=", 10), (">=", 10), ("&&", 5), + ("||", 4), ("??", 4), ("<<", 11), (">>", 11), ("+=", 2), ("-=", 2), + ("*=", 2), ("/=", 2), ("%=", 2), ("&=", 2), ("|=", 2), ("^=", 2), + ("=>", 2), ("=", 2), ("?", 3), (":", 3), ("|", 6), ("^", 7), ("&", 8), + ("<", 10), (">", 10), ("+", 12), ("-", 12), ("*", 13), ("/", 13), + ("%", 13), (",", 1), ("!", 14), ("~", 14), +) +# Word-spelled operators. They are only recognisable while the whitespace is +# still there, which is why the re-wrap comparison runs on a SPACE-COLLAPSED +# variant of each line rather than the space-STRIPPED one used to match lines: +# `'value' in ctx` collapses to `'value' in ctx` but strips to `'value'inctx`, +# where `in` is indistinguishable from the tail of an identifier and its binding +# power of 10 would silently read as a primary's 21. +_JS_WORD_BINARY = {"in": 10, "instanceof": 10} +_JS_WORD_UNARY = {"typeof": 14, "void": 14, "delete": 14, "await": 14} +# A word before `(` that is an OPERATOR rather than a callee, with the binding +# power the parenthesised expression must beat to lose its parentheses. +_JS_PREFIX_WORD_CONTEXT = { + "return": 0, "case": 0, # a full Expression, comma operator included + "of": 2, "else": 2, "do": 2, # an AssignmentExpression + "in": 10, "instanceof": 10, + **_JS_WORD_UNARY, +} +# …and the words that make the parentheses load-bearing outright. +_JS_PREFIX_WORD_REFUSE = {"new", "yield", "function", "class"} +# Nothing may be squeezed against a `)` we are about to delete: `(a || b).c`, +# `(f || g)(x)`, `(a || b)[0]`, `(x)?.y` and `(a)++` all read differently once +# the parentheses are gone. `/` is NOT in this list, though it IS refused on the +# LEFT: a `/` before `(` may open a regular expression, whose parentheses are +# capture groups and not grouping at all, but a `/` after `)` can only be +# division — a regular expression may not follow an operand. +_JS_SUFFIX_REFUSE_CHARS = ".([`" +# TypeScript assertions bind looser than they look: `a || b as string` asserts +# only `b`, so `(a || b) as string` is a different expression and the +# parentheses stay. +_JS_TYPE_ASSERTION_WORDS = {"as", "satisfies"} +# The operators whose re-grouping provably preserves the value, so a pair of +# parentheses may go even at EQUAL binding power. `+` is deliberately absent: +# `1 + (2 + '3')` is `'123'` and `1 + 2 + '3'` is `'33'`. +_JS_ASSOCIATIVE_OPS = {"||", "&&", "??"} +# The binding power of an expression with no top-level operator at all — an +# identifier, a literal, a member/call chain. Nothing binds tighter, so a pair of +# parentheses around one is always removable. +_JS_PRIMARY_PREC = 21 +_JS_MAX_PAREN_PASSES = 30 +# How many opcodes either side of one the re-wrap comparison may reach for the +# rest of the construct, and the line ceiling that stops it becoming "compare the +# whole file" — see `_js_rewrap_in_context`. +_JS_CONTEXT_REACH = (0, 1, 2, 3, 4) +_JS_CONTEXT_MAX_LINES = 120 def _unify_quote_style(literal: str) -> str: @@ -196,14 +312,384 @@ def _unify_quote_style(literal: str) -> str: return "'" + body.replace('\\"', '"') + "'" -def _normalise_code_line(line: str) -> str | None: +_JS_BRACKET_PAIR = {"(": ")", "[": "]", "{": "}"} + + +def _js_scan(text: str) -> tuple[list[bool], list[int]] | None: + """``(is_literal_char, bracket_depth)`` per character, or ``None`` when the + text does not scan cleanly — an unterminated quote or a bracket closed by the + wrong kind. ``None`` means every caller refuses, which is the safe direction. + + A region of a diff is a FRAGMENT, not a program: it routinely opens a brace + it never closes (``for (…) {`` is one whole opcode) or closes one it never + opened. Those are tolerated — the depth simply runs on, negative if need be — + because refusing them was measured to cost 30 of pipelinq#820's findings for + no safety at all: an unmatched bracket has no pair, so no pair is analysed. + + A template literal is literal text EXCEPT inside ``${…}``, which is ordinary + expression code and holds parentheses of its own. Both are modelled: the + ``${`` pushes a brace whose matching ``}`` hands the scanner back to template + text, so a nested template inside an interpolation nests correctly, and the + ``in_template`` flag needs no stack of its own. + """ + n = len(text) + literal = [False] * n + depth = [0] * n + stack: list[str] = [] + level = 0 + in_template = False + i = 0 + while i < n: + c = text[i] + depth[i] = level + if in_template: + literal[i] = True + if c == "\\": + if i + 1 < n: + literal[i + 1] = True + depth[i + 1] = level + i += 2 + continue + if c == "`": + in_template = False + i += 1 + continue + if text[i:i + 2] == "${": + # The `{` is pushed like any other bracket, so the + # interpolation's own brackets nest and its `}` is what hands + # the scanner back to template text. + depth[i + 1] = level + stack.append("${") + level += 1 + in_template = False + i += 2 + continue + i += 1 + continue + if c in "'\"": + j = i + 1 + while j < n and text[j] != c: + j += 2 if text[j] == "\\" else 1 + if j >= n: + return None + for k in range(i, j + 1): + literal[k] = True + depth[k] = level + i = j + 1 + continue + if c == "`": + literal[i] = True + in_template = True + i += 1 + continue + if c in "([{": + stack.append(c) + level += 1 + elif c in ")]}": + if stack and stack[-1] == "${": + if c != "}": + return None + stack.pop() + level -= 1 + depth[i] = level + # NOT marked literal: it is the interpolation's delimiter, and a + # `)` that ends right before it has to be able to see a closer on + # its right rather than "a string follows, refuse". + in_template = True + i += 1 + continue + if stack: + if _JS_BRACKET_PAIR[stack[-1]] != c: + return None + stack.pop() + level -= 1 + depth[i] = level + i += 1 + return (literal, depth) if not in_template else None + + +def _js_word_at(text: str, end: int) -> str: + """The identifier ending at ``end`` (exclusive), or ``""``.""" + start = end + while start > 0 and _JS_IDENT_CHAR.match(text[start - 1]): + start -= 1 + return text[start:end] + + +def _js_operator_ending_at(text: str, end: int) -> int | None: + """Binding power of the symbolic operator ending at ``end`` (exclusive).""" + if text[end - 2:end] in ("++", "--"): + return None + for spelling, prec in _JS_OPERATORS: + if text[:end].endswith(spelling): + return prec + return None + + +def _js_operator_starting_at(text: str, start: int) -> int | None: + """Binding power of the symbolic operator starting at ``start``.""" + if text[start:start + 2] in ("++", "--"): + return None + for spelling, prec in _JS_OPERATORS: + if text.startswith(spelling, start): + return prec + return None + + +def _js_inner_binding(inner: str, at_statement_start: bool) -> tuple[int, set[str]] | None: + """``(lowest binding power holding ``inner`` together, its top-level operator + spellings)``, or ``None`` when the expression is one this analysis refuses to + reason about. + + Every top-level operator counts, prefix ones included — ``(-a) * b`` is read + as bound by ``-`` at 12, so it loses to ``*`` at 13 and keeps its + parentheses. Over-counting like that can only ever make the answer smaller, + and a smaller answer only ever refuses. + """ + scanned = _js_scan(inner) + if scanned is None: + return None + literal, depth = scanned + stripped = inner.strip() + if not stripped: + return None + # An object literal, a function or a class expression AT THE HEAD OF A + # STATEMENT: there the parentheses are what stop it being read as a block or + # a declaration. In expression position — after `return`, after an operator — + # `{ a: 1 }[k]` is already an object literal and the parentheses are only + # grouping, so the refusal would be pure cost. + if at_statement_start and (stripped[0] == "{" or re.match(r"^(?:function|class)\b", stripped)): + return None + # `a++ + b` and `a + ++b` are the same characters once the spaces go, so an + # increment anywhere in the expression refuses it. Literal-aware, or a CSS + # class called `badge--off` would refuse every expression mentioning it — + # which is exactly what it did until pipelinq#820 named the method. + code_only = "".join("\x00" if lit else ch for lit, ch in zip(literal, inner)) + if "++" in code_only or "--" in code_only: + return None + prec = _JS_PRIMARY_PREC + ops: set[str] = set() + i = 0 + n = len(inner) + while i < n: + if literal[i] or depth[i] > 0: + i += 1 + continue + c = inner[i] + if _JS_IDENT_CHAR.match(c): + j = i + while j < n and _JS_IDENT_CHAR.match(inner[j]): + j += 1 + word = inner[i:j] + if word in _JS_WORD_BINARY: + prec = min(prec, _JS_WORD_BINARY[word]) + ops.add(word) + elif word in _JS_WORD_UNARY: + prec = min(prec, _JS_WORD_UNARY[word]) + ops.add(word) + elif word in _JS_TYPE_ASSERTION_WORDS: + return None + i = j + continue + if c in "'\" ": + i += 1 + continue + if c in "([{)]}": + i += 1 + continue + spelling = _js_operator_spelling_at(inner, i) + if spelling is None: + # `++`, `--`, `.`, `;`, `#`, or anything else unmodelled. + if c in ".;#": + i += 1 + continue + return None + prec = min(prec, dict(_JS_OPERATORS)[spelling]) + ops.add(spelling) + i += len(spelling) + return prec, ops + + +def _js_removable_paren(text: str, literal: list[bool], depth: list[int]) -> tuple[int, int] | None: + """The first grouping-parenthesis pair in ``text`` that provably does not + change the parse. ``None`` when there is none.""" + n = len(text) + for i in range(n): + if literal[i] or text[i] != "(": + continue + close = _js_matching_close(text, literal, depth, i) + if close is None: + continue + context = _js_left_context(text, literal, i) + if context is None: + continue + left, at_statement_start = context + right = _js_right_context(text, literal, close) + if right is None: + continue + binding = _js_inner_binding(text[i + 1:close], at_statement_start) + if binding is None: + continue + inner, ops = binding + if inner > max(left, right): + return (i, close) + # EQUAL binding power, but the operator is one whose two groupings are + # the same value. `a || (b || c)` and `a || b || c` differ only in where + # a short circuit is written down. Restricted to the three operators for + # which that is true of ANY operands — `+` is not one of them + # (`1 + (2 + '3')` is `'123'`, `1 + 2 + '3'` is `'33'`), and neither is + # floating-point `*`. + if inner == max(left, right) and ops and ops <= _JS_ASSOCIATIVE_OPS: + if len(ops) == 1 and _js_neighbour_spellings(text, literal, i, close) <= ops: + return (i, close) + return None + + +def _js_neighbour_spellings(text: str, literal: list[bool], open_at: int, close_at: int) -> set[str]: + """The symbolic operators immediately left of ``(`` and right of ``)``.""" + out: set[str] = set() + i = open_at + while i > 0 and text[i - 1] == " ": + i -= 1 + for spelling, _ in _JS_OPERATORS: + if i and not literal[i - 1] and text[:i].endswith(spelling): + out.add(spelling) + break + j = close_at + 1 + while j < len(text) and text[j] == " ": + j += 1 + if j < len(text) and not literal[j]: + spelling = _js_operator_spelling_at(text, j) + if spelling is not None: + out.add(spelling) + return out + + +def _js_operator_spelling_at(text: str, start: int) -> str | None: + """The symbolic operator starting at ``start``, longest spelling first.""" + if text[start:start + 2] in ("++", "--"): + return None + for spelling, _ in _JS_OPERATORS: + if text.startswith(spelling, start): + return spelling + return None + + +def _js_matching_close(text: str, literal: list[bool], depth: list[int], open_at: int) -> int | None: + """The ``)`` that closes the ``(`` at ``open_at``, or ``None`` if the + fragment does not contain it. Depth dipping BELOW the opener's level before a + candidate is reached means the fragment is malformed there, so the search + stops rather than pairing across the gap.""" + want = depth[open_at] + for j in range(open_at + 1, len(text)): + if literal[j]: + continue + if depth[j] < want: + return None + if text[j] == ")" and depth[j] == want: + return j + return None + + +def _js_left_context( + text: str, literal: list[bool], open_at: int, +) -> tuple[int, bool] | None: + """``(binding power the parenthesised expression must beat on its left, is it + at the head of a statement)``, or ``None`` to refuse — a call, an unmodelled + neighbour, or a keyword whose parentheses are part of the syntax rather than + grouping.""" + i = open_at + while i > 0 and text[i - 1] == " ": + i -= 1 + if i == 0: + # Start of the region: an AssignmentExpression may stand here, and it is + # the head of a statement. + return (2, True) + prev = text[i - 1] + if literal[i - 1]: + return None # a call on a string, or an unmodelled neighbour + if _JS_IDENT_CHAR.match(prev): + word = _js_word_at(text, i) + # A MEMBER whose name happens to spell a keyword is a callee, not an + # operator. `axios.delete(url)` is a call; reading its `delete` as the + # unary operator hands the parentheses a binding power of 14 and deletes + # them, welding `axios.deleteurl`. Found on pipelinq's forecastApi.js. + if text[:i - len(word)].rstrip(" ").endswith("."): + return None + if word in _JS_PREFIX_WORD_REFUSE: + return None + if word in _JS_PREFIX_WORD_CONTEXT: + return (_JS_PREFIX_WORD_CONTEXT[word], False) + return None # a callee, or `if` / `for` / `while` / `switch` / `catch` + if prev in ")]}." and prev != "}": + return None # a call, an index, or a member access + if prev == "}": + return (2, True) # the end of the previous block: a statement head + if prev == "/": + return None # a regular expression's own parentheses are capture groups + if prev == ";": + return (2, True) + if prev in "([{,:?": + return (2, False) + prec = _js_operator_ending_at(text, i) + return None if prec is None else (prec, False) + + +def _js_right_context(text: str, literal: list[bool], close_at: int) -> int | None: + """Binding power the parenthesised expression must beat on its right.""" + i = close_at + 1 + while i < len(text) and text[i] == " ": + i += 1 + if i >= len(text): + return 0 + nxt = text[i] + if literal[i]: + # A string cannot follow an operand, so this is the next statement — + # but a template literal could be a TAGGED template, which is a call. + return None + if nxt in _JS_SUFFIX_REFUSE_CHARS or text[i:i + 2] in ("++", "--", "?."): + return None + if _JS_IDENT_CHAR.match(nxt): + word = re.match(r"[A-Za-z0-9_$]+", text[i:]).group(0) + if word in _JS_WORD_BINARY: + return _JS_WORD_BINARY[word] + if word in _JS_TYPE_ASSERTION_WORDS: + return None + # `(a) b` is not an expression in any dialect this gate reads, so the + # word begins the NEXT statement — the region joined two of them. The + # parenthesised expression therefore ends here, at statement level. + return 2 + if nxt in ")]},;:": + return 2 + return _js_operator_starting_at(text, i) + + +def _js_drop_redundant_parens(text: str) -> str | None: + """``text`` with every provably-redundant grouping parenthesis removed, or + ``None`` when it could not be scanned. Applied to BOTH sides, so it is a + canonical form, not a rewrite of one of them.""" + for _ in range(_JS_MAX_PAREN_PASSES): + scanned = _js_scan(text) + if scanned is None: + return None + pair = _js_removable_paren(text, *scanned) + if pair is None: + return text + i, j = pair + text = text[:i] + text[i + 1:j] + text[j + 1:] + return text + + +def _normalise_code_line(line: str, is_php: bool = True) -> str | None: """A layout-independent key for ``line``, or ``None`` if the line carries no - code identity at all (blank, or a bare docblock ``*``, or a lone brace). + code identity at all (blank, a bare docblock ``*``, or — in PHP — a lone + opening brace). Applied to BOTH sides of the comparison, so the only question it answers is "are these two lines the same program". What it deliberately equates: - * brace placement — K&R vs Allman (``.github#395``, and gate-14's ``#391``); + * brace placement — K&R vs Allman (``.github#395``, and gate-14's ``#391``). + PHP only; see ``_NORM_TRAILING_BRACE_RE`` for why JS keeps its brace; * indentation and intra-line spacing, including operator and cast spacing (``(array) $x`` / ``(array)$x``) and docblock column alignment; * quote style, under the narrow rule in ``_unify_quote_style``. @@ -222,59 +708,160 @@ def _normalise_code_line(line: str) -> str | None: that one is — or one half of it does not parse, so it cannot equate two different WORKING programs. """ + return _normalise_line(line, is_php, collapse_whitespace=False) + + +def _normalise_line(line: str, is_php: bool, collapse_whitespace: bool) -> str | None: + """The shared body of the two key variants — see ``_normalise_code_line`` + (``collapse_whitespace=False``) and ``_spaced_code_line`` (``True``).""" s = line.strip() if s == "" or s == "*": return None - s = _NORM_TRAILING_BRACE_RE.sub("", s) - if s == "": - return None + if is_php: + s = _NORM_TRAILING_BRACE_RE.sub("", s) + if s == "": + return None + replacement = " " if collapse_whitespace else "" + if not is_php and "`" in s: + masked = _mask_literals_with_scanner(s, replacement) + if masked is not None: + return masked out: list[str] = [] pos = 0 for m in _NORM_STRING_RE.finditer(s): - out.append(_NORM_WS_RE.sub("", s[pos:m.start()])) + out.append(_NORM_WS_RE.sub(replacement, s[pos:m.start()])) out.append(_unify_quote_style(m.group(0))) pos = m.end() - out.append(_NORM_WS_RE.sub("", s[pos:])) + out.append(_NORM_WS_RE.sub(replacement, s[pos:])) return "".join(out) -def _comparison_keys(text: str, is_php: bool) -> tuple[list[int], list[str], list[str]]: - """``(line_numbers, keys, keys_with_trailing_comma_kept)`` for ``text``. +def _mask_literals_with_scanner(s: str, replacement: str) -> str | None: + """``_normalise_line``'s body for a line carrying a TEMPLATE LITERAL. + + ``_NORM_STRING_RE`` knows about ``'`` and ``"`` only, so a backtick's text + was being whitespace-stripped like code and ``` `not installed. ` ``` read + as ``` `not installed.` ```. The scanner marks quasi text as literal and + ``${…}`` as the code it is, which is exactly the distinction wanted. + + ``None`` when the line does not scan (a fragment, e.g. a Vue attribute split + across lines), which falls back to the regex — the pre-existing behaviour. + """ + scanned = _js_scan(s) + if scanned is None: + return None + literal, _ = scanned + out: list[str] = [] + i = 0 + n = len(s) + while i < n: + if not literal[i]: + j = i + while j < n and not literal[j]: + j += 1 + out.append(_NORM_WS_RE.sub(replacement, s[i:j])) + i = j + continue + j = i + while j < n and literal[j]: + j += 1 + run = s[i:j] + out.append(_unify_quote_style(run) if run[:1] in ("'", '"') else run) + i = j + return "".join(out) + - Two variants of the same key, index-aligned, because the two comparisons - below want different things from a trailing comma. Line-vs-line, a PHP - trailing comma is a no-op and must be ignored — php-cs-fixer adds one to - every multi-line parameter list, which is 2 of procest's 185 findings on its - own. Region-vs-region (the re-wrap rule) has to keep it, because there the - commas are load-bearing punctuation of the joined statement. +def _spaced_code_line(line: str, is_php: bool) -> str | None: + """``_normalise_code_line`` with runs of whitespace COLLAPSED to one space + instead of removed. Same lines kept, same literals preserved, same brace + rule — only the JS re-wrap comparison uses it, and only because a word- + spelled operator (``in``, ``instanceof``, ``typeof``, ``await``) is + unrecognisable once the spaces around it are gone.""" + return _normalise_line(line, is_php, collapse_whitespace=True) + + +def _comparison_keys( + text: str, is_php: bool, +) -> tuple[list[int], list[str], list[str], list[str]]: + """``(line_numbers, keys, keys_with_trailing_comma_kept, spaced_keys)``. + + Three variants of the same key, index-aligned, because the comparisons below + want different things from the same line. Line-vs-line, a trailing comma is + a no-op and must be ignored — php-cs-fixer adds one to every multi-line + parameter list (2 of procest's 185 findings on its own) and prettier's + ``trailingComma: "all"`` does the same to every JS construct it breaks. + Region-vs-region (the re-wrap rule) has to keep it, because there the commas + are load-bearing punctuation of the joined statement. The JS re-wrap rule + additionally needs the spaces back, for the reason in ``_spaced_code_line``. + + The JS half of the comma rule (``.github#435``) is refused on any line + carrying an ELISION marker, because there a comma is an ELEMENT: ``[a, , b]`` + has three entries and ``[a, b]`` has two. Only ONE trailing comma is ever + dropped, so ``a,,`` can never be reduced to ``a``. """ numbers: list[int] = [] keys: list[str] = [] keys_full: list[str] = [] + keys_spaced: list[str] = [] for i, raw in enumerate(text.splitlines()): - k = _normalise_code_line(raw) + k = _normalise_code_line(raw, is_php) if k is None: continue numbers.append(i + 1) keys_full.append(k) - keys.append(k[:-1] if is_php and k.endswith(",") else k) - return numbers, keys, keys_full - - -def _is_pure_rewrap(base_region: list[str], head_region: list[str], is_php: bool) -> bool: - """True if the two regions are the SAME CHARACTERS, distributed differently - across lines — a line-length reflow such as ``'a '.`` + ``'b'`` becoming - ``'a '`` + ``.'b'``, or a long call broken after an argument. - - PHP ONLY, and never across a line comment. Both restrictions are about the - one thing a line break can do besides layout: END something. JavaScript has - automatic semicolon insertion, so joining ``return`` and ``x`` changes what - the function returns; PHP has no ASI. And in either language a ``//`` runs - to the end of the line, so inserting a break after one UNCOMMENTS whatever - followed — the same characters, a different program. Refusing the rule - whenever the region contains a comment introducer costs only precision. + keys.append(k[:-1] if k.endswith(",") and _may_drop_trailing_comma(k) else k) + keys_spaced.append("" if is_php else (_spaced_code_line(raw, is_php) or "")) + return numbers, keys, keys_full, keys_spaced + + +def _may_drop_trailing_comma(key: str) -> bool: + """True if the final comma of ``key`` is punctuation rather than a hole.""" + return not any(marker in key for marker in _ELISION_MARKERS) + + +def _drop_punctuation_commas(joined: str) -> str: + """Remove every comma that sits immediately before its own closer.""" + return _TRAILING_COMMA_BEFORE_CLOSER_RE.sub("", joined) + + +def _js_asi_hazard(region: list[str]) -> bool: + """True if any line break INSIDE ``region`` is one that JavaScript's + automatic semicolon insertion gives a meaning to. + + Two directions, and both are needed. A break AFTER a restricted production + (``return`` / ``throw`` / ``break`` / ``continue`` / ``yield`` / ``async``) + terminates the statement, so ``return`` + ``buildThing(a)`` returns + ``undefined`` where the joined line returns the thing. A break BEFORE + ``++`` / ``--`` / ``=>`` is equally load-bearing in the other direction: + ``x`` + ``++y`` is two statements and ``x++y`` is not a program at all. + """ + for i in range(len(region) - 1): + if _JS_RESTRICTED_TAIL_RE.search(region[i]): + return True + if _JS_NO_PRECEDING_BREAK_RE.match(region[i + 1]): + return True + return False + + +def _is_pure_rewrap(base_region: list[str], head_region: list[str]) -> bool: + """The PHP arm: true if the two regions are the SAME CHARACTERS, distributed + differently across lines — a line-length reflow such as ``'a '.`` + ``'b'`` + becoming ``'a '`` + ``.'b'``, or a long call broken after an argument. + + Never across a line comment. That restriction is about the one thing a line + break can do besides layout: END something. A ``//`` runs to the end of its + line, so inserting a break after one UNCOMMENTS whatever followed and joining + two lines COMMENTS OUT whatever follows the ``//`` — the same characters, a + different program. Refusing the rule whenever the region contains a comment + introducer costs only precision. + + ``.github#435`` gave JS its own arm (``_is_pure_js_rewrap``) rather than a + branch in here, so that every byte of this function's behaviour — and so of + the PHP half of #395 — is unchanged. Verified against the pre-#435 checker on + 3,054 real (base, head) PHP file pairs from openregister's history: no + difference on any of them. """ - if not is_php or not base_region or not head_region: + if not base_region or not head_region: return False joined_base = "".join(base_region) if joined_base != "".join(head_region): @@ -282,17 +869,148 @@ def _is_pure_rewrap(base_region: list[str], head_region: list[str], is_php: bool return "//" not in joined_base and "#" not in joined_base +def _is_pure_js_rewrap(base_region: list[str], head_region: list[str]) -> bool: + """The JS/TS/Vue arm of the re-wrap rule (``.github#435``). Both regions are + ``_spaced_code_line`` keys, NOT the space-stripped ones. + + prettier re-wraps everything it touches — an argument list, an object + literal, a mustache, a CSS declaration block — adds a trailing comma + wherever it breaks, and re-prints parentheses from its own precedence table. + None of that is a changed method, but four JS-specific hazards make "same + characters, different line breaks" an unsafe test on its own, and each is + REFUSED rather than reasoned about: + + * a ``//`` anywhere in either region (see ``_is_pure_rewrap``) — including + one inside a string, e.g. a URL. That is deliberately blunt: this + normaliser has no parser, and the cost of the bluntness is precision + while the cost of getting it wrong is a method that changed and was never + reported; + * a line break INSIDE a template literal, which is a CHARACTER of the + resulting string. Detected by the backtick PARITY of each line: if every + line on both sides carries an even number of backticks then no literal can + span a break, and an odd count anywhere refuses the whole region. A + backtick that stays on one line — which is every one prettier produces + when it breaks an argument list around a URL — is no hazard at all, and + refusing those outright cost 21 of pipelinq#820's findings; + * an ELISION marker (``,,`` or ``[,``) anywhere in either region. There a + comma is an element of an array and the trailing-comma rule must not touch + it; + * an ASI-sensitive break — see ``_js_asi_hazard``. + + What survives all four is compared in a canonical form: punctuation commas + dropped, provably-redundant parentheses dropped, then whitespace removed. A + region whose parentheses cannot be scanned safely falls back to the + characters-identical test, which is the pre-#435 behaviour. + """ + joined_base = " ".join(base_region) + joined_head = " ".join(head_region) + for region, joined in ((base_region, joined_base), (head_region, joined_head)): + if "//" in joined: + return False + if any(line.count("`") % 2 for line in region): + return False + if any(marker in _NORM_WS_RE.sub("", joined) for marker in _ELISION_MARKERS): + return False + if _js_asi_hazard(base_region) or _js_asi_hazard(head_region): + return False + return _js_canonical(joined_base) == _js_canonical(joined_head) + + +def _js_rewrap_in_context( + base_spaced: list[str], head_spaced: list[str], + opcodes: list[tuple[str, int, int, int, int]], at: int, +) -> bool: + """``_is_pure_js_rewrap`` on a window that grows OPCODE BY OPCODE until the + re-flow fits inside it (``.github#435``). + + One opcode is often not the whole re-wrap. prettier turning:: + + return a + || b + + into:: + + return ( + a + || b + ) + + leaves the ``|| b`` line byte-identical, so ``SequenceMatcher`` calls it + EQUAL and splits the reflow into two opcodes with an untouched line wedged + between: a replace that opens a parenthesis it never closes, and a lone + ``)`` insert. Neither half is analysable alone; together they are one reflow. + + The window therefore grows to whole neighbouring OPCODES, never to a raw line + count. That is the difference between working and not: the two sides of this + reflow have different line counts, so "two more lines each" lands the base at + ``},`` and the head at ``)``, comparing different constructs forever. An + opcode boundary is a point where base index and head index provably + correspond, so every window this builds is aligned at both ends. + + Widening can only ever SUPPRESS, never report, and each rung demands full + canonical equality of a LARGER pair of regions — a stronger claim than the + last, not a weaker one. Only the head lines of THIS opcode are dropped from + the scope; the neighbours are re-judged on their own turn. + """ + for reach in _JS_CONTEXT_REACH: + lo, hi = max(0, at - reach), min(len(opcodes) - 1, at + reach) + _, i1, _, j1, _ = opcodes[lo] + _, _, i2, _, j2 = opcodes[hi] + if (i2 - i1) > _JS_CONTEXT_MAX_LINES or (j2 - j1) > _JS_CONTEXT_MAX_LINES: + break + if _is_pure_js_rewrap(base_spaced[i1:i2], head_spaced[j1:j2]): + return True + return False + + +def _js_canonical(joined: str) -> str: + """The form two JS regions are compared in: redundant parentheses gone, + whitespace OUTSIDE a string literal gone, punctuation commas gone. Applied to + both sides. + + The whitespace is removed literal-by-literal rather than with one blanket + ``sub`` because the region arrives SPACED — that is the whole point of + ``_spaced_code_line`` — and a blanket strip would reach inside the literals + too. ``'not installed. '`` -> ``'not installed.'`` is text a user reads, and + #395's first rule is that this gate keeps seeing it. + """ + stripped = _js_drop_redundant_parens(joined) + if stripped is not None: + joined = stripped + scanned = _js_scan(joined) + if scanned is None: + # Unscannable — fall back to the regex, which is what the PHP side uses + # and which cannot see a template literal. + out: list[str] = [] + pos = 0 + for m in _NORM_STRING_RE.finditer(joined): + out.append(_NORM_WS_RE.sub("", joined[pos:m.start()])) + out.append(m.group(0)) + pos = m.end() + out.append(_NORM_WS_RE.sub("", joined[pos:])) + return _drop_punctuation_commas("".join(out)) + literal, _ = scanned + kept = "".join( + ch for lit, ch in zip(literal, joined) if lit or not ch.isspace() + ) + return _drop_punctuation_commas(kept) + + def _substantively_changed_lines(base_text: str, head_text: str, is_php: bool) -> set[int]: """1-based line numbers in ``head_text`` that are not present, unchanged, in ``base_text`` once both sides are normalised by ``_normalise_code_line``.""" - _, base_keys, base_full = _comparison_keys(base_text, is_php) - head_numbers, head_keys, head_full = _comparison_keys(head_text, is_php) + _, base_keys, base_full, base_spaced = _comparison_keys(base_text, is_php) + head_numbers, head_keys, head_full, head_spaced = _comparison_keys(head_text, is_php) matcher = SequenceMatcher(None, base_keys, head_keys, autojunk=False) + opcodes = matcher.get_opcodes() changed: set[int] = set() - for tag, i1, i2, j1, j2 in matcher.get_opcodes(): + for at, (tag, i1, i2, j1, j2) in enumerate(opcodes): if tag not in ("replace", "insert"): continue - if _is_pure_rewrap(base_full[i1:i2], head_full[j1:j2], is_php): + if is_php: + if _is_pure_rewrap(base_full[i1:i2], head_full[j1:j2]): + continue + elif _js_rewrap_in_context(base_spaced, head_spaced, opcodes, at): continue for j in range(j1, j2): changed.add(head_numbers[j]) @@ -364,6 +1082,10 @@ def changed_lines(base_ref: str, cwd: Path) -> dict[str, set[int]]: 921 PHP files: 185 findings before, 0 after, and PHP's own tokeniser agrees — the 54 files those findings named are token-identical to their base once whitespace, comments, trailing commas and quote style are set aside. + + The same is true of `@nextcloud/prettier-config` for JS/TS/Vue + (`.github#435`). Measured on ConductionNL/pipelinq#820, 324 files: 468 + findings before, 11 after, with `development` at PASS on both sides. """ diff = _git(["diff", "-U0", "--diff-filter=ACMR", f"{base_ref}...HEAD"], cwd) three_dot = True diff --git a/hydra-gates/scripts/lib/test_check_spec_coverage.py b/hydra-gates/scripts/lib/test_check_spec_coverage.py index ce94a0a3..614f41c2 100644 --- a/hydra-gates/scripts/lib/test_check_spec_coverage.py +++ b/hydra-gates/scripts/lib/test_check_spec_coverage.py @@ -305,11 +305,15 @@ def test_a_new_parameter_through_the_trailing_comma_is_still_reported(self): self.assertIn(4, self._changed(base, head), "an ADDED parameter is not a trailing comma") - def test_trailing_comma_is_not_normalised_in_javascript(self): - # `[1, 2,]` and `[1, 2,,]` differ in JS (elision), so the rule is PHP-only. + def test_trailing_comma_is_normalised_in_javascript_too(self): + # `.github#435` REVERSED the PHP-only scope of this rule, and the + # reversal is the whole reason the assertion changed rather than moved: + # `[1, 2]` and `[1, 2,]` are both two-element arrays, in every engine + # since ES5. The hazard #395 named is a HOLE, and the control for it is + # the next test, not this one. base = "const a = [1, 2]\n" head = "const a = [1, 2,]\n" - self.assertEqual(self._changed(base, head, is_php=False), {1}) + self.assertEqual(self._changed(base, head, is_php=False), set()) # --- quote style ------------------------------------------------------- def test_quote_style_alone_is_not_a_change(self): @@ -364,8 +368,11 @@ def test_rewrap_is_not_applied_across_a_line_comment(self): self.assertTrue(self._changed(base, head), "uncommenting a statement is not a re-wrap") - def test_rewrap_is_not_applied_to_javascript(self): - # ASI: `return` on its own line returns undefined. + def test_rewrap_across_an_asi_boundary_is_still_a_change_in_javascript(self): + # ASI: `return` on its own line returns undefined. `.github#435` gave JS + # the re-wrap rule but NOT across a restricted production, so this + # assertion is unchanged from #395 — it is now the control for the + # guard rather than for the absence of the rule. base = " return buildThing(a)\n" head = " return\n buildThing(a)\n" self.assertTrue(self._changed(base, head, is_php=False), @@ -396,6 +403,251 @@ def test_an_added_method_is_entirely_in_scope(self): self.assertEqual(len(changed), 3, changed) +class JsNormalisationTest(unittest.TestCase): + """`.github#435` — the JS/TS/Vue half of #395's normalisation. + + Same contract as `NormalisationTest`: every rule ships with the change it + must STILL see. A rule with only an A-arm is indistinguishable from having + switched the frontend half of gate-16 off, which is the exact failure this + gate exists to prevent — so the B-arms outnumber the A-arms here. + + MEASURED: pipelinq#820 (`feat/nextcloud-prettier`, 324 files) went from 468 + findings to 11 with no change to what `development` reports. + """ + + def _changed(self, base: str, head: str) -> set[int]: + return csc._substantively_changed_lines(base, head, is_php=False) + + def _same(self, base: str, head: str) -> bool: + return csc._js_canonical(base) == csc._js_canonical(head) + + # --- brace placement --------------------------------------------------- + def test_a_mustache_split_over_lines_is_not_a_change(self): + # The reason JS keeps its trailing `{`: stripping one leaves `{{` as `{` + # and the halves stop matching their own single-line base. + base = " {{ item.title }}\n" + head = "\t\t\t{{\n\t\t\t\titem.title\n\t\t\t}}\n" + self.assertEqual(self._changed(base, head), set()) + + def test_a_changed_expression_in_a_split_mustache_is_still_reported(self): + base = " {{ item.title }}\n" + head = "\t\t\t{{\n\t\t\t\titem.subtitle\n\t\t\t}}\n" + self.assertTrue(self._changed(base, head)) + + def test_a_multiline_import_is_not_a_change(self): + base = "import { CnAppRoot, CnObjectSidebar } from '@conduction/nextcloud-vue'\n" + head = ("import {\n\tCnAppRoot,\n\tCnObjectSidebar,\n" + "} from '@conduction/nextcloud-vue'\n") + self.assertEqual(self._changed(base, head), set()) + + def test_an_added_import_specifier_is_still_reported(self): + base = "import { CnAppRoot, CnObjectSidebar } from '@conduction/nextcloud-vue'\n" + head = ("import {\n\tCnAppRoot,\n\tCnObjectSidebar,\n\tbuiltinIntegrations,\n" + "} from '@conduction/nextcloud-vue'\n") + self.assertTrue(self._changed(base, head)) + + # --- trailing comma / elision ------------------------------------------ + def test_an_elision_is_not_a_trailing_comma(self): + # THE hazard #395 named. `[a, , b]` has three entries; `[a, b]` has two. + base = "const a = [x, y]\n" + head = "const a = [x, , y]\n" + self.assertEqual(self._changed(base, head), {1}) + + def test_an_elision_survives_a_rewrap(self): + base = "const a = [x, , y]\n" + head = "const a = [\n\tx,\n\t,\n\ty,\n]\n" + self.assertTrue(self._changed(base, head), + "a hole must never be normalised away as punctuation") + + def test_a_broken_argument_list_with_a_trailing_comma_is_not_a_change(self): + base = "\t\tawait axios.put(generateUrl('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/apps/x/y'), delta)\n" + head = "\t\tawait axios.put(\n\t\t\tgenerateUrl('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/apps/x/y'),\n\t\t\tdelta,\n\t\t)\n" + self.assertEqual(self._changed(base, head), set()) + + def test_an_added_argument_through_the_trailing_comma_is_still_reported(self): + base = "\t\tawait axios.put(generateUrl('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/apps/x/y'), delta)\n" + head = ("\t\tawait axios.put(\n\t\t\tgenerateUrl('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/apps/x/y'),\n" + "\t\t\tdelta,\n\t\t\t{ force: true },\n\t\t)\n") + self.assertTrue(self._changed(base, head)) + + # --- ASI --------------------------------------------------------------- + def test_a_join_across_a_restricted_production_is_still_a_change(self): + for word in ("return", "throw", "break", "continue", "yield"): + with self.subTest(word=word): + base = f"\t{word}\n\tvalue\n" + head = f"\t{word} value\n" + self.assertTrue(self._changed(base, head), + f"a line break after `{word}` ends the statement") + + def test_a_break_before_an_increment_is_still_a_change(self): + base = "\tcount\n\t++other\n" + head = "\tcount ++other\n" + self.assertTrue(self._changed(base, head)) + + # --- line comments ----------------------------------------------------- + def test_a_rewrap_that_uncomments_code_is_still_a_change(self): + base = "\t// fixme flag = true\n" + head = "\t// fixme\n\tflag = true\n" + self.assertTrue(self._changed(base, head), + "inserting a break after `//` uncomments what followed") + + def test_a_rewrap_that_comments_out_code_is_still_a_change(self): + base = "\tconst a = 1 // note\n\tconst b = 2\n" + head = "\tconst a = 1 // note const b = 2\n" + self.assertTrue(self._changed(base, head), + "joining onto a `//` line comments out what follows") + + # --- template literals ------------------------------------------------- + def test_a_line_local_template_literal_does_not_block_the_rewrap(self): + base = "\t\tconst u = generateUrl(`/apps/x/${id}/${action}`)\n" + head = "\t\tconst u = generateUrl(\n\t\t\t`/apps/x/${id}/${action}`,\n\t\t)\n" + self.assertEqual(self._changed(base, head), set()) + + def test_whitespace_inside_a_template_literal_is_still_a_change(self): + base = "\t\tconst m = `not installed. `\n" + head = "\t\tconst m = `not installed.`\n" + self.assertEqual(self._changed(base, head), {1}, + "a template literal's text is text a user reads") + + def test_a_changed_interpolation_is_still_reported(self): + base = "\t\tconst u = generateUrl(`/apps/x/${id}`)\n" + head = "\t\tconst u = generateUrl(\n\t\t\t`/apps/x/${otherId}`,\n\t\t)\n" + self.assertTrue(self._changed(base, head)) + + def test_a_break_inside_a_template_literal_is_still_a_change(self): + # A newline inside a template literal is a CHARACTER of the string. + base = "\t\tconst m = `alpha beta`\n" + head = "\t\tconst m = `alpha\nbeta`\n" + self.assertTrue(self._changed(base, head)) + + # --- redundant parentheses --------------------------------------------- + def test_a_return_wrapped_in_parentheses_is_not_a_change(self): + base = "\t\t\treturn this.a !== this.b\n\t\t\t\t|| this.c !== this.d\n" + head = "\t\t\treturn (\n\t\t\t\tthis.a !== this.b\n\t\t\t\t|| this.c !== this.d\n\t\t\t)\n" + self.assertEqual(self._changed(base, head), set()) + + def test_a_changed_operand_inside_a_parenthesised_return_is_still_reported(self): + base = "\t\t\treturn this.a !== this.b\n\t\t\t\t|| this.c !== this.d\n" + head = "\t\t\treturn (\n\t\t\t\tthis.a !== this.b\n\t\t\t\t|| this.c !== this.e\n\t\t\t)\n" + self.assertTrue(self._changed(base, head)) + + def test_a_precedence_changing_paren_edit_is_still_a_change(self): + """THE control for the whole paren canonicaliser. + + Each pair is the same characters apart from one parenthesis, and each + pair means two different things. If any of these ever equate, gate-16 + has stopped reporting a real operator-precedence bug. + """ + pairs = [ + ("(a || b) && c", "a || b && c"), + ("(a + b) * c", "a + b * c"), + ("a - (b - c)", "a - b - c"), + ("a + (b + c)", "a + b + c"), + ("f((a, b))", "f(a, b)"), + ("x = (a, b)", "x = a, b"), + ("(a ? b : c) ? d : e", "a ? b : c ? d : e"), + ("!(a && b)", "!a && b"), + ("(a || b).c", "a || b.c"), + ("(f || g)(x)", "f || g(x)"), + ("(a || b)[0]", "a || b[0]"), + ("(await x) ** 2", "await x ** 2"), + ("typeof (a + b)", "typeof a + b"), + ("new (a.b)()", "new a.b()"), + ("('k' in ctx) + 1", "'k' in ctx + 1"), + ("(a = 1) || b", "a = 1 || b"), + ("(a, b) => y", "a, b => y"), + ("({ a: 1 })", "{ a: 1 }"), + ("/(a)/.test(s)", "/a/.test(s)"), + ("(a || b) as string", "a || b as string"), + ] + for tighter, looser in pairs: + with self.subTest(pair=tighter): + self.assertFalse(self._same(tighter, looser), + f"{tighter!r} and {looser!r} are different programs") + + def test_a_redundant_paren_prettier_reprints_is_not_a_change(self): + """…and the other half: the ones that ARE the same program.""" + pairs = [ + ("return (a || b)", "return a || b"), + ("x = (a && b) ? c : d", "x = a && b ? c : d"), + ("map[s] || (s || '-')", "map[s] || s || '-'"), + ("x = (await f()) || {}", "x = await f() || {}"), + ("a ? b : (c ? d : e)", "a ? b : c ? d : e"), + ("x ? (a) : b", "x ? a : b"), + ("(x) => y", "x => y"), + ("('k' in ctx) && q", "'k' in ctx && q"), + ("for (const c of (x || [])) {", "for (const c of x || []) {"), + ("((a - b) ** 2) / c", "(a - b) ** 2 / c"), + ("return ({ a: 1 }[k] || 'z')", "return { a: 1 }[k] || 'z'"), + ("value: `${(this.d?.rate || 0)}%`", "value: `${this.d?.rate || 0}%`"), + ] + for wrapped, bare in pairs: + with self.subTest(pair=wrapped): + self.assertTrue(self._same(wrapped, bare), + f"{wrapped!r} and {bare!r} are one program") + + def test_a_member_named_like_a_keyword_is_a_call_not_an_operator(self): + # `axios.delete(url)` is a call. Reading its `delete` as the unary + # operator hands the parentheses a binding power of 14 and welds + # `axios.deleteurl`. Found on pipelinq's forecastApi.js. + base = "\t\tconst r = await axios.delete(generateUrl(base + '/x/' + id))\n" + head = "\t\tconst r = await axios.delete(\n\t\t\tgenerateUrl(base + '/x/' + id),\n\t\t)\n" + self.assertEqual(self._changed(base, head), set()) + self.assertFalse(self._same("axios.delete(url)", "axios.deleteurl")) + + # --- the ordinary changes, through every rule above -------------------- + def test_a_renamed_method_is_still_reported(self): + base = "\t\tfetchThings () {\n\t\t\treturn this.load()\n\t\t},\n" + head = "\t\tfetchItems() {\n\t\t\treturn this.load()\n\t\t},\n" + self.assertIn(1, self._changed(base, head)) + + def test_an_added_parameter_is_still_reported(self): + base = "\t\tsave (id) {\n\t\t\treturn this.put(id)\n\t\t},\n" + head = "\t\tsave(id, force) {\n\t\t\treturn this.put(id)\n\t\t},\n" + self.assertIn(1, self._changed(base, head)) + + def test_a_changed_value_in_a_rewrapped_expression_is_still_reported(self): + base = "\t\tconst total = a * 2 + b\n" + head = "\t\tconst total =\n\t\t\ta * 3\n\t\t\t+ b\n" + self.assertTrue(self._changed(base, head)) + + def test_a_changed_string_content_is_still_reported(self): + base = "\t\tshowError(t('app', 'Could not reveal address.'))\n" + head = "\t\tshowError(\n\t\t\tt('app', 'Could not reveal the address.'),\n\t\t)\n" + self.assertTrue(self._changed(base, head), + "re-quoting and re-wrapping must not carry a CONTENT edit through") + + def test_quote_style_alone_is_not_a_change_in_javascript(self): + base = '\t\tconst m = "PDF extraction failed"\n' + head = "\t\tconst m = 'PDF extraction failed'\n" + self.assertEqual(self._changed(base, head), set()) + + def test_an_added_method_is_entirely_in_scope_in_javascript(self): + base = "export default {\n\tmethods: {\n\t},\n}\n" + head = ("export default {\n\tmethods: {\n\t\tsave() {\n\t\t\treturn 1\n" + "\t\t},\n\t},\n}\n") + changed = self._changed(base, head) + self.assertTrue({3, 4} <= changed, changed) + + def test_a_semicolon_to_newline_split_is_still_a_change(self): + # prettier's `semi: false` turns `a; b` into two lines. Equating them + # needs the same ASI argument the guard above refuses to make, so this + # is REFUSED — measured cost, 3 findings on pipelinq#820. + base = "\t\trun(i) { const a = load(); a.splice(i, 1); emit(a) },\n" + head = "\t\trun(i) {\n\t\t\tconst a = load()\n\t\t\ta.splice(i, 1)\n\t\t\temit(a)\n\t\t},\n" + self.assertTrue(self._changed(base, head)) + + # --- the narrowing can only ever narrow -------------------------------- + def test_the_narrowing_is_an_intersection_with_git(self): + """`_drop_cosmetic_only` may only REMOVE lines from git's answer.""" + base = "const a = [1, 2]\n" + head = "const a = [\n\t1,\n\t2,\n]\n" + changed = csc._substantively_changed_lines(base, head, is_php=False) + git_says = {1, 2, 3, 4} + self.assertTrue(changed <= git_says | set(), + "normalisation returned a line git never called added") + + class DiffScopeFullRunTest(unittest.TestCase): """End-to-end: a real git repo where only one method is in the diff.""" diff --git a/hydra-gates/scripts/lib/test_gate16_spec_coverage_scope.sh b/hydra-gates/scripts/lib/test_gate16_spec_coverage_scope.sh index 550ec7ac..1d0cfea3 100644 --- a/hydra-gates/scripts/lib/test_gate16_spec_coverage_scope.sh +++ b/hydra-gates/scripts/lib/test_gate16_spec_coverage_scope.sh @@ -390,6 +390,101 @@ else _ok "gate-16 does NOT name buildRowLabel — the reformatted-but-unchanged method beside it stays out of scope" fi +# =========================================================================== +echo +echo "== arm 5 — a PRETTIER reformat is not a set of changed methods either ==" +# =========================================================================== +# +# `.github#435`. #395 normalised PHP and deliberately left JS/TS/Vue alone, +# because a JS trailing comma can be an array ELISION, a JS re-wrap can cross an +# ASI boundary, and a re-wrap across a `//` uncomments what followed. Those +# hazards are real; they are the SPECIFICATION for the JS rules, not a reason to +# have none. MEASURED on ConductionNL/pipelinq#820 (`feat/nextcloud-prettier`, +# 324 files, the same gate package on both sides): `development` PASS, the +# reformat 468 findings before and 11 after. +# +# Same shape as arm 4, and for the same reason: 5a alone would be passed by a +# normalisation that had simply switched the frontend half of gate-16 off. +# +# 5a reformat ONLY -> gate-16 must report NOTHING +# 5b reformat + ONE character -> gate-16 must still report THAT method, and +# must not report the three beside it +gf_build_repo "${WORK}/prettier" "${SRC}" +gf_commit_all "${WORK}/prettier" "base: app before the prettier reformat" +gf_mark_base "${WORK}/prettier" +cp "${SRC}/src/views/ReflowedView.vue.prettier" \ + "${WORK}/prettier/src/views/ReflowedView.vue" +rm -f "${WORK}/prettier/src/views/ReflowedView.vue.prettier" \ + "${WORK}/prettier/src/views/ReflowedView.vue.prettier-changed" +gf_commit_paths "${WORK}/prettier" "style: adopt @nextcloud/prettier-config" \ + src/views/ReflowedView.vue + +# PRE-CONDITION 1 — the reformat has to BE a diff. +_churn5="$(cd "${WORK}/prettier" && git diff --numstat "$(git rev-parse refs/remotes/origin/development)"...HEAD -- src/views/ReflowedView.vue)" +_churn5_added="$(printf '%s' "${_churn5}" | awk '{print $1}')" +if [ "${_churn5_added:-0}" -ge 10 ]; then + _ok "pre-condition: the prettier reformat rewrites ${_churn5_added} lines of ReflowedView.vue — 5a is judging a real diff" +else + _bad "pre-condition: the prettier reformat produced ${_churn5_added:-0} added line(s). The two fixture variants must differ, or arm 5a is vacuous." +fi + +# PRE-CONDITION 2 — the methods must be findable subject matter. +_pc5="$(cd "${WORK}/prettier" && python3 "${CHECKER}" . --mode report 2>&1)" +if printf '%s' "${_pc5}" | grep -qF 'ReflowedView.vue::totalWeights'; then + _ok "pre-condition: report mode names ReflowedView::totalWeights as untagged — it IS in scope and IS uncovered" +else + _bad "pre-condition: report mode does not name ReflowedView::totalWeights, so arms 5a/5b are not measuring an in-scope untagged frontend method at all" +fi + +_out5="$(gf_run_wrapper "${WORK}/prettier" "${WORK}/log-prettier")" +_v5="$(gf_verdict "${_out5}" 16)" +case "${_v5}" in + *PASS*) _ok "gate-16 PASSES on a prettier-only reformat — a re-wrap, a trailing comma and a reprinted parenthesis are not changed methods (#435)" ;; + *FAIL*) _bad ".github#435 is LIVE: gate-16 reports changed methods on a diff whose only content is prettier's line breaks, trailing commas, arrow parentheses and mustache wrapping. Got: ${_v5:0:160}" ;; + "") _bad "gate-16 emitted no verdict at all on the prettier arm" ;; + *) _bad "gate-16 gave an unrecognised verdict on the prettier arm: ${_v5:0:160}" ;; +esac +if grep -qF 'ReflowedView.vue' "${WORK}/log-prettier/hydra-gate-spec-coverage.log" 2>/dev/null; then + _bad "gate-16 named ReflowedView.vue on a formatting-only diff — even without a FAIL that is #435 still in the log, and it is what a reviewer is asked to act on" +else + _ok "gate-16 writes no finding for the prettier-reformatted Vue file" +fi + +# --- 5b — the same reformat with ONE character changed --------------------- +# `.prettier` and `.prettier-changed` differ in exactly one byte: the `+` of +# `carry + w` is a `-`. If 5b goes quiet, the JS normalisation has become a +# false-negative machine and gate-16 has stopped watching every `.vue` file in +# the fleet — strictly worse than the 468 false positives it replaced. +gf_build_repo "${WORK}/prettier-plus" "${SRC}" +gf_commit_all "${WORK}/prettier-plus" "base: app before the prettier reformat" +gf_mark_base "${WORK}/prettier-plus" +cp "${SRC}/src/views/ReflowedView.vue.prettier-changed" \ + "${WORK}/prettier-plus/src/views/ReflowedView.vue" +rm -f "${WORK}/prettier-plus/src/views/ReflowedView.vue.prettier" \ + "${WORK}/prettier-plus/src/views/ReflowedView.vue.prettier-changed" +gf_commit_paths "${WORK}/prettier-plus" "style: adopt prettier, and flip one operator" \ + src/views/ReflowedView.vue + +_out5b="$(gf_run_wrapper "${WORK}/prettier-plus" "${WORK}/log-prettier-plus")" +_v5b="$(gf_verdict "${_out5b}" 16)" +case "${_v5b}" in + *FAIL*) _ok "gate-16 still FAILS when a Vue method's BODY changed inside the reformat — ${_v5b:0:90}" ;; + *) _bad "gate-16 did NOT report a Vue method whose operator changed, because the change arrived inside a prettier-reformatted file. Got: ${_v5b:0:160}" ;; +esac +_log5b="${WORK}/log-prettier-plus/hydra-gate-spec-coverage.log" +if grep -qF 'ReflowedView.vue::totalWeights' "${_log5b}" 2>/dev/null; then + _ok "gate-16 NAMES totalWeights — the Vue method whose body actually changed" +else + _bad "gate-16 failed without naming totalWeights; a verdict with no evidence cannot be acted on" +fi +for _neighbour in buildLabel mayEdit persistRow; do + if grep -qF "::${_neighbour}" "${_log5b}" 2>/dev/null; then + _bad "gate-16 also named ${_neighbour}, which carries the SAME reformat and no change. One real change is re-opening the whole file — #395 at file granularity, arriving through the JS path." + else + _ok "gate-16 does NOT name ${_neighbour} — the reformatted-but-unchanged method beside it stays out of scope" + fi +done + echo echo "== summary ==" echo " passed: ${_pass_n}" diff --git a/hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue b/hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue new file mode 100644 index 00000000..255e0c55 --- /dev/null +++ b/hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue @@ -0,0 +1,76 @@ + + + + + + diff --git a/hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue.prettier b/hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue.prettier new file mode 100644 index 00000000..5d5aeaf8 --- /dev/null +++ b/hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue.prettier @@ -0,0 +1,84 @@ + + + + + + diff --git a/hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue.prettier-changed b/hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue.prettier-changed new file mode 100644 index 00000000..dcaaeea7 --- /dev/null +++ b/hydra-gates/scripts/test-fixtures/spec-coverage-scope/app/src/views/ReflowedView.vue.prettier-changed @@ -0,0 +1,84 @@ + + + + + +