From a9a6b71b0c5d4a4910d95cabfab478d36065829c Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 1 Jul 2022 14:56:33 +0200 Subject: [PATCH 1/5] Add MaxIfThenShortWidth to FormatConfig. --- src/Fantomas.Core.Tests/AppTests.fs | 43 ++++++++------- .../CompilerDirectivesTests.fs | 4 +- src/Fantomas.Core.Tests/DotGetTests.fs | 18 +++--- .../Fantomas.Core.Tests.fsproj | 1 + src/Fantomas.Core.Tests/IfThenElseTests.fs | 30 +++++----- .../KeepIndentInBranchTests.fs | 7 ++- src/Fantomas.Core.Tests/ListTests.fs | 7 ++- .../MaxIfThenShortWidthTests.fs | 55 +++++++++++++++++++ ...ockBracketsOnSameColumnArrayOrListTests.fs | 2 +- ...ApplicationsInConditionExpressionsTests.fs | 32 +++++------ src/Fantomas.Core.Tests/OperatorTests.fs | 6 +- src/Fantomas.Core/CodePrinter.fs | 39 +++++++++++++ src/Fantomas.Core/FormatConfig.fs | 5 ++ src/Fantomas.Core/SourceParser.fs | 8 +++ 14 files changed, 190 insertions(+), 67 deletions(-) create mode 100644 src/Fantomas.Core.Tests/MaxIfThenShortWidthTests.fs diff --git a/src/Fantomas.Core.Tests/AppTests.fs b/src/Fantomas.Core.Tests/AppTests.fs index bde01b17b4..656b930a6a 100644 --- a/src/Fantomas.Core.Tests/AppTests.fs +++ b/src/Fantomas.Core.Tests/AppTests.fs @@ -486,35 +486,38 @@ let ``parenthesis around composed function expression, 1341`` () = [] let ``parenthesis around short composed function expression, tuple, 1700`` () = - formatSourceString false """((=) (ownerName, username))""" config - |> should - equal - """((=) (ownerName, username)) -""" + formatSourceString false "((=) (ownerName, username))" { config with InsertFinalNewline = false } + |> should equal "((=) (ownerName, username))" [] let ``parenthesis around short composed function expression, tuple in if, 1700`` () = - formatSourceString false """if ((=) (ownerName, username)) then 6""" config - |> should - equal - """if ((=) (ownerName, username)) then 6 -""" + formatSourceString + false + "if ((=) (ownerName, username)) then 6" + { config with + MaxIfThenShortWidth = 20 + InsertFinalNewline = false } + |> should equal "if ((=) (ownerName, username)) then 6" [] let ``parenthesis around short composed function expression, no tuple, 1700`` () = - formatSourceString false """((=) ownerName)""" config - |> should - equal - """((=) ownerName) -""" + formatSourceString + false + """((=) ownerName)""" + { config with + MaxIfThenShortWidth = 20 + InsertFinalNewline = false } + |> should equal """((=) ownerName)""" [] let ``parenthesis around short composed function expression, no tuple in if, 1700, part 2`` () = - formatSourceString false """if ((=) ownerName) then 6""" config - |> should - equal - """if ((=) ownerName) then 6 -""" + formatSourceString + false + "if ((=) ownerName) then 6" + { config with + MaxIfThenShortWidth = 20 + InsertFinalNewline = false } + |> should equal "if ((=) ownerName) then 6" [] let ``parenthesis around simple function expression`` () = diff --git a/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs b/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs index f010a465dc..6cd38e26f8 100644 --- a/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs +++ b/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs @@ -1646,7 +1646,7 @@ module Dbg = let print _ = () #endif """ - config + { config with MaxIfThenShortWidth = 10 } |> prepend newline |> should equal @@ -1704,7 +1704,7 @@ module Dbg = let print _ = () #endif """ - config + { config with MaxIfThenShortWidth = 10 } |> prepend newline |> should equal diff --git a/src/Fantomas.Core.Tests/DotGetTests.fs b/src/Fantomas.Core.Tests/DotGetTests.fs index b306ebbd12..3bc0c01b4f 100644 --- a/src/Fantomas.Core.Tests/DotGetTests.fs +++ b/src/Fantomas.Core.Tests/DotGetTests.fs @@ -947,15 +947,19 @@ let PublishValueDefn cenv env declKind (vspec: Val) = equal """ let PublishValueDefn cenv env declKind (vspec: Val) = - if (declKind = ModuleOrMemberBinding) - && ((GetCurrAccumulatedModuleOrNamespaceType env) - .ModuleOrNamespaceKind = Namespace) - && (Option.isNone vspec.MemberInfo) then + if + (declKind = ModuleOrMemberBinding) + && ((GetCurrAccumulatedModuleOrNamespaceType env) + .ModuleOrNamespaceKind = Namespace) + && (Option.isNone vspec.MemberInfo) + then errorR (Error(FSComp.SR.tcNamespaceCannotContainValues (), vspec.Range)) - if (declKind = ExtrinsicExtensionBinding) - && ((GetCurrAccumulatedModuleOrNamespaceType env) - .ModuleOrNamespaceKind = Namespace) then + if + (declKind = ExtrinsicExtensionBinding) + && ((GetCurrAccumulatedModuleOrNamespaceType env) + .ModuleOrNamespaceKind = Namespace) + then errorR (Error(FSComp.SR.tcNamespaceCannotContainExtensionMembers (), vspec.Range)) () diff --git a/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj b/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj index f0c53d23ad..bac51ac3d0 100644 --- a/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj +++ b/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj @@ -111,6 +111,7 @@ + diff --git a/src/Fantomas.Core.Tests/IfThenElseTests.fs b/src/Fantomas.Core.Tests/IfThenElseTests.fs index a95aaca425..1c0b05a16d 100644 --- a/src/Fantomas.Core.Tests/IfThenElseTests.fs +++ b/src/Fantomas.Core.Tests/IfThenElseTests.fs @@ -6,13 +6,13 @@ open Fantomas.Core.Tests.TestHelper [] let ``single line if without else`` () = - formatSourceString false "if foo then bar" config - |> prepend newline - |> should - equal - """ -if foo then bar -""" + formatSourceString + false + "if foo then bar" + { config with + InsertFinalNewline = false + MaxIfThenShortWidth = 4 } + |> should equal "if foo then bar" [] let ``if without else, if is longer`` () = @@ -54,7 +54,7 @@ let ``short if then without else`` () = """ if a then b """ - config + { config with MaxIfThenShortWidth = 2 } |> prepend newline |> should equal @@ -74,9 +74,11 @@ if foo && bar && meh then aha |> should equal """ -if foo - && bar - && meh then +if + foo + && bar + && meh +then aha """ @@ -1786,10 +1788,12 @@ module UtxoCoinAccount = (amount: TransferAmount) (password: string) = - if (baseAccount.PublicAddress.Equals( + if + (baseAccount.PublicAddress.Equals( destination, StringComparison.InvariantCultureIgnoreCase - )) then + )) + then raise DestinationEqualToOrigin """ diff --git a/src/Fantomas.Core.Tests/KeepIndentInBranchTests.fs b/src/Fantomas.Core.Tests/KeepIndentInBranchTests.fs index ff17ea3112..c556634aac 100644 --- a/src/Fantomas.Core.Tests/KeepIndentInBranchTests.fs +++ b/src/Fantomas.Core.Tests/KeepIndentInBranchTests.fs @@ -674,11 +674,14 @@ lock lockingObj (fun () -> equal """ lock lockingObj (fun () -> - if not thing then printfn "" + if not thing then + printfn "" match error with | Some error -> - if foo then () + if foo then + () + thing () false | None -> diff --git a/src/Fantomas.Core.Tests/ListTests.fs b/src/Fantomas.Core.Tests/ListTests.fs index 05cea061b1..6f51d0d6f3 100644 --- a/src/Fantomas.Core.Tests/ListTests.fs +++ b/src/Fantomas.Core.Tests/ListTests.fs @@ -173,7 +173,7 @@ let ``array comprehensions`` () = let a1 = [| for i in 1 .. 10 -> i * i |] let a2 = [| 0 .. 99 |] let a3 = [| for n in 1 .. 100 do if isPrime n then yield n |]""" - config + { config with MaxIfThenShortWidth = 20 } |> prepend newline |> should equal @@ -1918,7 +1918,8 @@ let original_input = [ """ { config with MaxIfThenElseShortWidth = 120 - MaxArrayOrListWidth = 120 } + MaxArrayOrListWidth = 120 + MaxIfThenShortWidth = 120 } |> prepend newline |> should equal @@ -1964,7 +1965,7 @@ let wrong = [ if true then 2 ] """ - config + { config with MaxIfThenShortWidth = 20 } |> prepend newline |> should equal diff --git a/src/Fantomas.Core.Tests/MaxIfThenShortWidthTests.fs b/src/Fantomas.Core.Tests/MaxIfThenShortWidthTests.fs new file mode 100644 index 0000000000..cd32a09ea2 --- /dev/null +++ b/src/Fantomas.Core.Tests/MaxIfThenShortWidthTests.fs @@ -0,0 +1,55 @@ +module Fantomas.Core.Tests.MaxIfThenShortWidthTests + +open NUnit.Framework +open FsUnit +open Fantomas.Core.Tests.TestHelper + +[] +let ``default behavior of MaxIfThenShortWidth`` () = + formatSourceString + false + """ +if a then () +""" + config + |> prepend newline + |> should + equal + """ +if a then + () +""" + +[] +let ``keep entire expression in one line`` () = + formatSourceString + false + """ +if a then () +""" + { config with MaxIfThenShortWidth = 3 } + |> prepend newline + |> should + equal + """ +if a then () +""" + +[] +let ``always put then on next line if the ifExpr is multiline`` () = + formatSourceString + false + """ +if // comment makes expr multiline + a then b +""" + { config with MaxIfThenShortWidth = 100 } + |> prepend newline + |> should + equal + """ +if // comment makes expr multiline + a +then + b +""" diff --git a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnArrayOrListTests.fs b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnArrayOrListTests.fs index c5b6bdbc42..5531e4dcef 100644 --- a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnArrayOrListTests.fs +++ b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnArrayOrListTests.fs @@ -105,7 +105,7 @@ let ``array comprehensions`` () = """ let a1 = [| 0 .. 99 |] let a2 = [| for n in 1 .. 100 do if isPrime n then yield n |]""" - config + { config with MaxIfThenShortWidth = 20 } |> prepend newline |> should equal diff --git a/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs b/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs index 934fb507b3..a1cc0443e2 100644 --- a/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs +++ b/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs @@ -150,8 +150,9 @@ if MyGrandFunctionThatTakesASingleArgument ( myEvenGranderArgumentNameThatGoesOn equal """ if - MyGrandFunctionThatTakesASingleArgument - (myEvenGranderArgumentNameThatGoesOnForEverAndEver) + MyGrandFunctionThatTakesASingleArgument( + myEvenGranderArgumentNameThatGoesOnForEverAndEver + ) then () """ @@ -248,21 +249,18 @@ module Web3ServerSeedList = | Some rpcResponseEx -> if rpcResponseEx.RpcError <> null then if - (not - ( - rpcResponseEx.RpcError.Message.Contains - "pruning=archive" - )) - && (not - ( - rpcResponseEx.RpcError.Message.Contains - "header not found" - )) - && (not - ( - rpcResponseEx.RpcError.Message.Contains - "missing trie node" - )) + (not ( + rpcResponseEx.RpcError.Message.Contains + "pruning=archive" + )) + && (not ( + rpcResponseEx.RpcError.Message.Contains + "header not found" + )) + && (not ( + rpcResponseEx.RpcError.Message.Contains + "missing trie node" + )) then raise UnexpectedRpcResponseError | _ -> () diff --git a/src/Fantomas.Core.Tests/OperatorTests.fs b/src/Fantomas.Core.Tests/OperatorTests.fs index a7d23662fe..bc4a15f0c5 100644 --- a/src/Fantomas.Core.Tests/OperatorTests.fs +++ b/src/Fantomas.Core.Tests/OperatorTests.fs @@ -1116,8 +1116,10 @@ module Foo = module Foo = let bar () = - if not - <| RuntimeInformation.IsOSPlatform OSPlatform.Windows then + if + not + <| RuntimeInformation.IsOSPlatform OSPlatform.Windows + then raise <| PlatformNotSupportedException ( "Blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah" diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index f9f71e58b5..518a09a82c 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -1974,6 +1974,45 @@ and genExpr astContext synExpr ctx = | Sequentials es -> let items = List.collect (collectMultilineItemForSynExpr astContext) es atCurrentColumn (colWithNlnWhenItemIsMultilineUsingConfig items) + + | IfThenWithoutElse (ifKw, ifExpr, thenKw, thenExpr) -> + let genIf = genTriviaFor SynExpr_IfThenElse_If ifKw !- "if" + let genThen = genTriviaFor SynExpr_IfThenElse_Then thenKw !- "then" + + let shortIfExpr = + genIf + +> sepSpace + +> genExpr astContext ifExpr + +> sepSpace + +> genThen + + let longIfExpr = + genIf + +> indent + +> sepNln + +> genExpr astContext ifExpr + +> unindent + +> sepNln + +> genThen + + leadingExpressionIsMultiline (expressionFitsOnRestOfLine shortIfExpr longIfExpr) (fun isMultiline ctx -> + if isMultiline then + (indent + +> sepNln + +> genExpr astContext thenExpr + +> unindent) + ctx + else + isShortExpression + ctx.Config.MaxIfThenShortWidth + (sepSpace +> genExpr astContext thenExpr) + (indent + +> sepNln + +> genExpr astContext thenExpr + +> unindent) + ctx) + |> atCurrentColumnIndent + // A generalization of IfThenElse | ElIf ((_, ifKw, isElif, e1, thenKw, e2) :: es, (elseKw, elseOpt), _) -> // https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-if-expressions diff --git a/src/Fantomas.Core/FormatConfig.fs b/src/Fantomas.Core/FormatConfig.fs index ce9b7e2a28..e6b75ba8d0 100644 --- a/src/Fantomas.Core/FormatConfig.fs +++ b/src/Fantomas.Core/FormatConfig.fs @@ -115,6 +115,10 @@ type FormatConfig = [] SpaceAroundDelimiter: bool + [] + [] + MaxIfThenShortWidth: Num + [] [] MaxIfThenElseShortWidth: Num @@ -220,6 +224,7 @@ type FormatConfig = SpaceBeforeSemicolon = false SpaceAfterSemicolon = true SpaceAroundDelimiter = true + MaxIfThenShortWidth = 0 MaxIfThenElseShortWidth = 40 MaxInfixOperatorExpression = 50 MaxRecordWidth = 40 diff --git a/src/Fantomas.Core/SourceParser.fs b/src/Fantomas.Core/SourceParser.fs index d739203de9..1ae7136462 100644 --- a/src/Fantomas.Core/SourceParser.fs +++ b/src/Fantomas.Core/SourceParser.fs @@ -1065,6 +1065,14 @@ let (|IfThenElse|_|) = | SynExpr.IfThenElse _ as e -> Some e | _ -> None +let (|IfThenWithoutElse|_|) = + function + | SynExpr.IfThenElse (ifExpr, thenExpr, None, _, _, _, trivia) -> + match ifExpr with + | IfThenElse _ -> None + | _ -> Some(trivia.IfKeyword, ifExpr, trivia.ThenKeyword, thenExpr) + | _ -> None + let rec (|ElIf|_|) = function | SynExpr.IfThenElse (e1, From 5d7497990972394260daa4490aa0216c8d50b810 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 1 Jul 2022 15:25:26 +0200 Subject: [PATCH 2/5] Apply the same rule for if/then formatting to match/with and match!/with. --- ...ApplicationsInConditionExpressionsTests.fs | 36 +++--- .../PatternMatchingTests.fs | 65 +++++----- src/Fantomas.Core/CodePrinter.fs | 113 +++++++++++------- 3 files changed, 121 insertions(+), 93 deletions(-) diff --git a/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs b/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs index a1cc0443e2..162fc622bd 100644 --- a/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs +++ b/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs @@ -21,12 +21,11 @@ let foo () = """ let foo () = match - b.TryGetValue - ( - longlonglonglonglong, - b - ) - with + b.TryGetValue( + longlonglonglonglong, + b + ) + with | true, i -> Some i | false, _ -> failwith "" """ @@ -48,9 +47,10 @@ let foo () = """ let foo () = match - b.TryGetValue - (longlonglonglonglong) - with + b.TryGetValue( + longlonglonglonglong + ) + with | true, i -> Some i | false, _ -> failwith "" """ @@ -177,9 +177,10 @@ let foo () = let foo () = async { match! - b.TryGetValue - (longlonglonglonglong) - with + b.TryGetValue( + longlonglonglonglong + ) + with | true, i -> Some i | false, _ -> failwith "" } @@ -205,12 +206,11 @@ let foo () = let foo () = async { match! - b.TryGetValue - ( - longlonglonglonglong, - b - ) - with + b.TryGetValue( + longlonglonglonglong, + b + ) + with | true, i -> Some i | false, _ -> failwith "" } diff --git a/src/Fantomas.Core.Tests/PatternMatchingTests.fs b/src/Fantomas.Core.Tests/PatternMatchingTests.fs index c0fa1f4c1c..735ec461a2 100644 --- a/src/Fantomas.Core.Tests/PatternMatchingTests.fs +++ b/src/Fantomas.Core.Tests/PatternMatchingTests.fs @@ -656,7 +656,7 @@ let MethInfoIsUnseen g m ty minfo = (fun fsAttribs -> Some bar) (fun provAttribs -> Some(CheckProvidedAttributesForUnseen provAttribs m)) (fun _provAttribs -> None) - with + with | Some res -> res | None -> false @@ -702,7 +702,7 @@ let MethInfoIsUnseen g m ty minfo = #else (fun _provAttribs -> None) #endif - with + with | Some res -> res | None -> false @@ -1143,12 +1143,11 @@ match x (Map.tryFind somelongidentifier a + Option.defaultValue longidentifier) equal """ match - x - ( - Map.tryFind somelongidentifier a - + Option.defaultValue longidentifier - ) - with + x ( + Map.tryFind somelongidentifier a + + Option.defaultValue longidentifier + ) +with | _ -> () """ @@ -1294,7 +1293,7 @@ match match u with | null -> "" | s -> s - with +with | "" -> x | _ -> failwith "" """ @@ -1324,7 +1323,7 @@ match match! u with | null -> "" | s -> s - with +with | "" -> x | _ -> failwith "" """ @@ -1354,7 +1353,7 @@ match! match u with | null -> "" | s -> s - with +with | "" -> x | _ -> failwith "" """ @@ -1384,7 +1383,7 @@ match! match! u with | null -> "" | s -> s - with +with | "" -> x | _ -> failwith "" """ @@ -1898,13 +1897,12 @@ match structuralTypes |> List.tryFind (fst >> checkIfFieldTypeSupportsComparison """ match structuralTypes - |> List.tryFind - ( - fst - >> checkIfFieldTypeSupportsComparison tycon - >> not - ) - with + |> List.tryFind ( + fst + >> checkIfFieldTypeSupportsComparison tycon + >> not + ) +with | _ -> () """ @@ -1923,13 +1921,12 @@ match! structuralTypes |> List.tryFind (fst >> checkIfFieldTypeSupportsCompariso """ match! structuralTypes - |> List.tryFind - ( - fst - >> checkIfFieldTypeSupportsComparison tycon - >> not - ) - with + |> List.tryFind ( + fst + >> checkIfFieldTypeSupportsComparison tycon + >> not + ) +with | _ -> () """ @@ -2021,7 +2018,7 @@ match Caching.Instance.TryRetrieveLastCompoundBalanceLoooooooooooooooooooooooooooooooooooooooooooongFuncName address currency - with +with | None -> false | Some balance -> someRetrievedBalance = balance """ @@ -2043,7 +2040,7 @@ match! Caching.Instance.TryRetrieveLastCompoundBalanceLoooooooooooooooooooooooooooooooooooooooooooongFuncName address currency - with +with | None -> false | Some balance -> someRetrievedBalance = balance """ @@ -2083,7 +2080,8 @@ match // foo equal """ match // foo - a with + a +with | B b -> () """ @@ -2102,7 +2100,8 @@ match! // foo equal """ match! // foo - a with + a +with | B b -> () """ @@ -2143,10 +2142,12 @@ match! // foo! equal """ match // foo - a with + a +with | _ -> () match! // foo! - a with + a +with | _ -> () """ diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index 518a09a82c..0e302930d6 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -1390,30 +1390,10 @@ and genExpr astContext synExpr ctx = +> sepNln +> genClauses astContext cs | Match (matchRange, e, withRange, cs) -> - let genMatchExpr = - genTriviaFor SynExpr_Match_Match matchRange !- "match " - +> autoIndentAndNlnWhenWriteBeforeNewlineNotEmpty ( - expressionFitsOnRestOfLine - (genExpr astContext e - +> genWithAfterMatch SynExpr_Match_With withRange) - (genExprInIfOrMatch astContext e - +> (sepNlnUnlessLastEventIsNewline - +> (genWithAfterMatch SynExpr_Match_With withRange))) - ) - + let genMatchExpr = genMatchWith astContext matchRange e withRange atCurrentColumn (genMatchExpr +> sepNln +> genClauses astContext cs) | MatchBang (matchRange, e, withRange, cs) -> - let genMatchExpr = - genTriviaFor SynExpr_MatchBang_Match matchRange !- "match! " - +> autoIndentAndNlnWhenWriteBeforeNewlineNotEmpty ( - expressionFitsOnRestOfLine - (genExpr astContext e - +> genWithAfterMatch SynExpr_MatchBang_With withRange) - (genExprInIfOrMatch astContext e - +> (sepNlnUnlessLastEventIsNewline - +> (genWithAfterMatch SynExpr_MatchBang_With withRange))) - ) - + let genMatchExpr = genMatchBangWith astContext matchRange e withRange atCurrentColumn (genMatchExpr +> sepNln +> genClauses astContext cs) | TraitCall (tps, msg, e) -> genTyparList astContext tps @@ -1423,7 +1403,6 @@ and genExpr astContext synExpr ctx = +> sepCloseT +> sepSpace +> genExpr astContext e - | Paren (_, ILEmbedded r, rpr, _) -> fun ctx -> let expr = @@ -1976,26 +1955,7 @@ and genExpr astContext synExpr ctx = atCurrentColumn (colWithNlnWhenItemIsMultilineUsingConfig items) | IfThenWithoutElse (ifKw, ifExpr, thenKw, thenExpr) -> - let genIf = genTriviaFor SynExpr_IfThenElse_If ifKw !- "if" - let genThen = genTriviaFor SynExpr_IfThenElse_Then thenKw !- "then" - - let shortIfExpr = - genIf - +> sepSpace - +> genExpr astContext ifExpr - +> sepSpace - +> genThen - - let longIfExpr = - genIf - +> indent - +> sepNln - +> genExpr astContext ifExpr - +> unindent - +> sepNln - +> genThen - - leadingExpressionIsMultiline (expressionFitsOnRestOfLine shortIfExpr longIfExpr) (fun isMultiline ctx -> + leadingExpressionIsMultiline (genIfThen astContext ifKw ifExpr thenKw) (fun isMultiline ctx -> if isMultiline then (indent +> sepNln @@ -3290,6 +3250,73 @@ and genAppWithLambda astContext sep (e, es, lpr, lambda, rpr, pr) = expressionFitsOnRestOfLine short long +and genControlExpressionStart + astContext + (startKeywordType: FsAstType) + (startKeywordRange: range) + (startKeywordText: string) + (innerExpr: SynExpr) + (endKeywordType: FsAstType) + (endKeywordRange: range) + (endKeywordText: string) + = + let genStartKeyword = + genTriviaFor startKeywordType startKeywordRange !-startKeywordText + + let genEndKeyword = genTriviaFor endKeywordType endKeywordRange !-endKeywordText + + let shortIfExpr = + genStartKeyword + +> sepSpace + +> genExpr astContext innerExpr + +> sepSpace + +> genEndKeyword + + let longIfExpr = + genStartKeyword + +> indent + +> sepNln + +> genExpr astContext innerExpr + +> unindent + +> sepNln + +> genEndKeyword + + expressionFitsOnRestOfLine shortIfExpr longIfExpr + +and genIfThen astContext (ifKeyword: range) (ifExpr: SynExpr) (thenKeyword: range) = + genControlExpressionStart + astContext + SynExpr_IfThenElse_If + ifKeyword + "if" + ifExpr + SynExpr_IfThenElse_Then + thenKeyword + "then" + +and genMatchWith astContext (matchKeyword: range) (matchExpr: SynExpr) (withKeyword: range) = + genControlExpressionStart + astContext + SynExpr_Match_Match + matchKeyword + "match" + matchExpr + SynExpr_Match_With + withKeyword + "with" + +and genMatchBangWith astContext (matchKeyword: range) (matchExpr: SynExpr) (withKeyword: range) = + genControlExpressionStart + astContext + SynExpr_MatchBang_Match + matchKeyword + "match!" + matchExpr + SynExpr_MatchBang_With + withKeyword + "with" + +// TODO: remove 😁 and genExprInIfOrMatch astContext (e: SynExpr) (ctx: Context) : Context = let short = sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace From 8d92dd2849583720b62676fef9d9358e3c8b6284 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 4 Jul 2022 09:27:56 +0200 Subject: [PATCH 3/5] Revisit all If/Then/Else formatting according to new rules. --- src/Fantomas.Core.Tests/ActivePatternTests.fs | 2 +- src/Fantomas.Core.Tests/AppTests.fs | 11 +- src/Fantomas.Core.Tests/CommentTests.fs | 7 +- .../CompilerDirectivesTests.fs | 6 +- .../ComputationExpressionTests.fs | 4 +- .../ControlStructureTests.fs | 2 +- .../DynamicOperatorTests.fs | 2 +- src/Fantomas.Core.Tests/IfThenElseTests.fs | 129 +++--- .../KeepIndentInBranchTests.fs | 6 +- src/Fantomas.Core.Tests/LetBindingTests.fs | 28 +- src/Fantomas.Core.Tests/ListTests.fs | 2 +- .../MaxIfThenShortWidthTests.fs | 43 +- ...ockBracketsOnSameColumnArrayOrListTests.fs | 2 +- ...ApplicationsInConditionExpressionsTests.fs | 22 +- src/Fantomas.Core.Tests/TupleTests.fs | 6 +- src/Fantomas.Core/AstTransformer.fs | 18 +- src/Fantomas.Core/CodePrinter.fs | 385 ++++++++++-------- src/Fantomas.Core/Context.fs | 8 +- src/Fantomas.Core/SourceParser.fs | 17 +- 19 files changed, 421 insertions(+), 279 deletions(-) diff --git a/src/Fantomas.Core.Tests/ActivePatternTests.fs b/src/Fantomas.Core.Tests/ActivePatternTests.fs index da8da31094..d141c3bd4e 100644 --- a/src/Fantomas.Core.Tests/ActivePatternTests.fs +++ b/src/Fantomas.Core.Tests/ActivePatternTests.fs @@ -64,7 +64,7 @@ let (|ParseRegex|_|) regex str = { config with MaxValueBindingWidth = 30 MaxFunctionBindingWidth = 30 - MaxIfThenElseShortWidth = 70 } + MaxIfThenElseShortWidth = 75 } |> prepend newline |> should equal diff --git a/src/Fantomas.Core.Tests/AppTests.fs b/src/Fantomas.Core.Tests/AppTests.fs index 656b930a6a..f50057c24e 100644 --- a/src/Fantomas.Core.Tests/AppTests.fs +++ b/src/Fantomas.Core.Tests/AppTests.fs @@ -495,18 +495,13 @@ let ``parenthesis around short composed function expression, tuple in if, 1700`` false "if ((=) (ownerName, username)) then 6" { config with - MaxIfThenShortWidth = 20 + MaxIfThenShortWidth = 40 InsertFinalNewline = false } |> should equal "if ((=) (ownerName, username)) then 6" [] let ``parenthesis around short composed function expression, no tuple, 1700`` () = - formatSourceString - false - """((=) ownerName)""" - { config with - MaxIfThenShortWidth = 20 - InsertFinalNewline = false } + formatSourceString false """((=) ownerName)""" { config with InsertFinalNewline = false } |> should equal """((=) ownerName)""" [] @@ -515,7 +510,7 @@ let ``parenthesis around short composed function expression, no tuple in if, 170 false "if ((=) ownerName) then 6" { config with - MaxIfThenShortWidth = 20 + MaxIfThenShortWidth = 25 InsertFinalNewline = false } |> should equal "if ((=) ownerName) then 6" diff --git a/src/Fantomas.Core.Tests/CommentTests.fs b/src/Fantomas.Core.Tests/CommentTests.fs index c7546d1bf7..b774f6b404 100644 --- a/src/Fantomas.Core.Tests/CommentTests.fs +++ b/src/Fantomas.Core.Tests/CommentTests.fs @@ -495,7 +495,9 @@ if a then () else // Comment 1 -if b then +if + b +then () // Comment 2 else @@ -993,7 +995,8 @@ else 0""" equal """ if //comment - true then + true +then 1 else 0 diff --git a/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs b/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs index 6cd38e26f8..1aafbb4505 100644 --- a/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs +++ b/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs @@ -1646,7 +1646,7 @@ module Dbg = let print _ = () #endif """ - { config with MaxIfThenShortWidth = 10 } + { config with MaxIfThenShortWidth = 30 } |> prepend newline |> should equal @@ -1704,7 +1704,7 @@ module Dbg = let print _ = () #endif """ - { config with MaxIfThenShortWidth = 10 } + { config with MaxIfThenShortWidth = 30 } |> prepend newline |> should equal @@ -2504,7 +2504,7 @@ let ``comment after compiler define`` () = let tcrefObjTy, enclosingDeclaredTypars, renaming, objTy = FreshenTyconRef m rigid tcref declaredTyconTypars #endif """ - config + { config with MaxIfThenElseShortWidth = 20 } |> prepend newline |> should equal diff --git a/src/Fantomas.Core.Tests/ComputationExpressionTests.fs b/src/Fantomas.Core.Tests/ComputationExpressionTests.fs index bb226997e6..1e21da1f02 100644 --- a/src/Fantomas.Core.Tests/ComputationExpressionTests.fs +++ b/src/Fantomas.Core.Tests/ComputationExpressionTests.fs @@ -1909,7 +1909,9 @@ let create: Highlighter = |> List.ofSeq |> FormattedText.fromList """ - { config with MaxIfThenElseShortWidth = 80 } + { config with + MaxIfThenElseShortWidth = 80 + MaxIfThenShortWidth = 80 } |> prepend newline |> should equal diff --git a/src/Fantomas.Core.Tests/ControlStructureTests.fs b/src/Fantomas.Core.Tests/ControlStructureTests.fs index 799edaeee7..f7c9c7baae 100644 --- a/src/Fantomas.Core.Tests/ControlStructureTests.fs +++ b/src/Fantomas.Core.Tests/ControlStructureTests.fs @@ -389,7 +389,7 @@ let ``if/elif without else`` () = if true then () elif true then () """ - config + { config with MaxIfThenShortWidth = 20 } |> prepend newline |> should equal diff --git a/src/Fantomas.Core.Tests/DynamicOperatorTests.fs b/src/Fantomas.Core.Tests/DynamicOperatorTests.fs index 0513158476..d14368272e 100644 --- a/src/Fantomas.Core.Tests/DynamicOperatorTests.fs +++ b/src/Fantomas.Core.Tests/DynamicOperatorTests.fs @@ -31,7 +31,7 @@ let ``keep () when dynamic operator inside boolean expr, #476`` () = NoColor |> Input.Color """ - config + { config with MaxIfThenElseShortWidth = 5 } |> prepend newline |> should equal diff --git a/src/Fantomas.Core.Tests/IfThenElseTests.fs b/src/Fantomas.Core.Tests/IfThenElseTests.fs index 1c0b05a16d..464c093cb7 100644 --- a/src/Fantomas.Core.Tests/IfThenElseTests.fs +++ b/src/Fantomas.Core.Tests/IfThenElseTests.fs @@ -11,7 +11,7 @@ let ``single line if without else`` () = "if foo then bar" { config with InsertFinalNewline = false - MaxIfThenShortWidth = 4 } + MaxIfThenShortWidth = 15 } |> should equal "if foo then bar" [] @@ -54,7 +54,7 @@ let ``short if then without else`` () = """ if a then b """ - { config with MaxIfThenShortWidth = 2 } + { config with MaxIfThenShortWidth = 12 } |> prepend newline |> should equal @@ -225,8 +225,10 @@ let ``multiline condition`` () = |> should equal """ -if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - && bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb) then +if + (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + && bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb) +then x else y @@ -344,7 +346,8 @@ let ``comment after if`` () = equal """ if // meh - x then + x +then 0 else 1 @@ -362,7 +365,8 @@ let ``comment after if branch`` () = |> should equal """ -if x // meh +if + x // meh then 0 else @@ -453,7 +457,9 @@ let ``comment after else keyword before if keyword`` () = if a then b else // meh -if c then +if + c +then d else e @@ -474,7 +480,8 @@ let ``comment after else if keyword`` () = if a then b else if // meh - c then + c +then d else e @@ -495,7 +502,8 @@ let ``comment after elif keyword`` () = if a then b elif // meh - c then + c +then d else e @@ -516,7 +524,8 @@ let ``comment after else if boolean expression`` () = """ if a then b -else if c // meh +else if + c // meh then d else @@ -538,7 +547,8 @@ let ``comment after elif boolean expression`` () = """ if a then b -elif c // meh +elif + c // meh then d else @@ -558,12 +568,9 @@ let ``comment after else if then keyword`` () = |> should equal """ -if a then - b -else if c then // meh - d -else - e +if a then b +else if c then d // meh +else e """ [] @@ -579,12 +586,9 @@ let ``comment after elif then keyword`` () = |> should equal """ -if a then - b -elif c then // meh - d -else - e +if a then b +elif c then d // meh +else e """ [] @@ -702,7 +706,8 @@ if a then b else // foo if // bar - c then + c +then d else e @@ -923,7 +928,7 @@ else // c10 """ if // c1 a // c2 - then // c3 +then // c3 b // c4 else // c5 if // c6 @@ -1299,7 +1304,8 @@ let foo result total = equal """ let foo result total = - if result = 0 // there's a comment here + if + result = 0 // there's a comment here then total // and another one else @@ -1512,11 +1518,10 @@ let code = " let code = if - System.Text.RegularExpressions.Regex.IsMatch - ( - d.Name, - \"\"\"^[a-zA-Z][a-zA-Z0-9']+$\"\"\" - ) + System.Text.RegularExpressions.Regex.IsMatch( + d.Name, + \"\"\"^[a-zA-Z][a-zA-Z0-9']+$\"\"\" + ) then d.Name elif d.NamespaceToOpen.IsSome then @@ -1704,8 +1709,10 @@ let private tryGetUrlWithExactMatch (urlPattern: string) (document: Document) = - if (UMX.untag pathPattern) - .Equals(UMX.untag document.Name, System.StringComparison.Ordinal) then + if + (UMX.untag pathPattern) + .Equals(UMX.untag document.Name, System.StringComparison.Ordinal) + then Some(urlPattern, normalizeRepoPath (UMX.cast pathPattern), document) else None @@ -1715,8 +1722,10 @@ let private tryGetUrlWithExactMatch (urlPattern: string) (document: Document) = - if (UMX.untag pathPattern) - .Equals(UMX.untag document.Name, System.StringComparison.Ordinal) then + if + (UMX.untag pathPattern) + .Equals(UMX.untag document.Name, System.StringComparison.Ordinal) + then Some(urlPattern, normalizeRepoPath (UMX.cast pathPattern), document) else None @@ -1826,11 +1835,9 @@ let x = // Original input: let x = if - not - ( - f - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - ) + not ( + f aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + ) then 1 else @@ -1838,9 +1845,11 @@ let x = // Formatted output of the above, in for a second format: let x = - if (not ( + if + (not ( f aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - )) then + )) + then 1 else 2 @@ -2055,14 +2064,13 @@ else () equal """ if - Uri.Compare - ( - foo, - bar, - UriComponents.Host ||| UriComponents.Path, - UriFormat.UriEscaped, - StringComparison.CurrentCulture - ) = 0 + Uri.Compare( + foo, + bar, + UriComponents.Host ||| UriComponents.Path, + UriFormat.UriEscaped, + StringComparison.CurrentCulture + ) = 0 then () else @@ -2190,8 +2198,9 @@ type internal Foo private () = static member Bar: int option = if thing = 1 then printfn "hi" - else if veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLong - |> Seq.forall (fun (u: VeryVeryVeryVeryVeryVeryVeryLong) -> u.Length = 0) // + else if + veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLong + |> Seq.forall (fun (u: VeryVeryVeryVeryVeryVeryVeryLong) -> u.Length = 0) // then printfn "hi" else @@ -2201,8 +2210,9 @@ type internal Foo2 private () = static member Bar: int option = if thing = 1 then printfn "hi" - else if veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLong - |> Seq.forall (fun (u: VeryVeryVeryVeryVeryVeryVeryLong) -> u.Length = 0) // + else if + veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLong + |> Seq.forall (fun (u: VeryVeryVeryVeryVeryVeryVeryLong) -> u.Length = 0) // then printfn "hi" else @@ -2396,7 +2406,8 @@ if (function | CompExpr _ -> true | _ -> false) - es then + es +then shortExpression ctx else expressionFitsOnRestOfLine shortExpression longExpression ctx @@ -2421,8 +2432,12 @@ module Foo = module Foo = let bar = if - Regex("long long long long long long long long long") - .Match(s) + Regex( + "long long long long long long long long long" + ) + .Match( + s + ) .Success then None diff --git a/src/Fantomas.Core.Tests/KeepIndentInBranchTests.fs b/src/Fantomas.Core.Tests/KeepIndentInBranchTests.fs index c556634aac..830987f613 100644 --- a/src/Fantomas.Core.Tests/KeepIndentInBranchTests.fs +++ b/src/Fantomas.Core.Tests/KeepIndentInBranchTests.fs @@ -528,8 +528,10 @@ let rec getEndCol (r: Range) (tokenizer: FSharpLineTokenizer) lexState = | Some (tok), state -> Debug.WriteLine("End token: {0}", sprintf "%A" tok |> box) - if tok.RightColumn >= r.EndColumn - && isSignificantToken tok then + if + tok.RightColumn >= r.EndColumn + && isSignificantToken tok + then tok.RightColumn else lexState := state diff --git a/src/Fantomas.Core.Tests/LetBindingTests.fs b/src/Fantomas.Core.Tests/LetBindingTests.fs index 6ed464b8b7..18269b39dd 100644 --- a/src/Fantomas.Core.Tests/LetBindingTests.fs +++ b/src/Fantomas.Core.Tests/LetBindingTests.fs @@ -394,7 +394,19 @@ let ``comment trivia before simple sequence doesn't force remaining to get offse [] let ``no extra newline should be added between IfThenElse within Sequential, 588`` () = - shouldNotChangeAfterFormat + formatSourceString + false + """ +let x = + if true then printfn "a" + elif true then printfn "b" + + if true then 1 else 0 +""" + { config with MaxIfThenShortWidth = 50 } + |> prepend newline + |> should + equal """ let x = if true then printfn "a" @@ -1344,12 +1356,14 @@ let internal sepSpace = // ignore multiple spaces, space on start of file, after newline // TODO: this is inefficient - maybe remember last char written? fun (ctx: Context) -> - if (not ctx.WriterInitModel.IsDummy - && let s = dump ctx in - - s = "" - || s.EndsWith " " - || s.EndsWith Environment.NewLine) then + if + (not ctx.WriterInitModel.IsDummy + && let s = dump ctx in + + s = "" + || s.EndsWith " " + || s.EndsWith Environment.NewLine) + then ctx else (!- " ") ctx diff --git a/src/Fantomas.Core.Tests/ListTests.fs b/src/Fantomas.Core.Tests/ListTests.fs index 6f51d0d6f3..7019580355 100644 --- a/src/Fantomas.Core.Tests/ListTests.fs +++ b/src/Fantomas.Core.Tests/ListTests.fs @@ -173,7 +173,7 @@ let ``array comprehensions`` () = let a1 = [| for i in 1 .. 10 -> i * i |] let a2 = [| 0 .. 99 |] let a3 = [| for n in 1 .. 100 do if isPrime n then yield n |]""" - { config with MaxIfThenShortWidth = 20 } + { config with MaxIfThenShortWidth = 25 } |> prepend newline |> should equal diff --git a/src/Fantomas.Core.Tests/MaxIfThenShortWidthTests.fs b/src/Fantomas.Core.Tests/MaxIfThenShortWidthTests.fs index cd32a09ea2..5679c0caad 100644 --- a/src/Fantomas.Core.Tests/MaxIfThenShortWidthTests.fs +++ b/src/Fantomas.Core.Tests/MaxIfThenShortWidthTests.fs @@ -27,7 +27,7 @@ let ``keep entire expression in one line`` () = """ if a then () """ - { config with MaxIfThenShortWidth = 3 } + { config with MaxIfThenShortWidth = 12 } |> prepend newline |> should equal @@ -53,3 +53,44 @@ if // comment makes expr multiline then b """ + +[] +let ``apply same rules for nested if/then/else without else expr`` () = + formatSourceString + false + """ +if a then b +elif c then d +elif e then f +""" + config + |> prepend newline + |> should + equal + """ +if a then + b +elif c then + d +elif e then + f +""" + +[] +let ``apply same rules for nested if/then/else without else expr, MaxIfThenShortWidth = 15`` () = + formatSourceString + false + """ +if a then b +elif c then d +elif e then f +""" + { config with MaxIfThenShortWidth = 15 } + |> prepend newline + |> should + equal + """ +if a then b +elif c then d +elif e then f +""" diff --git a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnArrayOrListTests.fs b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnArrayOrListTests.fs index 5531e4dcef..ff4c3010bf 100644 --- a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnArrayOrListTests.fs +++ b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnArrayOrListTests.fs @@ -105,7 +105,7 @@ let ``array comprehensions`` () = """ let a1 = [| 0 .. 99 |] let a2 = [| for n in 1 .. 100 do if isPrime n then yield n |]""" - { config with MaxIfThenShortWidth = 20 } + { config with MaxIfThenShortWidth = 25 } |> prepend newline |> should equal diff --git a/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs b/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs index 162fc622bd..f64263bf67 100644 --- a/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs +++ b/src/Fantomas.Core.Tests/MultilineFunctionApplicationsInConditionExpressionsTests.fs @@ -124,12 +124,11 @@ let c = let c = if bar - |> Seq.exists - ( - (|KeyValue|) - >> snd - >> (=) (Some i) - ) + |> Seq.exists ( + (|KeyValue|) + >> snd + >> (=) (Some i) + ) then false else @@ -288,12 +287,11 @@ let c = true elif bar - |> Seq.exists - ( - (|KeyValue|) - >> snd - >> (=) (Some i) - ) + |> Seq.exists ( + (|KeyValue|) + >> snd + >> (=) (Some i) + ) then false else diff --git a/src/Fantomas.Core.Tests/TupleTests.fs b/src/Fantomas.Core.Tests/TupleTests.fs index 98e0b04eaf..12f6b4a260 100644 --- a/src/Fantomas.Core.Tests/TupleTests.fs +++ b/src/Fantomas.Core.Tests/TupleTests.fs @@ -260,8 +260,10 @@ let y = "" elif args.StartsWith("(") then args - elif v.CurriedParameterGroups.Count > 1 - && (not verboseMode) then + elif + v.CurriedParameterGroups.Count > 1 + && (not verboseMode) + then " " + args else sprintf "(%s)" args), diff --git a/src/Fantomas.Core/AstTransformer.fs b/src/Fantomas.Core/AstTransformer.fs index e6c96f2965..d809d8f48f 100644 --- a/src/Fantomas.Core/AstTransformer.fs +++ b/src/Fantomas.Core/AstTransformer.fs @@ -502,25 +502,29 @@ and visitSynExpr (synExpr: SynExpr) : TriviaNode list = processSequence finalContinuation continuations (fun nodes -> mkSynExprNode SynExpr_SequentialOrImplicitYield synExpr range (sortChildren [| yield! nodes |])) - | SourceParser.ElIf ((_leadingElseKw, ifKw, isElif, ifExpr, thenKw, thenExpr) :: es, (elseKw, elseExpr), range) -> + | SourceParser.ElIf ((_leadingElseKw, ifKw, isElif, ifExpr, thenKw, thenExpr) :: es, elseInfo, range) -> let elifs = es |> List.collect (fun (_, _, _, e1, _, e2) -> [ visit e1; visit e2 ]) + let elseExprNodes, elseKeywordNodes = + match elseInfo with + | None -> [], [] + | Some (elseKw, elseExpr) -> [ visit elseExpr ], [ mkNode SynExpr_IfThenElse_Else elseKw ] + let continuations: ((TriviaNode list -> TriviaNode list) -> TriviaNode list) list = [ yield visit ifExpr yield visit thenExpr yield! elifs - yield! (Option.toList elseExpr |> List.map visit) ] + yield! elseExprNodes ] processSequence finalContinuation continuations (fun nodes -> - let elseNode elseKw = - mkNodeOption SynExpr_IfThenElse_Else elseKw - let elifKeywords = es |> List.collect (fun (elseKw, ifKw, isElif, _, thenKw, _) -> - [ yield! Option.toList (elseNode elseKw) + [ yield! + (mkNodeOption SynExpr_IfThenElse_Else elseKw + |> Option.toList) yield mkNode (if isElif then @@ -544,7 +548,7 @@ and visitSynExpr (synExpr: SynExpr) : TriviaNode list = ifKw yield mkNode SynExpr_IfThenElse_Then thenKw yield! elifKeywords - yield! Option.toList (elseNode elseKw) + yield! elseKeywordNodes yield! nodes |])) | SynExpr.IfThenElse (ifExpr, thenExpr, elseExpr, _, _, range, trivia) -> diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index 0e302930d6..8d1b186104 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -1954,169 +1954,164 @@ and genExpr astContext synExpr ctx = let items = List.collect (collectMultilineItemForSynExpr astContext) es atCurrentColumn (colWithNlnWhenItemIsMultilineUsingConfig items) - | IfThenWithoutElse (ifKw, ifExpr, thenKw, thenExpr) -> - leadingExpressionIsMultiline (genIfThen astContext ifKw ifExpr thenKw) (fun isMultiline ctx -> - if isMultiline then - (indent - +> sepNln - +> genExpr astContext thenExpr - +> unindent) - ctx - else - isShortExpression - ctx.Config.MaxIfThenShortWidth - (sepSpace +> genExpr astContext thenExpr) + // if condExpr then thenExpr + | ElIf ([ None, ifKw, false, ifExpr, thenKw, thenExpr ], None, _) -> + leadingExpressionResult + (genIfThen astContext ifKw ifExpr thenKw) + (fun ((lineCountBefore, columnBefore), (lineCountAfter, columnAfter)) ctx -> + // Check if the `if expr then` is already multiline or cross the max_line_length. + let isMultiline = + lineCountAfter > lineCountBefore + || columnAfter > ctx.Config.MaxLineLength + + if isMultiline then (indent +> sepNln +> genExpr astContext thenExpr +> unindent) - ctx) + ctx + else + // Check if the entire expression is will still fit on one line, respecting MaxIfThenShortWidth + let remainingMaxLength = + ctx.Config.MaxIfThenShortWidth + - (columnAfter - columnBefore) + + isShortExpression + remainingMaxLength + (sepSpace +> genExpr astContext thenExpr) + (indent + +> sepNln + +> genExpr astContext thenExpr + +> unindent) + ctx) |> atCurrentColumnIndent - // A generalization of IfThenElse - | ElIf ((_, ifKw, isElif, e1, thenKw, e2) :: es, (elseKw, elseOpt), _) -> - // https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-if-expressions - let hasElfis = not (List.isEmpty es) - let hasElse = Option.isSome elseOpt - - let genIf ifKeywordRange isElif = - (ifElse isElif (!- "elif ") (!- "if ") - |> genTriviaFor - (if isElif then - SynExpr_IfThenElse_Elif - else - SynExpr_IfThenElse_If) - ifKeywordRange) - +> sepSpace + // if condExpr then thenExpr else elseExpr + | ElIf ([ None, ifKw, false, ifExpr, thenKw, thenExpr ], Some (elseKw, elseExpr), _) -> + let genElse = genTriviaFor SynExpr_IfThenElse_Else elseKw !- "else" - let genThen thenRange = - !- "then " - |> genTriviaFor SynExpr_IfThenElse_Then thenRange + leadingExpressionResult + (genIfThen astContext ifKw ifExpr thenKw) + (fun ((lineCountBefore, columnBefore), (lineCountAfter, columnAfter)) ctx -> + let long = + indent + +> sepNln + +> genExpr astContext thenExpr + +> unindent + +> sepNln + +> genElse + +> indent + +> sepNln + +> genExpr astContext elseExpr + +> unindent - let genElse elseRange = - !- "else " - |> genTriviaFor SynExpr_IfThenElse_Else elseRange + // Check if the `if expr then` is already multiline or cross the max_line_length. + let isMultiline = + lineCountAfter > lineCountBefore + || columnAfter > ctx.Config.MaxLineLength - let genElifOneliner (elseKw, ifKw, isElif, e1, thenKw, e2) = - optSingle genElse elseKw - +> sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace - +> genIf ifKw isElif - +> sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace - +> genExpr astContext e1 - +> sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace - +> genThen thenKw - +> sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace - +> genExpr astContext e2 + // If the `thenExpr` is also an SynExpr.IfThenElse, it will not be valid code if put on one line. + // ex: if cond then if a then b else c else e2 + let thenExprIsIfThenElse = + match thenExpr with + | IfThenElse _ -> true + | _ -> false - let genElifMultiLine (elseKw, ifKw, isElif, e1, thenKw, e2) = - optSingle genElse elseKw - +> sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace - +> genIf ifKw isElif - +> autoIndentAndNlnWhenWriteBeforeNewlineNotEmpty (genExprInIfOrMatch astContext e1) - +> sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace - +> genThen thenKw - +> indent - +> sepNln - +> genExpr astContext e2 - +> unindent + if isMultiline || thenExprIsIfThenElse then + long ctx + else + // Check if the entire expression is will still fit on one line, respecting MaxIfThenShortWidth + let remainingMaxLength = + ctx.Config.MaxIfThenElseShortWidth + - (columnAfter - columnBefore) + + isShortExpression + remainingMaxLength + (sepSpace + +> genExpr astContext thenExpr + +> sepSpace + +> genElse + +> sepSpace + +> genExpr astContext elseExpr) + long + ctx) + |> atCurrentColumnIndent - let genShortElse e elseRange = - optSingle - (fun e -> - sepSpace - +> optSingle genElse elseRange - +> genExpr astContext e) - e + // At least one `elif` or `else if` is present + // Optional else branch + | ElIf (branches, elseInfo, _) -> + // multiple branches but no else expr + // use the same threshold check as for if-then + // Everything should fit on one line + let areAllShort ctx = + let anyThenExprIsIfThenElse = + branches + |> List.exists (fun (_, _, _, _, _, thenExpr) -> + match thenExpr with + | IfThenElse _ -> true + | _ -> false) - let genOneliner elseOpt = - genIf ifKw isElif - +> genExpr astContext e1 - +> sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace - +> genThen thenKw - +> genExpr astContext e2 - +> genShortElse elseOpt elseKw + let checkIfLine (elseKwOpt, ifKw, isElif, condExpr, thenKw, thenExpr) = + genIfOrElseIfOrElifThen astContext elseKwOpt ifKw isElif condExpr thenKw + +> sepSpace + +> genExpr astContext thenExpr + + let linesToCheck = + match elseInfo with + | None -> List.map checkIfLine branches + | Some (elseKw, elseExpr) -> + // This may appear a bit odd that we are adding the `else elseExpr` before the `if expr then expr` lines but purely for this check this doesn't matter. + // Each lines needs to fit on one line in order for us to format the short way + (genTriviaFor SynExpr_IfThenElse_Else elseKw !- "else" + +> sepSpace + +> genExpr astContext elseExpr) + :: (List.map checkIfLine branches) + + let lineCheck () = + linesToCheck + |> List.forall (fun lineCheck -> + let maxWidth = + if elseInfo.IsSome then + ctx.Config.MaxIfThenElseShortWidth + else + ctx.Config.MaxIfThenShortWidth - let isIfThenElse = - function - | SynExpr.IfThenElse _ -> true - | _ -> false + not (exceedsWidth maxWidth lineCheck ctx)) - let longIfThenElse = - genIf ifKw isElif - // f.ex. if // meh - // x - // bool expr x should be indented - +> autoIndentAndNlnWhenWriteBeforeNewlineNotEmpty ( - genExprInIfOrMatch astContext e1 - +> sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace - ) - +> genThen thenKw - +> indent - +> sepNln - +> genExpr astContext e2 - +> unindent - +> onlyIf (hasElfis || hasElse) sepNln - +> col sepNln es genElifMultiLine - +> opt id elseOpt (fun e4 -> - onlyIf (List.isNotEmpty es) sepNln - +> optSingle genElse elseKw + not anyThenExprIsIfThenElse && lineCheck () + + let shortExpr = + col sepNln branches (fun (elseKwOpt, ifKw, isElif, condExpr, thenKw, thenExpr) -> + genIfOrElseIfOrElifThen astContext elseKwOpt ifKw isElif condExpr thenKw + +> sepSpace + +> genExpr astContext thenExpr) + +> optSingle + (fun (elseKw, elseExpr) -> + sepNln + +> genTriviaFor SynExpr_IfThenElse_Else elseKw !- "else" + +> sepSpace + +> genExpr astContext elseExpr) + elseInfo + + let longExpr = + col sepNln branches (fun (elseKwOpt, ifKw, isElif, condExpr, thenKw, thenExpr) -> + genIfOrElseIfOrElifThen astContext elseKwOpt ifKw isElif condExpr thenKw +> indent +> sepNln - +> genExpr astContext e4 + +> genExpr astContext thenExpr +> unindent) + +> optSingle + (fun (elseKw, elseExpr) -> + sepNln + +> genTriviaFor SynExpr_IfThenElse_Else elseKw !- "else" + +> indent + +> sepNln + +> genExpr astContext elseExpr + +> unindent) + elseInfo - let shortIfThenElif (ctx: Context) = - // Try and format if each conditional follow the one-liner rules - // Abort if something is too long - let shortCtx, isShort = - let exprs = - [ yield genOneliner None - yield! (List.map genElifOneliner es) - yield! - (Option.map (fun _ -> genShortElse elseOpt elseKw) elseOpt - |> Option.toList) ] - - let lastIndex = List.length exprs - 1 - - exprs - |> List.indexed - |> List.fold - (fun (acc, allLinesShort) (idx, expr) -> - if allLinesShort then - let lastLine, lastColumn = acc.WriterModel.Lines.Length, acc.Column - - let nextCtx = expr acc - - let currentLine, currentColumn = nextCtx.WriterModel.Lines.Length, nextCtx.Column - - let isStillShort = - lastLine = currentLine - && (currentColumn - lastColumn - <= acc.Config.MaxIfThenElseShortWidth) - - (ifElse (lastIndex > idx) sepNln sepNone nextCtx, isStillShort) - else - ctx, false) - (ctx, true) - - if isShort then - shortCtx - else - longIfThenElse ctx - - let expr = - if hasElfis && not (isIfThenElse e2) then - shortIfThenElif - elif isIfThenElse e2 then - // If branch expression is an if/then/else expressions. - // Always go with long version in this case - longIfThenElse - else - let shortExpression = genOneliner elseOpt - let longExpression = longIfThenElse - (fun ctx -> isShortExpression ctx.Config.MaxIfThenElseShortWidth shortExpression longExpression ctx) - - atCurrentColumnIndent expr + ifElseCtx areAllShort shortExpr longExpr + |> atCurrentColumnIndent | IdentExpr ident -> genIdent ident @@ -3250,38 +3245,68 @@ and genAppWithLambda astContext sep (e, es, lpr, lambda, rpr, pr) = expressionFitsOnRestOfLine short long -and genControlExpressionStart +and genControlExpressionStartCore astContext - (startKeywordType: FsAstType) - (startKeywordRange: range) - (startKeywordText: string) - (innerExpr: SynExpr) - (endKeywordType: FsAstType) - (endKeywordRange: range) - (endKeywordText: string) + enterStartKeyword + genStartKeyword + leaveStartKeyword + innerExpr + enterEndKeyword + genEndKeyword + leaveEndKeyword = - let genStartKeyword = - genTriviaFor startKeywordType startKeywordRange !-startKeywordText - - let genEndKeyword = genTriviaFor endKeywordType endKeywordRange !-endKeywordText - let shortIfExpr = genStartKeyword - +> sepSpace + +> leaveStartKeyword + +> sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace +> genExpr astContext innerExpr +> sepSpace + +> enterEndKeyword +> genEndKeyword let longIfExpr = genStartKeyword + +> leaveStartKeyword +> indent +> sepNln +> genExpr astContext innerExpr +> unindent +> sepNln + +> enterEndKeyword +> genEndKeyword - expressionFitsOnRestOfLine shortIfExpr longIfExpr + // A code comment before the start keyword should not make the expression long. + enterStartKeyword + +> expressionFitsOnRestOfLine shortIfExpr longIfExpr + +> leaveEndKeyword + +and genControlExpressionStart + astContext + (startKeywordType: FsAstType) + (startKeywordRange: range) + (startKeywordText: string) + (innerExpr: SynExpr) + (endKeywordType: FsAstType) + (endKeywordRange: range) + (endKeywordText: string) + = + let enterStartKeyword = enterNodeFor startKeywordType startKeywordRange + let genStartKeyword = !-startKeywordText + let leaveStartKeyword = leaveNodeFor startKeywordType startKeywordRange + + let enterEndKeyword = enterNodeFor endKeywordType endKeywordRange + let genEndKeyword = !-endKeywordText + let leaveEndKeyword = leaveNodeFor endKeywordType endKeywordRange + + genControlExpressionStartCore + astContext + enterStartKeyword + genStartKeyword + leaveStartKeyword + innerExpr + enterEndKeyword + genEndKeyword + leaveEndKeyword and genIfThen astContext (ifKeyword: range) (ifExpr: SynExpr) (thenKeyword: range) = genControlExpressionStart @@ -3294,6 +3319,44 @@ and genIfThen astContext (ifKeyword: range) (ifExpr: SynExpr) (thenKeyword: rang thenKeyword "then" +and genIfOrElseIfOrElifThen + astContext + (elseKwOpt: range option) + (ifKw: range) + (isElif: bool) + (condExpr: SynExpr) + (thenKw: range) + = + let enterStartKeyword, genStartKeyword, leaveStartKeyword = + match elseKwOpt with + | Some elseKw -> + enterNodeFor SynExpr_IfThenElse_Else elseKw, + !- "else" + +> leaveNodeFor SynExpr_IfThenElse_Else elseKw + +> sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace + +> enterNodeFor SynExpr_IfThenElse_If ifKw + +> !- "if", + leaveNodeFor SynExpr_IfThenElse_If ifKw + | None -> + if isElif then + enterNodeFor SynExpr_IfThenElse_Elif ifKw, !- "elif", leaveNodeFor SynExpr_IfThenElse_Elif ifKw + else + enterNodeFor SynExpr_IfThenElse_If ifKw, !- "if", leaveNodeFor SynExpr_IfThenElse_If ifKw + + let enterEndKeyword = enterNodeFor SynExpr_IfThenElse_Then thenKw + let genEndKeyword = !- "then" + let leaveEndKeyword = leaveNodeFor SynExpr_IfThenElse_Then thenKw + + genControlExpressionStartCore + astContext + enterStartKeyword + genStartKeyword + leaveStartKeyword + condExpr + enterEndKeyword + genEndKeyword + leaveEndKeyword + and genMatchWith astContext (matchKeyword: range) (matchExpr: SynExpr) (withKeyword: range) = genControlExpressionStart astContext @@ -3316,7 +3379,7 @@ and genMatchBangWith astContext (matchKeyword: range) (matchExpr: SynExpr) (with withKeyword "with" -// TODO: remove 😁 +// TODO: remove when revisiting KeepIndentInBranch and genExprInIfOrMatch astContext (e: SynExpr) (ctx: Context) : Context = let short = sepNlnWhenWriteBeforeNewlineNotEmpty sepSpace diff --git a/src/Fantomas.Core/Context.fs b/src/Fantomas.Core/Context.fs index 724b79c471..56b7495f68 100644 --- a/src/Fantomas.Core/Context.fs +++ b/src/Fantomas.Core/Context.fs @@ -1072,9 +1072,15 @@ let futureNlnCheck f (ctx: Context) = let exceedsWidth maxWidth f (ctx: Context) = let dummyCtx: Context = ctx.WithDummy(Queue.empty, keepPageWidth = true) + let currentLines = dummyCtx.WriterModel.Lines.Length let currentColumn = dummyCtx.Column let ctxAfter: Context = f dummyCtx - (ctxAfter.Column - currentColumn) > maxWidth + let linesAfter = ctxAfter.WriterModel.Lines.Length + let columnAfter = ctxAfter.Column + + linesAfter > currentLines + || (columnAfter - currentColumn) > maxWidth + || currentColumn > ctx.Config.MaxLineLength /// Similar to col, skip auto newline for index 0 let colAutoNlnSkip0i f' (c: seq<'T>) f (ctx: Context) = diff --git a/src/Fantomas.Core/SourceParser.fs b/src/Fantomas.Core/SourceParser.fs index 1ae7136462..d6882c75ad 100644 --- a/src/Fantomas.Core/SourceParser.fs +++ b/src/Fantomas.Core/SourceParser.fs @@ -1065,14 +1065,6 @@ let (|IfThenElse|_|) = | SynExpr.IfThenElse _ as e -> Some e | _ -> None -let (|IfThenWithoutElse|_|) = - function - | SynExpr.IfThenElse (ifExpr, thenExpr, None, _, _, _, trivia) -> - match ifExpr with - | IfThenElse _ -> None - | _ -> Some(trivia.IfKeyword, ifExpr, trivia.ThenKeyword, thenExpr) - | _ -> None - let rec (|ElIf|_|) = function | SynExpr.IfThenElse (e1, @@ -1091,7 +1083,12 @@ let rec (|ElIf|_|) = ) | SynExpr.IfThenElse (e1, e2, e3, _, _, range, trivia) -> - Some([ (None, trivia.IfKeyword, trivia.IsElif, e1, trivia.ThenKeyword, e2) ], (trivia.ElseKeyword, e3), range) + let elseInfo = + match trivia.ElseKeyword, e3 with + | Some elseKw, Some elseExpr -> Some(elseKw, elseExpr) + | _ -> None + + Some([ (None, trivia.IfKeyword, trivia.IsElif, e1, trivia.ThenKeyword, e2) ], elseInfo, range) | _ -> None let (|Record|_|) = @@ -1862,7 +1859,7 @@ let (|KeepIndentMatch|_|) (e: SynExpr) = let (|KeepIndentIfThenElse|_|) (e: SynExpr) = match e with - | ElIf (branches, (_, Some elseExpr), _) -> + | ElIf (branches, Some (_, elseExpr), _) -> let branchBodies = branches |> List.map (fun (_, _, _, e, _, _) -> e) if shouldNotIndentBranch elseExpr branchBodies then From 8ecca916466ca606b73042c113ec5955c9d7e0e3 Mon Sep 17 00:00:00 2001 From: nojaf Date: Thu, 7 Jul 2022 09:24:56 +0200 Subject: [PATCH 4/5] Add documentation for fsharp_max_if_then_short_width. --- docs-old/Documentation.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs-old/Documentation.md b/docs-old/Documentation.md index a1ee97e075..1f19df1035 100644 --- a/docs-old/Documentation.md +++ b/docs-old/Documentation.md @@ -122,6 +122,7 @@ fsharp_space_after_comma=true fsharp_space_before_semicolon=false fsharp_space_after_semicolon=true fsharp_space_around_delimiter=true +fsharp_max_if_then_short_width=0 fsharp_max_if_then_else_short_width=40 fsharp_max_infix_operator_expression=50 fsharp_max_record_width=40 @@ -442,6 +443,26 @@ let a = [1;2;3] let b = [|4;5;6|] ``` +### fsharp_max_if_then_short_width + +Control the maximum length for which if/then expression without an else expression can be on one line. +The [Microsoft F# style guide](https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-if-expressions) recommends to never write such an expression in one line. +> If the else expression is absent, it is recommended to never to write the entire expression in one line. +Default = 0. + +`defaultConfig` + +```fsharp +if a then + () +``` + +`{ defaultConfig with MaxIfThenShortWidth = 15 }` + +```fsharp +if a then () +``` + ### fsharp_max_if_then_else_short_width Fantomas by default follows the if/then/else conventions listed in the [Microsoft F# style guide](https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-if-expressions). From 41b8de4fc0a5c2590c25d7cded7b774e1c7afd5c Mon Sep 17 00:00:00 2001 From: nojaf Date: Thu, 7 Jul 2022 09:32:37 +0200 Subject: [PATCH 5/5] Add changelog entry. --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a65e113c4e..31121b7499 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ * Restore the CodeFormatter.MakeRange public API. [#2306](https://github.com/fsprojects/fantomas/pull/2306) * Update FCS to 'Add arrow to SynType.Fun trivia.', commit 5a5a5f6cd07aa4a8326baa07d4f7af1305ced6f4 * Update FCS to 'Fix setter first', commit 267d0a57f217df756d9ac33c6aa4ffbfe3b53097 +* Update style of long if/match expressions (See [fslang-design#646](https://github.com/fsharp/fslang-design/issues/646)). [#2334](https://github.com/fsprojects/fantomas/pull/2334) + +### Added +* Add setting `fsharp_max_if_then_short_width`. [#2299](https://github.com/fsprojects/fantomas/issues/2299) ## [5.0.0-alpha-010] - 2022-06-27