Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@

#### :nail_care: Polish

- Omit unnecessary parentheses around coercions where the surrounding syntax already delimits the expression, while preserving required grouping. https://github.com/rescript-lang/rescript/pull/8614
- Print external declarations in signatures and type errors with their processed attributes instead of the `"#rescript-external"` placeholder, and print inline constants using `@inline` syntax. https://github.com/rescript-lang/rescript/pull/8581
- Improve diagnostics for dynamic imports of local values and attempts to use `import` as a first-class value. https://github.com/rescript-lang/rescript/pull/8582
- Allow inferred labeled functions to be called with labels in any order by removing legacy curried-arrow commutation locks. https://github.com/rescript-lang/rescript/pull/8547
Expand Down
39 changes: 22 additions & 17 deletions compiler/syntax/src/res_parens.ml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Parsetree_viewer = Res_parsetree_viewer
type kind = Parenthesized | Braced of Location.t | Nothing

let expr expr =
let expr_with_coercion_kind coercion_kind expr =
let opt_braces, _ = Parsetree_viewer.process_braces_attr expr in
match opt_braces with
| Some ({Location.loc = braces_loc}, _) -> Braced braces_loc
Expand All @@ -12,9 +12,13 @@ let expr expr =
Pexp_constraint ({pexp_desc = Pexp_pack _}, {ptyp_desc = Ptyp_package _});
} ->
Nothing
| {pexp_desc = Pexp_coerce _} -> coercion_kind
| {pexp_desc = Pexp_constraint _} -> Parenthesized
| _ -> Nothing)

let expr expr = expr_with_coercion_kind Parenthesized expr
let expr_allowing_coercion expr = expr_with_coercion_kind Nothing expr

let expr_record_row_rhs ~optional e =
let kind = expr e in
match kind with
Expand Down Expand Up @@ -50,9 +54,9 @@ let call_expr expr =
Nothing
| {
pexp_desc =
( Pexp_assert _ | Pexp_fun _ | Pexp_constraint _ | Pexp_setfield _
| Pexp_match _ | Pexp_try _ | Pexp_while _ | Pexp_for _ | Pexp_for_of _
| Pexp_for_await_of _ | Pexp_ifthenelse _ );
( Pexp_assert _ | Pexp_fun _ | Pexp_constraint _ | Pexp_coerce _
| Pexp_setfield _ | Pexp_match _ | Pexp_try _ | Pexp_while _ | Pexp_for _
| Pexp_for_of _ | Pexp_for_await_of _ | Pexp_ifthenelse _ );
} ->
Parenthesized
| _ when Parsetree_viewer.expr_is_await expr -> Parenthesized
Expand All @@ -72,7 +76,7 @@ let structure_expr expr =
Pexp_constraint ({pexp_desc = Pexp_pack _}, {ptyp_desc = Ptyp_package _});
} ->
Nothing
| {pexp_desc = Pexp_constraint _} -> Parenthesized
| {pexp_desc = Pexp_constraint _ | Pexp_coerce _} -> Parenthesized
| _ -> Nothing)

let unary_expr_operand expr =
Expand Down Expand Up @@ -100,8 +104,8 @@ let unary_expr_operand expr =
Nothing
| {
pexp_desc =
( Pexp_assert _ | Pexp_fun _ | Pexp_constraint _ | Pexp_setfield _
| Pexp_extension _ (* readability? maybe remove *)
( Pexp_assert _ | Pexp_fun _ | Pexp_constraint _ | Pexp_coerce _
| Pexp_setfield _ | Pexp_extension _ (* readability? maybe remove *)
| Pexp_object_literal _ (* ({"a": 1})["a"] *)
| Pexp_object_set _ (* (o["x"] = v)["y"] *) | Pexp_match _ | Pexp_try _
| Pexp_while _ | Pexp_for _ | Pexp_for_of _ | Pexp_for_await_of _
Expand All @@ -125,7 +129,8 @@ let binary_expr_operand ~is_lhs expr =
| {pexp_desc = Pexp_fun _}
when Parsetree_viewer.is_underscore_apply_sugar expr ->
Nothing
| {pexp_desc = Pexp_constraint _ | Pexp_fun _} -> Parenthesized
| {pexp_desc = Pexp_constraint _ | Pexp_coerce _ | Pexp_fun _} ->
Parenthesized
| expr when Parsetree_viewer.is_binary_expression expr -> Parenthesized
| expr when Parsetree_viewer.is_ternary_expr expr -> Parenthesized
| {pexp_desc = Pexp_assert _} when is_lhs -> Parenthesized
Expand Down Expand Up @@ -182,7 +187,7 @@ let flatten_operand_rhs parent_operator rhs =
false
| Pexp_fun {params = {p_pat = {ppat_desc = Ppat_var {txt = "__x"}}} :: _} ->
false
| Pexp_fun _ | Pexp_setfield _ | Pexp_constraint _ -> true
| Pexp_fun _ | Pexp_setfield _ | Pexp_constraint _ | Pexp_coerce _ -> true
| _ when Parsetree_viewer.is_ternary_expr rhs -> true
| _ -> false

Expand Down Expand Up @@ -220,9 +225,9 @@ let assert_or_await_expr_rhs ?(in_await = false) expr =
Nothing
| {
pexp_desc =
( Pexp_assert _ | Pexp_fun _ | Pexp_constraint _ | Pexp_setfield _
| Pexp_match _ | Pexp_try _ | Pexp_while _ | Pexp_for _ | Pexp_for_of _
| Pexp_for_await_of _ | Pexp_ifthenelse _ );
( Pexp_assert _ | Pexp_fun _ | Pexp_constraint _ | Pexp_coerce _
| Pexp_setfield _ | Pexp_match _ | Pexp_try _ | Pexp_while _ | Pexp_for _
| Pexp_for_of _ | Pexp_for_await_of _ | Pexp_ifthenelse _ );
} ->
Parenthesized
| _ when (not in_await) && Parsetree_viewer.expr_is_await expr ->
Expand Down Expand Up @@ -267,9 +272,9 @@ let field_expr expr =
pexp_desc =
( Pexp_assert _ | Pexp_extension _ (* %extension.x vs (%extension).x *)
| Pexp_object_literal _ (* ({"a": 1})["a"] *) | Pexp_fun _
| Pexp_constraint _ | Pexp_setfield _ | Pexp_match _ | Pexp_try _
| Pexp_while _ | Pexp_for _ | Pexp_for_of _ | Pexp_for_await_of _
| Pexp_ifthenelse _ );
| Pexp_constraint _ | Pexp_coerce _ | Pexp_setfield _ | Pexp_match _
| Pexp_try _ | Pexp_while _ | Pexp_for _ | Pexp_for_of _
| Pexp_for_await_of _ | Pexp_ifthenelse _ );
} ->
Parenthesized
| _ when Parsetree_viewer.expr_is_await expr -> Parenthesized
Expand All @@ -286,11 +291,11 @@ let ternary_operand expr =
Pexp_constraint ({pexp_desc = Pexp_pack _}, {ptyp_desc = Ptyp_package _});
} ->
Nothing
| {pexp_desc = Pexp_constraint _} -> Parenthesized
| {pexp_desc = Pexp_constraint _ | Pexp_coerce _} -> Parenthesized
| _ when Res_parsetree_viewer.is_fun_expr expr -> (
let _, _parameters, return_expr = Parsetree_viewer.fun_expr expr in
match return_expr.pexp_desc with
| Pexp_constraint _ -> Parenthesized
| Pexp_constraint _ | Pexp_coerce _ -> Parenthesized
| _ -> Nothing)
| _ -> Nothing)

Expand Down
5 changes: 5 additions & 0 deletions compiler/syntax/src/res_parens.mli
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
type kind = Parenthesized | Braced of Location.t | Nothing

val expr : Parsetree.expression -> kind

(* Unlike [expr], this does not request parentheses for a top-level coercion.
Use only where the surrounding grammar delimits the expression, such as call
arguments and collection elements. *)
val expr_allowing_coercion : Parsetree.expression -> kind
val structure_expr : Parsetree.expression -> kind

val unary_expr_operand : Parsetree.expression -> kind
Expand Down
61 changes: 38 additions & 23 deletions compiler/syntax/src/res_printer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1716,7 +1716,7 @@ and print_spread_dict_expr ~state parts (expr : Parsetree.expression) cmt_tbl =
in
let spread_doc =
let doc = print_expression ~state spread_expr cmt_tbl in
match Parens.expr spread_expr with
match Parens.expr_allowing_coercion spread_expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc spread_expr braces
| Nothing -> doc
Expand Down Expand Up @@ -2419,7 +2419,7 @@ and print_value_binding ~state ~rec_flag (vb : Parsetree.value_binding) cmt_tbl
print_typ_expr ~state pvc_type cmt_tbl;
Doc.text " =";
Doc.line;
print_expression_with_comments ~state expr cmt_tbl;
print_expression_with_comments_and_parens ~state expr cmt_tbl;
]);
])
| {
Expand Down Expand Up @@ -2463,7 +2463,8 @@ and print_value_binding ~state ~rec_flag (vb : Parsetree.value_binding) cmt_tbl
Doc.concat
[
Doc.line;
print_expression_with_comments ~state expr cmt_tbl;
print_expression_with_comments_and_parens ~state expr
cmt_tbl;
];
]);
])
Expand All @@ -2490,7 +2491,8 @@ and print_value_binding ~state ~rec_flag (vb : Parsetree.value_binding) cmt_tbl
Doc.concat
[
Doc.line;
print_expression_with_comments ~state expr cmt_tbl;
print_expression_with_comments_and_parens ~state expr
cmt_tbl;
];
]);
]))
Expand Down Expand Up @@ -3032,10 +3034,17 @@ and print_expression_with_comments ~state expr cmt_tbl : Doc.t =
let doc = print_expression ~state expr cmt_tbl in
print_comments doc cmt_tbl expr.Parsetree.pexp_loc

and print_expression_with_comments_and_parens ~state expr cmt_tbl =
let doc = print_expression_with_comments ~state expr cmt_tbl in
match Parens.expr expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc

and print_expression_args ~state (args : Parsetree.expression list) cmt_tbl =
let print_arg expr =
let doc = print_expression_with_comments ~state expr cmt_tbl in
match Parens.expr expr with
match Parens.expr_allowing_coercion expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc
Expand Down Expand Up @@ -3261,7 +3270,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl =
Doc.line;
Doc.dotdotdot;
(let doc = print_expression_with_comments ~state expr cmt_tbl in
match Parens.expr expr with
match Parens.expr_allowing_coercion expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc);
Expand All @@ -3283,7 +3292,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl =
let doc =
print_expression_with_comments ~state expr cmt_tbl
in
match Parens.expr expr with
match Parens.expr_allowing_coercion expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc)
Expand Down Expand Up @@ -3315,7 +3324,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl =
let doc =
print_expression_with_comments ~state expr cmt_tbl
in
match Parens.expr expr with
match Parens.expr_allowing_coercion expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc)
Expand Down Expand Up @@ -3344,7 +3353,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl =
let doc =
print_expression_with_comments ~state expr cmt_tbl
in
match Parens.expr expr with
match Parens.expr_allowing_coercion expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc)
Expand Down Expand Up @@ -3378,7 +3387,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl =
Doc.concat
[
Doc.dotdotdot;
(match Parens.expr expr with
(match Parens.expr_allowing_coercion expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc);
Expand Down Expand Up @@ -3728,9 +3737,15 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl =
print_cases ~state cases cmt_tbl;
]
| Pexp_coerce (expr, (), typ) ->
let doc_expr = print_expression_with_comments ~state expr cmt_tbl in
let doc_expr =
print_expression_with_comments_and_parens ~state expr cmt_tbl
in
let doc_typ = print_typ_expr ~state typ cmt_tbl in
Doc.concat [Doc.lparen; doc_expr; Doc.text " :> "; doc_typ; Doc.rparen]
let doc = Doc.concat [doc_expr; Doc.text " :> "; doc_typ] in
Comment thread
cknitt marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve coercion grouping in plain-expression delimiters

When a coercion is the sole expression in an attribute/extension payload (for example, @a((x :> t))) or in unpack((x :> t)), removing the coercion's own parentheses produces @a(x :> t) or unpack(x :> t). Those contexts ultimately invoke plain parse_expr (parse_structure_item_region for the payload and parse_atomic_module_expr for unpack), which leaves :> unconsumed, so formatting valid input makes it fail to reparse; these printers need to apply Parens.expr or otherwise retain the inner grouping.

AGENTS.md reference: AGENTS.md:L135-L139

Useful? React with 👍 / 👎.

(* Keep attributes on the coercion rather than its operand. *)
if Parsetree_viewer.has_printable_attributes e.pexp_attributes then
add_parens doc
else doc
| Pexp_object_get (parent_expr, label) ->
print_object_get_doc ~state parent_expr label cmt_tbl
| Pexp_object_set (obj, member, rhs) ->
Expand Down Expand Up @@ -4231,7 +4246,7 @@ and print_array_spread_apply ~state sub_lists cmt_tbl =
(* Print expression without leading comments (they're already extracted) *)
let expr_doc =
let doc = print_expression ~state expr cmt_tbl in
match Parens.expr expr with
match Parens.expr_allowing_coercion expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc
Expand Down Expand Up @@ -4263,7 +4278,7 @@ and print_array_spread_apply ~state sub_lists cmt_tbl =
(List.map
(fun expr ->
let doc = print_expression_with_comments ~state expr cmt_tbl in
match Parens.expr expr with
match Parens.expr_allowing_coercion expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc)
Expand Down Expand Up @@ -4298,7 +4313,7 @@ and print_list_spread_apply ~state sub_lists cmt_tbl =
comma_before_spread;
Doc.dotdotdot;
(let doc = print_expression_with_comments ~state expr cmt_tbl in
match Parens.expr expr with
match Parens.expr_allowing_coercion expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc);
Expand All @@ -4319,7 +4334,7 @@ and print_list_spread_apply ~state sub_lists cmt_tbl =
(List.map
(fun expr ->
let doc = print_expression_with_comments ~state expr cmt_tbl in
match Parens.expr expr with
match Parens.expr_allowing_coercion expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc)
Expand Down Expand Up @@ -4419,7 +4434,7 @@ and print_pexp_apply ~state expr cmt_tbl =
let member =
let member_doc =
let doc = print_expression_with_comments ~state member_expr cmt_tbl in
match Parens.expr member_expr with
match Parens.expr_allowing_coercion member_expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc member_expr braces
| Nothing -> doc
Expand Down Expand Up @@ -4466,7 +4481,7 @@ and print_pexp_apply ~state expr cmt_tbl =
let member =
let member_doc =
let doc = print_expression_with_comments ~state member_expr cmt_tbl in
match Parens.expr member_expr with
match Parens.expr_allowing_coercion member_expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc member_expr braces
| Nothing -> doc
Expand Down Expand Up @@ -4885,7 +4900,7 @@ and print_jsx_prop ~state prop cmt_tbl =
[
Doc.lbrace;
Doc.dotdotdot;
print_expression_with_comments ~state value cmt_tbl;
print_expression_with_comments_and_parens ~state value cmt_tbl;
Doc.rbrace;
])
in
Expand Down Expand Up @@ -5119,7 +5134,7 @@ and print_arguments ~state ~partial
| [(Nolabel, arg)] when Parsetree_viewer.is_huggable_expression arg ->
let arg_doc =
let doc = print_expression_with_comments ~state arg cmt_tbl in
match Parens.expr arg with
match Parens.expr_allowing_coercion arg with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc arg braces
| Nothing -> doc
Expand Down Expand Up @@ -5233,7 +5248,7 @@ and print_argument ~state (arg_lbl, arg) cmt_tbl =
in
let printed_expr =
let doc = print_expression_with_comments ~state expr cmt_tbl in
match Parens.expr expr with
match Parens.expr_allowing_coercion expr with
| Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc
Expand Down Expand Up @@ -5291,7 +5306,7 @@ and print_case ~state (case : Parsetree.case) cmt_tbl =
[
Doc.line;
Doc.text "if ";
print_expression_with_comments ~state expr cmt_tbl;
print_expression_with_comments_and_parens ~state expr cmt_tbl;
])
in
let should_inline_rhs =
Expand Down Expand Up @@ -5853,7 +5868,7 @@ and print_payload ~state (payload : Parsetree.payload) cmt_tbl =
[
Doc.line;
Doc.text "if ";
print_expression_with_comments ~state expr cmt_tbl;
print_expression_with_comments_and_parens ~state expr cmt_tbl;
]
| None -> Doc.nil
in
Expand Down
2 changes: 1 addition & 1 deletion packages/dev-playground/src/CompilerApi.res
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ let applyConfig = (
~experimentalFeatures: array<PlaygroundConfig.experimentalFeature>,
) => {
if hasFunction(instance, "setModuleSystem") {
instance->Instance.setModuleSystem((moduleSystem :> string))
instance->Instance.setModuleSystem(moduleSystem :> string)
}
if hasFunction(instance, "setWarnFlags") {
instance->Instance.setWarnFlags(warnFlags === "" ? defaultConfig.warnFlags : warnFlags)
Expand Down
2 changes: 1 addition & 1 deletion packages/dev-playground/src/UrlState.res
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ let applyUrlState = (~encoded, ~config: PlaygroundConfig.t) => {
params->UrlSearchParams.delete("sourceMapSourcesContent")
params->UrlSearchParams.delete("sourceMapRoot")
| sourceMapMode =>
params->UrlSearchParams.set("sourceMap", (sourceMapMode :> string))
params->UrlSearchParams.set("sourceMap", sourceMapMode :> string)
params->UrlSearchParams.set(
"sourceMapSourcesContent",
config.sourceMapSourcesContent ? "true" : "false",
Expand Down
Loading
Loading