Skip to content

Add parens to funcs in return position of docs rendered type annotations - #10606

Open
jrrrp wants to merge 1 commit into
roc-lang:mainfrom
jrrrp:fix-incorrect-doc-parens
Open

Add parens to funcs in return position of docs rendered type annotations#10606
jrrrp wants to merge 1 commit into
roc-lang:mainfrom
jrrrp:fix-incorrect-doc-parens

Conversation

@jrrrp

@jrrrp jrrrp commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

(Zulip thread)

Before, if a function returned a function, the return type that was rendered to the docs was not surrounded with parentheses, leading to the following invalid syntax:
tagged_value : a -> b -> SqliteStmt => Try(...
(Example from basic-cli docs)

Similarly, functions in tag payloads would not be parenthesized in the docs. The following returns a binary function enclosed in Foo
f = || Foo(|a, b| a + b)
but leads to the below type in the docs, regardless of whether the type was inferred or manually annotated in the source:

f : () -> [
    Foo(
        Dec, Dec -> Dec,
    ),
]

This describes a different type: a tag Foo having a payload with two fields, one Dec and one Dec -> Dec.


needs_parens was flipped to true for the arguments of tuples, tag payloads, and function return types, which ensures safe wrapping, although it now leads to redundant parentheses in the docs with unary functions in tags and tuples.

h : () -> [
    Bar(
        (a -> a), # Extra level of nested parens
    ),
]

@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes incorrectly rendered type annotations in generated docs when function types appear in certain positions. Without parentheses, multi-arg functions in tag payloads, tuple elements, type-application arguments, and function return positions could produce ambiguous or invalid Roc syntax (e.g., Foo(Dec, Dec -> Dec) being parsed as two payload args instead of one function).

  • Sets needs_parens = true for tag payload args, tuple elements, and type-application args, preventing commas inside multi-arg function types from being confused with delimiter commas.
  • Sets needs_parens = true for the return type of a function, ensuring effectful inner functions like b -> SqliteStmt => Try(…) are correctly parenthesized.
  • As an acknowledged trade-off, unary functions in those positions now receive redundant parentheses (e.g., Bar((a -> a))), which is valid Roc syntax.

Confidence Score: 4/5

The changes are correct and conservative — all seven one-line flips fix genuine ambiguities in generated docs, with the only downside being acknowledged redundant parens around unary functions.

The logic is straightforward and the fix matches the existing pattern for function arguments which were already true before this PR. The one gap is record field types, which carry the same latent ambiguity for multi-arg functions but are pre-existing and not worsened by this change.

Files Needing Attention: Record field type rendering in src/docs/render_html.zig around lines 2206-2226 deserves a follow-up to close the remaining gap where needs_parens stays false.

Important Files Changed

Filename Overview
src/docs/render_html.zig Seven targeted needs_parens = false to true changes fix function-type disambiguation in tag payloads, tuple elements, apply-args, and function return types; record field types remain false and carry the same latent ambiguity for multi-arg functions.

Comments Outside Diff (1)

  1. src/docs/render_html.zig, line 2206-2210 (link)

    P2 Record field types still use needs_parens = false

    This PR correctly fixes function-type disambiguation for tags, tuples, apply-args, and return types, but record field types remain unguarded. A multi-arg function like a, b -> c as a record field renders as { fieldName : a, b -> c }, where the comma after a is indistinguishable from a field separator, so the annotation parses as two fields (fieldName : a and the orphaned b -> c). The same fix (needs_parens = true on field.type) would resolve it, following the pattern applied everywhere else in this diff.

Reviews (1): Last reviewed commit: "Add parens to funcs in return position o..." | Re-trigger Greptile

@jrrrp

jrrrp commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Greptile review noted a case where records containing functions will not have parentheses, unlike the changes made that apply to function return types etc.
E.g., the generated annotation for {key: |v1, _v2| v1} being {key: a, a -> a}.

However, the annotation {key: a, a -> a} currently parses as expected/unambiguously to a record containing a binary function. Whether a parenthesised version would be more readable is a separate question.

@rtfeldman

Copy link
Copy Markdown
Contributor

@jrrrp Thanks for the contribution! I definitely like the idea of the formatter adding the parens for you automatically, but it's important that the formatter never introduce nested parens like ((foo -> bar)) - among other reasons because it will make the formatting unstable, and running roc fmt a second time will delete them. 😄

That requires a more involved fix - I can either do that if you like, or if you want to take a crack at it in this PR, feel free!

@jrrrp

jrrrp commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Ah, but this PR should only affect the rendered roc docs annotations, to at least make sure they were parenthesised in the listed cases. I don't think it affects either the formatter or the compiler error type renderers (though it should probably all be brought under one roof eventually?)

I can add a follow up here, to ensure redundant parens aren't introduced in the docs? I think the only case is when a 1-ary function is included in a tag payload or tuple (though not in plain parens).


I also just realised the PR comment was a little ambiguous about the effects, sorry! Should be better now if that helps / changes things.

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.

2 participants