diff --git a/CHANGELOG.md b/CHANGELOG.md index f1844c4b0b..3d0ecfeb81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/compiler/syntax/src/res_parens.ml b/compiler/syntax/src/res_parens.ml index 0c25e46853..6d958dd52d 100644 --- a/compiler/syntax/src/res_parens.ml +++ b/compiler/syntax/src/res_parens.ml @@ -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 @@ -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 @@ -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 @@ -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 = @@ -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 _ @@ -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 @@ -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 @@ -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 -> @@ -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 @@ -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) diff --git a/compiler/syntax/src/res_parens.mli b/compiler/syntax/src/res_parens.mli index 3ce0218c84..8d304823f4 100644 --- a/compiler/syntax/src/res_parens.mli +++ b/compiler/syntax/src/res_parens.mli @@ -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 diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index 892e22b574..a70d524a8b 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -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 @@ -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; ]); ]) | { @@ -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; ]; ]); ]) @@ -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; ]; ]); ])) @@ -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 @@ -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); @@ -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) @@ -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) @@ -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) @@ -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); @@ -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 + (* 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) -> @@ -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 @@ -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) @@ -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); @@ -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) @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 = @@ -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 diff --git a/packages/dev-playground/src/CompilerApi.res b/packages/dev-playground/src/CompilerApi.res index b9f232dcc4..361204c69e 100644 --- a/packages/dev-playground/src/CompilerApi.res +++ b/packages/dev-playground/src/CompilerApi.res @@ -235,7 +235,7 @@ let applyConfig = ( ~experimentalFeatures: array, ) => { 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) diff --git a/packages/dev-playground/src/UrlState.res b/packages/dev-playground/src/UrlState.res index 68e234a16b..5213a0c46b 100644 --- a/packages/dev-playground/src/UrlState.res +++ b/packages/dev-playground/src/UrlState.res @@ -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", diff --git a/tests/syntax_tests/data/printer/expr/coerce.res b/tests/syntax_tests/data/printer/expr/coerce.res index 1d5f309de6..2a35400250 100644 --- a/tests/syntax_tests/data/printer/expr/coerce.res +++ b/tests/syntax_tests/data/printer/expr/coerce.res @@ -24,3 +24,77 @@ let foo = (~a=(3:int:>int), b) => 34 // let x : int1 :> int2 = 3 :> int3 let x = (/* c0 */ x /* c1 */ :> /* c2 */ int /* c3 */) + +// Delimited expression positions need no extra parentheses. +foo(v :> b) +foo((v :> b)) +foo(~arg=(v :> b), ~optional=?(v :> b)) +foo((v :> b), x => x) +foo(x => x, (v :> b)) +let tuple = ((v :> b), (w :> c)) +let array = [(v :> b), (w :> c)] +let spreadArray = [...(vs :> array), (w :> b)] +let list = list{(v :> b), ...(vs :> list)} +let constructor = Some((v :> b)) +let variant = #Value((v :> b)) + +// Preserve grouping when the surrounding expression needs it. +let result = x => (x :> b) +let callback = foo(x => (x :> b)) +let coercedFunction = (x => x) :> (a => b) +let call = (f :> (a => b))(v) +let field = (v :> b).name +let objectField = (v :> b)["name"] +let equalLeft = (v :> b) == w +let equalRight = v == (w :> b) +let unary = !(v :> b) +let awaited = await (v :> b) +let nested = (v :> b) :> c +let record = {field: (v :> b)} +let conditional = condition ? (v :> b) : (w :> b) +let block = {v :> b} +let default = (~arg=(v :> b)) => arg +let jsx = b)}> {(v :> b)} + +// Comments must stay attached when parentheses disappear. +foo(/* before */ (v /* operand */ :> /* type */ b) /* after */) +foo((v :> b) // trailing line comment +) +let attributed = (@foo v) :> b +let coercionAttribute = @foo (v :> b) +let long = functionWithAVeryLongName((valueWithAVeryLongName :> typeWithAVeryLongName)) + +let arrayIndex = values[(index :> int)] +values[(index :> int)] = (value :> b) +let recordSpread = {...(value :> b), field: value} +let bracedOperand = {value} :> b +let blockSequence = {foo(); value :> b} +let checked = assert(value :> bool) +(value :> b) +let ternaryCallback = condition ? (x => (x :> b)) : other +let piped = (value :> b)->foo +let blockTail = {foo(); (value :> b)} +let blockHead = {(value :> b); foo()} +let dictSpread = dict{...(value :> dict), "field": value} + +// A constrained operand needs parentheses when the coercion is a binding RHS. +let constrainedOperand = (x: t) :> u +let constrainedChain = ((x: t) :> u) :> v +let constrainedBlock = {(x: t) :> u} +call((x: t) :> u) + +// A following JSX element must not be read as coercion type arguments. +let beforeJsx = () => { + let value = (x :> string) +