Keep regexp statements parseable after formatting

[fhammerschmidt]

Formatting a parenthesized regexp statement after a binding or another expression removed the parentheses, causing its opening / to be parsed as division continuing the previous statement. Keep parentheses around statements whose printed expression starts with a regexp, including pipes, binary expressions, ternaries, object and field reads and writes, bracket access, chained calls, and PPX-generated ## accesses.

Track whether the previous structure item or block row can continue as an expression, so first statements and statements following declarations such as type, open, or module keep their existing formatting. Statement attributes already separate expressions and do not need an extra regexp wrapper. Follow the printer's operand-parenthesis rules to avoid wrapping expressions whose left edge is already protected. Share the statement-printing logic across block and structure contexts, and inspect placeholder bodies without allocating a rewritten AST. Preserve attributes on placeholder calls, include attributes in statement locations when preserving blank lines, and parenthesize attributed ternary conditions to retain their scope. Middle and final ternary operands keep their existing formatting.

Keep protection on the expression's first and last lines so narrow formatting widths remain stable and preserve comments. This fixes #8688 without changing existing division rules. Extending the newline heuristic for unary operators to / would change valid multiline division such as a\n /2, while whitespace-leading regexps would remain ambiguous. Handwritten regexp statements after another expression still require parentheses or a semicolon.

Intended for backporting to v12 after merging on master.

Validation:

• make compiler and dune build @fmt.
• node scripts/test.js -ounit -mocha -build -docstrings: 267 OUnit tests, 591 runtime tests, all 42 build fixtures, CommonJS and stdlib tests, and 744 documentation examples. The regexp fixture checks formatting, reparsing, -check-lam, and runtime behavior, including attributed calls and placeholders.
• make test-syntax and make test-syntax-roundtrip.
• ReScript formatting of the build fixture and Biome checks for its JavaScript test.
• Comment preservation and formatting stability at widths 20, 40, 80, 100, and 120, including multiline integer and float division.
• Snapshot regressions for first statements, statements after type/open/module, statement attributes, PPX ## accesses, attributed expressions, and a regexp ternary used as a binary operand.
• 35 AST-preservation checks across five widths cover statement boundaries and attribute scope in ordinary and nested ternaries. Runtime regressions cover regexp statements immediately after open and module aliases.

[codecov]

Codecov Report

❌ Patch coverage is 96.77419% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.57%. Comparing base (bfbc9ee) to head (db22b97).

Files with missing lines	Patch %	Lines
compiler/syntax/src/res_parens.ml	93.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8720      +/-   ##
==========================================
+ Coverage   80.54%   80.57%   +0.02%     
==========================================
  Files         447      447              
  Lines       61166    61210      +44     
==========================================
+ Hits        49264    49317      +53     
+ Misses      11902    11893       -9     

Files with missing lines	Coverage Δ
compiler/syntax/src/res_printer.ml	93.95% <100.00%> (+0.33%)	⬆️
compiler/syntax/src/res_parens.ml	87.63% <93.33%> (+0.67%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pkg-pr-new]

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@8720

@rescript/belt

npm i https://pkg.pr.new/@rescript/belt@8720

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@8720

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8720

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@8720

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@8720

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@8720

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@8720

commit: db22b97

[fhammerschmidt]

@codex review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-09T14:38:45.333459Z	c922ca1	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ad914954a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[fhammerschmidt]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7ef23231f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[fhammerschmidt]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: c922ca108f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cknitt]

Thanks for picking this up! I ran a review with Claude, and it found a few things. Some of them may not hold up, so please push back where they're wrong.

1. Fix this in the parser instead? (compiler/syntax/src/res_core.ml, parse_binary_expr)
Right now only the formatter is changed. Handwritten code like

let t = s
/a/->RegExp.test(t)

still parses as s / a / ... and gives a confusing type error. The parser already handles this case for -, -., < and % with Scanner.is_binary_op plus a line-number check. Adding Forwardslash | ForwardslashDot there would fix parsing and formatting together, and the starts_with_regexp walk in the printer would probably not be needed. If we keep the printer approach, the points below apply.

2. Unneeded parentheses at the top level (res_printer.ml, print_structure_item)
At the top level, a statement that starts with a regexp always gets parentheses, but blocks only add them after another statement (acc <> []). So a file that starts with /a/->RegExp.test("a")->Console.log, or has that line right after a type or open, gets rewritten to (/a/->...). That causes formatting churn in existing code that was never ambiguous, and the two contexts behave differently.

3. ## is not handled (res_parens.ml, starts_with_regexp)
Pexp_apply with ## as the function prints as parent["member"], so the regexp comes first. starts_with_regexp stops at the ## ident and returns false. An AST from a PPX like Pexp_apply(##, [/a/; x]) therefore prints as /a/["x"] without parentheses and reparses as division.

4. Double parentheses (res_parens.ml, starts_with_regexp)
The check also looks inside parts the printer already wraps in parentheses, so we get output like:

• (/a/ ? 1 : 2) + 1 → ((/a/ ? 1 : 2) + 1)
• (@attr /a/)->f → ((@attr /a/)->f)

5. statement_expr can be simpler (res_parens.ml)
It calls expr e again and returns a kind, but every caller has already matched Nothing. The Parenthesized / Braced arms in the callers can never be reached. Exposing starts_with_regexp : expression -> bool and writing | Nothing when Parens.starts_with_regexp e && acc <> [] -> add_statement_parens doc would avoid the second classification and the dead arms.

6. Same logic in three places (res_printer.ml)
The nested match that picks statement parentheses appears twice in collect_rows (the Pexp_sequence branch and the final-expression branch) and once more in print_structure_item. A shared helper (say print_statement_expr ~is_first) would mean future fixes, such as #3, only go in one place.

7. Extra allocation for _ placeholders (res_parens.ml)
For every f(_, x) statement, rewrite_underscore_apply builds a rewritten AST only to check its left edge. Matching the placeholder body directly would avoid that.

Suggested tests, whichever approach you go with: a regexp statement as the first top-level item, one after type/open, a ## access on a regexp, and a regexp ternary used as a binary operand.

[fhammerschmidt]

@cknitt Points 2–7 are now implemented:

• 2: Track whether the preceding structure item can continue as an expression. First regexp statements and those after type or open keep their existing formatting.
• 3: Follow the receiver of PPX-generated ## accesses, with an AST regression test.
• 4: Follow the printer’s existing operand-parenthesis rules, so already-protected operands do not cause an extra statement-level pair.
• 5–6: Expose starts_with_regexp : expression -> bool, remove the redundant classification, and share print_statement_expr across the three statement-printing paths.
• 7: Inspect placeholder bodies directly rather than allocating a rewritten AST just to check the left edge.

The suggested tests are included: first top-level regexp, regexp after type/open, PPX ##, and a regexp ternary used as a binary operand. Further checks caught attribute-related cases; the printer now preserves placeholder attributes, accounts for attribute locations when preserving blank lines, and keeps attributes scoped to their ternary operands.

For 1, I kept the formatter approach because adding Forwardslash | ForwardslashDot to the existing newline heuristic would change valid multiline division, for example:

let b = a
  /2

That currently means a / 2. Since the slash has no whitespace on its right, the proposed heuristic would instead end the preceding expression and attempt to parse a regexp. The equivalent /.2. case has the same problem. Conversely, a whitespace-leading regexp such as / a/ still satisfies the binary-operator whitespace check, so the proposal would not resolve every regexp ambiguity.

This PR therefore preserves the existing division rules and prevents formatting from removing necessary statement separation. Handwritten regexp statements following another expression still need parentheses or a semicolon. The parser alternative would need a broader parsing decision.

Validation passes: compiler build, syntax and roundtrip suites, 267 OUnit tests, 591 runtime tests, all 42 build fixtures, 744 documentation examples, and formatting/changelog checks. Comment preservation and formatting stability are checked at widths 20, 40, 80, 100, and 120.

[cknitt]

Re-reviewed in Claude:

1. The ternary change fixes a separate bug but is broader than it needs to be. The new rule in res_parens.ml (ternary_operand) puts parentheses around any ternary operand that has an attribute.
   • The bug is real: on master, the statement (@a x) ? 1 : 2 is formatted as @a x ? 1 : 2, which reparses with @a on the whole statement instead of on x.
   • The churn: the rule also changes the middle and last operands, and every non-statement position, where master was already correct. For example, c ? @inline x : y becomes c ? (@inline x) : y, and let x = @attrOnCondition truth ? true : false gets parentheses too. Limiting it to the condition, or only to statements, would avoid that.
   • Not documented: the changelog fragment only describes regexps, not this fix.
2. Blocks and top level still behave differently. Inside { … }, a regexp statement right after open Foo or module M = Foo still gets parentheses, because the block check is just "is there an earlier row" (acc <> []). At the top level, the same code stays bare. The block check could skip those rows the same way the top-level one does.
3. Extra parens after a statement attribute. let x = 1\n@foo (/a/->f) now formats as @foo\n(/a/->f). Master leaves it bare, and that's fine because the @foo line already ends the previous statement. Harmless churn.
4. Layout nit: a long protected pipe chain is printed as (/aaa/\n->RegExp.test(…)\n->ignore), with the -> lines not indented and the ) on the last line. That is deliberate (it keeps comments and repeated formatting stable) and it is idempotent, but it looks odd.

[fhammerschmidt]

f76591a43 addresses points 1–3: ternary attribute protection is limited to conditions and documented; blocks now follow the same declaration boundaries as top level; statement attributes avoid redundant regexp parentheses. Added 35 AST-preservation checks across five widths, plus runtime regressions. Point 4’s pipe layout remains intentional to preserve comments and formatting stability.