From 818c5a956be2fdaf6f4f756c0bd8e5277cd99f38 Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Fri, 22 Nov 2019 11:48:47 +0200 Subject: [PATCH] Fix: print line comment after arrow in lamda between paranthesis (#557) * WIP: refactor IfThenElse * WIP: refactor IfThenElse, more comments after keywords * WIP: refactor IfThenElse, comments everywhere * WIP: refactor IfThenElse, fix all tests * WIP: refactor IfThenElse, added MaxIfThenElseShortWidth setting * WIP: refactor IfThenElse, added MaxIfThenElseShortWidth setting to global tool * Updated Documentation.md with --maxIfThenElseShortWidth * Removed warnings introduced by IfThenElse refactor * merged with master --- docs/Documentation.md | 21 + src/Fantomas.CoreGlobalTool/Program.fs | 15 +- src/Fantomas.Tests/ActivePatternTests.fs | 13 +- src/Fantomas.Tests/CommentTests.fs | 43 +- src/Fantomas.Tests/ControlStructureTests.fs | 21 +- src/Fantomas.Tests/DynamicOperatorTests.fs | 3 +- src/Fantomas.Tests/Fantomas.Tests.fsproj | 1 + src/Fantomas.Tests/IfThenElseTests.fs | 786 ++++++++++++++++++ src/Fantomas.Tests/InterfaceTests.fs | 3 +- src/Fantomas.Tests/LambdaTests.fs | 10 + src/Fantomas.Tests/LetBindingTests.fs | 3 +- src/Fantomas.Tests/PipingTests.fs | 6 +- src/Fantomas.Tests/RecordTests.fs | 3 +- src/Fantomas.Tests/TokenParserTests.fs | 7 +- src/Fantomas.Tests/TriviaTests.fs | 24 +- src/Fantomas.Tests/TupleTests.fs | 3 +- src/Fantomas.Tests/TypeDeclarationTests.fs | 6 +- .../VerboseSyntaxConversionTests.fs | 3 +- src/Fantomas/CodePrinter.fs | 329 ++++++-- src/Fantomas/Context.fs | 42 +- src/Fantomas/Fantomas.fsproj | 1 + src/Fantomas/FormatConfig.fs | 22 +- src/Fantomas/RangeHelpers.fs | 18 + src/Fantomas/TokenParser.fs | 6 +- src/Fantomas/Trivia.fs | 12 + src/Fantomas/TriviaContext.fs | 59 ++ src/Fantomas/TriviaHelpers.fs | 26 +- src/Fantomas/Utils.fs | 2 + 28 files changed, 1329 insertions(+), 159 deletions(-) create mode 100644 src/Fantomas.Tests/IfThenElseTests.fs create mode 100644 src/Fantomas/RangeHelpers.fs diff --git a/docs/Documentation.md b/docs/Documentation.md index d47585eaa0..5648c83828 100644 --- a/docs/Documentation.md +++ b/docs/Documentation.md @@ -183,6 +183,27 @@ match meh with will remain the same, the newline after `->` was detected and preserved. + +- `--maxIfThenElseShortWidth `: `number` if being set, controls when if/then/else expressions will be formatted as single line or as multiple lines. + +Fantomas tries to follow [the F# style guide](https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-if-expressions) as close as possible when formatting if expressions. + +The style guide says: + +> If either cond, e1 or e2 are longer, but not multi-line: + +```fsharp +if cond +then e1 +else e2 +``` + +But what exactly is longer right? By default Fantomas will use 40 width to determine if the expression needs to be formatted to the example above or remain as a oneliner (`if cond then e1 else e2`). + + +So if either `cond`, `e1` or `e2` is longer than `maxIfThenElseShortWidth` but not multiline, it will format on three lines. +See [unit tests](https://github.com/fsprojects/fantomas/blob/9d4b499c09a1f06f5485835817844657cc51215b/src/Fantomas.Tests/IfThenElseTests.fs#L734) for more examples. + That said, most of the preferences are very simple. But they demonstrate the flexibility of Fantomas on a set of configurations. More preferences will be added depending on use cases. diff --git a/src/Fantomas.CoreGlobalTool/Program.fs b/src/Fantomas.CoreGlobalTool/Program.fs index 2567caef3f..1be8e53379 100644 --- a/src/Fantomas.CoreGlobalTool/Program.fs +++ b/src/Fantomas.CoreGlobalTool/Program.fs @@ -46,6 +46,7 @@ let [] indentOnTryWithText = "Enable indentation on try/with block (def let [] reorderOpenDeclarationText = "Enable reordering open declarations (default = false)." let [] spaceAroundDelimiterText = "Disable spaces after starting and before ending of lists, arrays, sequences and records (default = true)." let [] keepNewlineAfterText = "Keep newlines found after = in let bindings, -> in pattern matching and chained function calls (default = false)." +let [] maxIfThenElseShortWidthText = "Set the max length of any expression in an if expression before formatting on multiple lines (default = 40)." let [] strictModeText = "Enable strict mode (ignoring directives and comments and printing literals in canonical forms) (default = false)." let time f = @@ -129,6 +130,7 @@ let main _args = let spaceAroundDelimiter = ref FormatConfig.Default.SpaceAroundDelimiter let keepNewlineAfter = ref FormatConfig.Default.KeepNewlineAfter let strictMode = ref FormatConfig.Default.StrictMode + let maxIfThenElseShortWidth = ref FormatConfig.Default.MaxIfThenElseShortWidth let handleOutput s = if not !stdOut then @@ -163,6 +165,13 @@ let main _args = eprintfn "Page width should be at least 60." exit 1 + let handleMaxIfThenElseShortWidth i = + if i >= 0 then + maxIfThenElseShortWidth := i + else + eprintfn "Max IfThenElse short width should be greater than zero." + exit 1 + let fileToFile (inFile : string) (outFile : string) config = try printfn "Processing %s" inFile @@ -244,7 +253,8 @@ let main _args = ArgInfo("--reorderOpenDeclaration", ArgType.Set reorderOpenDeclaration, reorderOpenDeclarationText); ArgInfo("--noSpaceAroundDelimiter", ArgType.Clear spaceAroundDelimiter, spaceAroundDelimiterText); - ArgInfo("--keepNewlineAfter", ArgType.Set keepNewlineAfter, keepNewlineAfterText); + ArgInfo("--keepNewlineAfter", ArgType.Set keepNewlineAfter, keepNewlineAfterText) + ArgInfo("--maxIfThenElseShortWidth", ArgType.Int handleMaxIfThenElseShortWidth, maxIfThenElseShortWidthText); ArgInfo("--strictMode", ArgType.Set strictMode, strictModeText) |] ArgParser.Parse(options, handleInput, sprintf "Fantomas %sCheck out https://github.com/fsprojects/fantomas/blob/master/docs/Documentation.md#using-the-command-line-tool for more info." Environment.NewLine ) @@ -262,7 +272,8 @@ let main _args = ReorderOpenDeclaration = !reorderOpenDeclaration SpaceAroundDelimiter = !spaceAroundDelimiter StrictMode = !strictMode - KeepNewlineAfter = !keepNewlineAfter } + KeepNewlineAfter = !keepNewlineAfter + MaxIfThenElseShortWidth = !maxIfThenElseShortWidth } // Handle inputs via pipeline let isKeyAvailable = ref false diff --git a/src/Fantomas.Tests/ActivePatternTests.fs b/src/Fantomas.Tests/ActivePatternTests.fs index df10262000..bcf21376d5 100644 --- a/src/Fantomas.Tests/ActivePatternTests.fs +++ b/src/Fantomas.Tests/ActivePatternTests.fs @@ -47,18 +47,15 @@ let (|ParseRegex|_|) regex str = |> prepend newline |> should equal """ let (|Even|Odd|) input = - if input % 2 = 0 then Even - else Odd + if input % 2 = 0 then Even else Odd let (|Integer|_|) (str: string) = let mutable intvalue = 0 - if System.Int32.TryParse(str, &intvalue) then Some(intvalue) - else None + if System.Int32.TryParse(str, &intvalue) then Some(intvalue) else None let (|ParseRegex|_|) regex str = let m = Regex(regex).Match(str) - if m.Success then - Some(List.tail [ for x in m.Groups -> x.Value ]) - else - None + if m.Success + then Some(List.tail [ for x in m.Groups -> x.Value ]) + else None """ diff --git a/src/Fantomas.Tests/CommentTests.fs b/src/Fantomas.Tests/CommentTests.fs index e2cd8beab0..215d144c5a 100644 --- a/src/Fantomas.Tests/CommentTests.fs +++ b/src/Fantomas.Tests/CommentTests.fs @@ -326,12 +326,15 @@ else """ config |> prepend newline |> should equal """ -if a then () +if a then + () else - // Comment 1 - if b then () - // Comment 2 - else () +// Comment 1 +if b then + () +// Comment 2 +else + () """ [] @@ -643,12 +646,13 @@ type substring = /// An integer that indicates the lexical relationship between the two comparands. static member CompareOrdinal(strA: substring, strB: substring) = // If both substrings are empty they are considered equal, regardless of their offset or underlying string. - if strA.Length = 0 && strB.Length = 0 then 0 - elif + if strA.Length = 0 && strB.Length = 0 then + 0 - // OPTIMIZATION : If the substrings have the same (identical) underlying string - // and offset, the comparison value will depend only on the length of the substrings. - strA.String == strB.String && strA.Offset = strB.Offset then compare strA.Length strB.Length + // OPTIMIZATION : If the substrings have the same (identical) underlying string + // and offset, the comparison value will depend only on the length of the substrings. + elif strA.String == strB.String && strA.Offset = strB.Offset then + compare strA.Length strB.Length else (* Structural comparison on substrings -- this uses the same comparison @@ -675,8 +679,10 @@ if true then //comment else 0""" config |> prepend newline |> should equal """ -if true then 1 //comment -else 0 +if true then //comment + 1 +else + 0 """ [] @@ -687,8 +693,11 @@ if //comment else 0""" config |> prepend newline |> should equal """ -if true then 1 //comment -else 0 +if //comment + true then + 1 +else + 0 """ [] @@ -699,6 +708,8 @@ else //comment 0""" config |> prepend newline |> should equal """ -if true then 1 -else 0 //comment +if true then + 1 +else //comment + 0 """ \ No newline at end of file diff --git a/src/Fantomas.Tests/ControlStructureTests.fs b/src/Fantomas.Tests/ControlStructureTests.fs index 5514b9d664..11e8348e2d 100644 --- a/src/Fantomas.Tests/ControlStructureTests.fs +++ b/src/Fantomas.Tests/ControlStructureTests.fs @@ -27,8 +27,7 @@ then printfn "You are only %d years old and already learning F#? Wow!" age""" co let rec tryFindMatch pred list = match list with | head :: tail -> - if pred (head) then Some(head) - else tryFindMatch pred tail + if pred (head) then Some(head) else tryFindMatch pred tail | [] -> None let test x y = @@ -138,8 +137,7 @@ let ``try/with and finally``() = let function1 x y = try try - if x = y then raise (InnerError("inner")) - else raise (OuterError("outer")) + if x = y then raise (InnerError("inner")) else raise (OuterError("outer")) with | Failure _ -> () | InnerError(str) -> printfn "Error1 %s" str @@ -222,9 +220,10 @@ let x = let x = if try true - with Failure _ -> false - then () - else () + with Failure _ -> false then + () + else + () """ [] @@ -297,8 +296,7 @@ let ``multiline if in tuple``() = """ config |> prepend newline |> should equal """ -((if true then 1 - else 2), 3) +((if true then 1 else 2), 3) """ // https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-if-expressions @@ -435,9 +433,8 @@ let a ex = if null = ex then fooo() None - elif - // this was None - ex.GetType() = typeof then + // this was None + elif ex.GetType() = typeof then Some ex else None diff --git a/src/Fantomas.Tests/DynamicOperatorTests.fs b/src/Fantomas.Tests/DynamicOperatorTests.fs index d62f9b9518..669e2db442 100644 --- a/src/Fantomas.Tests/DynamicOperatorTests.fs +++ b/src/Fantomas.Tests/DynamicOperatorTests.fs @@ -28,7 +28,8 @@ let ``keep () when dynamic operator inside boolean expr, #476`` () = |> prepend newline |> should equal """ let fieldColor (fieldNameX: string) = - if f.errors?(fieldNameY) && f.touched?(fieldNameZ) then IsDanger + if f.errors?(fieldNameY) && f.touched?(fieldNameZ) + then IsDanger else NoColor |> Input.Color """ \ No newline at end of file diff --git a/src/Fantomas.Tests/Fantomas.Tests.fsproj b/src/Fantomas.Tests/Fantomas.Tests.fsproj index 8aea943c4c..97d849e433 100644 --- a/src/Fantomas.Tests/Fantomas.Tests.fsproj +++ b/src/Fantomas.Tests/Fantomas.Tests.fsproj @@ -56,6 +56,7 @@ + diff --git a/src/Fantomas.Tests/IfThenElseTests.fs b/src/Fantomas.Tests/IfThenElseTests.fs new file mode 100644 index 0000000000..248a5eea6c --- /dev/null +++ b/src/Fantomas.Tests/IfThenElseTests.fs @@ -0,0 +1,786 @@ +module Fantomas.Tests.IfThenElseTests + +open NUnit.Framework +open FsUnit +open Fantomas.Tests.TestHelper + +[] +let ``single line if without else`` () = + formatSourceString false "if foo then bar" config + |> prepend newline + |> should equal """ +if foo then bar +""" + +[] +let ``single line if/then/else`` () = + formatSourceString false "if a then b else c" config + |> prepend newline + |> should equal """ +if a then b else c +""" + +[] +let ``single line if/then/elif/then/else`` () = + formatSourceString false "if a then b elif c then d else e" config + |> prepend newline + |> should equal """ +if a then b +elif c then d +else e +""" + +[] +let ``single line if/then/else if/then/else`` () = + formatSourceString false "if a then b else if c then d else e" config + |> prepend newline + |> should equal """ +if a then b +else if c then d +else e +""" + +[] +let ``single line if/then/else if/elif/then/else`` () = + formatSourceString false "if a then b else if c then d elif e then f else g" config + |> prepend newline + |> should equal """ +if a then b +else if c then d +elif e then f +else g +""" + +[] +let ``longer condition, not multi-line`` () = + formatSourceString false """if aaaaaaaaaBBBBBBBBBBccccccccccDDDDDDDDDeeeeeeeeeeeeeFFFFFFFFFFFggggggggg then 1 else 0 +""" ({ config with PageWidth = 80 }) + |> prepend newline + |> should equal """ +if aaaaaaaaaBBBBBBBBBBccccccccccDDDDDDDDDeeeeeeeeeeeeeFFFFFFFFFFFggggggggg +then 1 +else 0 +""" + +[] +let ``longer ifBranch, not multi-line`` () = + formatSourceString false """if x then aaaaaaaaaBBBBBBBBBBccccccccccDDDDDDDDDeeeeeeeeeeeeeFFFFFFFFFFFggggggggg else 0 +""" ({ config with PageWidth = 80 }) + |> prepend newline + |> should equal """ +if x +then aaaaaaaaaBBBBBBBBBBccccccccccDDDDDDDDDeeeeeeeeeeeeeFFFFFFFFFFFggggggggg +else 0 +""" + +[] +let ``longer else branch, not multi-line`` () = + formatSourceString false """if x then 1 else aaaaaaaaaBBBBBBBBBBccccccccccDDDDDDDDDeeeeeeeeeeeeeFFFFFFFFFFFggggggggg +""" ({ config with PageWidth = 80 }) + |> prepend newline + |> should equal """ +if x +then 1 +else aaaaaaaaaBBBBBBBBBBccccccccccDDDDDDDDDeeeeeeeeeeeeeFFFFFFFFFFFggggggggg +""" + +[] +let ``longer if else branch, not multi-line`` () = + formatSourceString false """if aaaaaaaaaaaa then bbbbbbbbbbbb else if cccccccccccc then ddddddddddd else eeeeeee +""" ({ config with PageWidth = 80 }) + |> prepend newline + |> should equal """ +if aaaaaaaaaaaa then bbbbbbbbbbbb +else if cccccccccccc then ddddddddddd +else eeeeeee +""" + +[] +let ``longer if else branch, longer elif branch, not multi-line`` () = + formatSourceString false """if aaaaaa then bbbbbb else if ccccccc then ddddddd elif eeeee then ffffff else gggggg +""" ({ config with PageWidth = 80 }) + |> prepend newline + |> should equal """ +if aaaaaa then bbbbbb +else if ccccccc then ddddddd +elif eeeee then ffffff +else gggggg +""" + +[] +let ``multiline condition`` () = + formatSourceString false """if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa && bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb) then + x else y +""" ({ config with PageWidth = 80 }) + |> prepend newline + |> should equal """ +if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + && bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb) then + x +else + y +""" + +[] +let ``multiline if branch`` () = + formatSourceString false """if a then + let x = 2 + x + 2 +else y +""" ({ config with PageWidth = 80 }) + |> prepend newline + |> should equal """ +if a then + let x = 2 + x + 2 +else + y +""" + +[] +let ``multiline else branch`` () = + formatSourceString false """if a then + x +else + let y = 7; + y + 9 +""" ({ config with PageWidth = 80 }) + |> prepend newline + |> should equal """ +if a then + x +else + let y = 7 + y + 9 +""" + +[] +let ``multiline else if branch`` () = + formatSourceString false """if a then + x else if b then + let y = 7; + y + 9 + else + 99 +""" ({ config with PageWidth = 80 }) + |> prepend newline + |> should equal """ +if a then + x +else if b then + let y = 7 + y + 9 +else + 99 +""" + +[] +let ``multiline else if branch, multiline elif branch`` () = + formatSourceString false """if a then + x else if b then + let y = 7; + y + 9 + elif c then + let z = 8 + z - 7 + else + 99 +""" ({ config with PageWidth = 80 }) + |> prepend newline + |> should equal """ +if a then + x +else if b then + let y = 7 + y + 9 +elif c then + let z = 8 + z - 7 +else + 99 +""" + +[] +let ``comment after if`` () = + formatSourceString false """if // meh + x then 0 else 1 +""" config + |> prepend newline + |> should equal """ +if // meh + x then + 0 +else + 1 +""" + +[] +let ``comment after if branch`` () = + formatSourceString false """if x // meh + then 0 else 1 +""" config + |> prepend newline + |> should equal """ +if x // meh +then + 0 +else + 1 +""" + +[] +let ``comment after if branch then keyword`` () = + formatSourceString false """if x then // meh + 0 else 1 +""" config + |> prepend newline + |> should equal """ +if x then // meh + 0 +else + 1 +""" + +[] +let ``comment after if branch expression`` () = + formatSourceString false """if x then + 0 // meh + else 1 +""" config + |> prepend newline + |> should equal """ +if x then 0 // meh +else 1 +""" + +[] +let ``comment after else keyword`` () = + formatSourceString false """if x then 0 else // meh + 1 +""" config + |> prepend newline + |> should equal """ +if x then + 0 +else // meh + 1 +""" + + +[] +let ``comment after else branch expression`` () = + formatSourceString false """if x then 0 else 1 // meh +""" config + |> prepend newline + |> should equal """ +if x then 0 else 1 // meh +""" + +[] +let ``comment after else keyword before if keyword`` () = + formatSourceString false """if a then b else // meh + if c then d else e +""" config + |> prepend newline + |> should equal """ +if a then + b +else // meh +if c then + d +else + e +""" + +[] +let ``comment after else if keyword`` () = + formatSourceString false """if a then b else if // meh + c then d else e +""" config + |> prepend newline + |> should equal """ +if a then + b +else if // meh + c then + d +else + e +""" + +[] +let ``comment after elif keyword`` () = + formatSourceString false """if a then b elif // meh + c then d else e +""" config + |> prepend newline + |> should equal """ +if a then + b +elif // meh + c then + d +else + e +""" + +[] +let ``comment after else if boolean expression`` () = + formatSourceString false """if a then b else if + c // meh + then d else e +""" config + |> prepend newline + |> should equal """ +if a then + b +else if c // meh +then + d +else + e +""" + +[] +let ``comment after elif boolean expression`` () = + formatSourceString false """if a then b elif + c // meh + then d else e +""" config + |> prepend newline + |> should equal """ +if a then + b +elif c // meh +then + d +else + e +""" + +[] +let ``comment after else if then keyword`` () = + formatSourceString false """if a then b else if + c then // meh + d else e +""" config + |> prepend newline + |> should equal """ +if a then + b +else if c then // meh + d +else + e +""" + +[] +let ``comment after elif then keyword`` () = + formatSourceString false """if a then b elif + c then // meh + d else e +""" config + |> prepend newline + |> should equal """ +if a then + b +elif c then // meh + d +else + e +""" + +[] +let ``comment after else if branch expression`` () = + formatSourceString false """if a then b else if + c then + d // meh + else e +""" config + |> prepend newline + |> should equal """ +if a then b +else if c then d // meh +else e +""" + +[] +let ``comment after multi line else branch expression`` () = + formatSourceString false """ +if a then b +else if c then d +else + e // meh + f +""" config + |> prepend newline + |> should equal """ +if a then + b +else if c then + d +else + e // meh + f +""" + +[] +let ``comment after multi line elif branch expression`` () = + formatSourceString false """ +if a then b +elif c then + d + e // meh +else + f +""" config + |> prepend newline + |> should equal """ +if a then + b +elif c then + d + e // meh +else + f +""" + +[] +let ``comment after multi line else if branch expression`` () = + formatSourceString false """ +if a then b +else if c then + d + e // meh +else + f +""" config + |> prepend newline + |> should equal """ +if a then + b +else if c then + d + e // meh +else + f +""" + +[] +let ``comment after else & if keyword`` () = + formatSourceString false """ +if a then b +else // foo +if // bar + c then d else e +""" config + |> prepend newline + |> should equal """ +if a then + b +else // foo +if // bar + c then + d +else + e +""" + +[] +let ``block comment if keyword`` () = + formatSourceString false """ +if (* meh *) a then b +else c +""" config + |> prepend newline + |> should equal """ +if (* meh *) a then b else c +""" + +[] +let ``block comment if bool expr`` () = + formatSourceString false """ +if a (* meh *) then b +else c +""" config + |> prepend newline + |> should equal """ +if a (* meh *) then b else c +""" + +[] +let ``block comment then keyword`` () = + formatSourceString false """ +if a then (* meh *) b +else c +""" config + |> prepend newline + |> should equal """ +if a then (* meh *) b else c +""" + +[] +let ``block comment if branch expr`` () = + formatSourceString false """ +if a then b (* meh *) +else c +""" config + |> prepend newline + |> should equal """ +if a then b (* meh *) else c +""" + +[] +let ``block comment else keyword`` () = + formatSourceString false """ +if a then b +else (* meh *) c +""" config + |> prepend newline + |> should equal """ +if a then b else (* meh *) c +""" + +[] +let ``block comment else branch expr`` () = + formatSourceString false """ +if a then b +else c (* meh *) +""" config + |> prepend newline + |> should equal """ +if a then b else c (* meh *) +""" + +[] +let ``block comment between else and if keyword`` () = + formatSourceString false """ +if a then b +else (* meh *) if c then d +else e +""" config + |> prepend newline + |> should equal """ +if a then b +else (* meh *) if c then d +else e +""" + +[] +let ``block comment after else if keyword`` () = + formatSourceString false """ +if a then b +else if (* meh *) c then d +else e +""" config + |> prepend newline + |> should equal """ +if a then b +else if (* meh *) c then d +else e +""" + +[] +let ``block comment after elif keyword`` () = + formatSourceString false """ +if a then b +elif (* meh *) c then d +else e +""" config + |> prepend newline + |> should equal """ +if a then b +elif (* meh *) c then d +else e +""" + +[] +let ``block comment after elif branch expr`` () = + formatSourceString false """ +if a then b +elif c (* meh *) then d +else e +""" config + |> prepend newline + |> should equal """ +if a then b +elif c (* meh *) then d +else e +""" + +[] +let ``block comment after else if branch expr`` () = + formatSourceString false """ +if a then b +else if c (* meh *) then d +else e +""" config + |> prepend newline + |> should equal """ +if a then b +else if c (* meh *) then d +else e +""" + +[] +let ``line comment after all fragments of IfThenElse expr`` () = + formatSourceString false """ +if // c1 + a // c2 +then // c3 + b // c4 +else // c5 +if // c6 + c // c7 + then // c8 + d // c9 +else // c10 + e // c11 +""" config + |> prepend newline + |> should equal """ +if // c1 + a // c2 + then // c3 + b // c4 +else // c5 +if // c6 + c // c7 + then // c8 + d // c9 +else // c10 + e // c11 +""" + +[] +let ``newlines before comment on elif`` () = + formatSourceString false """ +if strA.Length = 0 && strB.Length = 0 then 0 + +// OPTIMIZATION : If the substrings have the same (identical) underlying string +// and offset, the comparison value will depend only on the length of the substrings. +elif strA.String == strB.String && strA.Offset = strB.Offset then + compare strA.Length strB.Length + +else + -1 +""" config + |> prepend newline + |> should equal """ +if strA.Length = 0 && strB.Length = 0 then + 0 + +// OPTIMIZATION : If the substrings have the same (identical) underlying string +// and offset, the comparison value will depend only on the length of the substrings. +elif strA.String == strB.String && strA.Offset = strB.Offset then + compare strA.Length strB.Length + +else + -1 +""" + +[] +let ``simple if/else with long identifiers`` () = + formatSourceString false """ +if someveryveryveryverylongexpression then + someveryveryveryveryveryverylongexpression +else someveryveryveryverylongexpression +""" ({ config with PageWidth = 80 }) + |> prepend newline + |> should equal """ +if someveryveryveryverylongexpression +then someveryveryveryveryveryverylongexpression +else someveryveryveryverylongexpression +""" + +[] +let ``longer if branch, nothing multiline`` () = + formatSourceString false """ + if m.Success then Some (List.tail [ for x in m.Groups -> x.Value ]) else None +""" config + |> prepend newline + |> should equal """ +if m.Success +then Some(List.tail [ for x in m.Groups -> x.Value ]) +else None +""" + +[] +let ``almost longer if branch where the whole if/else is indented by letbinding`` () = + formatSourceString false """ +let (|Integer|_|) (str: string) = + let mutable intvalue = 0 + if System.Int32.TryParse(str, &intvalue) then Some(intvalue) + else None +""" config + |> prepend newline + |> should equal """ +let (|Integer|_|) (str: string) = + let mutable intvalue = 0 + if System.Int32.TryParse(str, &intvalue) then Some(intvalue) else None +""" + +[] +let ``longer elif condition`` () = + formatSourceString false """if a then b elif somethingABitLongerToForceDifferentStyle then c else d +""" config + |> prepend newline + |> should equal """ +if a then b +elif somethingABitLongerToForceDifferentStyle then c +else d +""" + +[] +let ``impact of MaxIfThenElseShortWidth setting, longer bool expression`` () = + let source = """if (tare + netWeight) = 10000 then a else b""" + + formatSourceString false source config + |> prepend newline + |> should equal """ +if (tare + netWeight) = 10000 then a else b +""" + + formatSourceString false source ({ config with MaxIfThenElseShortWidth = 20}) + |> prepend newline + |> should equal """ +if (tare + netWeight) = 10000 +then a +else b +""" + +[] +let ``impact of MaxIfThenElseShortWidth setting, longer if branch`` () = + let source = """if a then (tare + netWeight) + 10000 else 0""" + + formatSourceString false source config + |> prepend newline + |> should equal """ +if a then (tare + netWeight) + 10000 else 0 +""" + + formatSourceString false source ({ config with MaxIfThenElseShortWidth = 20}) + |> prepend newline + |> should equal """ +if a +then (tare + netWeight) + 10000 +else 0 +""" + +[] +let ``impact of MaxIfThenElseShortWidth setting, longer else branch`` () = + let source = """if a then 0 else (tare + netWeight) + 10000""" + + formatSourceString false source config + |> prepend newline + |> should equal """ +if a then 0 else (tare + netWeight) + 10000 +""" + + formatSourceString false source ({ config with MaxIfThenElseShortWidth = 20}) + |> prepend newline + |> should equal """ +if a +then 0 +else (tare + netWeight) + 10000 +""" \ No newline at end of file diff --git a/src/Fantomas.Tests/InterfaceTests.fs b/src/Fantomas.Tests/InterfaceTests.fs index 65ca8eff9f..1bb92f10af 100644 --- a/src/Fantomas.Tests/InterfaceTests.fs +++ b/src/Fantomas.Tests/InterfaceTests.fs @@ -164,8 +164,7 @@ type MyLogInteface() = member x.Print msg = printfn "%s" msg override x.GetLogFile environment = - if environment = "DEV" then "dev.log" - else sprintf "date-%s.log" environment + if environment = "DEV" then "dev.log" else sprintf "date-%s.log" environment member x.Info() = () override x.Version() = () diff --git a/src/Fantomas.Tests/LambdaTests.fs b/src/Fantomas.Tests/LambdaTests.fs index b32216ca79..36b9125635 100644 --- a/src/Fantomas.Tests/LambdaTests.fs +++ b/src/Fantomas.Tests/LambdaTests.fs @@ -5,6 +5,16 @@ open FsUnit open Fantomas.Tests.TestHelper [] +let ``keep comment after arrow`` () = + formatSourceString false """_Target "FSharpTypesDotNet" (fun _ -> // obsolete + ()) +""" ({ config with IndentSpaceNum = 2; PageWidth = 90 }) + |> prepend newline + |> should equal """ +_Target "FSharpTypesDotNet" (fun _ -> // obsolete + ()) +""" + let ``indent multiline lambda in parenthesis, 523`` () = formatSourceString false """let square = (fun b -> b*b diff --git a/src/Fantomas.Tests/LetBindingTests.fs b/src/Fantomas.Tests/LetBindingTests.fs index 70eab37654..c713691209 100644 --- a/src/Fantomas.Tests/LetBindingTests.fs +++ b/src/Fantomas.Tests/LetBindingTests.fs @@ -54,8 +54,7 @@ let f () = formatSourceString false codeSnippet config |> should equal """let f() = let x = 1 - if true then x - else x + if true then x else x """ [] diff --git a/src/Fantomas.Tests/PipingTests.fs b/src/Fantomas.Tests/PipingTests.fs index f33e828680..70c8d40ea6 100644 --- a/src/Fantomas.Tests/PipingTests.fs +++ b/src/Fantomas.Tests/PipingTests.fs @@ -20,10 +20,12 @@ let f x = someveryveryveryverylongexpression <|> if someveryveryveryverylongexpression then someveryveryveryverylongexpression - else someveryveryveryverylongexpression + else + someveryveryveryverylongexpression <|> if someveryveryveryverylongexpression then someveryveryveryverylongexpression - else someveryveryveryverylongexpression + else + someveryveryveryverylongexpression |> f """ diff --git a/src/Fantomas.Tests/RecordTests.fs b/src/Fantomas.Tests/RecordTests.fs index f4a2545858..44a964da0f 100644 --- a/src/Fantomas.Tests/RecordTests.fs +++ b/src/Fantomas.Tests/RecordTests.fs @@ -153,8 +153,7 @@ let ``should not break inside of if statements in records``() = TimeOut = TimeSpan.FromMinutes 5.; Package = null; Version = - if not isLocalBuild then buildVersion - else "0.1.0.0"; + if not isLocalBuild then buildVersion else "0.1.0.0"; OutputPath = "./xpkg"; Project = null; Summary = null; diff --git a/src/Fantomas.Tests/TokenParserTests.fs b/src/Fantomas.Tests/TokenParserTests.fs index 9da41076a3..cba669422b 100644 --- a/src/Fantomas.Tests/TokenParserTests.fs +++ b/src/Fantomas.Tests/TokenParserTests.fs @@ -277,7 +277,10 @@ elif true then ()""" let triviaNodes = getTriviaFromTokens tokens lineCount match triviaNodes with - | [{Item = Keyword({ Content = "if"})}; {Item = Keyword({ Content = "elif" })}] -> + | [{Item = Keyword({ Content = "if"})} + {Item = Keyword({ Content = "then"})} + {Item = Keyword({ Content = "elif" })} + {Item = Keyword({ Content = "then"})}] -> pass() | _ -> fail() @@ -392,4 +395,4 @@ let ``ident between tickets `` () = match triviaNodes with | [{ Item = IdentBetweenTicks("``/ operator combines paths``") }] -> pass() - | _ -> fail() \ No newline at end of file + | _ -> fail() diff --git a/src/Fantomas.Tests/TriviaTests.fs b/src/Fantomas.Tests/TriviaTests.fs index 169219ce8c..b6162987e5 100644 --- a/src/Fantomas.Tests/TriviaTests.fs +++ b/src/Fantomas.Tests/TriviaTests.fs @@ -327,8 +327,10 @@ elif true then ()""" |> List.head match triviaNodes with - | [{ Type = Token {Content = "if"}; ContentBefore = [Keyword({Content = "if"})] } - { Type = MainNode("SynExpr.IfThenElse"); ContentBefore = [Keyword({Content = "elif"})]}] -> + | [{ Type = Token {Content = "if"}; ContentItself = Some(Keyword({Content = "if"})) } + { Type = Token {Content = "then"}; ContentItself = Some(Keyword({Content = "then"})) } + { Type = Token {Content = "elif"}; ContentItself = Some(Keyword({Content = "elif"})) } + { Type = Token {Content = "then"}; ContentItself = Some(Keyword({Content = "then"})) }] -> pass() | _ -> fail() @@ -449,4 +451,20 @@ let foo = 42 match trivia with | [{ Type = MainNode("SynModuleOrNamespace.AnonModule") ContentBefore = [ Directive("#if SOMETHING"); Newline; Directive("#endif") ] }] -> pass() - | _ -> fail() \ No newline at end of file + | _ -> fail() + + +[] +let ``if keyword should be keyword itself`` () = + let source = "if meh then ()" + let trivia = + toTrivia source + |> List.head + + match trivia with + | [{ ContentItself = Some(Keyword({ TokenInfo = { TokenName = "IF" } })) + Type = TriviaNodeType.Token({ TokenInfo = { TokenName = "IF" } }) } + { ContentItself = Some(Keyword({ TokenInfo = { TokenName = "THEN" } })) + Type = TriviaNodeType.Token({ TokenInfo = { TokenName = "THEN" } }) }] -> + pass() + | _ -> fail() diff --git a/src/Fantomas.Tests/TupleTests.fs b/src/Fantomas.Tests/TupleTests.fs index 797bf35100..b5b38ba8d2 100644 --- a/src/Fantomas.Tests/TupleTests.fs +++ b/src/Fantomas.Tests/TupleTests.fs @@ -23,6 +23,5 @@ let ``multiline item in tuple - paren on its line`` () = else 2) """ config |> should equal """(x, - (if true then 1 - else 2)) + (if true then 1 else 2)) """ \ No newline at end of file diff --git a/src/Fantomas.Tests/TypeDeclarationTests.fs b/src/Fantomas.Tests/TypeDeclarationTests.fs index eed3cb4fba..46b534280a 100644 --- a/src/Fantomas.Tests/TypeDeclarationTests.fs +++ b/src/Fantomas.Tests/TypeDeclarationTests.fs @@ -335,8 +335,7 @@ type SpeedingTicket() = let CalculateFine(ticket: SpeedingTicket) = let delta = ticket.GetMPHOver(limit = 55, speed = 70) - if delta < 20 then 50.0 - else 100.0 + if delta < 20 then 50.0 else 100.0 """ [] @@ -612,7 +611,8 @@ type BlobHelper(Account: CloudStorageAccount) = new(configurationSettingName, hostedService) = CloudStorageAccount.SetConfigurationSettingPublisher(fun configName configSettingPublisher -> let connectionString = - if hostedService then RoleEnvironment.GetConfigurationSettingValue(configName) + if hostedService + then RoleEnvironment.GetConfigurationSettingValue(configName) else ConfigurationManager.ConnectionStrings.[configName].ConnectionString configSettingPublisher.Invoke(connectionString) |> ignore) BlobHelper(CloudStorageAccount.FromConfigurationSetting(configurationSettingName)) diff --git a/src/Fantomas.Tests/VerboseSyntaxConversionTests.fs b/src/Fantomas.Tests/VerboseSyntaxConversionTests.fs index 4a8ab47313..781a99b0cf 100644 --- a/src/Fantomas.Tests/VerboseSyntaxConversionTests.fs +++ b/src/Fantomas.Tests/VerboseSyntaxConversionTests.fs @@ -24,6 +24,5 @@ let div2 = 2 let f x = let r = x % div2 - if r = 1 then ("Odd") - else ("Even") + if r = 1 then ("Odd") else ("Even") """ diff --git a/src/Fantomas/CodePrinter.fs b/src/Fantomas/CodePrinter.fs index 0c3802dcf5..e55a49bc77 100644 --- a/src/Fantomas/CodePrinter.fs +++ b/src/Fantomas/CodePrinter.fs @@ -883,7 +883,8 @@ and genExpr astContext synExpr = | JoinIn(e1, e2) -> genExpr astContext e1 -- " in " +> genExpr astContext e2 | Paren(DesugaredLambda(cps, e)) -> let genLamba f = - sepOpenT -- "fun " +> col sepSpace cps (genComplexPats astContext) +> sepArrow + sepOpenT -- "fun " +> col sepSpace cps (genComplexPats astContext) + +> triviaAfterArrow synExpr.Range +> f astContext e +> sepCloseT ifElseCtx @@ -1112,77 +1113,261 @@ and genExpr astContext synExpr = atCurrentColumn (!- "if " +> ifElse (checkBreakForExpr e1) (genExpr astContext e1 ++ "then") (genExpr astContext e1 +- "then") -- " " +> preserveBreakNln astContext e2) // A generalization of IfThenElse - | ElIf((e1,e2, _, fullRange, _)::es, enOpt) -> - let printIfKeyword separator fallback range (ctx:Context) = - ctx.Trivia - |> List.tryFind (fun {Range = r} -> r = range) - |> Option.bind (fun tv -> - tv.ContentBefore - |> List.map (function | Keyword kw -> Some kw | _ -> None) - |> List.choose id - |> List.tryHead + | ElIf((e1,e2, _, _, _)::es, enOpt) -> + // https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-if-expressions + fun ctx -> + let elseKeywordRange = + ctx.Trivia + |> TriviaHelpers.``keyword tokens inside range`` ["IF"; "THEN"; "ELIF"; "ELSE"] synExpr.Range + |> fun tokens -> + // skip if .. then and take first else keyword + // ignore if third keyword is elif f.ex. + match tokens with + | ({ TokenName = "IF" },_)::({ TokenName = "THEN" }, _)::({ TokenName = "ELSE" }, et)::_ -> + Some et.Range + | _ -> None + + let elfis = + let lastEsIndex = (List.length es) - 1 + List.indexed es + |> List.map (fun (idx, (elf1, elf2, _, fullRange, _)) -> + if idx = lastEsIndex then + // In some scenarios the last else keyword of the 'else if' combination is not included in inner IfThenElse syn expr + // f.ex if a then b else // meh + // if c then d else e + let correctedRange = + match elseKeywordRange with + | Some er -> mkRange "else if" er.Start fullRange.End + | None -> fullRange + (elf1, elf2, correctedRange) + else + (elf1, elf2, fullRange) ) - |> Option.map (fun ({Content = kw}) -> sprintf "%s " kw |> separator) - |> Option.defaultValue (separator fallback) - <| ctx - - let ifTokenKw r f s = tokN r "IF" (printIfKeyword f s r) - let ifToken r f = tokN r "IF" f - let thenToken r f = tokN r "THEN" f - let elseToken r f = tokN r "ELSE" f - let anyExpressIsMultiline = - multiline e2 || (Option.map multiline enOpt |> Option.defaultValue false) || (List.exists (fun (_, e, _, _, _) -> multiline e) es) + let hasElfis = not (List.isEmpty elfis) - let printBranch prefix astContext expr = prefix +> ifElse anyExpressIsMultiline (breakNln astContext true expr) (preserveBreakNln astContext expr) - - // track how many indents was called, so we can correctly unindent. - // TODO: do it without mutable - let mutable indented = 0 - - atCurrentColumn ( - ifToken fullRange !-"if " +> ifElse (checkBreakForExpr e1) (genExpr astContext e1 +> thenToken fullRange !+"then") (genExpr astContext e1 +> thenToken fullRange !+-"then") -- " " - +> printBranch id astContext e2 - +> fun ctx -> colPost (rep indented unindent) sepNone es (fun (e1, e2, _, fullRangeInner, node) -> - let rangeBeforeInner = mkRange "" fullRange.Start fullRangeInner.Start - let elsePart = - ifTokenKw fullRangeInner (fun kw ctx -> - let hasContentBeforeIf = - ctx.Trivia - |> List.tryFind (fun tv -> tv.Range = fullRangeInner) - |> Option.map (fun tv -> - tv.ContentBefore - |> List.exists (fun cb -> - match cb with - | Comment(_) -> true - | _ -> false - ) - ) - |> Option.defaultValue false - - // Trivia knows if the keyword is "elif" or "else if" - // Next we need to be sure that the are no comments between else and if - match kw with - | "if " when hasContentBeforeIf -> - indented <- indented + 1 - (elseToken rangeBeforeInner !+"else" +> indent +> sepNln +> genTrivia fullRangeInner (ifToken node.Range !-"if ")) - | "if " -> - (elseToken rangeBeforeInner !+"else if ") - | _ (* "elif" *) -> - !+ kw - <| ctx - ) "if " - - elsePart +> - indent +> (genTrivia node.Range ( - (ifElse (checkBreakForExpr e1) - (genExpr astContext e1 +> thenToken node.Range !+"then") - (genExpr astContext e1 +> thenToken node.Range !+-"then") - -- " " +> unindent +> printBranch id astContext e2) - )) - ) ctx - +> opt sepNone enOpt (fun en -> printBranch (elseToken fullRange !+~"else ") astContext en) - ) + let commentAfterKeyword keyword rangePredicate (ctx: Context) = + ctx.Trivia + |> TriviaHelpers.``has content after after that matches`` + (fun t -> + let ttt = TriviaHelpers.``is token of type`` keyword t + let rrr = rangePredicate t.Range + ttt && rrr) + (function | Comment(LineCommentAfterSourceCode(_)) -> true | _ -> false) + + let hasCommentAfterBoolExpr = + TriviaHelpers.``has content after after that matches`` + (fun tn -> tn.Range = e1.Range) + (function | Comment(LineCommentAfterSourceCode(_)) -> true | _ -> false) + ctx.Trivia + + let hasCommentAfterIfKeyword = + commentAfterKeyword "IF" (RangeHelpers.``have same range start`` synExpr.Range) ctx + + let ``has line comment after source code for range`` range = + TriviaHelpers.``has content after after that matches`` + (fun tn -> tn.Range = range) + (function | Comment(LineCommentAfterSourceCode(_)) -> true | _ -> false) + ctx.Trivia + + let hasCommentAfterIfBranchExpr = ``has line comment after source code for range`` e2.Range + + let hasCommentAfterIfBranchThenKeyword = + commentAfterKeyword "THEN" (RangeHelpers.``range contains`` synExpr.Range) ctx + + let hasCommentAfterElseKeyword = + commentAfterKeyword "ELSE" (RangeHelpers.``range contains`` synExpr.Range) ctx + + let isConditionMultiline = + hasCommentAfterIfKeyword || + hasCommentAfterBoolExpr || + hasCommentAfterIfBranchThenKeyword || + futureNlnCheck (!- "if " +> genExpr astContext e1) ctx + + let isIfBranchMultiline = + futureNlnCheck (genExpr astContext e2) ctx + + let isElseBranchMultiline = + match enOpt with + | Some e3 -> + hasCommentAfterElseKeyword || + futureNlnCheck (!- " else " +> genExpr astContext e3) ctx + | None -> false + + let genIf ifElseRange = tokN ifElseRange "IF" (!- "if ") + let genThen ifElseRange = tokN ifElseRange "THEN" (!- "then ") + let genElse ifElseRange = tokN ifElseRange "ELSE" (!- "else ") + + let genElifOneliner ((elf1: SynExpr), (elf2: SynExpr), fullRange) = + let hasCommentAfterBoolExpr = + TriviaHelpers.``has content after after that matches`` + (fun tn -> tn.Range = elf1.Range) + (function | Comment(LineCommentAfterSourceCode(_)) -> true | _ -> false) + ctx.Trivia + let hasCommentAfterThenKeyword = + commentAfterKeyword "THEN" (RangeHelpers.``range contains`` fullRange) ctx + + TriviaContext.``else if / elif`` fullRange + +> genExpr astContext elf1 +> sepSpace + +> ifElse hasCommentAfterBoolExpr sepNln sepNone + +> genThen fullRange + +> ifElse hasCommentAfterThenKeyword sepNln sepNone + +> genExpr astContext elf2 + |> genTrivia fullRange + + let genElifTwoLiner ((elf1: SynExpr), (elf2: SynExpr), fullRange) = + let hasCommentAfterThenKeyword = + commentAfterKeyword "THEN" (RangeHelpers.``range contains`` fullRange) ctx + + TriviaContext.``else if / elif`` fullRange + +> genExpr astContext elf1 +> sepNln + +> genThen fullRange + +> ifElse hasCommentAfterThenKeyword sepNln sepNone + +> genExpr astContext elf2 + |> genTrivia fullRange + + let isAnyElifBranchMultiline = + elfis + |> List.exists (fun elf -> futureNlnCheck (genElifOneliner elf) ctx) + + let anyElifBranchHasCommentAfterBranchExpr = + elfis + |> List.exists (fun (_, e, _) -> ``has line comment after source code for range`` e.Range) + + let isAnyExpressionIsLongerButNotMultiline = + let longerSetting = ctx.Config.MaxIfThenElseShortWidth + let elseExceedsWith = + match enOpt with + | Some e4 -> exceedsWidth longerSetting (genExpr astContext e4) ctx + | None -> false + + exceedsWidth longerSetting (genExpr astContext e1) ctx || + exceedsWidth longerSetting (genExpr astContext e2) ctx || + elseExceedsWith + + let isAnyExpressionIsMultiline = isConditionMultiline || isIfBranchMultiline || isElseBranchMultiline || isAnyElifBranchMultiline + + let genElifMultiLine ((elf1: SynExpr), elf2, fullRange) (ctx: Context) = + let indentAfterThenKeyword = + ctx.Trivia + |> TriviaHelpers.``keyword tokens inside range`` ["IF"; "ELIF"] fullRange + |> List.tryHead + |> Option.map (fun (_, t) -> + if TriviaHelpers.``has line comment after`` t then + // don't indent because comment after if/elif keyword + // TriviaContext.``else if / elif`` fullRange will already placed indentation + sepNone + else + indent) + |> Option.defaultValue indent + + let hasCommentAfterBoolExpr = + TriviaHelpers.``has content after after that matches`` + (fun tn -> tn.Range = elf1.Range) + (function | Comment(LineCommentAfterSourceCode(_)) -> true | _ -> false) + ctx.Trivia + + let elifExpr = + TriviaContext.``else if / elif`` fullRange + +> genExpr astContext elf1 + +> ifElse hasCommentAfterBoolExpr sepNln sepSpace + +> genThen fullRange + +> indentAfterThenKeyword +> sepNln + +> genExpr astContext elf2 + +> unindent + + (elifExpr |> genTrivia fullRange) ctx + + let genOneliner = + genIf synExpr.Range +> genExpr astContext e1 +> sepSpace + +> genThen synExpr.Range +> genExpr astContext e2 + +> col sepNone elfis (fun elf -> sepSpace +> genElifOneliner elf) + +> opt id enOpt (fun e4 -> (sepSpace +> genElse synExpr.Range +> genExpr astContext e4)) + + // This is a simplistic check to see if everything fits on one line + let isOneLiner = + not hasElfis && + not isAnyExpressionIsLongerButNotMultiline && + not isAnyExpressionIsMultiline && + not hasCommentAfterIfBranchExpr && + not anyElifBranchHasCommentAfterBranchExpr && + not (futureNlnCheck genOneliner ctx) + + let formatIfElseExpr = + if isOneLiner then + // Indentation of conditionals depends on the sizes of the expressions that make them up. If cond, e1 and e2 are short, simply write them on one line: + // if cond then e1 else e2 + genOneliner + + elif not isOneLiner && not isAnyExpressionIsMultiline + && isAnyExpressionIsLongerButNotMultiline then + // If either cond, e1 or e2 are longer, but not multi-line: + // if cond + // then e1 + // else e2 + + genIf synExpr.Range +> genExpr astContext e1 +> sepNln + +> genThen synExpr.Range +> genExpr astContext e2 +> sepNln + +> colPost sepNln sepNln elfis genElifTwoLiner + +> opt id enOpt (fun e4 -> genElse synExpr.Range +> genExpr astContext e4) + + elif hasElfis && not isAnyExpressionIsMultiline then + // Multiple conditionals with elif and else are indented at the same scope as the if: + // if cond1 then e1 + // elif cond2 then e2 + // elif cond3 then e3 + // else e4 + + genIf synExpr.Range +> genExpr astContext e1 +> sepSpace + +> genThen synExpr.Range +> genExpr astContext e2 +> sepNln + +> colPost sepNln sepNln elfis genElifOneliner + +> opt id enOpt (fun e4 -> genElse synExpr.Range +> genExpr astContext e4) + + else if hasCommentAfterIfBranchExpr && not hasElfis then + // f.ex + // if x then 0 // meh + // else 1 + genIf synExpr.Range +> genExpr astContext e1 +> sepSpace + +> genThen synExpr.Range +> genExpr astContext e2 +> sepNln + +> opt id enOpt (fun e4 -> genElse synExpr.Range +> genExpr astContext e4) + + else + // If any of the expressions are multi-line: + // if cond then + // e1 + // else + // e2 + + genIf synExpr.Range + // f.ex. if // meh + // x + // bool expr x should be indented + +> ifElse hasCommentAfterIfKeyword (indent +> sepNln) sepNone + +> genExpr astContext e1 + +> ifElse hasCommentAfterBoolExpr sepNln sepSpace + +> genThen synExpr.Range + // f.ex if x then // meh + // 0 + // 0 should be indented + +> ifElse (hasCommentAfterIfBranchThenKeyword && not hasCommentAfterIfKeyword) indent sepNone + // f.ex. if x // + // then + // 0 + // 0 should be indented + +> ifElse (hasCommentAfterBoolExpr && not hasCommentAfterIfKeyword) indent sepNone + // normal scenario + // f.ex. if (longCondition + // && onMultipleLines) then + // x + +> ifElse (not hasCommentAfterIfKeyword && not hasCommentAfterBoolExpr && not hasCommentAfterIfBranchThenKeyword) indent sepNone + +> sepNln + +> genExpr astContext e2 +> unindent +> sepNln + +> colPost sepNln sepNln elfis genElifMultiLine + +> opt id enOpt (fun e4 -> genElse synExpr.Range +> + indent +> sepNln +> genExpr astContext e4 +> + unindent) + + (atCurrentColumn formatIfElseExpr) ctx // At this stage, all symbolic operators have been handled. | OptVar(s, isOpt) -> ifElse isOpt (!- "?") sepNone -- s @@ -2082,12 +2267,6 @@ and genConstBytes (bytes: byte []) (r: range) = and genTrivia (range: range) f = enterNode range +> f +> leaveNode range -and tok (range: range) (s: string) = - enterNodeToken range +> (!-s) +> leaveNodeToken range - -and tokN (range: range) (tokenName: string) f = - enterNodeTokenByName range tokenName +> f +> leaveNodeTokenByName range tokenName - and infixOperatorFromTrivia range fallback (ctx: Context) = ctx.Trivia |> List.choose(fun t -> diff --git a/src/Fantomas/Context.fs b/src/Fantomas/Context.fs index 1a801ba635..71a98374a9 100644 --- a/src/Fantomas/Context.fs +++ b/src/Fantomas/Context.fs @@ -2,6 +2,7 @@ module Fantomas.Context open System open FSharp.Compiler.Range +open Fantomas open Fantomas.FormatConfig open Fantomas.TriviaTypes @@ -134,12 +135,14 @@ let internal dump (ctx: Context) = m.Lines |> List.rev |> String.concat Environment.NewLine let internal dumpAndContinue (ctx: Context) = - let code = dump ctx + let m = applyWriterEvents ctx + let lines = m.Lines |> List.rev + let code = String.concat Environment.NewLine lines #if DEBUG printfn "%s" code #endif ctx - + type Context with member x.Column = (applyWriterEvents x).Column member x.ApplyWriterEvents = applyWriterEvents x @@ -409,7 +412,7 @@ let internal autoNlnCheck (f: _ -> Context) sep (ctx : Context) = let internal futureNlnCheckMem = Cache.memoizeBy (fun (f, ctx : Context) -> Cache.LambdaEqByRef f, ctx.MemoizeProjection) <| fun (f, ctx) -> if ctx.WriterInitModel.IsDummy || not ctx.BreakLines then (false, false) else // Create a dummy context to evaluate length of current operation - let dummyCtx = ctx.WithDummy([], keepPageWidth = true) |> f + let dummyCtx : Context = ctx.WithDummy([], keepPageWidth = true) |> f WriterEvents.isMultiline dummyCtx.WriterEvents, dummyCtx.Column > ctx.Config.PageWidth let internal futureNlnCheck f (ctx : Context) = @@ -426,6 +429,14 @@ let internal autoIndentNlnByFuture f = ifElseCtx (futureNlnCheck f) (indent +> s /// like autoNlnByFuture but don't do nln if there is another nln inside f let internal autoNlnByFutureLazy f = ifElseCtx (futureNlnCheckLazy f) (sepNln +> f) f +/// similar to futureNlnCheck but validates whether the expression is going over the max page width +/// This functions is does not use any caching +let internal exceedsWidth maxWidth f (ctx: Context) = + let dummyCtx : Context = ctx.WithDummy([], keepPageWidth = true) + let currentColumn = dummyCtx.Column + let ctxAfter : Context = f dummyCtx + (ctxAfter.Column - currentColumn) > maxWidth + /// Set a checkpoint to break at an appropriate column let internal autoNlnOrAddSep f sep (ctx : Context) = let isNln = autoNlnCheck f sep ctx @@ -493,17 +504,27 @@ let internal NewLineInfixOps = set ["|>"; "||>"; "|||>"; ">>"; ">>="] let internal NoBreakInfixOps = set ["="; ">"; "<";] let internal printTriviaContent (c: TriviaContent) (ctx: Context) = + let currentLastLine = + let m = applyWriterEvents ctx + m.Lines + |> List.tryHead + // Some items like #if of Newline should be printed on a newline // It is hard to always get this right in CodePrinter, so we detect it based on the current code. let addNewline = - dump ctx - |> String.normalizeThenSplitNewLine - |> Array.tryLast - |> Option.map (fun (line:string) -> line.Trim().Length > 1) + currentLastLine + |> Option.map(fun line -> line.Trim().Length > 1) + |> Option.defaultValue false + + let addSpace = + currentLastLine + |> Option.bind(fun line -> Seq.tryLast line |> Option.map (fun lastChar -> lastChar <> ' ')) |> Option.defaultValue false match c with - | Comment(LineCommentAfterSourceCode s) -> writerEvent (WriteBeforeNewline (" " + s)) + | Comment(LineCommentAfterSourceCode s) -> + let comment = sprintf "%s%s" (if addSpace then " " else String.empty) s + writerEvent (WriteBeforeNewline comment) | Comment(BlockComment(s, before, after)) -> ifElse (before && addNewline) sepNln sepNone +> sepSpace -- s +> sepSpace @@ -576,13 +597,12 @@ let private findTriviaTokenFromRange nodes (range:range) = nodes |> List.tryFind(fun n -> Trivia.isToken n && n.Range.Start = range.Start && n.Range.End = range.End) -let private findTriviaTokenFromName (range: range) nodes (tokenName:string) = +let internal findTriviaTokenFromName (range: range) nodes (tokenName:string) = nodes |> List.tryFind(fun n -> match n.Type with | Token(tn) when tn.TokenInfo.TokenName = tokenName -> - (range.Start.Line, range.Start.Column) <= (n.Range.Start.Line, n.Range.Start.Column) - && (range.End.Line, range.End.Column) >= (n.Range.End.Line, n.Range.End.Column) + RangeHelpers.``range contains`` range n.Range | _ -> false) let internal enterNodeWith f x (ctx: Context) = diff --git a/src/Fantomas/Fantomas.fsproj b/src/Fantomas/Fantomas.fsproj index 6ebdc73f59..de4b16852c 100644 --- a/src/Fantomas/Fantomas.fsproj +++ b/src/Fantomas/Fantomas.fsproj @@ -9,6 +9,7 @@ + diff --git a/src/Fantomas/FormatConfig.fs b/src/Fantomas/FormatConfig.fs index 350a292555..6962f43e19 100644 --- a/src/Fantomas/FormatConfig.fs +++ b/src/Fantomas/FormatConfig.fs @@ -11,19 +11,20 @@ type Num = int type FormatConfig = { /// Number of spaces for each indentation - IndentSpaceNum : Num; + IndentSpaceNum : Num /// The column where we break to new lines - PageWidth : Num; - SemicolonAtEndOfLine : bool; - SpaceBeforeArgument : bool; - SpaceBeforeColon : bool; - SpaceAfterComma : bool; - SpaceAfterSemicolon : bool; - IndentOnTryWith : bool; + PageWidth : Num + SemicolonAtEndOfLine : bool + SpaceBeforeArgument : bool + SpaceBeforeColon : bool + SpaceAfterComma : bool + SpaceAfterSemicolon : bool + IndentOnTryWith : bool /// Reordering and deduplicating open statements - ReorderOpenDeclaration : bool; - SpaceAroundDelimiter : bool; + ReorderOpenDeclaration : bool + SpaceAroundDelimiter : bool KeepNewlineAfter : bool + MaxIfThenElseShortWidth: Num /// Prettyprinting based on ASTs only StrictMode : bool } @@ -39,6 +40,7 @@ type FormatConfig = ReorderOpenDeclaration = false SpaceAroundDelimiter = true KeepNewlineAfter = false + MaxIfThenElseShortWidth = 40 StrictMode = false } static member create(indentSpaceNum, pageWith, semicolonAtEndOfLine, diff --git a/src/Fantomas/RangeHelpers.fs b/src/Fantomas/RangeHelpers.fs new file mode 100644 index 0000000000..e377a089bf --- /dev/null +++ b/src/Fantomas/RangeHelpers.fs @@ -0,0 +1,18 @@ +namespace Fantomas + +open FSharp.Compiler.Range + +[] +module RangeHelpers = + + /// Checks if range B is fully contained by range A + let ``range contains`` (a:range) (b:range) = + (a.Start.Line, a.Start.Column) <= (b.Start.Line, b.Start.Column) && + (a.End.Line, a.End.Column) >= (b.End.Line, b.End.Column) + + let ``have same range start`` (a:range) (b:range) = + a.StartLine = b.StartLine && a.StartColumn = b.StartColumn + + // check if b is after a + let ``range after`` (a: range) (b: range) = + (a.StartLine, a.StartColumn) < (b.StartLine, b.StartColumn) \ No newline at end of file diff --git a/src/Fantomas/TokenParser.fs b/src/Fantomas/TokenParser.fs index 00a35d7897..6300992d6a 100644 --- a/src/Fantomas/TokenParser.fs +++ b/src/Fantomas/TokenParser.fs @@ -202,7 +202,7 @@ let private getContentFromTokens tokens = |> List.map (fun t -> t.Content) |> String.concat String.Empty -let private keywordTrivia = ["IF"; "ELIF"; "OVERRIDE"; "MEMBER"; "DEFAULT"; "KEYWORD_STRING"; "QMARK"] +let private keywordTrivia = ["IF"; "ELIF"; "ELSE"; "THEN"; "OVERRIDE"; "MEMBER"; "DEFAULT"; "KEYWORD_STRING"; "QMARK"] let private numberTrivia = ["UINT8";"INT8";"UINT16";"INT16";"UINT32";"INT32";"UINT64";"IEEE32"; "DECIMAL";"IEEE64";"BIGNUM";"NATIVEINT";"UNATIVEINT"] @@ -316,7 +316,7 @@ let rec private getTriviaFromTokensThemSelves (config: FormatConfig) (allTokens: let directiveContent = directiveTokens |> List.map (fun t -> t.Content) - |> String.concat System.String.Empty + |> String.concat String.empty let range = getRangeBetween "directive" headToken (List.last directiveTokens) let info = @@ -437,7 +437,7 @@ let getTriviaFromTokens config (tokens: Token list) linesCount = fromTokens @ newLines |> List.sortBy (fun t -> t.Range.StartLine, t.Range.StartColumn) -let private tokenNames = ["LBRACE";"RBRACE"; "LPAREN";"RPAREN"; "LBRACK"; "RBRACK"; "BAR_LBRACK"; "BAR_RBRACK"; "EQUALS"; "IF"; "THEN"; "ELSE"; "BAR"; "RARROW"; "TRY"; "FINALLY"; "WITH"] +let private tokenNames = ["LBRACE";"RBRACE"; "LPAREN";"RPAREN"; "LBRACK"; "RBRACK"; "BAR_LBRACK"; "BAR_RBRACK"; "EQUALS"; "IF"; "THEN"; "ELSE"; "ELIF"; "BAR"; "RARROW"; "TRY"; "FINALLY"; "WITH"] let private tokenKinds = [FSharpTokenCharKind.Operator] let getTriviaNodesFromTokens (tokens: Token list) : TriviaNode list = diff --git a/src/Fantomas/Trivia.fs b/src/Fantomas/Trivia.fs index c057278e78..e49d6088a8 100644 --- a/src/Fantomas/Trivia.fs +++ b/src/Fantomas/Trivia.fs @@ -209,6 +209,13 @@ let private triviaBetweenAttributeAndLetBinding triviaNodes line = | Some (ai,a), Some (mdli,_) when (ai + 1 = mdli && a.Range.StartLine = a.Range.EndLine) -> Some a | _ -> None +let private findASTNodeOfTypeThatContains (nodes: TriviaNode list) typeName range = + nodes + |> List.filter (fun t -> + match t.Type with + | TriviaNodeType.MainNode(mnt) when (mnt = typeName) -> RangeHelpers.``range contains`` t.Range range + | _ -> false) + |> List.tryHead let private addTriviaToTriviaNode (triviaNodes: TriviaNode list) trivia = match trivia with @@ -264,6 +271,11 @@ let private addTriviaToTriviaNode (triviaNodes: TriviaNode list) trivia = findConstNodeAfter triviaNodes range |> updateTriviaNode (fun tn -> { tn with ContentBefore = List.appendItem tn.ContentBefore (Keyword(kw)) }) triviaNodes + | { Item = Keyword({ Content = keyword}); Range = range } when (keyword = "if" || keyword = "then" || keyword = "else" || keyword = "elif") -> + findNodeOnLineAndColumn triviaNodes range.StartLine range.StartColumn + |> Option.orElseWith(fun () -> findASTNodeOfTypeThatContains triviaNodes "SynExpr.IfThenElse" range) + |> updateTriviaNode (fun tn -> { tn with ContentItself = Some trivia.Item }) triviaNodes + | { Item = Keyword(keyword); Range = range } -> findNodeOnLineAndColumn triviaNodes range.StartLine range.StartColumn |> fun nodeOnLineAndColumn -> diff --git a/src/Fantomas/TriviaContext.fs b/src/Fantomas/TriviaContext.fs index 73f1a5c929..c09648f3d6 100644 --- a/src/Fantomas/TriviaContext.fs +++ b/src/Fantomas/TriviaContext.fs @@ -1,10 +1,17 @@ module internal Fantomas.TriviaContext open FSharp.Compiler.Ast +open FSharp.Compiler.Range open Fantomas open Fantomas.Context open Fantomas.TriviaTypes +let tok (range: range) (s: string) = + enterNodeToken range +> (!-s) +> leaveNodeToken range + +let tokN (range: range) (tokenName: string) f = + enterNodeTokenByName range tokenName +> f +> leaveNodeTokenByName range tokenName + let firstNewlineOrComment (es: SynExpr list) (ctx: Context) = es |> List.tryHead @@ -24,3 +31,55 @@ let firstNewlineOrComment (es: SynExpr list) (ctx: Context) = printTriviaContent head ctx' | _ -> sepNone ctx +let triviaAfterArrow (range: range) (ctx: Context) = + let hasCommentAfterArrow = + findTriviaTokenFromName range ctx.Trivia "RARROW" + |> Option.bind (fun t -> + t.ContentAfter + |> List.tryFind (function | Comment(LineCommentAfterSourceCode(_)) -> true | _ -> false) + ) + |> Option.isSome + ((tokN range "RARROW" sepArrow) +> ifElse hasCommentAfterArrow sepNln sepNone) ctx + +let ``else if / elif`` (rangeOfIfThenElse: range) (ctx: Context) = + let keywords = + ctx.Trivia + |> TriviaHelpers.``keyword tokens inside range`` ["ELSE";"IF";"ELIF"] rangeOfIfThenElse + |> List.map (fun (tok, t) -> (tok.TokenName, t)) + + let resultExpr = + match keywords with + | ("ELSE", elseTrivia)::("IF", ifTrivia)::_ -> + let commentAfterElseKeyword = TriviaHelpers.``has line comment after`` elseTrivia + let commentAfterIfKeyword = TriviaHelpers.``has line comment after`` ifTrivia + let triviaBeforeIfKeyword = + ctx.Trivia + |> List.filter (fun t -> + match t.Type with + | MainNode("SynExpr.IfThenElse") -> + RangeHelpers.``range contains`` rangeOfIfThenElse t.Range + && (RangeHelpers.``range after`` elseTrivia.Range t.Range) + | _ -> false + ) + |> List.tryHead + + tokN rangeOfIfThenElse "ELSE" (!- "else") +> + ifElse commentAfterElseKeyword sepNln sepSpace +> + opt sepNone triviaBeforeIfKeyword printContentBefore +> + tokN rangeOfIfThenElse "IF" (!- "if ") +> + ifElse commentAfterIfKeyword (indent +> sepNln) sepNone + + | ("ELIF",elifTok)::_ + | [("ELIF",elifTok)] -> + let commentAfterElIfKeyword = TriviaHelpers.``has line comment after`` elifTok + tokN rangeOfIfThenElse "ELIF" (!- "elif ") + +> ifElse commentAfterElIfKeyword (indent +> sepNln) sepNone + + | [] -> + // formatting from AST + !- "else if " + + | _ -> + failwith "Unexpected scenario when formatting else if / elif, please open an issue via https://jindraivanek.gitlab.io/fantomas-ui" + + resultExpr ctx \ No newline at end of file diff --git a/src/Fantomas/TriviaHelpers.fs b/src/Fantomas/TriviaHelpers.fs index 6982ac1845..9089e2eaaa 100644 --- a/src/Fantomas/TriviaHelpers.fs +++ b/src/Fantomas/TriviaHelpers.fs @@ -21,4 +21,28 @@ module TriviaHelpers = let internal ``has content after that ends with`` (findTrivia: TriviaNode -> bool) (contentAfterEnd: TriviaContent -> bool) (trivia: TriviaNode list) = List.tryFind findTrivia trivia |> Option.bind (fun t -> t.ContentAfter |> List.tryLast |> Option.map contentAfterEnd) - |> Option.defaultValue false \ No newline at end of file + |> Option.defaultValue false + + let internal ``is token of type`` tokenName (triviaNode: TriviaNode) = + match triviaNode.Type with + | Token({ TokenInfo = ti }) -> ti.TokenName = tokenName + | _ -> false + + let internal ``keyword tokens inside range`` keywords range (trivia: TriviaNode list) = + trivia + |> List.choose(fun t -> + match t.Type with + | TriviaNodeType.Token({ TokenInfo = { TokenName = tn } as tok }) + when ( RangeHelpers.``range contains`` range t.Range && List.contains tn keywords) -> + Some (tok, t) + | _ -> None + ) + + let internal ``has line comment after`` triviaNode = + triviaNode.ContentAfter + |> List.filter(fun tn -> + match tn with + | Comment(LineCommentAfterSourceCode(_)) -> true + | _ -> false + ) + |> (List.isEmpty >> not) \ No newline at end of file diff --git a/src/Fantomas/Utils.fs b/src/Fantomas/Utils.fs index eb37ac3946..d80d05ebc1 100644 --- a/src/Fantomas/Utils.fs +++ b/src/Fantomas/Utils.fs @@ -74,6 +74,8 @@ There is a problem with merging all the code back togheter. Please raise an issu |> String.concat Environment.NewLine + let empty = System.String.Empty + module Cache = let alreadyVisited<'key when 'key : not struct>() = let cache = System.Collections.Generic.HashSet<'key>([], HashIdentity.Reference)