Skip to content

Fix functor module type result-signature brace formatting#8519

Open
MavenRain wants to merge 4 commits into
rescript-lang:masterfrom
MavenRain:fix/7949-functor-result-brace
Open

Fix functor module type result-signature brace formatting#8519
MavenRain wants to merge 4 commits into
rescript-lang:masterfrom
MavenRain:fix/7949-functor-result-brace

Conversation

@MavenRain

Copy link
Copy Markdown

Fix functor module type result-signature brace formatting

Fixes #7949.

Problem

When a functor module type returns a signature, the formatter broke the
opening brace onto its own line:

// input
module Make: Pattern => {
  let fmt: event => string
}

// formatted (before)
module Make: Pattern =>
{
  let fmt: event => string
}

This is inconsistent with functor module expressions, which already keep the
brace on the same line as =>:

module Make = (P: Pattern) => {
  let x = 1
}

Cause

In print_mod_type (the Pmty_functor arm of compiler/syntax/src/res_printer.ml),
the arrow was followed by a breakable Doc.line. Because a signature result
force-breaks internally, the enclosing group always broke at that Doc.line,
pushing { to the next line. The sibling print_mod_functor (module
expressions) uses an unconditional hard space (Doc.text " => ") and so never
had this problem.

Fix

When the functor result is a signature, use a hard space before it (hug the
brace); otherwise keep the breakable Doc.line so long non-signature results
(idents, with constraints, curried arrows) can still wrap:

let arrow_sep =
  match return_type.pmty_desc with
  | Pmty_signature _ -> Doc.space
  | _ -> Doc.line
in

Testing

  • New printer test tests/syntax_tests/data/printer/modType/functorInterface.resi
    covering the reported .resi case plus curried params, a nested/curried arrow,
    an empty signature, and non-signature results (which must remain unchanged).
  • Two existing golden outputs that had captured the wrong behavior are updated
    (modType/functor.res.txt module type B = () => {, and
    modExpr/structure.res.txt module G0: (X: {}) => {).
  • make test-syntax and make test-syntax-roundtrip pass; the printer +
    idempotency corpus round-trips with no diff, and comments around => / inside
    the result signature are preserved.

Drafted with AI assistance; the change, tests, and verification were reviewed by me.

@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: 77aaf8f19b

ℹ️ 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
Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
@fhammerschmidt
fhammerschmidt requested a review from shulhi July 23, 2026 19:35

@shulhi shulhi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!


module ResultIdent: A => B

module ResultWith: A => (B with type t = int)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@MavenRain can you add module name with a longer name as well (more than 80 characters in one line) to test how the formatter handles that case?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added three long-name cases (each over 80 columns) to functorInterface.resi plus their golden output:

module ThisIsAVeryLongFunctorModuleTypeNameThatExceedsTheEightyCharacterLimit: Pattern => {
  let fmt: event => string
}

module SecondVeryLongFunctorModuleTypeNameWithMultipleShortParameters: (A, B) => {
  let x: int
}

module YetAnotherVeryLongFunctorModuleTypeNameHere: SomeVeryLongParameterModuleTypeName =>
SomeVeryLongResultingModuleTypeName

How the formatter handles them:

  • Signature results keep the brace hugged even past 80 columns (91 and 82 columns above). For Name: Pattern => { there is no break point that could rescue the width anyway: before this fix the printer emitted the same overlong header minus the brace and then put { alone on the next line, so hugging costs 2 characters on an already-overlong line and matches how module-expression functors print.
  • Long non-signature results still wrap at the breakable line after => (third case); that path is untouched by this PR.
  • One trait the new golden makes visible: the second case's short parameter tuple stays flat rather than breaking to rescue the width. That behavior is not new here: master prints the same header flat (just with { pushed to the next line), and module-expression functors keep short parameter tuples flat on comparable over-80-column headers as well. The fix only changes where the brace goes.

The full corpus regeneration shows no other golden moved, and the printer + idempotency roundtrip suite still passes; the new cases are idempotent. Reverting the fix breaks the new golden on exactly the two long-name signature cases, so they do exercise the changed code path.

Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>

module type B = () =>
{
module type B = () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sorry I should be more explicit @MavenRain, can you also add a similar test here? Just need to be sure that both .res and .resi are handled correctly.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added the mirrored three cases to functor.res plus their golden output:

module type ThisIsAVeryLongFunctorModuleTypeNameThatExceedsTheEightyCharacterLimit = Pattern => {
  let fmt: event => string
}

module type SecondVeryLongFunctorModuleTypeNameWithMultipleShortParameters = (A, B) => {
  let x: int
}

module type YetAnotherVeryLongFunctorModuleTypeNameHere = SomeVeryLongParameterModuleTypeName =>
SomeVeryLongResultingModuleTypeName

The .res output matches the .resi behavior shape for shape: signature results keep the brace hugged past 80 columns (97 and 88 columns here), the short parameter tuple stays flat, and the long non-signature result wraps at the break point after =>. The symmetry is structural: module type X = ... in .res and module X: ... in .resi both delegate to the same print_mod_type functor arm that this PR changed, so the brace placement rule is shared between the two syntaxes.

Full corpus regeneration moves no other golden, the printer + idempotency roundtrip suite still passes, and reverting the fix breaks this golden on exactly the two new signature-result cases (plus the pre-existing module type B = () => {} case), so the new cases do pin the changed code path on the .res side as well.

Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
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.

formatter formats braces incorrectly for functors in resi

2 participants