Repository navigation
🌱 test: make jsFunc skip the parameter list before brace-matching - #15210
hivecommons-hive[bot] wants to merge 1 commit into
Conversation
jsFunc brace-matched a page function from the first `{` after its
signature. A default parameter such as `opts = {}` closes that brace at
once, so the slice stopped at `function costSortTh(table, key, label,
opts = {}` and any node harness whose closure reached it failed with a
SyntaxError pointing at the next function. Scan the parameter list
first (balanced, string-aware) and start brace-matching at the body.
Pin the extractor on every top-level function in the shipped page so a
new signature shape fails here, naming the function, rather than as an
opaque node error in whichever harness happens to reach it first.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Changelog: this PR changes code but carries no changelog entry If it is user-visible — a feature, a fix an operator would notice, a This is a reminder, not a gate; it never blocks a merge. |
Open PR overlap checkBase branch:
Conflicts are checked with every PR merged onto the current tip of No conflicting open PRs or same-file overlaps were found. |
|
CI note: 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Looks correct to me.
Read jsFunc at src/pkg/dashboard/acmm_per_repo_ui_test.go:30-62 on this head: start is the index of "function "+name+"(", so the first ( that jsBodyStart (:76-108) finds from sig is the parameter list, never a brace from an earlier default. The scanner balances (/[/{ together, skips '/"/` literals with backslash escapes, and on depth==0 returns the next {; when no ( or no closing ) exists it falls back to the old first-{ behaviour, so unrecognised signatures degrade rather than fail. The len(html) return when no body brace follows makes the caller's loop fall through to its existing "unbalanced braces" fatal, which is the right outcome.
The table in js_func_default_param_test.go covers the shapes that matter (object default, string default containing })(, arrow default), and TestJsFuncSlicesEveryPageFunctionToItsBody runs against the real page so a new signature shape fails with the function named.
One note for whoever merges: the only red check, docker (contributor), cannot be caused by this diff (two _test.go files, no runtime code or Dockerfile change) — it reads as infrastructure, not a defect here.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Looks correct to me. Read jsBodyStart at head 59594a6e (src/pkg/dashboard/acmm_per_repo_ui_test.go:76-109) and the callers it changes.
- The scanner starts at the first
(after the signature, balances()[]{}, skips'/"/`literals (with backslash escapes), and returns the first{after the paren list closes. Both fallbacks (open < 0, depth never returning to 0) reproduce the previous first-{behaviour, so a shape it can't read degrades rather than regresses — matches the comment. jsFunc's brace loop (:41) still receives an index pointing at a{, so thedepthaccounting is unchanged.- The new
js_func_default_param_test.gorelies onindexHTML(clickable_avatars_test.go:12) andjsDeclRe(your_turn_ui_11566_test.go:28), both already in thedashboardpackage on v6; no name collisions withjsBodyOpenRe/jsBodyStart/TestJsFunc*. - The
function f(sep = '})(')case is the one that would have broken a naive paren counter; it is covered.
The red docker (contributor) check is not this diff: job 123197324013 failed in "Build Docker image" with failed to copy: httpReadSeeker: … could not fetch content descriptor sha256:e2de9651… not found — a registry layer pull failure on a PR that touches only two _test.go files. A rerun should clear it.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
Why
jsFunc(pkg/dashboard/acmm_per_repo_ui_test.go) brace-matches a page function from the first{after its signature. A default parameter such asopts = {}closes that brace immediately, so the slice is the truncatedfunction costSortTh(table, key, label, opts = {}and any node harness whose closure reaches it dies withSyntaxError: Unexpected token 'function'pointing at the next function — not the one at fault.On v6 today nothing in a your-turn closure reaches
costSortTh, so this is latent. The v5→v6 forward-merge #15194 makes it live: v5'srender() → renderStageTracker()plus the merge-celebration chain (… → stageTrackerScheduleVoices, whoseosc.connect(matches the page's SSEconnect()) reachesrenderCost → costSortTh, andTestYourTurnReleaseOwnerStepAction12635/TestYourTurnMenuDesignActionHandsIssueToArchitect11566fail on every dashboard shard (run 40795001748). The extractor is v6's; the fix belongs here so the merge can pick it up.What
jsBodyStartscans the parameter list (balanced()[]{}, string-aware) and starts brace-matching at the body's{; falls back to the old first-{behaviour if no parameter list closes.TestJsFuncSkipsDefaultParameterBraces— synthetic signatures (object/array/arrow/string defaults, async, plain).TestJsFuncSlicesEveryPageFunctionToItsBody— every top-level function in the shippedindex.htmlslices to a declaration that contains its body-opening){, so a future signature shape the extractor cannot read fails here, naming the function.TestJsFuncCostSortThSliceIsValidJavaScript— the realcostSortThslice passesnode --check.All three fail against the current
jsFuncand pass with the change, on both the v6 page and the #15194 head page (verified by compiling the helper + tests into a scratch binary;go testis unavailable on this host).Refs #15194 (forward-merge whose dashboard shards this unblocks once v6 is merged into its branch; the PR's other reds are separate semantic conflicts and are triaged on the PR).
Test-only change; changelog fragment not required (
*_test.goauto-exempt).Filed by quality agent (ACMM L4/L6 — full mode)
— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88 requested_by=@clubanderson