From 2e0b219eb1b5677a8a2d6924ad42ca48bb778575 Mon Sep 17 00:00:00 2001 From: Tom Gasson Date: Fri, 3 Jul 2020 09:15:37 -0700 Subject: [PATCH 1/2] Add `if let` syntax --- src/napkin_core.ml | 94 +++++++-- src/napkin_parsetree_viewer.ml | 68 +++++-- src/napkin_parsetree_viewer.mli | 11 +- src/napkin_printer.ml | 173 ++++++++-------- .../__snapshots__/parse.spec.js.snap | 73 ++++++- tests/parsing/grammar/expressions/if.js | 110 +++++++++- .../expr/__snapshots__/render.spec.js.snap | 189 ++++++++++++++++++ tests/printer/expr/if.js | 180 ++++++++++++++++- 8 files changed, 778 insertions(+), 120 deletions(-) diff --git a/src/napkin_core.ml b/src/napkin_core.ml index b00530e8..b0289eb1 100644 --- a/src/napkin_core.ml +++ b/src/napkin_core.ml @@ -83,6 +83,7 @@ end let jsxAttr = (Location.mknoloc "JSX", Parsetree.PStr []) let uncurryAttr = (Location.mknoloc "bs", Parsetree.PStr []) let ternaryAttr = (Location.mknoloc "ns.ternary", Parsetree.PStr []) +let ifLetAttr = (Location.mknoloc "ns.iflet", Parsetree.PStr []) let makeBracesAttr loc = (Location.mkloc "ns.braces" loc, Parsetree.PStr []) type typDefOrExt = @@ -2000,7 +2001,7 @@ and parseOperandExpr ~context p = | Try -> parseTryExpression p | If -> - parseIfExpression p + parseIfOrIfLetExpression p | For -> parseForExpression p | While -> @@ -2996,20 +2997,30 @@ and parseTryExpression p = let loc = mkLoc startPos p.prevEndPos in Ast_helper.Exp.try_ ~loc expr cases -and parseIfExpression p = - Parser.beginRegion p; - Parser.leaveBreadcrumb p Grammar.ExprIf; - let startPos = p.Parser.startPos in - Parser.expect If p; +and parseIfCondition p = Parser.leaveBreadcrumb p Grammar.IfCondition; (* doesn't make sense to try es6 arrow here? *) let conditionExpr = parseExpr ~context:WhenExpr p in Parser.eatBreadcrumb p; + conditionExpr + +and parseThenBranch p = Parser.leaveBreadcrumb p IfBranch; Parser.expect Lbrace p; let thenExpr = parseExprBlock p in Parser.expect Rbrace p; Parser.eatBreadcrumb p; + thenExpr + +and parseElseBranch p = + Parser.expect Lbrace p; + let blockExpr = parseExprBlock p in + Parser.expect Rbrace p; + blockExpr; + +and parseIfExpr startPos p = + let conditionExpr = parseIfCondition p in + let thenExpr = parseThenBranch p in let elseExpr = match p.Parser.token with | Else -> Parser.endRegion p; @@ -3018,12 +3029,9 @@ and parseIfExpression p = Parser.beginRegion p; let elseExpr = match p.token with | If -> - parseIfExpression p + parseIfOrIfLetExpression p | _ -> - Parser.expect Lbrace p; - let blockExpr = parseExprBlock p in - Parser.expect Rbrace p; - blockExpr + parseElseBranch p in Parser.eatBreadcrumb p; Parser.endRegion p; @@ -3033,9 +3041,55 @@ and parseIfExpression p = None in let loc = mkLoc startPos p.prevEndPos in - Parser.eatBreadcrumb p; Ast_helper.Exp.ifthenelse ~loc conditionExpr thenExpr elseExpr +and parseIfLetExpr startPos p = + let pattern = parsePattern p in + Parser.expect Equal p; + let conditionExpr = parseIfCondition p in + let thenExpr = parseThenBranch p in + let elseExpr = match p.Parser.token with + | Else -> + Parser.endRegion p; + Parser.leaveBreadcrumb p Grammar.ElseBranch; + Parser.next p; + Parser.beginRegion p; + let elseExpr = match p.token with + | If -> + parseIfOrIfLetExpression p + | _ -> + parseElseBranch p + in + Parser.eatBreadcrumb p; + Parser.endRegion p; + elseExpr + | _ -> + Parser.endRegion p; + let startPos = p.Parser.startPos in + let loc = mkLoc startPos p.prevEndPos in + Ast_helper.Exp.construct ~loc (Location.mkloc (Longident.Lident "()") loc) None + in + let loc = mkLoc startPos p.prevEndPos in + Ast_helper.Exp.match_ ~attrs:[ifLetAttr] ~loc conditionExpr [ + Ast_helper.Exp.case pattern thenExpr; + Ast_helper.Exp.case (Ast_helper.Pat.any ()) elseExpr; + ] + +and parseIfOrIfLetExpression p = + Parser.beginRegion p; + Parser.leaveBreadcrumb p Grammar.ExprIf; + let startPos = p.Parser.startPos in + Parser.expect If p; + let expr = match p.Parser.token with + | Let -> + Parser.next p; + parseIfLetExpr startPos p + | _ -> + parseIfExpr startPos p + in + Parser.eatBreadcrumb p; + expr; + and parseForRest hasOpeningParen pattern startPos p = Parser.expect In p; let e1 = parseExpr p in @@ -3098,6 +3152,14 @@ and parseWhileExpression p = let loc = mkLoc startPos p.prevEndPos in Ast_helper.Exp.while_ ~loc expr1 expr2 +and parsePatternGuard p = + match p.Parser.token with + | When -> + Parser.next p; + Some (parseExpr ~context:WhenExpr p) + | _ -> + None + and parsePatternMatchCase p = Parser.beginRegion p; Parser.leaveBreadcrumb p Grammar.PatternMatchCase; @@ -3105,13 +3167,7 @@ and parsePatternMatchCase p = | Token.Bar -> Parser.next p; let lhs = parsePattern p in - let guard = match p.Parser.token with - | When -> - Parser.next p; - Some (parseExpr ~context:WhenExpr p) - | _ -> - None - in + let guard = parsePatternGuard p in let () = match p.token with | EqualGreater -> Parser.next p | _ -> Recover.recoverEqualGreater p diff --git a/src/napkin_parsetree_viewer.ml b/src/napkin_parsetree_viewer.ml index 3df6e8b0..b2e61d63 100644 --- a/src/napkin_parsetree_viewer.ml +++ b/src/napkin_parsetree_viewer.ml @@ -42,18 +42,6 @@ open Parsetree in process false [] attrs - let collectIfExpressions expr = - let rec collect acc expr = match expr.pexp_desc with - | Pexp_ifthenelse (ifExpr, thenExpr, Some elseExpr) -> - collect ((ifExpr, thenExpr)::acc) elseExpr - | Pexp_ifthenelse (ifExpr, thenExpr, (None as elseExpr)) -> - let ifs = List.rev ((ifExpr, thenExpr)::acc) in - (ifs, elseExpr) - | _ -> - (List.rev acc, Some expr) - in - collect [] expr - let collectListExpressions expr = let rec collect acc expr = match expr.pexp_desc with | Pexp_construct ({txt = Longident.Lident "[]"}, _) -> @@ -165,7 +153,7 @@ open Parsetree let filterParsingAttrs attrs = List.filter (fun attr -> match attr with - | ({Location.txt = ("ns.ternary" | "ns.braces" | "bs" | "ns.namedArgLoc")}, _) -> false + | ({Location.txt = ("ns.ternary" | "ns.braces" | "bs" | "ns.iflet" | "ns.namedArgLoc")}, _) -> false | _ -> true ) attrs @@ -269,7 +257,7 @@ open Parsetree let hasAttributes attrs = List.exists (fun attr -> match attr with - | ({Location.txt = "bs" | "ns.ternary" | "ns.braces"}, _) -> false + | ({Location.txt = "bs" | "ns.ternary" | "ns.braces" | "ns.iflet"}, _) -> false | _ -> true ) attrs @@ -280,6 +268,52 @@ open Parsetree ) -> true | _ -> false + let rec hasIfLetAttribute attrs = + match attrs with + | [] -> false + | ({Location.txt="ns.iflet"},_)::_ -> true + | _::attrs -> hasIfLetAttribute attrs + + let isIfLetExpr expr = match expr with + | { + pexp_attributes = attrs; + pexp_desc = Pexp_match _ + } when hasIfLetAttribute attrs -> true + | _ -> false + + type ifConditionKind = + | If of Parsetree.expression + | IfLet of Parsetree.pattern * Parsetree.expression + + let collectIfExpressions expr = + let rec collect acc expr = match expr.pexp_desc with + | Pexp_ifthenelse (ifExpr, thenExpr, Some elseExpr) -> + collect ((If(ifExpr), thenExpr)::acc) elseExpr + | Pexp_ifthenelse (ifExpr, thenExpr, (None as elseExpr)) -> + let ifs = List.rev ((If(ifExpr), thenExpr)::acc) in + (ifs, elseExpr) + | Pexp_match (condition, [{ + pc_lhs = pattern; + pc_guard = None; + pc_rhs = thenExpr; + }; { + pc_rhs = {pexp_desc = Pexp_construct ({txt = Longident.Lident "()"}, _)} + }]) when isIfLetExpr expr -> + let ifs = List.rev ((IfLet(pattern, condition), thenExpr)::acc) in + (ifs, None) + | Pexp_match (condition, [{ + pc_lhs = pattern; + pc_guard = None; + pc_rhs = thenExpr; + }; { + pc_rhs = elseExpr; + }]) when isIfLetExpr expr -> + collect ((IfLet(pattern, condition), thenExpr)::acc) elseExpr + | _ -> + (List.rev acc, Some expr) + in + collect [] expr + let rec hasTernaryAttribute attrs = match attrs with | [] -> false @@ -371,13 +405,13 @@ open Parsetree let filterPrinteableAttributes attrs = List.filter (fun attr -> match attr with - | ({Location.txt="bs" | "ns.ternary"}, _) -> false + | ({Location.txt="bs" | "ns.ternary" | "ns.iflet"}, _) -> false | _ -> true ) attrs let partitionPrinteableAttributes attrs = List.partition (fun attr -> match attr with - | ({Location.txt="bs" | "ns.ternary"}, _) -> false + | ({Location.txt="bs" | "ns.ternary" | "ns.iflet"}, _) -> false | _ -> true ) attrs @@ -513,4 +547,4 @@ open Parsetree {ppat_desc = Ppat_var {txt="__x"}}, {pexp_desc = Pexp_apply _} ) -> true - | _ -> false \ No newline at end of file + | _ -> false diff --git a/src/napkin_parsetree_viewer.mli b/src/napkin_parsetree_viewer.mli index 8c853ec2..66ead5d7 100644 --- a/src/napkin_parsetree_viewer.mli +++ b/src/napkin_parsetree_viewer.mli @@ -13,12 +13,16 @@ val functorType: Parsetree.module_type -> (* filters @bs out of the provided attributes *) val processUncurriedAttribute: Parsetree.attributes -> bool * Parsetree.attributes +type ifConditionKind = + | If of Parsetree.expression + | IfLet of Parsetree.pattern * Parsetree.expression + (* if ... else if ... else ... is represented as nested expressions: if ... else { if ... } * The purpose of this function is to flatten nested ifs into one sequence. * Basically compute: ([if, else if, else if, else if], else) *) val collectIfExpressions: Parsetree.expression -> - (Parsetree.expression * Parsetree.expression) list * Parsetree.expression option + (ifConditionKind * Parsetree.expression) list * Parsetree.expression option val collectListExpressions: Parsetree.expression -> (Parsetree.expression list * Parsetree.expression option) @@ -62,6 +66,7 @@ val hasAttributes: Parsetree.attributes -> bool val isArrayAccess: Parsetree.expression -> bool val isTernaryExpr: Parsetree.expression -> bool +val isIfLetExpr: Parsetree.expression -> bool val collectTernaryParts: Parsetree.expression -> ((Parsetree.expression * Parsetree.expression) list * Parsetree.expression) @@ -90,7 +95,7 @@ val modExprApply : Parsetree.module_expr -> ( * Example: given a ptyp_arrow type, what are its arguments and what is the * returnType? *) - + val modExprFunctor : Parsetree.module_expr -> ( (Parsetree.attributes * string Asttypes.loc * Parsetree.module_type option) list * Parsetree.module_expr @@ -130,4 +135,4 @@ val classifyJsImport: Parsetree.value_description -> jsImportScope val rewriteUnderscoreApply: Parsetree.expression -> Parsetree.expression (* (__x) => f(a, __x, c) -----> f(a, _, c) *) -val isUnderscoreApplySugar: Parsetree.expression -> bool \ No newline at end of file +val isUnderscoreApplySugar: Parsetree.expression -> bool diff --git a/src/napkin_printer.ml b/src/napkin_printer.ml index 39d10714..83932dd2 100644 --- a/src/napkin_printer.ml +++ b/src/napkin_printer.ml @@ -2365,6 +2365,58 @@ and printExpressionWithComments expr cmtTbl = let doc = printExpression expr cmtTbl in printComments doc cmtTbl expr.Parsetree.pexp_loc +and printIfChain pexp_attributes ifs elseExpr cmtTbl = + let ifDocs = Doc.join ~sep:Doc.space ( + List.mapi (fun i (ifExpr, thenExpr) -> + let ifTxt = if i > 0 then Doc.text "else if " else Doc.text "if " in + match ifExpr with + | ParsetreeViewer.If ifExpr -> + let condition = + if ParsetreeViewer.isBlockExpr ifExpr then + printExpressionBlock ~braces:true ifExpr cmtTbl + else + let doc = printExpressionWithComments ifExpr cmtTbl in + match Parens.expr ifExpr with + | Parens.Parenthesized -> addParens doc + | Braced braces -> printBraces doc ifExpr braces + | Nothing -> Doc.ifBreaks (addParens doc) doc + in + Doc.concat [ + ifTxt; + Doc.group (condition); + Doc.space; + let thenExpr = match ParsetreeViewer.processBracesAttr thenExpr with + (* This case only happens when coming from Reason, we strip braces *) + | (Some _, expr) -> expr + | _ -> thenExpr + in + printExpressionBlock ~braces:true thenExpr cmtTbl; + ] + | IfLet (pattern, conditionExpr) -> + Doc.concat [ + ifTxt; + Doc.text "let "; + printPattern pattern cmtTbl; + Doc.text " = "; + printExpressionWithComments conditionExpr cmtTbl; + Doc.space; + printExpressionBlock ~braces:true thenExpr cmtTbl; + ] + ) ifs + ) in + let elseDoc = match elseExpr with + | None -> Doc.nil + | Some expr -> Doc.concat [ + Doc.text " else "; + printExpressionBlock ~braces:true expr cmtTbl; + ] + in + Doc.concat [ + printAttributes pexp_attributes cmtTbl; + ifDocs; + elseDoc; + ] + and printExpression (e : Parsetree.expression) cmtTbl = let printedExpression = match e.pexp_desc with | Parsetree.Pexp_constant c -> printConstant c @@ -2699,90 +2751,52 @@ and printExpression (e : Parsetree.expression) cmtTbl = ] | Pexp_setfield (expr1, longidentLoc, expr2) -> printSetFieldExpr e.pexp_attributes expr1 longidentLoc expr2 e.pexp_loc cmtTbl - | Pexp_ifthenelse (_ifExpr, _thenExpr, _elseExpr) -> - if ParsetreeViewer.isTernaryExpr e then - let (parts, alternate) = ParsetreeViewer.collectTernaryParts e in - let ternaryDoc = match parts with - | (condition1, consequent1)::rest -> - Doc.group (Doc.concat [ - printTernaryOperand condition1 cmtTbl; - Doc.indent ( - Doc.concat [ - Doc.line; - Doc.indent ( + | Pexp_ifthenelse (_ifExpr, _thenExpr, _elseExpr) when ParsetreeViewer.isTernaryExpr e -> + let (parts, alternate) = ParsetreeViewer.collectTernaryParts e in + let ternaryDoc = match parts with + | (condition1, consequent1)::rest -> + Doc.group (Doc.concat [ + printTernaryOperand condition1 cmtTbl; + Doc.indent ( + Doc.concat [ + Doc.line; + Doc.indent ( + Doc.concat [ + Doc.text "? "; + printTernaryOperand consequent1 cmtTbl + ] + ); + Doc.concat ( + List.map (fun (condition, consequent) -> Doc.concat [ + Doc.line; + Doc.text ": "; + printTernaryOperand condition cmtTbl; + Doc.line; Doc.text "? "; - printTernaryOperand consequent1 cmtTbl + printTernaryOperand consequent cmtTbl; ] - ); - Doc.concat ( - List.map (fun (condition, consequent) -> - Doc.concat [ - Doc.line; - Doc.text ": "; - printTernaryOperand condition cmtTbl; - Doc.line; - Doc.text "? "; - printTernaryOperand consequent cmtTbl; - ] - ) rest - ); - Doc.line; - Doc.text ": "; - Doc.indent (printTernaryOperand alternate cmtTbl); - ] - ) - ]) - | _ -> Doc.nil - in - let attrs = ParsetreeViewer.filterTernaryAttributes e.pexp_attributes in - let needsParens = match ParsetreeViewer.filterParsingAttrs attrs with - | [] -> false | _ -> true - in - Doc.concat [ - printAttributes attrs cmtTbl; - if needsParens then addParens ternaryDoc else ternaryDoc; - ] - else - let (ifs, elseExpr) = ParsetreeViewer.collectIfExpressions e in - let ifDocs = Doc.join ~sep:Doc.space ( - List.mapi (fun i (ifExpr, thenExpr) -> - let ifTxt = if i > 0 then Doc.text "else if " else Doc.text "if " in - let condition = - if ParsetreeViewer.isBlockExpr ifExpr then - printExpressionBlock ~braces:true ifExpr cmtTbl - else - let doc = printExpressionWithComments ifExpr cmtTbl in - match Parens.expr ifExpr with - | Parens.Parenthesized -> addParens doc - | Braced braces -> printBraces doc ifExpr braces - | Nothing -> Doc.ifBreaks (addParens doc) doc - in - Doc.concat [ - ifTxt; - Doc.group (condition); - Doc.space; - let thenExpr = match ParsetreeViewer.processBracesAttr thenExpr with - (* This case only happens when coming from Reason, we strip braces *) - | (Some _, expr) -> expr - | _ -> thenExpr - in - printExpressionBlock ~braces:true thenExpr cmtTbl; - ] - ) ifs - ) in - let elseDoc = match elseExpr with - | None -> Doc.nil - | Some expr -> Doc.concat [ - Doc.text " else "; - printExpressionBlock ~braces:true expr cmtTbl; - ] + ) rest + ); + Doc.line; + Doc.text ": "; + Doc.indent (printTernaryOperand alternate cmtTbl); + ] + ) + ]) + | _ -> Doc.nil + in + let attrs = ParsetreeViewer.filterTernaryAttributes e.pexp_attributes in + let needsParens = match ParsetreeViewer.filterParsingAttrs attrs with + | [] -> false | _ -> true in Doc.concat [ - printAttributes e.pexp_attributes cmtTbl; - ifDocs; - elseDoc; + printAttributes attrs cmtTbl; + if needsParens then addParens ternaryDoc else ternaryDoc; ] + | Pexp_ifthenelse (_ifExpr, _thenExpr, _elseExpr) -> + let (ifs, elseExpr) = ParsetreeViewer.collectIfExpressions e in + printIfChain e.pexp_attributes ifs elseExpr cmtTbl | Pexp_while (expr1, expr2) -> let condition = let doc = printExpressionWithComments expr1 cmtTbl in @@ -3008,6 +3022,9 @@ and printExpression (e : Parsetree.expression) cmtTbl = Doc.text " catch "; printCases cases cmtTbl; ] + | Pexp_match (_, [_;_]) when ParsetreeViewer.isIfLetExpr e -> + let (ifs, elseExpr) = ParsetreeViewer.collectIfExpressions e in + printIfChain e.pexp_attributes ifs elseExpr cmtTbl | Pexp_match (expr, cases) -> let exprDoc = let doc = printExpressionWithComments expr cmtTbl in diff --git a/tests/parsing/grammar/expressions/__snapshots__/parse.spec.js.snap b/tests/parsing/grammar/expressions/__snapshots__/parse.spec.js.snap index f3b27d11..3dbc89b3 100644 --- a/tests/parsing/grammar/expressions/__snapshots__/parse.spec.js.snap +++ b/tests/parsing/grammar/expressions/__snapshots__/parse.spec.js.snap @@ -503,7 +503,78 @@ let ifElseIfThen = if foo = bar then f () else if foo = bar2 then f1 () else if foo = bar3 then f2 () else f3 () -let x = (if true then 1 else 2) + (if false then 2 else 3)" +let x = (if true then 1 else 2) + (if false then 2 else 3) +;;((match foo () with | Some x -> doSomethingWithX x | _ -> ())[@ns.iflet ]) +;;((match foo () with + | Some x -> doSomethingWithX x + | _ -> doSomethingElse ())[@ns.iflet ]) +;;((match foo () with + | Some x -> doSomethingWithX x + | _ -> + (((match bar () with + | Some y -> doSomethingWithY y + | _ -> doSomethingElse ())) + [@ns.iflet ]))[@ns.iflet ]) +;;((match foo () with + | Some x -> doSomethingWithX x + | _ -> if n > 10 then doSomethingWithForN () else doSomethingElse ()) + [@ns.iflet ]) +;;if n > 10 + then doSomethingWithForN () + else + (((match foo () with + | Some x -> doSomethingWithX x + | _ -> doSomethingElse ())) + [@ns.iflet ]) +;;if n > 10 + then ((doSomethingWithForN ())[@aa ]) + else + (((match ((foo ())[@dd ]) with + | ((Some ((x)[@cc ]))[@bb ]) -> ((doSomethingWithY x)[@ee ]) + | _ -> ((doSomethingElse ())[@ff ]))) + [@ns.iflet ]) +;;((match foo () with + | Some x -> doSomethingWithX x + | _ -> + (((match bar () with + | Some y -> doSomethingWithY y + | _ -> + (((match baz () with | Some z -> doSomethingWithZ z | _ -> ())) + [@ns.iflet ]))) + [@ns.iflet ]))[@ns.iflet ]) +;;((match foo () with + | Some (Thing (With { many = Internal [|Components;q|] })) as p -> + doSomethingWithE e + | _ -> ())[@ns.iflet ]) +let a = ((match foo () with | Some x -> x | _ -> 123)[@ns.iflet ]) +let getZ nested = + ((match nested.origin with + | Some point -> (((match point.z with | Some z -> z | _ -> 0)) + [@ns.iflet ]) + | _ -> 0) + [@ns.iflet ]) +;;((match foo () with + | Some (Thing (With { many = Internal [|Components;q|] })) as p -> + ((((match foo () with + | Other (Thing (With { many = Internal [|Components;q|] })) as p + -> doSomethingElse e + | _ -> ())) + [@ns.iflet ]); + doSomethingWithE e) + | _ -> + (((match foo () with + | Some (Thing (With { many = Internal [|Components;q|] })) as p -> + doSomethingWithE e + | _ -> + (((match foo () with + | Some (Thing (With { many = Internal [|Components;q|] })) + as p -> doSomethingWithE e + | _ -> ())) + [@ns.iflet ]))) + [@ns.iflet ]))[@ns.iflet ]) +let a = + ((if b then ((match foo () with | Some x -> 1 | _ -> 2)[@ns.iflet ]) else 3) + [@ns.ternary ])" `; exports[`infix.js 1`] = ` diff --git a/tests/parsing/grammar/expressions/if.js b/tests/parsing/grammar/expressions/if.js index 06a96bfa..30709dce 100644 --- a/tests/parsing/grammar/expressions/if.js +++ b/tests/parsing/grammar/expressions/if.js @@ -14,7 +14,7 @@ let ifThenElse = if foo { lala } else { doStuff(x, y, z,) -} +} let ifElseIfThen = if foo == bar { @@ -28,3 +28,111 @@ let ifElseIfThen = } let x = if true { 1 } else { 2 } + if false { 2 } else { 3 } + +// Basic +if let Some(x) = foo() { + doSomethingWithX(x) +} + +// Else branch +if let Some(x) = foo(){ + doSomethingWithX(x) +} else { + doSomethingElse() +} + +// Else-if support +if let Some(x) = foo(){ + doSomethingWithX(x) +} else if let Some(y) = bar(){ + doSomethingWithY(y) +} else { + doSomethingElse() +} + +// Mixed conditions, pattern start +if let Some(x) = foo(){ + doSomethingWithX(x) +} else if n > 10 { + doSomethingWithForN() +} else { + doSomethingElse() +} + +// Mixed conditions, condition start +if n > 10 { + doSomethingWithForN() +} else if let Some(x) = foo() { + doSomethingWithX(x) +} else { + doSomethingElse() +} + +// Maintains attrs correctly +if n > 10 { + @aa + doSomethingWithForN() +} else if let @bb Some(@cc x) = @dd foo() { + @ee + doSomethingWithY(x) +} else { + @ff + doSomethingElse() +} + +if let Some(x) = foo() { + doSomethingWithX(x) +} else if let Some(y) = bar() { + doSomethingWithY(y) +} else if let Some(z) = baz() { + doSomethingWithZ(z) +} + +// full destructuring +if let Some(Thing(With({ + many: Internal([Components, q]), +}))) as p = foo() { + doSomethingWithE(e) +} + +// Assignment +let a = if let Some(x) = foo() { + x +} else { + 123 +} + +// Nesting +let getZ = nested => + if let Some(point) = nested.origin { + if let Some(z) = point.z { + z + } else { + 0 + } + } else { + 0 + } + +// Complex nesting +if let Some(Thing(With({many: Internal([Components, q])}))) as p = foo() { + if let Other(Thing(With({many: Internal([Components, q])}))) as p = foo() { + doSomethingElse(e) + } + doSomethingWithE(e) +} else if let Some(Thing(With({ + many: Internal([Components, q]), +}))) as p = foo() { + doSomethingWithE(e) +} else if let Some(Thing(With({ + many: Internal([Components, q]), +}))) as p = foo() { + doSomethingWithE(e) +} + +// Mixed with ternary +let a = b ? if let Some(x) = foo() { + 1 +} else { + 2 +} : 3 diff --git a/tests/printer/expr/__snapshots__/render.spec.js.snap b/tests/printer/expr/__snapshots__/render.spec.js.snap index 309a91c5..28ca6ad5 100644 --- a/tests/printer/expr/__snapshots__/render.spec.js.snap +++ b/tests/printer/expr/__snapshots__/render.spec.js.snap @@ -2198,6 +2198,195 @@ let x = if inclusions[index] = (uid, url) { onChange(inclusions) } + +// Basic +if let Some(x) = foo() { + doSomethingWithX(x) +} + +// Else branch +if let Some(x) = foo() { + doSomethingWithX(x) +} else { + doSomethingElse() +} + +// Else-if support +if let Some(x) = foo() { + doSomethingWithX(x) +} else if let Some(y) = bar() { + doSomethingWithY(y) +} else { + doSomethingElse() +} + +// Mixed conditions, pattern start +if let Some(x) = foo() { + doSomethingWithX(x) +} else if n > 10 { + doSomethingWithForN() +} else { + doSomethingElse() +} + +// Mixed conditions, condition start +if n > 10 { + doSomethingWithForN() +} else if let Some(x) = foo() { + doSomethingWithX(x) +} else { + doSomethingElse() +} + +// Maintains attrs correctly +if n > 10 { + @aa + doSomethingWithForN() +} else if let @bb Some(@cc x) = @dd +foo() { + @ee + doSomethingWithY(x) +} else { + @ff + doSomethingElse() +} + +if let Some(x) = foo() { + doSomethingWithX(x) +} else if let Some(y) = bar() { + doSomethingWithY(y) +} else if let Some(z) = baz() { + doSomethingWithZ(z) +} + +// full destructuring +if let Some(Thing(With({many: Internal([Components, q])}))) as p = foo() { + doSomethingWithE(e) +} + +// Assignment +let a = if let Some(x) = foo() { + x +} else { + 123 +} + +// Nesting +let getZ = nested => + if let Some(point) = nested.origin { + if let Some(z) = point.z { + z + } else { + 0 + } + } else { + 0 + } + +// Deep nesting +let getZ = nested => + if let Some(nested) = nested.a { + if let Some(nested) = nested.b { + if let Some(nested) = nested.c { + if let Some(nested) = nested.d { + if let Some(nested) = nested.e { + if let Some(nested) = nested.f { + if let Some(nested) = nested.g { + z + } else { + 0 + } + } else { + 0 + } + } else if let Some(nested) = nested.g { + z + } else { + 0 + } + } else if a { + b + } else { + c + } + } else { + 0 + } + } else { + 0 + } + } else { + 0 + } + +// Break testing +if let Some( + suuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName, +) = anotherSuuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName() { + foo() +} + +if let Some( + suuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName, +) = anotherSuuuuuuuuuuuuuuuuuuuuuuuuuuuperLongNameanotherSuuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName( + withSomeArgsThatAreAlsoLarge, + withManyArgsThatAreAlsoLarge, +) { + foo() +} + +if let { + suuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName, + suuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName2, + suuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName3, +} = buildMyRecord() { + foo() +} + +// Complex nesting +if let Some(Thing(With({many: Internal([Components, q])}))) as p = foo() { + if let Other(Thing(With({many: Internal([Components, q])}))) as p = foo() { + doSomethingElse(e) + } + doSomethingWithE(e) +} else if let Some(Thing(With({ + many: Internal([Components, q]), +}))) as p = foo() { + doSomethingWithE(e) +} else if let Some(Thing(With({ + many: Internal([Components, q]), +}))) as p = foo() { + doSomethingWithE(e) +} + +// Ternary +let a = b + ? if let Some(x) = foo() { + 1 + } else { + 2 + } + : 3 + +let a = b + ? 1 + : if let Some(x) = foo() { + 1 + } else { + 2 + } + +let a = b + ? if let Some(x) = foo() { + 1 + } else { + 2 + } + : if let Some(x) = foo() { + 1 + } else { + 2 + } " `; diff --git a/tests/printer/expr/if.js b/tests/printer/expr/if.js index c7f49a9e..2fb4401a 100644 --- a/tests/printer/expr/if.js +++ b/tests/printer/expr/if.js @@ -11,7 +11,7 @@ let name = if true { let name = if true { user.name } else if false { - user.lastName + user.lastName } else { defaultName } @@ -37,3 +37,181 @@ let x = @attr if truth { if inclusions[index] = (uid, url) { onChange(inclusions) } + + +// Basic +if let Some(x) = foo() { + doSomethingWithX(x) +} + +// Else branch +if let Some(x) = foo(){ + doSomethingWithX(x) +} else { + doSomethingElse() +} + +// Else-if support +if let Some(x) = foo(){ + doSomethingWithX(x) +} else if let Some(y) = bar(){ + doSomethingWithY(y) +} else { + doSomethingElse() +} + +// Mixed conditions, pattern start +if let Some(x) = foo(){ + doSomethingWithX(x) +} else if n > 10 { + doSomethingWithForN() +} else { + doSomethingElse() +} + +// Mixed conditions, condition start +if n > 10 { + doSomethingWithForN() +} else if let Some(x) = foo() { + doSomethingWithX(x) +} else { + doSomethingElse() +} + +// Maintains attrs correctly +if n > 10 { + @aa + doSomethingWithForN() +} else if let @bb Some(@cc x) = @dd foo() { + @ee + doSomethingWithY(x) +} else { + @ff + doSomethingElse() +} + +if let Some(x) = foo() { + doSomethingWithX(x) +} else if let Some(y) = bar() { + doSomethingWithY(y) +} else if let Some(z) = baz() { + doSomethingWithZ(z) +} + +// full destructuring +if let Some(Thing(With({ + many: Internal([Components, q]), +}))) as p = foo() { + doSomethingWithE(e) +} + +// Assignment +let a = if let Some(x) = foo() { + x +} else { + 123 +} + +// Nesting +let getZ = nested => + if let Some(point) = nested.origin { + if let Some(z) = point.z { + z + } else { + 0 + } + } else { + 0 + } + +// Deep nesting +let getZ = nested => + if let Some(nested) = nested.a { + if let Some(nested) = nested.b { + if let Some(nested) = nested.c { + if let Some(nested) = nested.d { + if let Some(nested) = nested.e { + if let Some(nested) = nested.f { + if let Some(nested) = nested.g { + z + } else { + 0 + } + } else { + 0 + } + } else if let Some(nested) = nested.g { + z + } else { + 0 + } + } else { + a ? b : c + } + } else { + 0 + } + } else { + 0 + } + } else { + 0 + } + +// Break testing +if let Some(suuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName) = anotherSuuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName() { + foo() +} + +if let Some(suuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName) = anotherSuuuuuuuuuuuuuuuuuuuuuuuuuuuperLongNameanotherSuuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName(withSomeArgsThatAreAlsoLarge, withManyArgsThatAreAlsoLarge) { + foo() +} + +if let { suuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName, suuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName2, suuuuuuuuuuuuuuuuuuuuuuuuuuuperLongName3 } = buildMyRecord() { + foo() +} + +// Complex nesting +if let Some(Thing(With({many: Internal([Components, q])}))) as p = foo() { + if let Other(Thing(With({many: Internal([Components, q])}))) as p = foo() { + doSomethingElse(e) + } + doSomethingWithE(e) +} else if let Some(Thing(With({ + many: Internal([Components, q]), +}))) as p = foo() { + doSomethingWithE(e) +} else if let Some(Thing(With({ + many: Internal([Components, q]), +}))) as p = foo() { + doSomethingWithE(e) +} + +// Ternary +let a = b + ? if let Some(x) = foo() { + 1 + } else { + 2 + } + : 3 + +let a = b + ? 1 + : if let Some(x) = foo() { + 1 + } else { + 2 + } + +let a = b + ? if let Some(x) = foo() { + 1 + } else { + 2 + } + : if let Some(x) = foo() { + 1 + } else { + 2 + } From 2131063f6a8a428e20aef32e5b73ec8a78b9505a Mon Sep 17 00:00:00 2001 From: Tom Gasson Date: Sat, 4 Jul 2020 13:08:42 -0700 Subject: [PATCH 2/2] Suppress fragile match warning for iflet --- src/napkin_core.ml | 3 +- src/napkin_parsetree_viewer.ml | 45 ++++++++++++++----- src/napkin_parsetree_viewer.mli | 1 + src/napkin_printer.ml | 4 +- .../__snapshots__/parse.spec.js.snap | 37 ++++++++------- 5 files changed, 60 insertions(+), 30 deletions(-) diff --git a/src/napkin_core.ml b/src/napkin_core.ml index b0289eb1..b7aaa479 100644 --- a/src/napkin_core.ml +++ b/src/napkin_core.ml @@ -84,6 +84,7 @@ let jsxAttr = (Location.mknoloc "JSX", Parsetree.PStr []) let uncurryAttr = (Location.mknoloc "bs", Parsetree.PStr []) let ternaryAttr = (Location.mknoloc "ns.ternary", Parsetree.PStr []) let ifLetAttr = (Location.mknoloc "ns.iflet", Parsetree.PStr []) +let suppressFragileMatchWarningAttr = (Location.mknoloc "warning", Parsetree.PStr [Ast_helper.Str.eval (Ast_helper.Exp.constant (Pconst_string ("-4", None)))]) let makeBracesAttr loc = (Location.mkloc "ns.braces" loc, Parsetree.PStr []) type typDefOrExt = @@ -3070,7 +3071,7 @@ and parseIfLetExpr startPos p = Ast_helper.Exp.construct ~loc (Location.mkloc (Longident.Lident "()") loc) None in let loc = mkLoc startPos p.prevEndPos in - Ast_helper.Exp.match_ ~attrs:[ifLetAttr] ~loc conditionExpr [ + Ast_helper.Exp.match_ ~attrs:[ifLetAttr; suppressFragileMatchWarningAttr] ~loc conditionExpr [ Ast_helper.Exp.case pattern thenExpr; Ast_helper.Exp.case (Ast_helper.Pat.any ()) elseExpr; ] diff --git a/src/napkin_parsetree_viewer.ml b/src/napkin_parsetree_viewer.ml index b2e61d63..d8258f56 100644 --- a/src/napkin_parsetree_viewer.ml +++ b/src/napkin_parsetree_viewer.ml @@ -255,9 +255,30 @@ open Parsetree else false + let rec hasIfLetAttribute attrs = + match attrs with + | [] -> false + | ({Location.txt="ns.iflet"},_)::_ -> true + | _::attrs -> hasIfLetAttribute attrs + + let isIfLetExpr expr = match expr with + | { + pexp_attributes = attrs; + pexp_desc = Pexp_match _ + } when hasIfLetAttribute attrs -> true + | _ -> false + let hasAttributes attrs = List.exists (fun attr -> match attr with | ({Location.txt = "bs" | "ns.ternary" | "ns.braces" | "ns.iflet"}, _) -> false + (* Remove the fragile pattern warning for iflet expressions *) + | ({Location.txt="warning"}, PStr [{ + pstr_desc = Pstr_eval ({ + pexp_desc = Pexp_constant ( + Pconst_string ("-4", None) + ) + }, _) + }]) -> not (hasIfLetAttribute attrs) | _ -> true ) attrs @@ -268,18 +289,6 @@ open Parsetree ) -> true | _ -> false - let rec hasIfLetAttribute attrs = - match attrs with - | [] -> false - | ({Location.txt="ns.iflet"},_)::_ -> true - | _::attrs -> hasIfLetAttribute attrs - - let isIfLetExpr expr = match expr with - | { - pexp_attributes = attrs; - pexp_desc = Pexp_match _ - } when hasIfLetAttribute attrs -> true - | _ -> false type ifConditionKind = | If of Parsetree.expression @@ -352,6 +361,18 @@ open Parsetree | _ -> true ) attrs + let filterFragileMatchAttributes attrs = + List.filter (fun attr -> match attr with + | ({Location.txt="warning"}, PStr [{ + pstr_desc = Pstr_eval ({ + pexp_desc = Pexp_constant ( + Pconst_string ("-4", _) + ) + }, _) + }]) -> false + | _ -> true + ) attrs + let isJsxExpression expr = let rec loop attrs = match attrs with diff --git a/src/napkin_parsetree_viewer.mli b/src/napkin_parsetree_viewer.mli index 66ead5d7..6e15a720 100644 --- a/src/napkin_parsetree_viewer.mli +++ b/src/napkin_parsetree_viewer.mli @@ -74,6 +74,7 @@ val parametersShouldHug: funParamKind list -> bool val filterTernaryAttributes: Parsetree.attributes -> Parsetree.attributes +val filterFragileMatchAttributes: Parsetree.attributes -> Parsetree.attributes val isJsxExpression: Parsetree.expression -> bool val hasJsxAttribute: Parsetree.attributes -> bool diff --git a/src/napkin_printer.ml b/src/napkin_printer.ml index 83932dd2..514c6f50 100644 --- a/src/napkin_printer.ml +++ b/src/napkin_printer.ml @@ -2411,8 +2411,9 @@ and printIfChain pexp_attributes ifs elseExpr cmtTbl = printExpressionBlock ~braces:true expr cmtTbl; ] in + let attrs = ParsetreeViewer.filterFragileMatchAttributes pexp_attributes in Doc.concat [ - printAttributes pexp_attributes cmtTbl; + printAttributes attrs cmtTbl; ifDocs; elseDoc; ] @@ -3072,6 +3073,7 @@ and printExpression (e : Parsetree.expression) cmtTbl = | Pexp_newtype _ | Pexp_setfield _ | Pexp_ifthenelse _ -> true + | Pexp_match _ when ParsetreeViewer.isIfLetExpr e -> true | Pexp_construct _ when ParsetreeViewer.hasJsxAttribute e.pexp_attributes -> true | _ -> false in diff --git a/tests/parsing/grammar/expressions/__snapshots__/parse.spec.js.snap b/tests/parsing/grammar/expressions/__snapshots__/parse.spec.js.snap index 3dbc89b3..4df7d44e 100644 --- a/tests/parsing/grammar/expressions/__snapshots__/parse.spec.js.snap +++ b/tests/parsing/grammar/expressions/__snapshots__/parse.spec.js.snap @@ -504,35 +504,36 @@ let ifElseIfThen = then f () else if foo = bar2 then f1 () else if foo = bar3 then f2 () else f3 () let x = (if true then 1 else 2) + (if false then 2 else 3) -;;((match foo () with | Some x -> doSomethingWithX x | _ -> ())[@ns.iflet ]) +;;((match foo () with | Some x -> doSomethingWithX x | _ -> ()) + [@ns.iflet ][@warning \\"-4\\"]) ;;((match foo () with | Some x -> doSomethingWithX x - | _ -> doSomethingElse ())[@ns.iflet ]) + | _ -> doSomethingElse ())[@ns.iflet ][@warning \\"-4\\"]) ;;((match foo () with | Some x -> doSomethingWithX x | _ -> (((match bar () with | Some y -> doSomethingWithY y | _ -> doSomethingElse ())) - [@ns.iflet ]))[@ns.iflet ]) + [@ns.iflet ][@warning \\"-4\\"]))[@ns.iflet ][@warning \\"-4\\"]) ;;((match foo () with | Some x -> doSomethingWithX x | _ -> if n > 10 then doSomethingWithForN () else doSomethingElse ()) - [@ns.iflet ]) + [@ns.iflet ][@warning \\"-4\\"]) ;;if n > 10 then doSomethingWithForN () else (((match foo () with | Some x -> doSomethingWithX x | _ -> doSomethingElse ())) - [@ns.iflet ]) + [@ns.iflet ][@warning \\"-4\\"]) ;;if n > 10 then ((doSomethingWithForN ())[@aa ]) else (((match ((foo ())[@dd ]) with | ((Some ((x)[@cc ]))[@bb ]) -> ((doSomethingWithY x)[@ee ]) | _ -> ((doSomethingElse ())[@ff ]))) - [@ns.iflet ]) + [@ns.iflet ][@warning \\"-4\\"]) ;;((match foo () with | Some x -> doSomethingWithX x | _ -> @@ -540,26 +541,27 @@ let x = (if true then 1 else 2) + (if false then 2 else 3) | Some y -> doSomethingWithY y | _ -> (((match baz () with | Some z -> doSomethingWithZ z | _ -> ())) - [@ns.iflet ]))) - [@ns.iflet ]))[@ns.iflet ]) + [@ns.iflet ][@warning \\"-4\\"]))) + [@ns.iflet ][@warning \\"-4\\"]))[@ns.iflet ][@warning \\"-4\\"]) ;;((match foo () with | Some (Thing (With { many = Internal [|Components;q|] })) as p -> doSomethingWithE e - | _ -> ())[@ns.iflet ]) -let a = ((match foo () with | Some x -> x | _ -> 123)[@ns.iflet ]) + | _ -> ())[@ns.iflet ][@warning \\"-4\\"]) +let a = ((match foo () with | Some x -> x | _ -> 123) + [@ns.iflet ][@warning \\"-4\\"]) let getZ nested = ((match nested.origin with | Some point -> (((match point.z with | Some z -> z | _ -> 0)) - [@ns.iflet ]) + [@ns.iflet ][@warning \\"-4\\"]) | _ -> 0) - [@ns.iflet ]) + [@ns.iflet ][@warning \\"-4\\"]) ;;((match foo () with | Some (Thing (With { many = Internal [|Components;q|] })) as p -> ((((match foo () with | Other (Thing (With { many = Internal [|Components;q|] })) as p -> doSomethingElse e | _ -> ())) - [@ns.iflet ]); + [@ns.iflet ][@warning \\"-4\\"]); doSomethingWithE e) | _ -> (((match foo () with @@ -570,10 +572,13 @@ let getZ nested = | Some (Thing (With { many = Internal [|Components;q|] })) as p -> doSomethingWithE e | _ -> ())) - [@ns.iflet ]))) - [@ns.iflet ]))[@ns.iflet ]) + [@ns.iflet ][@warning \\"-4\\"]))) + [@ns.iflet ][@warning \\"-4\\"]))[@ns.iflet ][@warning \\"-4\\"]) let a = - ((if b then ((match foo () with | Some x -> 1 | _ -> 2)[@ns.iflet ]) else 3) + ((if b + then ((match foo () with | Some x -> 1 | _ -> 2) + [@ns.iflet ][@warning \\"-4\\"]) + else 3) [@ns.ternary ])" `;