From 241327860e3d8039328bdb3c7282776e5000e27e Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Tue, 11 Dec 2018 13:22:07 +0000 Subject: [PATCH] Fix stackoverflow in unparseExprRecurse This is an alternative to #2289 that also renames `literalAttrs` to `stylisticAttrs` as request in #2223's code review --- formatTest/unit_tests/expected_output/jsx.re | 3 +++ formatTest/unit_tests/input/jsx.re | 3 +++ src/reason-parser/reason_attributes.ml | 24 +++++++++++++------- src/reason-parser/reason_pprint_ast.ml | 15 ++++++------ src/reason-parser/reason_syntax_util.ml | 14 ++++++------ src/reason-parser/reason_syntax_util.mli | 2 +- src/reason-parser/reason_toolchain.ml | 2 +- src/refmt/reason_implementation_printer.ml | 2 +- 8 files changed, 40 insertions(+), 25 deletions(-) diff --git a/formatTest/unit_tests/expected_output/jsx.re b/formatTest/unit_tests/expected_output/jsx.re index 6db2fdf5d..e2c22d2f8 100644 --- a/formatTest/unit_tests/expected_output/jsx.re +++ b/formatTest/unit_tests/expected_output/jsx.re @@ -612,3 +612,6 @@ ReasonReact.(<> {string("Test")} ); ?id className={Cn.make({"a": b})} onClick> {"Submit" |> ste} ; + +// shouldn't result in a stack overflow +Belt.Option.getWithDefault("")} />; diff --git a/formatTest/unit_tests/input/jsx.re b/formatTest/unit_tests/input/jsx.re index a5edde7fe..5c57c7a9b 100644 --- a/formatTest/unit_tests/input/jsx.re +++ b/formatTest/unit_tests/input/jsx.re @@ -489,3 +489,6 @@ ReasonReact.(<> {string("Test")} ); ; + +// shouldn't result in a stack overflow +Belt.Option.getWithDefault("")} />; diff --git a/src/reason-parser/reason_attributes.ml b/src/reason-parser/reason_attributes.ml index 784c6cb6a..f520c7d33 100644 --- a/src/reason-parser/reason_attributes.ml +++ b/src/reason-parser/reason_attributes.ml @@ -8,7 +8,7 @@ type attributesPartition = { docAttrs : attributes; stdAttrs : attributes; jsxAttrs : attributes; - literalAttrs : attributes; + stylisticAttrs : attributes; uncurried : bool } @@ -16,7 +16,7 @@ type attributesPartition = { let rec partitionAttributes ?(partDoc=false) ?(allowUncurry=true) attrs : attributesPartition = match attrs with | [] -> - {arityAttrs=[]; docAttrs=[]; stdAttrs=[]; jsxAttrs=[]; literalAttrs=[]; uncurried = false} + {arityAttrs=[]; docAttrs=[]; stdAttrs=[]; jsxAttrs=[]; stylisticAttrs=[]; uncurried = false} | (({txt = "bs"}, PStr []) as attr)::atTl -> let partition = partitionAttributes ~partDoc ~allowUncurry atTl in if allowUncurry then @@ -37,10 +37,10 @@ let rec partitionAttributes ?(partDoc=false) ?(allowUncurry=true) attrs : attrib {partition with docAttrs=doc::partition.docAttrs} | (({txt="reason.raw_literal"}, _) as attr) :: atTl -> let partition = partitionAttributes ~partDoc ~allowUncurry atTl in - {partition with literalAttrs=attr::partition.literalAttrs} + {partition with stylisticAttrs=attr::partition.stylisticAttrs} | (({txt="reason.preserve_braces"}, _) as attr) :: atTl -> let partition = partitionAttributes ~partDoc ~allowUncurry atTl in - {partition with literalAttrs=attr::partition.literalAttrs} + {partition with stylisticAttrs=attr::partition.stylisticAttrs} | atHd :: atTl -> let partition = partitionAttributes ~partDoc ~allowUncurry atTl in {partition with stdAttrs=atHd::partition.stdAttrs} @@ -59,9 +59,9 @@ let extract_raw_literal attrs = in loop [] attrs -let without_literal_attrs attrs = +let without_stylistic_attrs attrs = let rec loop acc = function - | attr :: rest when (partitionAttributes [attr]).literalAttrs != [] -> + | attr :: rest when (partitionAttributes [attr]).stylisticAttrs != [] -> loop acc rest | [] -> List.rev acc | attr :: rest -> loop (attr :: acc) rest @@ -71,6 +71,14 @@ let without_literal_attrs attrs = let is_preserve_braces_attr ({txt}, _) = txt = "reason.preserve_braces" -let has_preserve_braces_attrs literalAttrs = - (List.filter is_preserve_braces_attr literalAttrs) != [] +let has_preserve_braces_attrs stylisticAttrs = + (List.filter is_preserve_braces_attr stylisticAttrs) != [] +let maybe_remove_stylistic_attrs attrs should_preserve = + if should_preserve then + attrs + else + List.filter (function + | ({txt="reason.raw_literal"}, _) -> true + | _ -> false) + attrs diff --git a/src/reason-parser/reason_pprint_ast.ml b/src/reason-parser/reason_pprint_ast.ml index 84a10a33e..e64ea61a9 100644 --- a/src/reason-parser/reason_pprint_ast.ml +++ b/src/reason-parser/reason_pprint_ast.ml @@ -3919,15 +3919,16 @@ let printer = object(self:'self) let x = self#process_underscore_application x in (* If there are any attributes, render unary like `(~-) x [@ppx]`, and infix like `(+) x y [@attr]` *) - let {arityAttrs; stdAttrs; jsxAttrs; literalAttrs; uncurried} = + let {arityAttrs; stdAttrs; jsxAttrs; stylisticAttrs; uncurried} = partitionAttributes ~allowUncurry:(Reason_heuristics.bsExprCanBeUncurried x) x.pexp_attributes in + let stylisticAttrs = Reason_attributes.maybe_remove_stylistic_attrs stylisticAttrs preserve_braces in let () = if uncurried then Hashtbl.add uncurriedTable x.pexp_loc true in - let x = {x with pexp_attributes = (literalAttrs @ arityAttrs @ stdAttrs @ jsxAttrs) } in + let x = {x with pexp_attributes = (stylisticAttrs @ arityAttrs @ stdAttrs @ jsxAttrs) } in (* If there's any attributes, recurse without them, then apply them to the ends of functions, or simplify infix printings then append. *) if stdAttrs != [] then - let withoutVisibleAttrs = {x with pexp_attributes=(literalAttrs @ arityAttrs @ jsxAttrs)} in + let withoutVisibleAttrs = {x with pexp_attributes=(stylisticAttrs @ arityAttrs @ jsxAttrs)} in let attributesAsList = (List.map self#attribute stdAttrs) in let itms = match self#unparseExprRecurse withoutVisibleAttrs with | SpecificInfixPrecedence ({reducePrecedence}, wrappedRule) -> @@ -4412,7 +4413,7 @@ let printer = object(self:'self) let dotdotdotChild = match expr with | {pexp_desc = Pexp_apply (funExpr, args)} when printedStringAndFixityExpr funExpr == Normal && - Reason_attributes.without_literal_attrs expr.pexp_attributes == [] -> + Reason_attributes.without_stylistic_attrs expr.pexp_attributes == [] -> begin match (self#formatFunAppl ~prefix:(atom "...") ~wrap:("{", "}") ~jsxAttrs:[] ~args ~funExpr ~applicationExpr:expr ()) with | [x] -> x | xs -> makeList xs @@ -4451,7 +4452,7 @@ let printer = object(self:'self) (self#formatNonSequencyExpression e) | Pexp_apply ({pexp_desc = Pexp_ident _} as funExpr, args) when printedStringAndFixityExpr funExpr == Normal && - Reason_attributes.without_literal_attrs expression.pexp_attributes == [] -> + Reason_attributes.without_stylistic_attrs expression.pexp_attributes == [] -> let lhs = makeList [atom lbl; atom "="] in begin match ( self#formatFunAppl @@ -6065,13 +6066,13 @@ let printer = object(self:'self) | _ -> assert false method should_preserve_requested_braces expr = - let {literalAttrs} = partitionAttributes expr.pexp_attributes in + let {stylisticAttrs} = partitionAttributes expr.pexp_attributes in match expr.pexp_desc with | Pexp_ifthenelse _ | Pexp_try _ -> false | _ -> preserve_braces && - Reason_attributes.has_preserve_braces_attrs literalAttrs + Reason_attributes.has_preserve_braces_attrs stylisticAttrs method simplest_expression x = let {stdAttrs; jsxAttrs} = partitionAttributes x.pexp_attributes in diff --git a/src/reason-parser/reason_syntax_util.ml b/src/reason-parser/reason_syntax_util.ml index 9f1b176c7..297cc355c 100644 --- a/src/reason-parser/reason_syntax_util.ml +++ b/src/reason-parser/reason_syntax_util.ml @@ -395,25 +395,25 @@ let identifier_mapper f super = end; } -let remove_literal_attrs_mapper_maker super = +let remove_stylistic_attrs_mapper_maker super = let open Ast_404 in let open Ast_mapper in { super with expr = begin fun mapper expr -> - let {Reason_attributes.literalAttrs; arityAttrs; docAttrs; stdAttrs; jsxAttrs} = + let {Reason_attributes.stylisticAttrs; arityAttrs; docAttrs; stdAttrs; jsxAttrs} = Reason_attributes.partitionAttributes ~allowUncurry:false expr.pexp_attributes in - let expr = if literalAttrs != [] then + let expr = if stylisticAttrs != [] then { expr with pexp_attributes = arityAttrs @ docAttrs @ stdAttrs @ jsxAttrs } else expr in super.expr mapper expr end; pat = begin fun mapper pat -> - let {Reason_attributes.literalAttrs; arityAttrs; docAttrs; stdAttrs; jsxAttrs} = + let {Reason_attributes.stylisticAttrs; arityAttrs; docAttrs; stdAttrs; jsxAttrs} = Reason_attributes.partitionAttributes ~allowUncurry:false pat.ppat_attributes in - let pat = if literalAttrs != [] then + let pat = if stylisticAttrs != [] then { pat with ppat_attributes = arityAttrs @ docAttrs @ stdAttrs @ jsxAttrs } else pat in @@ -421,8 +421,8 @@ let remove_literal_attrs_mapper_maker super = end; } -let remove_literal_attrs_mapper = - remove_literal_attrs_mapper_maker Ast_mapper.default_mapper +let remove_stylistic_attrs_mapper = + remove_stylistic_attrs_mapper_maker Ast_mapper.default_mapper (** escape_stars_slashes_mapper escapes all stars and slashes in an AST *) let escape_stars_slashes_mapper = diff --git a/src/reason-parser/reason_syntax_util.mli b/src/reason-parser/reason_syntax_util.mli index e96159644..1225f0d9a 100644 --- a/src/reason-parser/reason_syntax_util.mli +++ b/src/reason-parser/reason_syntax_util.mli @@ -39,7 +39,7 @@ val syntax_error_extension_node : Ast_404.Location.t -> string -> string Ast_404.Location.loc * Ast_404.Parsetree.payload -val remove_literal_attrs_mapper : Ast_404.Ast_mapper.mapper +val remove_stylistic_attrs_mapper : Ast_404.Ast_mapper.mapper val escape_stars_slashes_mapper : Ast_404.Ast_mapper.mapper -> Ast_404.Ast_mapper.mapper diff --git a/src/reason-parser/reason_toolchain.ml b/src/reason-parser/reason_toolchain.ml index 6929b49ed..90d321db9 100644 --- a/src/reason-parser/reason_toolchain.ml +++ b/src/reason-parser/reason_toolchain.ml @@ -495,7 +495,7 @@ module OCaml_syntax = struct (To_current.copy_signature signature) let format_implementation_with_comments (structure, _) formatter = let structure = - Reason_syntax_util.(apply_mapper_to_structure structure remove_literal_attrs_mapper) + Reason_syntax_util.(apply_mapper_to_structure structure remove_stylistic_attrs_mapper) in Pprintast.structure formatter (To_current.copy_structure structure) diff --git a/src/refmt/reason_implementation_printer.ml b/src/refmt/reason_implementation_printer.ml index 966396aa7..ac7a85a3c 100644 --- a/src/refmt/reason_implementation_printer.ml +++ b/src/refmt/reason_implementation_printer.ml @@ -51,7 +51,7 @@ let print printtype filename parsedAsML output_chan output_formatter = ) | `Binary -> fun (ast, _) -> let ast = - Reason_syntax_util.(apply_mapper_to_structure ast remove_literal_attrs_mapper) + Reason_syntax_util.(apply_mapper_to_structure ast remove_stylistic_attrs_mapper) in Ast_io.to_channel output_chan filename (Ast_io.Impl ((module OCaml_current),