Skip to content

Fix formatter forcing doc-commented inline-record exceptions onto multiple lines - #8622

Open
MavenRain wants to merge 1 commit into
rescript-lang:masterfrom
MavenRain:fix/7947-exception-inline-record-doc-comment
Open

Fix formatter forcing doc-commented inline-record exceptions onto multiple lines#8622
MavenRain wants to merge 1 commit into
rescript-lang:masterfrom
MavenRain:fix/7947-exception-inline-record-doc-comment

Conversation

@MavenRain

Copy link
Copy Markdown
Contributor

Summary

Fixes #7947. A doc comment on an exception with an inline-record argument forced the record onto multiple lines. One line changes: the inline-record branch of print_constructor_arguments now wraps its argument doc in a Doc.group, exactly as the tuple branch already did.

Problem

With a doc comment, the record is exploded:

/** foo */
exception Foo({name: string, msg: string})

printed as:

/** foo */
exception Foo({
  name: string,
  msg: string,
})

Nothing else in the same neighbourhood behaves this way. All four of these stay on one line today:

// same declaration, no doc comment
exception Bar({name: string, msg: string})

// doc comment, tuple argument instead of an inline record
/** foo */
exception Baz(string, string)

// attribute instead of a doc comment
@attr exception Qux({name: string, msg: string})

// doc comment, inline record in a variant constructor
/** foo */
type t = Foo({name: string, msg: string})

That asymmetry matches the issue thread. @nojaf asked:

Is this also the case with type (records, inline in variant)?

and the reporter answered:

no, just exception

The cause is in compiler/syntax/src/res_printer.ml, in print_constructor_arguments. The Pcstr_tuple branch ends with:

Doc.group (if indent then Doc.indent args else args)

while the Pcstr_record branch ended with a bare:

if indent then Doc.indent args else args

A doc comment is printed as a hard line break by print_exception_def (and by print_extension_constructor). Because the record argument had no group of its own, its soft_lines sat in the same group as that hard break, so the forced break propagated into the record. The tuple branch was immune only because its group isolated it.

Fix

           Doc.rbrace;
           Doc.rparen;
         ]
     in
-    if indent then Doc.indent args else args
+    Doc.group (if indent then Doc.indent args else args)

The two branches are now identical in this respect. Breaking still happens when the record genuinely does not fit in 100 columns, since the group breaks on width as usual.

The same code path serves extension constructors, so type t += /** doc */ Ext({name: string, msg: string}) keeps its record on one line for free.

Two things are deliberately left alone:

Testing

Printer fixtures added, with the expected files regenerated:

  • tests/syntax_tests/data/printer/structure/exception.res: doc-commented inline-record exceptions with one field and with two fields, plus a four-field case that is over 100 columns and still breaks.
  • tests/syntax_tests/data/printer/signature/exception.resi: the same one-field and two-field cases in a signature.
  • tests/syntax_tests/data/printer/structure/typeExtension.res: a doc-commented inline-record extension constructor.

No existing expected file changed. The new group only takes effect when an enclosing group is already broken, which no previous fixture exercised.

Gates, all green:

  • ROUNDTRIP_TEST=1 ./scripts/test_syntax.sh: no unstaged test differences, roundtrip tests succeeded.
  • bsc -format over the CI file list: 1813 files checked, 0 need formatting.
  • ocamlformat 0.29.0 on res_printer.ml: clean.

Manual check on the reported snippet: it prints on one line after the fix, and formatting the output a second time is idempotent.

AI-assisted; I reviewed and tested every change.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8c57cb29de

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread CHANGELOG.md Outdated
…tiple lines (rescript-lang#7947)

Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
@MavenRain
MavenRain force-pushed the fix/7947-exception-inline-record-doc-comment branch from 8c57cb2 to b4daa71 Compare September 4, 2026 13:26
@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.32%. Comparing base (bc382a4) to head (b4daa71).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8622      +/-   ##
==========================================
- Coverage   77.32%   77.32%   -0.01%     
==========================================
  Files         467      467              
  Lines       63342    63313      -29     
==========================================
- Hits        48982    48957      -25     
+ Misses      14360    14356       -4     
Files with missing lines Coverage Δ
compiler/syntax/src/res_printer.ml 92.95% <100.00%> (+0.01%) ⬆️

... and 2 files 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

pkg-pr-new Bot commented Sep 4, 2026

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@8622

@rescript/belt

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/belt@8622

@rescript/darwin-arm64

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

@rescript/darwin-x64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@8622

@rescript/win32-x64

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

commit: b4daa71

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

doc comment forces multiline exceptions

1 participant