Skip to content

Commit

Permalink
MaxValueBindingWidth and MaxFunctionBindingWidth settings (#866)
Browse files Browse the repository at this point in the history
* Rename MaxLetBindingWidth setting to MaxBindingWidth

* Implement MaxBindingWidth for members and update existing tests
Some of the existing tests were not passing after MaxBindingWidth was
implemented for members. The reason is that members did not respect
MaxLetBindingWidth before, only PageWidth.
Since members now also respect MaxBindingWidth, some results of existing
tests are different.

* Keep existing tests unmodified where possible

* Use a simpler implementation for MaxBindingWidth

* Add typed member binding tests

* Split MaxBindingWidth setting into
MaxValueBindingWidth and MaxFunctionBindingWidth

* Use List.isNotEmpty instead of List.length > 0

* Update active pattern test to use a smaller MaxFunctionBindingWidth

* Remove unused helper function in SourceParser.fs

Co-authored-by: Florian Verdonck <florian.verdonck@outlook.com>
  • Loading branch information
Bobface and nojaf committed Jun 10, 2020
1 parent c6e400e commit 50ed1b7
Show file tree
Hide file tree
Showing 26 changed files with 196 additions and 64 deletions.
3 changes: 2 additions & 1 deletion docs/Documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@ A default configuration file would look like
"MaxInfixOperatorExpression":50,
"MaxRecordWidth":40,
"MaxArrayOrListWidth":40,
"MaxLetBindingWidth":40,
"MaxValueBindingWidth":40,
"MaxFunctionBindingWidth":40,
"MultilineBlockBracketsOnSameColumn":false,
"NewlineBetweenTypeDefinitionAndMembers":false,
"StrictMode":false
Expand Down
2 changes: 1 addition & 1 deletion src/Fantomas.Tests/ActivePatternTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ 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""" ({ config with MaxLetBindingWidth = 30 })
else None""" ({ config with MaxValueBindingWidth = 30; MaxFunctionBindingWidth = 30 })
|> prepend newline
|> should equal """
let (|Even|Odd|) input =
Expand Down
2 changes: 1 addition & 1 deletion src/Fantomas.Tests/AttributeTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ type Funcs =
[<Extension>]
static member ToFunc (f: Action<_,_,_>) =
Func<_,_,_,_>(fun a b c -> f.Invoke(a,b,c))
""" config
""" { config with MaxFunctionBindingWidth = 120 }
|> should equal """[<Extension>]
type Funcs =
[<Extension>]
Expand Down
23 changes: 16 additions & 7 deletions src/Fantomas.Tests/ClassTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ type Shape2D(x0 : float, y0 : float) =
abstract member Rotate: float -> unit
default this.Rotate(angle) = rotAngle <- rotAngle + angle
""" config
""" { config with MaxValueBindingWidth = 120 }
|> prepend newline
|> should equal """
[<AbstractClass>]
Expand Down Expand Up @@ -181,7 +181,7 @@ let ``classes and implicit constructors``() =
let data = dataIn
do self.PrintMessage()
member this.PrintMessage() =
printf "Creating MyClass2 with Data %d" data""" config
printf "Creating MyClass2 with Data %d" data""" { config with MaxFunctionBindingWidth = 120 }
|> prepend newline
|> should equal """
type MyClass2(dataIn) as self =
Expand All @@ -197,7 +197,7 @@ let ``classes and private implicit constructors``() =
let data = dataIn
do self.PrintMessage()
member this.PrintMessage() =
printf "Creating MyClass2 with Data %d" data""" config
printf "Creating MyClass2 with Data %d" data""" { config with MaxFunctionBindingWidth = 120 }
|> prepend newline
|> should equal """
type MyClass2 private (dataIn) as self =
Expand All @@ -216,7 +216,7 @@ type Folder(pathIn: string) =
and File(filename: string, containingFolder: Folder) =
member __.Name = filename
member __.ContainingFolder = containingFolder""" config
member __.ContainingFolder = containingFolder""" { config with MaxValueBindingWidth = 120 }
|> prepend newline
|> should equal """
type Folder(pathIn: string) =
Expand Down Expand Up @@ -385,7 +385,7 @@ let ``indexed get long line``() =
type Exception with
member inline __.FirstLine =
__.Message.Split([|Environment.NewLine|], StringSplitOptions.RemoveEmptyEntries).[0]
""" config
""" { config with MaxValueBindingWidth = 120 }
|> should equal """open System
type Exception with
Expand Down Expand Up @@ -426,7 +426,7 @@ module Logging =
LogProvider.SetCurrentLogProvider(QuartzLoggerWrapper(loggerFunction))
let SetQuartzLogger l = LogProvider.SetCurrentLogProvider(l)
""" config
""" { config with MaxFunctionBindingWidth = 80 }
|> prepend newline
|> should equal """
namespace Quartz.Fsharp
Expand Down Expand Up @@ -465,7 +465,16 @@ module Logging =

[<Test>]
let ``no extra new lines between type members, 569``() =
shouldNotChangeAfterFormat """
formatSourceString false """
type A() =
member this.MemberA = if true then 0 else 1
member this.MemberB = if true then 2 else 3
member this.MemberC = 0""" { config with MaxValueBindingWidth = 120 }
|> prepend newline
|> should equal """
type A() =
member this.MemberA = if true then 0 else 1
Expand Down
2 changes: 1 addition & 1 deletion src/Fantomas.Tests/ComputationExpressionTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ let factors number =
{2L .. number / 2L}
|> Seq.filter (fun x -> number % x = 0L)""" ({ config with
MaxInfixOperatorExpression = 65
MaxLetBindingWidth = 65 })
MaxFunctionBindingWidth = 65 })
|> prepend newline
|> should equal """
let factors number = { 2L .. number / 2L } |> Seq.filter (fun x -> number % x = 0L)
Expand Down
2 changes: 2 additions & 0 deletions src/Fantomas.Tests/Fantomas.Tests.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@
<Compile Include="MultilineBlockBracketsOnSameColumnArrayOrListTests.fs" />
<Compile Include="NewlineBetweenTypeDefinitionAndMembersTests.fs" />
<Compile Include="KeepIfThenInSameLineTests.fs" />
<Compile Include="MaxValueBindingWidthTests.fs" />
<Compile Include="MaxFunctionBindingWidthTests.fs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\Fantomas\Fantomas.fsproj" />
Expand Down
11 changes: 4 additions & 7 deletions src/Fantomas.Tests/FormattingSelectionOnlyTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ let source = "
done;
done;;
Multiple9x9 ();;"
""" ({ config with MaxLetBindingWidth = 60; MaxRecordWidth = 50 })
""" ({ config with MaxValueBindingWidth = 120; MaxRecordWidth = 50 })
|> should equal """let config = { FormatConfig.Default with IndentSpaceNum = 2 }"""

[<Test>]
Expand Down Expand Up @@ -100,8 +100,7 @@ let ``should detect members and format appropriately``() =
type T () =
let items = []
override x.Reorder () =
items |> List.iter ignore
""" config
items |> List.iter ignore""" { config with MaxFunctionBindingWidth = 120 }
|> should equal """ override x.Reorder() = items |> List.iter ignore"""

[<Test>]
Expand All @@ -125,14 +124,12 @@ type Folder(pathIn : string) =
and File(filename: string, containingFolder: Folder) =
member __.Name = filename
member __.ContainingFolder = containingFolder
""" config
member __.ContainingFolder = containingFolder""" { config with MaxValueBindingWidth = 120 }
|> prepend newline
|> should equal """
and File(filename: string, containingFolder: Folder) =
member __.Name = filename
member __.ContainingFolder = containingFolder
"""
member __.ContainingFolder = containingFolder"""

[<Test>]
let ``should not add trailing whitespaces and preserve indentation``() =
Expand Down
8 changes: 4 additions & 4 deletions src/Fantomas.Tests/InterfaceTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type SomeClass1(x: int, y: float) =
type Interface3 =
inherit Interface1
inherit Interface2
abstract member Method3 : int -> int""" config
abstract member Method3 : int -> int""" { config with MaxFunctionBindingWidth = 120 }
|> prepend newline
|> should equal """
type IPrintable =
Expand All @@ -40,7 +40,7 @@ let ``should not add with to interface definitions with no members``() =
interface Infrastucture with
member this.Serialize sb = sb.AppendFormat("\"{0}\"", escape v)
member this.ToXml() = v :> obj
""" config
""" { config with MaxValueBindingWidth = 120 }
|> should equal """type Text(text: string) =
interface IDocument
Expand Down Expand Up @@ -88,7 +88,7 @@ let f () =
member x.ToString() = "INotifyEnumerableInternal"
interface INotifyEnumerableInternal<'T>
interface IEnumerable<_> with
member x.GetEnumerator() = null }""" config
member x.GetEnumerator() = null }""" { config with MaxValueBindingWidth = 120 }
|> prepend newline
|> should equal """
let f () =
Expand Down Expand Up @@ -150,7 +150,7 @@ type MyLogInteface() =
else
sprintf "date-%s.log" environment
member x.Info () = ()
override x.Version () = ()""" ({ config with PageWidth = 119 })
override x.Version () = ()""" ({ config with PageWidth = 119; MaxFunctionBindingWidth = 120 })
|> prepend newline
|> should equal """
type LogInterface =
Expand Down
4 changes: 2 additions & 2 deletions src/Fantomas.Tests/LambdaTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ let foo = Foo(fun () -> Foo.Create x).Value
[<Test>]
let ``line comment after lambda should not necessary make it multiline`` () =
formatSourceString false """let a = fun _ -> div [] [] // React.lazy is not compatible with SSR, so just use an empty div
""" ({ config with MaxLetBindingWidth = 150 })
""" ({ config with MaxFunctionBindingWidth = 150 })
|> prepend newline
|> should equal """
let a = fun _ -> div [] [] // React.lazy is not compatible with SSR, so just use an empty div
Expand Down Expand Up @@ -335,7 +335,7 @@ let projectIntoMap projection =
SpaceAfterComma = false
SpaceAroundDelimiter = false
MaxInfixOperatorExpression = 40
MaxLetBindingWidth = 60
MaxFunctionBindingWidth = 60
MultilineBlockBracketsOnSameColumn = true })
|> prepend newline
|> should equal """
Expand Down
4 changes: 2 additions & 2 deletions src/Fantomas.Tests/LetBindingTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ let f () =
x + y
"""

formatSourceString false codeSnippet ({ config with MaxLetBindingWidth = 50 })
formatSourceString false codeSnippet ({ config with MaxValueBindingWidth = 50 })
|> should equal """let f () =
let x = 1 (* the "in" keyword is available in F# *)
let y = 2
Expand Down Expand Up @@ -81,7 +81,7 @@ let ``DotGet on newline should be indented far enough`` () =
let tomorrow =
DateTimeOffset(n.Year, n.Month, n.Day, 0, 0, 0, n.Offset)
.AddDays(1.)
""" ({ config with MaxLetBindingWidth = 70 })
""" ({ config with MaxValueBindingWidth = 70 })
|> prepend newline
|> should equal """
let tomorrow = DateTimeOffset(n.Year, n.Month, n.Day, 0, 0, 0, n.Offset).AddDays(1.)
Expand Down
57 changes: 57 additions & 0 deletions src/Fantomas.Tests/MaxFunctionBindingWidthTests.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
module Fantomas.Tests.MaxFunctionBindingWidthTests

open NUnit.Framework
open FsUnit
open Fantomas.Tests.TestHelper

let config = { config with MaxFunctionBindingWidth = 20 }

[<Test>]
let ``should apply to function definition``() =
formatSourceString false """let a bbbbbbbbbbbbbbbbbbbbbbbbbb = bbbbbbbbbbbbbbbbbbbbbbbbbb + 1""" config
|> should equal """let a bbbbbbbbbbbbbbbbbbbbbbbbbb =
bbbbbbbbbbbbbbbbbbbbbbbbbb + 1
"""

[<Test>]
let ``should not apply to short function definition``() =
formatSourceString false """let a b = b + 1""" config
|> should equal """let a b = b + 1
"""

[<Test>]
let ``should apply to member function definition``() =
formatSourceString false """type T =
let aaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbb = bbbbbbbbbbbbbbbbbbb + 1
member this.cccccccccccccc dddddddddddddd = dddddddddddddd + 2
""" config
|> should equal """type T =
let aaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbb =
bbbbbbbbbbbbbbbbbbb + 1
member this.cccccccccccccc dddddddddddddd = dddddddddddddd + 2
"""

[<Test>]
let ``should apply to typed member function definition``() =
formatSourceString false """type T =
let aaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbb = bbbbbbbbbbbbbbbbbbb + 1
member this.cccccccccccccc dddddddddddddd: int = dddddddddddddd + 2
""" config
|> should equal """type T =
let aaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbb =
bbbbbbbbbbbbbbbbbbb + 1
member this.cccccccccccccc dddddddddddddd: int = dddddddddddddd + 2
"""

[<Test>]
let ``should not apply to short member function definition``() =
formatSourceString false """type T =
let a b = b + 1
member this.c d = d + 2
""" { config with MaxFunctionBindingWidth = 30 }
|> should equal """type T =
let a b = b + 1
member this.c d = d + 2
"""
59 changes: 59 additions & 0 deletions src/Fantomas.Tests/MaxValueBindingWidthTests.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
module Fantomas.Tests.MaxValueBindingWidthTests

open NUnit.Framework
open FsUnit
open Fantomas.Tests.TestHelper

let config = { config with MaxValueBindingWidth = 20; }

[<Test>]
let ``should apply to value definition``() =
formatSourceString false """let a = bbbbbbbbbbbbbbbbbbbbbbbbbb""" config
|> should equal """let a =
bbbbbbbbbbbbbbbbbbbbbbbbbb
"""

[<Test>]
let ``should not apply to short value definition``() =
formatSourceString false """let a = b""" config
|> should equal """let a = b
"""

[<Test>]
let ``should apply to member value definition``() =
formatSourceString false """type T =
let aaaaaaaaaaaaaaaaaaaa = bbbbbbbbbbbbbbbbbbb + 1
member this.ccccccccccccccccccccccccccccccc = dddddddddddddddddddddddddddd + 2
""" config
|> should equal """type T =
let aaaaaaaaaaaaaaaaaaaa =
bbbbbbbbbbbbbbbbbbb + 1
member this.ccccccccccccccccccccccccccccccc =
dddddddddddddddddddddddddddd + 2
"""

[<Test>]
let ``should apply to typed member value definition``() =
formatSourceString false """type T =
let aaaaaaaaaaaaaaaaaaaa = bbbbbbbbbbbbbbbbbbb + 1
member (this.ccccccccccccccccccccccccccccccc: int)= dddddddddddddddddddddddddddd + 2
""" config
|> should equal """type T =
let aaaaaaaaaaaaaaaaaaaa =
bbbbbbbbbbbbbbbbbbb + 1
member (this.ccccccccccccccccccccccccccccccc: int) =
dddddddddddddddddddddddddddd + 2
"""

[<Test>]
let ``should not apply to short member value definition``() =
formatSourceString false """type T =
let a = b + 1
member this.c = d + 2
""" { config with MaxFunctionBindingWidth = 30 }
|> should equal """type T =
let a = b + 1
member this.c = d + 2
"""
4 changes: 2 additions & 2 deletions src/Fantomas.Tests/ModuleTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ namespace global
type SomeType() =
member this.Print() =
global.System.Console.WriteLine("Hello World!")
""" config
""" { config with MaxFunctionBindingWidth = 120 }
|> prepend newline
|> should equal """
namespace global
Expand All @@ -243,7 +243,7 @@ let ``abstract`` = "abstract"
type SomeType() =
member this.``new``() =
System.Console.WriteLine("Hello World!")
""" config
""" { config with MaxFunctionBindingWidth = 120 }
|> prepend newline
|> should equal """
module ``member``
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ type Range =
{ From: float
To: float }
member this.Length = this.To - this.From
""" config
""" { config with MaxValueBindingWidth = 120 }
|> prepend newline
|> should equal """
type Range =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ let ``newline between record type and members`` () =
To : float
Name: string }
member this.Length = this.To - this.From
""" config
""" { config with MaxValueBindingWidth = 120 }
|> prepend newline
|> should equal """
type Range =
Expand All @@ -32,7 +32,7 @@ let ``existing newline between record type and members should not be duplicate``
Name: string }
member this.Length = this.To - this.From
""" config
""" { config with MaxValueBindingWidth = 120 }
|> prepend newline
|> should equal """
type Range =
Expand Down
2 changes: 1 addition & 1 deletion src/Fantomas.Tests/SignatureTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ let ``should not add parens in signature``() =
Handler : Map<string, string> -> HttpListenerContext -> string }
override x.ToString() = sprintf "%s %s" x.Verb x.Path
""" config
""" { config with MaxFunctionBindingWidth = 120 }
|> should equal """type Route =
{ Verb: string
Path: string
Expand Down
Loading

0 comments on commit 50ed1b7

Please sign in to comment.