Fix module expressions losing attributes, await or parens when formatting by cknitt · Pull Request #8735 · rescript-lang/rescript

cknitt · 2026-10-07T10:59:45Z

The formatter dropped or moved attributes and await on module expressions, and printed code that doesn't parse for module constraints and functors in some positions.

print_mod_expr never printed pmod_attributes, and several other printer paths print or take apart a module expression on their own:

Source	Formatted before this PR
`module M = @attr F({})`	`module M = F()`
`module M = (@attr F)({})`	`module M = F()`
`module M = @attr (X: S)`	`module M: S = X`
`F(@attr (X: S))`	`F(@attr X: S)` (attribute moves to `X`)
local `module M = @attr (X: S)`	`module M: S = X`
`(X) => @attr (Y: S)`	`(_: X): S => Y`
`(Z: T) => await (X: S)`	`(Z: T): S => X` (await dropped)
`include @attr F({type t = int})`	`include F({type t = int})`
`(await F(A))(B)`	`F(A, B)` (await dropped)
`(await F)(A)`	`await F(A)`
`H(@attr {})`	`H()`
`module M: T = (X: S)`	`module M: T = X: S` (doesn't parse)
`((X: S))(Z)`, `((Y: S) => {})(Z)`, `(%ext)(Z)`	parens dropped
`module type of (X: S)`, `module((X: S))`	parens dropped
`(module((X: S1)): module(S2))`	`module(X: S1: S2)` (doesn't parse)

Found while adding a fixture for #8734: module M = @inlined F({}) couldn't be kept in a formatted test file.

Changes

Print a module expression's attributes before it, doc comments first, on one line with it. Functors are the exception: their attributes are printed on the first parameter, as before.
Parsetree_viewer.mod_expr_has_attributes: the attributes print_attributes prints, or await. It guards every place that hoists a constraint (module bindings, local modules, functor results), flattens applications or merges nested functors, so those only apply to unannotated nodes.
A constraint with attributes or await prints its own parens: @attr (X: S), await @attr (X: S). Without them, the position decides: after module M: T =, in a functor's result, after module type of and inside module(...).- Applied functors are parenthesized when they're a constraint, a functor, an extension without payload, or carry attributes or await (Parens.mod_apply_callee).
A typed pack of a constraint keeps the general form (module((X: S1)): module(S2)), since module((X: S1): S2) parses as a functor.
A functor argument whose printed form starts with a doc comment is parenthesized, since the parser doesn't accept one there.
The include F({type t = ...}) shortcut and the F() shorthand only apply without attributes.
New parens hug the module expression, (M: {...}), like the existing binding parens.

No existing syntax snapshot changes.

How it was checked

Beyond the syntax tests, I formatted every combination of 15 contexts (bindings, module rec, local modules, include, functor bodies and arguments, applied functors, module(...), module type of, await) and about 450 module expressions with attributes and await on different nodes, nested two levels deep: 6750 cases. For each, the reparsed tree must equal the original and formatting must be idempotent.

master fails 4541 of them; this PR fails none that master passes.
The same holds with @JSX, @attr("payload"), @a.b and a doc comment in place of @attr.
Ignoring attribute order on a node, 6 remain: await written twice on one module, await @w (await X), which the parser stores as two res.await on the same node. That can't type-check and is left alone.
A separate check with comments in each changed position keeps all comments.
Formatting the repo's 4643 .res/.resi files differs from master only in the new tests and 4 parser fixtures whose module attributes master dropped.

Tests

printer/modExpr/attributes.res: attributes and await on each kind of module expression and in each of the positions above.
printer/modExpr/parens.res: unattributed constraints and functors in positions that need parens.

🤖 Generated with Claude Code

Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

codecov · 2026-10-07T11:13:58Z

Codecov Report

❌ Patch coverage is 95.29412% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.88%. Comparing base (fb2093e) to head (469cbbe).

Files with missing lines	Patch %	Lines
compiler/syntax/src/res_parsetree_viewer.ml	88.88%	2 Missing ⚠️
compiler/syntax/src/res_printer.ml	96.77%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8735      +/-   ##
==========================================
+ Coverage   79.87%   79.88%   +0.01%     
==========================================
  Files         464      464              
  Lines       63078    63126      +48     
==========================================
+ Hits        50382    50428      +46     
- Misses      12696    12698       +2

Files with missing lines	Coverage Δ
compiler/syntax/src/res_parens.ml	`83.70% <100.00%> (+0.24%)`	⬆️
compiler/syntax/src/res_parsetree_viewer.ml	`92.39% <88.88%> (+0.18%)`	⬆️
compiler/syntax/src/res_printer.ml	`93.50% <96.77%> (+0.01%)`	⬆️

🚀 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 · 2026-10-07T11:17:19Z

Open in StackBlitz

rescript

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

@rescript/belt

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

@rescript/darwin-arm64

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

@rescript/darwin-x64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

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

@rescript/win32-x64

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

commit: 469cbbe

github-actions · 2026-10-07T11:19:08Z

Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8735

cknitt · 2026-10-07T15:48:54Z

@codex review

chatgpt-codex-connector · 2026-10-07T15:49:17Z

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-08T05:23:08.516748Z	`469cbbe`	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: 7d925133e2

ℹ️ 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 · 2026-10-07T16:47:02Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 77e9cb704e

ℹ️ 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 · 2026-10-07T17:14:42Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8329512020

ℹ️ 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 · 2026-10-07T18:04:39Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fba1d121ce

ℹ️ 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".

@attr

print_mod_expr never printed pmod_attributes, so the formatter dropped attributes such as `module M = @attr F(X)`, `module M = @attr {}` and `include @attr F(X)`. Print them before the module expression, except on functors, whose attributes are already printed on their first parameter. Two placements need care to round-trip: - An attributed functor in an application is parenthesized, `(@attr F)(X)`, and an attributed inner application is no longer flattened into the outer one, `(@attr F(A))(B)`; otherwise the attribute would move to the whole application. - `await` is printed before the attributes, since `@attr await M` does not parse. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@attr

An attribute on a constraint was printed without parens, so it moved to the constrained module on reparse (`F(@attr (X: S))` printed as `F(@attr X: S)`, and likewise for `include`). On the right-hand side of a module binding it was dropped, because `module M = @attr (X: S)` was printed as `module M: S = X`. Parenthesize attributed constraints, keep them on the right-hand side of a binding, and don't add a second pair of parens after `include`. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Use the existing Parsetree_viewer.has_printable_attributes instead of filtering the attributes and comparing with [], and share the rule that an attributed module constraint prints its own parens between the printer and Parens.include_mod_expr. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@attr

`await @attr (X: S)` printed as `await (@attr X: S)`, moving the attribute from the constraint to X: await's parens enclosed the attributes but not the constraint itself. An attributed constraint now always prints its own parens, and await only adds parens around an unattributed one. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@attr

Besides print_mod_expr, several printer paths print or take apart a module expression on their own, and each dropped or moved attributes and await, or printed code that doesn't parse: - a module constraint was hoisted into a local module's binding (`module M = @attr (X: S)` lost @attr) and a functor's result signature, the latter also dropping await - the `include F({type t = ...})` shortcut ignored all attributes - application flattening and callee parens ignored await, so `(await F(A))(B)` printed as `F(A, B)` and `(await F)(A)` as `await F(A)` - nested functors were merged through an awaited inner functor - `F(@attr {})` printed as `F()` - applied constraints, functors and extensions lost their parens, as did constraints after `module M: T =`, `module type of` and inside `module(...)` Found by formatting and re-parsing every combination of 15 contexts and about 450 module expressions, comparing parsetrees. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- `mod_expr_has_attributes` now uses the same filter as `print_attributes`. It used `has_printable_attributes`, which also excludes `@JSX`, so `(@jsx F)(A)` printed as `@JSX F(A)` and `(@jsx F(A))(B)` lost the attribute. - A functor argument whose printed form starts with a doc comment is parenthesized, since the parser doesn't accept a doc comment there. - A constraint now prints its own parens whenever it has attributes or `await`, which replaces `Parens.attributed_mod_constraint` and the constraint case of `await`. - `print_mod_expr_constraint_parens` covers what `Parens.mod_expr_parens` did for module bindings, so that is removed. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Removing it changed how existing code is formatted: after a signature holding a single module, `} = (M')` lost its parens and printed as `} = M'`. Both parse the same, but this PR shouldn't reformat existing code (tests/tests/src/coercion_module_alias_test.res failed the format check). Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

mod_expr_has_attributes filtered the attributes only to test whether the result was empty, and the doc comment check filtered and partitioned them. Factor out is_parsing_attr and is_doc_comment_attribute and use List.exists instead. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

From another review pass: - `(module((X: S1)): module(S2))` printed as `module((X: S1): S2)`, which parses as a functor. A typed pack of a constraint now keeps the general form. - An applied extension with a payload got parens it doesn't need: `%ext(A)(B)` printed as `(%ext(A))(B)`. Only `(%ext)(B)` needs them. - A module expression's attributes and doc comments stay on one line with it, instead of a doc comment or a long attribute list breaking onto unindented lines. - One rule for when an applied module needs parens (`Parens.mod_apply_callee`) and one for when a constraint does (`Parens.mod_constraint`, formerly `include_mod_expr`), used everywhere. The doc comment check uses the former and accounts for functors and `await`, which print first, so it no longer adds redundant parens. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cknitt · 2026-10-08T05:18:43Z

@codex review

chatgpt-codex-connector · 2026-10-08T05:23:07Z

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 469cbbe960

ℹ️ 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".

`res.ternary` and `JSX` are now only part of the v0 PPX wire format: the parser produces `Pexp_ternary` and `Pexp_jsx_element`, and the v0 bridge converts both markers back into those nodes. Drop them from `is_parsing_attr`, `has_attributes` and `is_printable_attribute`. `is_printable_attribute` then only differed from `is_parsing_attr` in `res.patVariantSpread` and `res.dictPattern`, which the parser only puts on patterns, while the printable helpers are only used on expressions and module expressions. Define the printable helpers in terms of `is_parsing_attr`, so the printer has one list of internal attributes instead of two that disagreed (the cause of the `@JSX` review finding in #8735). `filter_printable_attributes` was the same as `filter_parsing_attrs` and is removed. A `@JSX` attribute written in source now gets the same parens as any other attribute, e.g. `(@jsx x) + 1`. Formatting of the repo's sources is unchanged. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

#8740) * Remove obsolete attributes from the printer's filters and merge them `res.ternary` and `JSX` are now only part of the v0 PPX wire format: the parser produces `Pexp_ternary` and `Pexp_jsx_element`, and the v0 bridge converts both markers back into those nodes. Drop them from `is_parsing_attr`, `has_attributes` and `is_printable_attribute`. `is_printable_attribute` then only differed from `is_parsing_attr` in `res.patVariantSpread` and `res.dictPattern`, which the parser only puts on patterns, while the printable helpers are only used on expressions and module expressions. Define the printable helpers in terms of `is_parsing_attr`, so the printer has one list of internal attributes instead of two that disagreed (the cause of the `@JSX` review finding in #8735). `filter_printable_attributes` was the same as `filter_parsing_attrs` and is removed. A `@JSX` attribute written in source now gets the same parens as any other attribute, e.g. `(@jsx x) + 1`. Formatting of the repo's sources is unchanged. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add CHANGELOG entry for #8740 Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test the formatting of @jsx and @res.ternary written in source Printer cases for both marker names, and an ordinary attribute, on operands of binary operators, pipes, unary operators, field and array access, calls, ternaries and containers. Before the previous commit, @jsx lost its parens, e.g. @jsx x + 1, and @res.ternary was dropped. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

cknitt added a commit that referenced this pull request Oct 7, 2026

Add CHANGELOG link for #8735

7d92513

Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cknitt marked this pull request as ready for review October 7, 2026 15:47

chatgpt-codex-connector Bot reviewed Oct 7, 2026

View reviewed changes

Comment thread compiler/syntax/src/res_printer.ml Outdated

chatgpt-codex-connector Bot reviewed Oct 7, 2026

View reviewed changes

Comment thread compiler/syntax/src/res_parens.ml Outdated

chatgpt-codex-connector Bot reviewed Oct 7, 2026

View reviewed changes

Comment thread compiler/syntax/src/res_parsetree_viewer.ml Outdated

Comment thread compiler/syntax/src/res_printer.ml Outdated

Comment thread compiler/syntax/src/res_printer.ml

Comment thread compiler/syntax/src/res_printer.ml Outdated

cknitt changed the title ~~Keep attributes on module expressions when formatting~~ Fix module expressions losing attributes, await or parens when formatting Oct 7, 2026

cknitt mentioned this pull request Oct 7, 2026

Remove the call-site @inlined attribute #8734

Merged

chatgpt-codex-connector Bot reviewed Oct 7, 2026

View reviewed changes

Comment thread compiler/syntax/src/res_parsetree_viewer.ml Outdated

cknitt and others added 10 commits October 8, 2026 04:49

Add CHANGELOG link for #8735

44f0901

Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cknitt force-pushed the print-module-expr-attributes branch from 45b832a to 469cbbe Compare October 8, 2026 05:16

cknitt requested a review from cristianoc October 8, 2026 05:23

cknitt mentioned this pull request Oct 8, 2026

Represent await on module expressions with a Pmod_await node #8738

Merged

cknitt added this pull request to stack #8739 October 8, 2026 06:31

cknitt mentioned this pull request Oct 8, 2026

Remove obsolete attributes from the formatter's filters and merge them #8740

Merged

cristianoc approved these changes Oct 8, 2026

View reviewed changes

cknitt merged commit ac96983 into master Oct 8, 2026
24 checks passed

cknitt deleted the print-module-expr-attributes branch October 8, 2026 08:28

Conversation

cknitt commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

How it was checked

Tests

Uh oh!

codecov Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pkg-pr-new Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 7, 2026

Uh oh!

cknitt commented Oct 7, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

cknitt commented Oct 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

cknitt commented Oct 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cknitt commented Oct 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

cknitt commented Oct 8, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cknitt commented Oct 7, 2026 •

edited

Loading

codecov Bot commented Oct 7, 2026 •

edited

Loading

pkg-pr-new Bot commented Oct 7, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 7, 2026 •

edited

Loading