fix(gate-16): a prettier reformat is not a set of changed methods either

[rubenvdlinde]

Extends .github#395's cosmetic-reformat normalisation from PHP to JS / TS / Vue.

The defect

The fleet is adopting @nextcloud/prettier-config (useTabs: true, semi: false, trailingComma: "all"). On pipelinq#820, with the same gate package on both sides:

gate	development	PR
gate-6 orphan-auth	FAIL 3	FAIL 3
gate-7 no-admin-idor	FAIL 50	FAIL 50
gate-57 orphaned-write	FAIL 14	FAIL 14
gate-16 spec-coverage	PASS	FAIL 468

468 false findings on one app, blocking a 17-app rollout. The "fix" a hurried author reaches for is annotating 468 untouched methods with @spec, which bakes the defect into the codebase for good.

#395 excluded JS trailing commas and JS re-wraps for three reasons that are real: array elision, ASI, and a re-wrap across a //. Those are the specification for how to handle JS safely, not a reason to leave it unhandled.

Measured, on the live prettier PRs

Base = each app's merge-base with development. Before = check_spec_coverage.py at c26f9a3; after = this branch.

app	PR	changed files	development	before	after
pipelinq	#820	324	0	468	11
shillinq	#545	200	0	197	1
scholiq	#329	112	0	125	2
openregister	#2466	372	0	29	1
				819	15

development is # count=0 on all four with both versions, so every number in the "before" column is attributable to the reformat and nothing else.

What is normalised

The line-level key already removed whitespace outside literals and unified quote style for JS. Added:

• the opening brace is kept in JS. PHP strips a trailing { because php-cs-fixer moves it; prettier never does, and stripping it actively corrupts the comparison — a mustache split as <span>{{ + x + }}</span> loses one {, a multi-line import { loses the brace that opens the specifier list. A }-only line was already kept, so this makes the two halves of a pair symmetric rather than adding a rule;

• a trailing comma, line-level and region-level, refused on any line or region carrying an elision marker;

• a re-wrap — the same characters across different line breaks — with the four refusals below;

• a parenthesis prettier re-printed. Unavoidable: prettier does not edit text, it re-prints the tree, so every redundant parenthesis the author wrote is gone and every one its own line-breaking needs has appeared. Both directions occur in one 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)
  

  A pair is dropped only when the expression inside binds strictly tighter than both neighbours. Strictly, so associativity never enters the argument — a - (b - c) and (a - b) - c are both refused rather than one of them proved. The one exception is ||/&&/?? at equal power, where re-grouping provably preserves the value; + is deliberately not in that set, because 1 + (2 + '3') is '123' and 1 + 2 + '3' is '33'.

  The scan runs on a space-collapsed variant of each line, not the space-stripped one used to match lines. That is load-bearing: 'value' in ctx 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.

What is refused, and why

Each of these costs precision and each is deliberate. When the analysis could not show the two forms were one program, it says the method changed.

• an elision — [a, , b] has three entries and [a, b] has two. ,, or [, anywhere in either region refuses the whole region, and the line-level rule drops at most one trailing comma, so a,, can never reduce to a;
• an ASI-sensitive break — both directions: a break after return/throw/break/continue/yield/async, and a break before ++/--/=>;
• a // anywhere in either region, including one inside a string such as a URL. Deliberately blunt: there is no parser here, and the cost of the bluntness is precision while the cost of being wrong is a changed method nobody hears about. Covers both directions — a break after a // uncomments what followed, joining onto one comments out what follows;
• a line break inside a template literal, which is a character of the resulting string. Detected by backtick parity per line rather than by refusing every backtick: a literal that stays on one line, which is every one prettier produces when it breaks an argument list around a URL, is no hazard, and refusing those outright cost 21 findings on pipelinq alone;
• ; → newline (semi: false applied to a line carrying several statements). Equating those needs the same ASI argument the guard above exists to refuse. Cost: 3 findings;
• a regex literal in the expression — a backslash outside a literal is unmodelled, so the paren analysis declines. Cost: 2 findings;
• as / satisfies — a || b as string asserts only b, so (a || b) as string is a different expression;
• a member named like a keyword. 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 by the residue analysis, not by a test — it is now both;
• an increment anywhere in the expression, because a++ + b and a + ++b are the same characters once the spaces are gone. Literal-aware: a CSS class called badge--off used to refuse every expression that mentioned it.

The 15 that remain

Not zero, and I would rather say so than reach further. They are:

• 10 where a parenthesis reflow the analysis refuses, or whose partner the growing window never reaches — mostly (await someCall({ … 17 lines … })), where the added ( and its ) are several opcodes apart;
• 3 semi: false on multi-statement lines (pipelinq QuestionEditor.vue);
• 2 a regex literal inside the reflowed expression.

Every one is a false positive: an author is asked about a method that only moved. None is a false negative.

The anti-widening half

A gate that stops reporting is worse than one that over-reports. Every rule ships with a control proving a real change still travels through it.

JsNormalisationTest, 29 tests (unittest subTests expand to 55 assertions), each an A-arm paired with its B-arm:

still suppressed	still reported
a split mustache	a changed expression inside one
a multi-line import	an added import specifier
a broken argument list + trailing comma	an added argument through it
[1, 2] → [1, 2,]	[x, y] → [x, , y], and an elision through a re-wrap
a line-local template literal	whitespace inside one; a changed interpolation; a break inside one
return (…)	a changed operand inside a parenthesised return
12 redundant-paren pairs prettier really reprints	20 precedence-changing pairs
axios.delete(…) reflowed	axios.delete(url) ≠ axios.deleteurl
quote style	a changed string's content
	a renamed method · an added parameter · a changed value in a re-wrapped expression · a join across each of 5 restricted productions · a break before ++ · a re-wrap that uncomments · a re-wrap that comments out · ; → newline · an added method entirely in scope

The 20-pair table is the control for the whole paren canonicaliser. Each pair is the same characters apart from one parenthesis and each pair means two different things — (a || b) && c, (a + b) * c, a - (b - c), f((a, b)), (a ? b : c) ? d : e, !(a && b), (a || b).c, (await x) ** 2, new (a.b)(), ('k' in ctx) + 1, /(a)/.test(s), ({ a: 1 }), and eight more. If any ever equate, gate-16 has stopped reporting a real operator-precedence bug.

Shell arm 5, through bin/hydra-gates on a real git repo, with two pre-conditions so a silent PASS cannot be mistaken for a correct one:

• 5a — the prettier reformat alone: gate-16 PASSES and writes no finding;
• 5b — the same reformat with one byte different (carry + w → carry - w): gate-16 FAILS, names totalWeights, and does not name buildLabel, mayEdit or persistRow beside it.

ReflowedView.vue.prettier and .prettier-changed differ in exactly one character (cmp -l = 1 line).

Mutation-checked against the pre-#435 checker, both suites:

python  pre-fix: 8 FAILED + 32 errors      fixed: 66 passed
shell   pre-fix: 29 passed / 4 FAILED      fixed: 33 passed / 0 failed

On real data, twice — the arm that separates "468 → 11" from "the frontend half is off". One genuine one-line change committed on top of each reformat:

app	method	mutation	before	after
pipelinq	ProjectDetail.vue::statusLabel	'Paused' → 'Suspended'	11, not named	12, named
shillinq	AdministratieSwitcher.vue::activeLabel	formatLabel(active) → formatLabel(active, true)	1, not named	2, named

ProjectDetail.vue carries 43 untagged in-scope methods, all reformatted. Exactly one is reported: the one that changed. (App-wide, report mode counts 1,480 untagged methods; the gate reports 12.)

And at population scale, on ordinary work. Two mutated methods prove the rules do not blind those two; this asks whether the frontend half still reports what it used to across pipelinq's history. 25 commit ranges chosen for producing a non-empty frontend finding set — 756 findings in total — old checker vs new: identical on all 25. Ranges that reported nothing on both sides were skipped rather than counted; 36 of the first 40 sampled were empty-vs-empty, and counting those as agreement would have been a comparison of two blank strings.

PHP is unchanged

_is_pure_rewrap kept its PHP body byte-for-byte and JS got its own arm rather than a branch inside it. Verified two ways:

• 3,054 real (base, head) PHP file pairs from openregister's history through _substantively_changed_lines(…, is_php=True), old vs new: 0 differ;
• the whole gate on 6 commit ranges chosen for producing a non-zero PHP finding count (2/4/2/1/1/2): output identical on all six. Six more ranges were discarded first for reporting zero on both sides — a comparison of two empty strings is not a control.

NormalisationTest's 17 PHP assertions are unchanged and pass. Two JS assertions changed, both deliberately, both documented in place:

• test_trailing_comma_is_not_normalised_in_javascript → ..._is_normalised_in_javascript_too. The old assertion was semantically wrong: [1, 2] and [1, 2,] are both two-element arrays in every engine since ES5. The hazard gate-16 counts a brace-style reformat as a modified method: 1 finding becomes 185 on a formatting-only PR (sibling of #388/#391) #395 named is a hole, and that now has its own two controls;
• test_rewrap_is_not_applied_to_javascript → test_rewrap_across_an_asi_boundary_is_still_a_change_in_javascript. Same code, same assertion, renamed: it is now the control for the ASI guard rather than for the absence of the rule.

Also fixed, found on the way

The line-level key stripped whitespace inside template literals — `not installed. ` read as `not installed.`. Pre-existing (_NORM_STRING_RE knows ' and " only) and strictly a blindness, so it is closed here rather than filed: a line carrying a backtick is now masked with the scanner, which marks quasi text as literal and ${…} as the code it is.

Cost

One extra scan per JS line carrying a backtick, plus the paren canonicalisation per non-matching opcode. pipelinq#820: 7.4s → 12.2s. openregister#2466: 9.5s → 17.5s. The window that reaches for the rest of a construct is bounded at 4 opcodes and 120 lines either side; raising it to 16 opcodes / 400 lines was measured to close zero further findings.

Unchanged

--mode report, the empty-scope contract (#361), the COVERAGE: line, spec_tags_removed, and the intersection property — _drop_cosmetic_only still only ever removes lines from git's own answer, so nothing here can widen the scope.

🤖 Generated with Claude Code