From 3a10e8876591ae376693568bafc29896a38f32bf Mon Sep 17 00:00:00 2001 From: Josh DeGraw <18509575+josh-degraw@users.noreply.github.com> Date: Tue, 9 Jan 2024 10:06:46 -0700 Subject: [PATCH] Fix issues with multiline generic type parameters (#2852) * Expose TransformAST api. (#2868) * Expose TransformAST api. * Add additional unit test. * Revert changelog entry * Fix issues with multiline generic type parameters * Refactor a few things per feedback * Updates * Rebase and add new test * Add one extra space to ensure expr remains an application. * Only add extra space when in SynExpr.App * Rebase and add new test * Add test * WIP * Update tests * Refactor test configs * Fix issue and update newly broken tests * Format CodePrinter * Use more typical unit test template. * Add additional unit tests. * Add changelog entry --------- Co-authored-by: Florian Verdonck --- CHANGELOG.md | 5 + src/Fantomas.Core.Tests/ClassTests.fs | 6 +- src/Fantomas.Core.Tests/CodeFormatterTests.fs | 1 + src/Fantomas.Core.Tests/DallasTests.fs | 13 +- src/Fantomas.Core.Tests/LetBindingTests.fs | 2 +- ...ineBetweenTypeDefinitionAndMembersTests.fs | 8 +- src/Fantomas.Core.Tests/SignatureTests.fs | 11 +- .../TypeAnnotationTests.fs | 463 +++++++++++++++++- .../TypeDeclarationTests.fs | 62 ++- src/Fantomas.Core/CodePrinter.fs | 63 ++- src/Fantomas.Core/Context.fs | 2 +- 11 files changed, 592 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88857aa072..57dce7d890 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 6.3.0-alpha-006 - 2024-01-09 + +### Changed +* Aligned bracket style in anonymous record is not respected. [#2706](https://github.com/fsprojects/fantomas/issues/2706) [style guide](https://github.com/fsharp/fslang-design/issues/756) + ## 6.3.0-alpha-005 - 2023-12-22 ### Changed diff --git a/src/Fantomas.Core.Tests/ClassTests.fs b/src/Fantomas.Core.Tests/ClassTests.fs index 0bcaec3ad9..782d63095c 100644 --- a/src/Fantomas.Core.Tests/ClassTests.fs +++ b/src/Fantomas.Core.Tests/ClassTests.fs @@ -1187,8 +1187,10 @@ module rec Xterm [] type Terminal = abstract onKey: - IEvent<{| key: string - domEvent: KeyboardEvent |}> with get, set + IEvent< + {| key: string + domEvent: KeyboardEvent |} + > with get, set abstract onLineFeed: IEvent with get, set """ diff --git a/src/Fantomas.Core.Tests/CodeFormatterTests.fs b/src/Fantomas.Core.Tests/CodeFormatterTests.fs index b396a37a48..f44801cc39 100644 --- a/src/Fantomas.Core.Tests/CodeFormatterTests.fs +++ b/src/Fantomas.Core.Tests/CodeFormatterTests.fs @@ -1,5 +1,6 @@ module Fantomas.Core.Tests.CodeFormatterTests +open Fantomas.Core.SyntaxOak open NUnit.Framework open Fantomas.FCS.Text open Fantomas.Core diff --git a/src/Fantomas.Core.Tests/DallasTests.fs b/src/Fantomas.Core.Tests/DallasTests.fs index 36238d6007..0b1ad27234 100644 --- a/src/Fantomas.Core.Tests/DallasTests.fs +++ b/src/Fantomas.Core.Tests/DallasTests.fs @@ -1515,11 +1515,14 @@ let autoCompleteItems: cmap * - (Position -> option) * - FSharp.Compiler.Syntax.ParsedInput> = + : cmap< + DeclName, + DeclarationListItem * + Position * + string * + (Position -> option) * + FSharp.Compiler.Syntax.ParsedInput + > = cmap () """ diff --git a/src/Fantomas.Core.Tests/LetBindingTests.fs b/src/Fantomas.Core.Tests/LetBindingTests.fs index 06aa4eb3f2..9a200e94ed 100644 --- a/src/Fantomas.Core.Tests/LetBindingTests.fs +++ b/src/Fantomas.Core.Tests/LetBindingTests.fs @@ -1800,7 +1800,7 @@ module PoorlyIndented = select name from things where id = :id - " > + " > dependency cmd.AsyncExecute(id = thingId) diff --git a/src/Fantomas.Core.Tests/NewlineBetweenTypeDefinitionAndMembersTests.fs b/src/Fantomas.Core.Tests/NewlineBetweenTypeDefinitionAndMembersTests.fs index aac4e5812c..1db3c91ccb 100644 --- a/src/Fantomas.Core.Tests/NewlineBetweenTypeDefinitionAndMembersTests.fs +++ b/src/Fantomas.Core.Tests/NewlineBetweenTypeDefinitionAndMembersTests.fs @@ -387,10 +387,10 @@ let ``multiline abstract member without constraints, 2175`` () = type FuseSortFunctionItem = abstract Item: key: string -> - U2<{| ``$``: string |}, ResizeArray<{| ``$``: - string - idx: - float |}>> with get, set + U2< + {| ``$``: string |}, + ResizeArray<{| ``$``: string; idx: float |}> + > with get, set abstract X: int """ diff --git a/src/Fantomas.Core.Tests/SignatureTests.fs b/src/Fantomas.Core.Tests/SignatureTests.fs index 41df952e01..784ae08d83 100644 --- a/src/Fantomas.Core.Tests/SignatureTests.fs +++ b/src/Fantomas.Core.Tests/SignatureTests.fs @@ -1554,9 +1554,14 @@ namespace Foo type Bar = member Hello : thing : - XLongLongLongLongLongLongLongLong 'a, bool -> 'b, bool -> 'c, bool -> 'd, bool -> ('e -> 'f) -> 'g, ('h - -> 'i) - -> 'j> * + XLongLongLongLongLongLongLongLong< + bool -> 'a, + bool -> 'b, + bool -> 'c, + bool -> 'd, + bool -> ('e -> 'f) -> 'g, + ('h -> 'i) -> 'j + > * item : int list -> LongLongLongLongLongLongLongLongLongLongLongLongLongLongLongLong """ diff --git a/src/Fantomas.Core.Tests/TypeAnnotationTests.fs b/src/Fantomas.Core.Tests/TypeAnnotationTests.fs index 1b632b83c2..d24c12d665 100644 --- a/src/Fantomas.Core.Tests/TypeAnnotationTests.fs +++ b/src/Fantomas.Core.Tests/TypeAnnotationTests.fs @@ -2,6 +2,7 @@ module Fantomas.Core.Tests.TypeAnnotationTests open NUnit.Framework open FsUnit +open Fantomas.Core open Fantomas.Core.Tests.TestHelpers [] @@ -72,9 +73,13 @@ type X = equal """ type X = - Teq + Teq< + int, + list int, + System.DateTime array, + // + int + > """ [] @@ -116,7 +121,11 @@ type CancellableTaskResultBuilderBase with and ^Awaiter: (member GetResult: unit -> Result<'TResult1, 'Error>)> ( sm: - byref>>, + byref< + ResumableStateMachine< + CancellableTaskResultStateMachineData<'TOverall, 'Error> + > + >, task: CancellationToken -> ^TaskLike, continuation: ('TResult1 @@ -124,3 +133,449 @@ type CancellableTaskResultBuilderBase with ) : bool = true """ + +[] +let ``aligned bracket style in anonymous record is respected, 2706`` () = + formatSourceString + """ +let private asJson (arm: IArmResource) = + arm.JsonModel + |> convertTo<{| + kind: string + properties: {| statisticsEnabled: bool |} + |}> +""" + { config with + MultilineBracketStyle = Aligned } + |> prepend newline + |> should + equal + """ +let private asJson (arm: IArmResource) = + arm.JsonModel + |> convertTo< + {| + kind: string + properties: {| statisticsEnabled: bool |} + |} + > +""" + +[] +let ``aligned bracket style in anonymous record is respected for multiple types, 2706`` () = + formatSourceString + """ +let private asJson (arm: IArmResource) = + arm.JsonModel + |> convertTo<{| + kind: string + properties: {| statisticsEnabled: bool |} + |},{| + kind: string + properties: {| statisticsEnabled: bool |} + |} + > +""" + { config with + MultilineBracketStyle = Aligned } + |> prepend newline + |> should + equal + """ +let private asJson (arm: IArmResource) = + arm.JsonModel + |> convertTo< + {| + kind: string + properties: {| statisticsEnabled: bool |} + |}, + {| + kind: string + properties: {| statisticsEnabled: bool |} + |} + > +""" + +[] +let ``cramped bracket style in anonymous record is respected for multiple types, 2706`` () = + formatSourceString + """ +let private asJson (arm: IArmResource) = + arm.JsonModel + |> convertTo<{| + kind: string + properties: {| statisticsEnabled: bool |} + |},{| + kind: string + properties: {| statisticsEnabled: bool |} + |} + > +""" + { config with + MultilineBracketStyle = Cramped } + |> prepend newline + |> should + equal + """ +let private asJson (arm: IArmResource) = + arm.JsonModel + |> convertTo< + {| kind: string + properties: {| statisticsEnabled: bool |} |}, + {| kind: string + properties: {| statisticsEnabled: bool |} |} + > +""" + +[] +let ``stroustrup bracket style in anonymous record is respected for multiple types, 2706`` () = + formatSourceString + """ +let private asJson (arm: IArmResource) = + arm.JsonModel + |> convertTo<{| + kind: string + properties: {| statisticsEnabled: bool |} + |},{| + kind: string + properties: {| statisticsEnabled: bool |} + |} + > +""" + { config with + MultilineBracketStyle = Stroustrup } + |> prepend newline + |> should + equal + """ +let private asJson (arm: IArmResource) = + arm.JsonModel + |> convertTo< + {| + kind: string + properties: {| statisticsEnabled: bool |} + |}, + {| + kind: string + properties: {| statisticsEnabled: bool |} + |} + > +""" + +let alignedMaxLine30 = + { config with + MaxLineLength = 30 + MultilineBracketStyle = Aligned } + +[] +let ``type application including nested multiline function type`` () = + formatSourceString + """ +let bv = unbox 'b>> bf +""" + alignedMaxLine30 + |> prepend newline + |> should + equal + """ +let bv = + unbox< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + > + bf +""" + +[] +let ``multiline type argument with AppLongIdentAndSingleParenArg`` () = + formatSourceString + """ +path.Replace< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + >("../../../", "....") +""" + { config with + MaxLineLength = 30 + SpaceBeforeUppercaseInvocation = true + SpaceBeforeClassConstructor = true + SpaceBeforeMember = true + SpaceBeforeColon = true + SpaceBeforeSemicolon = true + MultilineBracketStyle = Aligned + AlignFunctionSignatureToIndentation = true + AlternativeLongMemberDefinitions = true + MultiLineLambdaClosingNewline = true + NewlineBetweenTypeDefinitionAndMembers = false } + |> prepend newline + |> should + equal + """ +path.Replace< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + > ( + "../../../", + "...." +) +""" + +[] +let ``multiline type argument with AppLongIdentAndSingleParenArg 2`` () = + formatSourceString + """ +path.Replace< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + >("../../../", "....") +""" + { alignedMaxLine30 with + SpaceBeforeClassConstructor = true } + |> prepend newline + |> should + equal + """ +path.Replace< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + >( + "../../../", + "...." +) +""" + +[] +let ``multiline type argument with AppLongIdentAndSingleParenArg 3`` () = + formatSourceString + """ +path.Replace< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + >("../../../", "....") +""" + alignedMaxLine30 + |> prepend newline + |> should + equal + """ +path.Replace< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + >( + "../../../", + "...." +) +""" + +[] +let ``multiline type argument with AppSingleParenArg`` () = + formatSourceString + """ +someFunc< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + >(a,b) +""" + alignedMaxLine30 + |> prepend newline + |> should + equal + """ +someFunc< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + > ( + a, + b +) +""" + +[] +let ``multiline type argument with AppWithLambda`` () = + formatSourceString + """ +someFunc< + Bar< + 'innerContextLongLongLong, + 'bb -> 'b + > + > (fun x -> x) +""" + alignedMaxLine30 + |> prepend newline + |> should + equal + """ +someFunc< + Bar< + 'innerContextLongLongLong, + 'bb -> 'b + > + > + (fun x -> x) +""" + +[] +let ``multiline type argument with NestedIndexWithoutDot`` () = + formatSourceString + """ +something< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + >["thing"][8](a,b) +""" + alignedMaxLine30 + |> prepend newline + |> should + equal + """ +something< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + >["thing"][8](a, b) +""" + +[] +let ``multiline type argument with EndsWithDualListApp`` () = + formatSourceString + """ +div< + Bar< + 'innerContextLongLongLong, + 'bb -> 'b + > + > [ ClassName "container" ] [ str "meh" ] +""" + alignedMaxLine30 + |> prepend newline + |> should + equal + """ +div< + Bar< + 'innerContextLongLongLong, + 'bb -> 'b + > + > + [ ClassName "container" ] + [ str "meh" ] +""" + +[] +let ``multiline type argument with elmish EndsWithDualListApp`` () = + formatSourceString + """ +div< + Bar< + 'innerContextLongLongLong, + 'bb -> 'b + > + > [ ClassName "container" ] [ str "meh" ] +""" + { alignedMaxLine30 with + ExperimentalElmish = true } + |> prepend newline + |> should + equal + """ +div< + Bar< + 'innerContextLongLongLong, + 'bb -> 'b + > + > + [ ClassName "container" ] [ + str "meh" + ] +""" + +[] +let ``multiline type argument with EndsWithSingleListApp`` () = + formatSourceString + """ +input< + Bar< + 'innerContextLongLongLong, + 'bb -> 'b + > + > [ Type "text" ] +""" + alignedMaxLine30 + |> prepend newline + |> should + equal + """ +input< + Bar< + 'innerContextLongLongLong, + 'bb -> 'b + > + > + [ Type "text" ] +""" + +[] +let ``multiline type argument with index without dot`` () = + formatSourceString + """ +XYZ.app int -> int -> string>[tellMeWhy { return wouldSomeoneWriteThisCode }] +""" + alignedMaxLine30 + |> prepend newline + |> should + equal + """ +XYZ.app< + int + -> int + -> int + -> string + >[tellMeWhy { + return + wouldSomeoneWriteThisCode + }] +""" + +[] +let ``multiline type argument with Index with dot`` () = + formatSourceString + """ +XYZ.app int -> int -> string>.[tellMeWhy { return wouldSomeoneWriteThisCode }] +""" + alignedMaxLine30 + |> prepend newline + |> should + equal + """ +XYZ.app< + int + -> int + -> int + -> string + >.[tellMeWhy { + return + wouldSomeoneWriteThisCode +}] +""" diff --git a/src/Fantomas.Core.Tests/TypeDeclarationTests.fs b/src/Fantomas.Core.Tests/TypeDeclarationTests.fs index 3fbc55d9ff..3dc368c1a7 100644 --- a/src/Fantomas.Core.Tests/TypeDeclarationTests.fs +++ b/src/Fantomas.Core.Tests/TypeDeclarationTests.fs @@ -2818,8 +2818,12 @@ and [] Bar<'context, 'a> = (fun inner -> if inner then let bv = - unbox 'b>> + unbox< + Foo< + 'innerContextLongLongLong, + 'bb -> 'b + > + > bf this.InnerEquals af bf cont @@ -2832,6 +2836,60 @@ and [] Bar<'context, 'a> = } """ +[] +let ``multiple nested generic types`` () = + formatSourceString + """ +let bv = + unbox< + Fooadfadadfdadfadfadfadfadfadfsfdsfadfadadfada< + Foo< + innerContextLongLongLong, + bb + > + > + > + bf +""" + { config with MaxLineLength = 10 } + |> prepend newline + |> should + equal + """ +let bv = + unbox< + Fooadfadadfdadfadfadfadfadfadfsfdsfadfadadfada< + Foo< + innerContextLongLongLong, + bb + > + > + > + bf +""" + +[] +let ``Trivia inside multiline generic type parameters`` () = + formatSourceString + """ +type X = + Teq< // + int + // + > +""" + config + |> prepend newline + |> should + equal + """ +type X = + Teq< // + int + // + > +""" + [] let ``a huge amount of type declarations`` () = let sourceCode = diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index d1f2476efe..7d4549d10f 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -1183,27 +1183,19 @@ let genExpr (e: Expr) = else indentSepNlnUnindent f ctx - (genExpr node.FunctionExpr + let genFuncExpr = + match node.FunctionExpr with + | Expr.TypeApp node -> genTypeApp true node + | _ -> genExpr node.FunctionExpr + + (genFuncExpr +> ensureArgumentsAreNotAlignedWithFunctionName (col sepNln node.Arguments genExpr)) ctx expressionFitsOnRestOfLine shortExpression longExpression ctx |> genNode node - | Expr.TypeApp node -> - fun ctx -> - let startColum = ctx.Column - - genNode - node - (genExpr node.Identifier - +> genSingleTextNode node.LessThan - +> colGenericTypeParameters node.TypeParameters - // we need to make sure each expression in the function application has offset at least greater than - // See: https://github.com/fsprojects/fantomas/issues/1611 - +> addFixedSpaces startColum - +> genSingleTextNode node.GreaterThan) - ctx + | Expr.TypeApp node -> expressionFitsOnRestOfLine (genTypeApp false node) (genTypeApp true node) | Expr.TryWithSingleClause node -> let genClause = let clauseNode = node.Clause @@ -2030,6 +2022,22 @@ let genAppSingleParenArgExpr (addSpace: Context -> Context) (node: ExprAppSingle expressionFitsOnRestOfLine short long |> genNode node +/// When called from `SynExpr.App` we need to ensure the node.GreaterThan is placed one space further than the start column. +/// This is to ensure the application remains an application. +let genTypeApp (addAdditionalColumnOffset: bool) (node: ExprTypeAppNode) (ctx: Context) : Context = + let startColumn = ctx.Column + (if addAdditionalColumnOffset then 1 else 0) + + genNode + node + (genExpr node.Identifier + +> genSingleTextNode node.LessThan + +> colGenericTypeParameters node.TypeParameters + // we need to make sure each expression in the function application has offset at least greater than + // See: https://github.com/fsprojects/fantomas/issues/1611 + +> addFixedSpaces startColumn + +> genSingleTextNode node.GreaterThan) + ctx + let genClauses (clauses: MatchClauseNode list) = let lastIndex = clauses.Length - 1 @@ -2267,13 +2275,22 @@ let genKeepIdentMatchClause (startNode: Node) (e: Expr) ctx = indentSepNlnUnindent (genExpr e) ctx let colGenericTypeParameters typeParameters = - coli sepComma typeParameters (fun idx t -> - let leadingSpace = - match t with - | Type.Var n when idx = 0 && String.startsWithOrdinal "^" n.Text -> sepSpace - | _ -> sepNone + let genParameters sep = + coli sep typeParameters (fun idx t -> + let leadingSpace = + match t with + | Type.Var n when idx = 0 && String.startsWithOrdinal "^" n.Text -> sepSpace + | _ -> sepNone + + leadingSpace +> genType t) + + let long = indentSepNlnUnindent (genParameters (sepComma +> sepNln)) +> sepNln + let short = genParameters sepComma - leadingSpace +> genType t) + // Multiline text type params should be unmodified + match typeParameters with + | [ Type.StaticConstant(Constant.FromText textNode) ] when textNode.Text.Contains("\n") -> short + | _ -> expressionFitsOnRestOfLine short long let genFunctionNameWithMultilineLids (trailing: Context -> Context) (longIdent: IdentListNode) = match longIdent.Content with @@ -3130,8 +3147,10 @@ let genType (t: Type) = +> optSingle genIdentListNodeWithDot node.PostIdentifier +> genSingleTextNode node.LessThen +> addExtraSpace - +> col sepComma node.Arguments genType + +> leadingExpressionIsMultiline (colGenericTypeParameters node.Arguments) (fun isMultiline -> + onlyIf isMultiline (!- " ")) +> addExtraSpace + // TODO: I think we need to add a space here +> genSingleTextNode node.GreaterThan |> genNode node | Type.StructTuple node -> diff --git a/src/Fantomas.Core/Context.fs b/src/Fantomas.Core/Context.fs index b6b2a60a5d..31540dcb3f 100644 --- a/src/Fantomas.Core/Context.fs +++ b/src/Fantomas.Core/Context.fs @@ -662,7 +662,7 @@ let leadingExpressionResult leadingExpression continuationExpression (ctx: Conte continuationExpression ((lineCountBefore, columnBefore), (lineCountAfter, columnAfter)) contextAfterLeading -/// A leading expression is not consider multiline if it has a comment before it. +/// A leading expression is not considered multiline if it has a comment before it. /// For example /// let a = 7 /// // foo