Add parens to funcs in return position of docs rendered type annotations - #10606
Add parens to funcs in return position of docs rendered type annotations#10606jrrrp wants to merge 1 commit into
Conversation
Greptile SummaryThis 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.,
Confidence Score: 4/5The 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.
|
| 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)
-
src/docs/render_html.zig, line 2206-2210 (link)Record field types still use
needs_parens = falseThis 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 -> cas a record field renders as{ fieldName : a, b -> c }, where the comma afterais indistinguishable from a field separator, so the annotation parses as two fields (fieldName : aand the orphanedb -> c). The same fix (needs_parens = trueonfield.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
|
Greptile review noted a case where records containing functions will not have parentheses, unlike the changes made that apply to function return types etc. However, the annotation |
|
@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 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! |
|
Ah, but this PR should only affect the rendered 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. |
(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
Foof = || 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:
This describes a different type: a tag
Foohaving a payload with two fields, oneDecand oneDec -> Dec.needs_parenswas 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.