From 25b6f95b1e17b0e54e4c8fe807ee3d748043d5b6 Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Sun, 14 May 2023 21:53:12 +0200 Subject: [PATCH] Take commas into account (#2880) * Update FCS to e267bb9f8d590feed1b94b469d78cfce61afecad. * Use commas as nodes in PatTupleNode. * Respect commas in SynSimplePats.SimplePats. * Pick correct comma in pattern in mkPropertyGetSetBinding. * Update src/Fantomas.Core.Tests/TypeDeclarationTests.fs Co-authored-by: dawe * 6.0.3 release --------- Co-authored-by: dawe --- CHANGELOG.md | 7 +- Directory.Build.props | 2 +- src/Fantomas.Core.Tests/CommentTests.fs | 6 +- .../CompilerDirectivesTests.fs | 36 ++++++ src/Fantomas.Core.Tests/LetBindingTests.fs | 25 ++++ .../TypeDeclarationTests.fs | 33 ++++++ src/Fantomas.Core/ASTTransformer.fs | 107 +++++++++++------- src/Fantomas.Core/CodePrinter.fs | 49 +++++--- src/Fantomas.Core/SyntaxOak.fs | 20 +++- 9 files changed, 218 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd96eab8bb..f470d52d18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,14 @@ # Changelog -## [Unreleased] +## [6.0.3] - 2023-05-14 ### Fixed * Preserves quotes around type parameter names. [#2875](https://github.com/fsprojects/fantomas/issues/2875) +* Additional whitespace for LineCommentAfterSourceCode when last character is a `,`. [#2589](https://github.com/fsprojects/fantomas/issues/2589) +* Tupled parameter wrapped in conditional directive. [#2877](https://github.com/fsprojects/fantomas/issues/2877) + +### Changed +* Update FCS to 'Add commas to tuple pat and simple pats', commit e267bb9f8d590feed1b94b469d78cfce61afecad ## [6.0.2] - 2023-05-05 diff --git a/Directory.Build.props b/Directory.Build.props index e9a6b4ab0f..eb91c9c62f 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -39,7 +39,7 @@ Some common use cases include: - ba6647ebf5b94823c4d6fafd1e7d5f806d915ee0 + e267bb9f8d590feed1b94b469d78cfce61afecad 2.8.28 6.0.1 diff --git a/src/Fantomas.Core.Tests/CommentTests.fs b/src/Fantomas.Core.Tests/CommentTests.fs index 2448ca0300..996c811baa 100644 --- a/src/Fantomas.Core.Tests/CommentTests.fs +++ b/src/Fantomas.Core.Tests/CommentTests.fs @@ -2080,8 +2080,8 @@ type CreateFSharpManifestResourceName public () = ( fileName: string, linkFileName: string, - rootNamespace: string, // may be null - dependentUponFileName: string, // may be null + rootNamespace: string, // may be null + dependentUponFileName: string, // may be null binaryStream: Stream // may be null ) : string = () @@ -2570,7 +2570,7 @@ type MyType2 = type MyType = member _.MyMethod ( - [] inputA: string, // my comment 1 + [] inputA: string, // my comment 1 [] inputB: string // my comment 2 ) = inputA diff --git a/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs b/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs index 460142e85c..a678882456 100644 --- a/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs +++ b/src/Fantomas.Core.Tests/CompilerDirectivesTests.fs @@ -3286,3 +3286,39 @@ module A = let f x = x + x """ + +[] +let ``conditional directives around last tuple pattern, 2877`` () = + formatSourceString + false + """ +// Link all the assemblies together and produce the input typecheck accumulator +let CombineImportedAssembliesTask + ( + a, + b +#if !NO_TYPEPROVIDERS + , c +#endif + ) = + + () +""" + config + |> prepend newline + |> should + equal + """ +// Link all the assemblies together and produce the input typecheck accumulator +let CombineImportedAssembliesTask + ( + a, + b +#if !NO_TYPEPROVIDERS + , + c +#endif + ) = + + () +""" diff --git a/src/Fantomas.Core.Tests/LetBindingTests.fs b/src/Fantomas.Core.Tests/LetBindingTests.fs index 3c21233243..dba78cf6d6 100644 --- a/src/Fantomas.Core.Tests/LetBindingTests.fs +++ b/src/Fantomas.Core.Tests/LetBindingTests.fs @@ -2140,3 +2140,28 @@ let inline (!!) (x: ^a) : ^b = ((^a or ^b): (static member op_Implicit: ^a -> ^b let inline (!!) (x: ^a) : ^b = ((^a or ^b): (static member op_Implicit: ^a -> ^b) x) """ + +[] +let ``avoid additional whitespace after comma, 2589`` () = + formatSourceString + false + """ +let x + ( + a: string, // test + b: string // test + ) = + print "hello" +""" + config + |> prepend newline + |> should + equal + """ +let x + ( + a: string, // test + b: string // test + ) = + print "hello" +""" diff --git a/src/Fantomas.Core.Tests/TypeDeclarationTests.fs b/src/Fantomas.Core.Tests/TypeDeclarationTests.fs index 97f2baf403..979052c77a 100644 --- a/src/Fantomas.Core.Tests/TypeDeclarationTests.fs +++ b/src/Fantomas.Core.Tests/TypeDeclarationTests.fs @@ -3587,3 +3587,36 @@ type ArrayBuffer = abstract byteLength: int abstract slice: ``begin``: int * ?``end``: int -> ArrayBuffer """ + +[] +let ``trivia before comma in primary constructor`` () = + formatSourceString + false + """ +type Meh + ( + a, + b +#if !NO_TYPEPROVIDERS + , c +#endif + ) = + class end +""" + config + |> prepend newline + |> should + equal + """ +type Meh + ( + a, + b +#if !NO_TYPEPROVIDERS + , + c +#endif + ) = + class + end +""" diff --git a/src/Fantomas.Core/ASTTransformer.fs b/src/Fantomas.Core/ASTTransformer.fs index bd0dea112a..09cedff756 100644 --- a/src/Fantomas.Core/ASTTransformer.fs +++ b/src/Fantomas.Core/ASTTransformer.fs @@ -1573,6 +1573,18 @@ let (|PatParameter|_|) (p: SynPat) = let mkUnit (StartEndRange 1 (lpr, m, rpr)) = UnitNode(stn "(" lpr, stn ")" rpr, m) +let mkTuplePat (creationAide: CreationAide) (pats: SynPat list) (commas: range list) (m: range) = + match pats with + | [] -> failwith "SynPat.Tuple with no elements" + | head :: tail -> + let rest = + assert (tail.Length = commas.Length) + + List.zip commas tail + |> List.collect (fun (c, e) -> [ yield Choice2Of2(stn "," c); yield Choice1Of2(mkPat creationAide e) ]) + + PatTupleNode([ yield Choice1Of2(mkPat creationAide head); yield! rest ], m) + let mkPat (creationAide: CreationAide) (p: SynPat) = let patternRange = p.Range @@ -1650,8 +1662,8 @@ let mkPat (creationAide: CreationAide) (p: SynPat) = | SynPat.Paren(p, StartEndRange 1 (lpr, _, rpr)) -> PatParenNode(stn "(" lpr, mkPat creationAide p, stn ")" rpr, patternRange) |> Pattern.Paren - | SynPat.Tuple(false, ps, _) -> PatTupleNode(List.map (mkPat creationAide) ps, patternRange) |> Pattern.Tuple - | SynPat.Tuple(true, ps, _) -> + | SynPat.Tuple(false, ps, commas, _) -> mkTuplePat creationAide ps commas patternRange |> Pattern.Tuple + | SynPat.Tuple(true, ps, _, _) -> PatStructTupleNode(List.map (mkPat creationAide) ps, patternRange) |> Pattern.StructTuple | SynPat.ArrayOrList(isArray, ps, range) -> @@ -1862,7 +1874,8 @@ let mkExternBinding let identifier, openNode, parameters, closeNode = match pat with | SynPat.LongIdent( - longDotId = longDotId; argPats = SynArgPats.Pats [ SynPat.Tuple(_, ps, StartEndRange 1 (mOpen, _, mClose)) ]) -> + longDotId = longDotId + argPats = SynArgPats.Pats [ SynPat.Tuple(_, ps, _, StartEndRange 1 (mOpen, _, mClose)) ]) -> mkSynLongIdent longDotId, stn "(" mOpen, List.map mkExternPat ps, stn ")" mClose | _ -> failwith "expecting a SynPat.LongIdent for extern binding" @@ -2255,6 +2268,28 @@ let mkSynUnionCase fullRange ) +let mkSynSimplePat creationAide (pat: SynSimplePat) = + match pat with + | SynSimplePat.Attrib(SynSimplePat.Typed(SynSimplePat.Id(ident = ident; isOptional = isOptional), t, _), + attributes, + m) -> + Some( + SimplePatNode( + mkAttributes creationAide attributes, + isOptional, + mkIdent ident, + Some(mkType creationAide t), + m + ) + ) + | SynSimplePat.Typed(SynSimplePat.Id(ident = ident; isOptional = isOptional), t, m) -> + Some(SimplePatNode(mkAttributes creationAide [], isOptional, mkIdent ident, Some(mkType creationAide t), m)) + | SynSimplePat.Attrib(SynSimplePat.Id(ident = ident; isOptional = isOptional), attributes, m) -> + Some(SimplePatNode(mkAttributes creationAide attributes, isOptional, mkIdent ident, None, m)) + | SynSimplePat.Id(ident = ident; isOptional = isOptional; range = m) -> + Some(SimplePatNode(mkAttributes creationAide [], isOptional, mkIdent ident, None, m)) + | _ -> None + let mkImplicitCtor creationAide vis @@ -2263,43 +2298,30 @@ let mkImplicitCtor (self: (range * Ident) option) (xmlDoc: PreXmlDoc) = - let openNode, closeNode = + let openNode, pats, commas, closeNode = match pats with - | SynSimplePats.SimplePats(range = StartEndRange 1 (mOpen, _, mClose)) - | SynSimplePats.Typed(range = StartEndRange 1 (mOpen, _, mClose)) -> stn "(" mOpen, stn ")" mClose + | SynSimplePats.SimplePats(pats = pats; commaRanges = commas; range = StartEndRange 1 (mOpen, _, mClose)) -> + stn "(" mOpen, pats, commas, stn ")" mClose let pats = match pats with - | SynSimplePats.SimplePats(pats = pats) -> pats - | SynSimplePats.Typed _ -> [] - |> List.choose (function - | SynSimplePat.Attrib(SynSimplePat.Typed(SynSimplePat.Id(ident = ident; isOptional = isOptional), t, _), - attributes, - m) -> - Some( - SimplePatNode( - mkAttributes creationAide attributes, - isOptional, - mkIdent ident, - Some(mkType creationAide t), - m - ) - ) - | SynSimplePat.Typed(SynSimplePat.Id(ident = ident; isOptional = isOptional), t, m) -> - Some( - SimplePatNode( - mkAttributes creationAide [], - isOptional, - mkIdent ident, - Some(mkType creationAide t), - m - ) - ) - | SynSimplePat.Attrib(SynSimplePat.Id(ident = ident; isOptional = isOptional), attributes, m) -> - Some(SimplePatNode(mkAttributes creationAide attributes, isOptional, mkIdent ident, None, m)) - | SynSimplePat.Id(ident = ident; isOptional = isOptional; range = m) -> - Some(SimplePatNode(mkAttributes creationAide [], isOptional, mkIdent ident, None, m)) - | _ -> None) + | [] -> + // Unit pattern + [] + | head :: tail -> + let rest = + assert (tail.Length = commas.Length) + + List.zip commas tail + |> List.collect (fun (c, p) -> + match mkSynSimplePat creationAide p with + | None -> [] + | Some simplePat -> [ Choice2Of2(stn "," c); Choice1Of2 simplePat ]) + + [ match mkSynSimplePat creationAide head with + | None -> () + | Some simplePat -> yield Choice1Of2 simplePat + yield! rest ] let range = let startRange = @@ -2540,18 +2562,25 @@ let mkPropertyGetSetBinding let pats = match ps with - | [ SynPat.Tuple(false, [ p1; p2; p3 ], _) ] -> + | [ SynPat.Tuple(false, [ p1; p2; p3 ], [ comma ], _) ] -> let mTuple = unionRanges p1.Range p2.Range [ PatParenNode( stn "(" Range.Zero, - Pattern.Tuple(PatTupleNode([ mkPat creationAide p1; mkPat creationAide p2 ], mTuple)), + Pattern.Tuple( + PatTupleNode( + [ Choice1Of2(mkPat creationAide p1) + Choice2Of2(stn "," comma) + Choice1Of2(mkPat creationAide p2) ], + mTuple + ) + ), stn ")" Range.Zero, mTuple ) |> Pattern.Paren mkPat creationAide p3 ] - | [ SynPat.Tuple(false, [ p1; p2 ], _) ] -> [ mkPat creationAide p1; mkPat creationAide p2 ] + | [ SynPat.Tuple(false, [ p1; p2 ], _, _) ] -> [ mkPat creationAide p1; mkPat creationAide p2 ] | ps -> List.map (mkPat creationAide) ps let range = unionRanges extraIdent.idRange e.Range diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index c1ea86dbff..8b762e8139 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -398,12 +398,12 @@ let genExpr (e: Expr) = +> sepSpace +> genExpr node.Arguments |> genNode node - | Expr.Tuple node -> genTuple node + | Expr.Tuple node -> genTupleExpr node | Expr.StructTuple node -> genSingleTextNode node.Struct +> sepSpace +> sepOpenT - +> genTuple node.Tuple + +> genTupleExpr node.Tuple +> genSingleTextNode node.ClosingParen |> genNode node | Expr.ArrayOrList node -> fun ctx -> genArrayOrList (ctx.Config.MultilineBracketStyle = Cramped) node ctx @@ -1833,7 +1833,7 @@ let genMultilineFunctionApplicationArguments (argExpr: Expr) = | _ -> genExpr parenNode.Expr |> argsInsideParenthesis parenNode | _ -> genExpr argExpr -let genTuple (node: ExprTupleNode) = +let genTupleExpr (node: ExprTupleNode) = // if a tuple element is an InfixApp with a lambda or if-then-else expression on the rhs, // we need to wrap the rhs in parenthesis to avoid a parse error caused by the higher precedence of "," over the rhs expression. // see 2819 @@ -2464,6 +2464,23 @@ let genTyparDecls (td: TyparDecls) = |> genNode node | TyparDecls.SinglePrefix node -> genTyparDecl true node +let genTuplePatLong (node: PatTupleNode) = + let padUntilAtCurrentColumn ctx = + addFixedSpaces ctx.WriterModel.AtColumn ctx + + col padUntilAtCurrentColumn node.Items (function + | Choice1Of2 p -> genPat p + | Choice2Of2 comma -> genSingleTextNode comma +> sepNln) + +let genTuplePat (node: PatTupleNode) = + let short = + col sepNone node.Items (function + | Choice1Of2 p -> genPat p + | Choice2Of2 comma -> genSingleTextNode comma +> addSpaceIfSpaceAfterComma) + + atCurrentColumn (expressionFitsOnRestOfLine short (atCurrentColumn (genTuplePatLong node))) + |> genNode node + let genPat (p: Pattern) = match p with | Pattern.OptionalVal n -> genSingleTextNode n @@ -2532,14 +2549,7 @@ let genPat (p: Pattern) = +> genPat node.Pattern +> genSingleTextNode node.ClosingParen |> genNode node - | Pattern.Tuple node -> - let padUntilAtCurrentColumn ctx = - addFixedSpaces ctx.WriterModel.AtColumn ctx - - expressionFitsOnRestOfLine - (col sepComma node.Patterns genPat) - (atCurrentColumn (col (padUntilAtCurrentColumn +> sepComma +> sepNln) node.Patterns genPat)) - |> genNode node + | Pattern.Tuple node -> genTuplePat node | Pattern.StructTuple node -> !- "struct " +> sepOpenT @@ -2745,9 +2755,7 @@ let genBinding (b: BindingNode) (ctx: Context) : Context = match parenNode.Pattern with | Pattern.Tuple tupleNode -> (genSingleTextNode parenNode.OpeningParen - +> indentSepNlnUnindent ( - (col (sepComma +> sepNln) tupleNode.Patterns genPat) |> genNode tupleNode - ) + +> indentSepNlnUnindent (genTuplePatLong tupleNode |> genNode tupleNode) +> sepNln +> genSingleTextNode parenNode.ClosingParen |> genNode parenNode), @@ -3158,11 +3166,18 @@ let genImplicitConstructor (node: ImplicitConstructorNode) = +> optSingle (fun t -> sepColon +> autoIndentAndNlnIfExpressionExceedsPageWidth (genType t)) node.Type |> genNode node - let shortPats = col sepComma node.Parameters genSimplePat + let shortPats = + col sepNone node.Items (function + | Choice1Of2 p -> genSimplePat p + | Choice2Of2 comma -> genSingleTextNode comma +> addSpaceIfSpaceAfterComma) let longPats = - indentSepNlnUnindent (col (sepComma +> sepNln) node.Parameters genSimplePat) - +> sepNln + let genPats = + col sepNone node.Items (function + | Choice1Of2 p -> genSimplePat p + | Choice2Of2 comma -> genSingleTextNode comma +> sepNln) + + indentSepNlnUnindent genPats +> sepNln let short = genXml node.XmlDoc diff --git a/src/Fantomas.Core/SyntaxOak.fs b/src/Fantomas.Core/SyntaxOak.fs index 4ff3c525b9..09d3f54b58 100644 --- a/src/Fantomas.Core/SyntaxOak.fs +++ b/src/Fantomas.Core/SyntaxOak.fs @@ -502,11 +502,16 @@ type PatParenNode(openingParen: SingleTextNode, pat: Pattern, closingParen: Sing member val Pattern = pat member val ClosingParen = closingParen -type PatTupleNode(pats: Pattern list, range) = +type PatTupleNode(items: Choice list, range) = inherit NodeBase(range) - override val Children: Node array = [| yield! (List.map Pattern.Node pats) |] - member val Patterns = pats + override val Children: Node array = + [| for item in items do + match item with + | Choice1Of2 p -> Pattern.Node p + | Choice2Of2 comma -> comma |] + + member val Items = items type PatStructTupleNode(pats: Pattern list, range) = inherit NodeBase(range) @@ -2229,7 +2234,7 @@ type ImplicitConstructorNode attributes: MultipleAttributeListNode option, accessibility: SingleTextNode option, openingParen: SingleTextNode, - parameters: SimplePatNode list, + items: Choice list, closingParen: SingleTextNode, self: AsSelfIdentifierNode option, range @@ -2241,7 +2246,10 @@ type ImplicitConstructorNode yield! noa attributes yield! noa accessibility yield openingParen - yield! nodes parameters + for item in items do + match item with + | Choice1Of2 node -> yield node + | Choice2Of2 comma -> yield comma yield closingParen yield! noa self |] @@ -2249,7 +2257,7 @@ type ImplicitConstructorNode member val Attributes = attributes member val Accessibility = accessibility member val OpeningParen = openingParen - member val Parameters = parameters + member val Items = items member val ClosingParen = closingParen member val Self = self