From cabbcf475f34d145252a7347d9037c7f59a78b6c Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Fri, 7 Oct 2022 21:23:56 +0200 Subject: [PATCH] Change style of base constructor calls. (#2555) * Change style of base constructor calls. * Add changelog entry for 5.1.0-alpha-005. --- CHANGELOG.md | 6 +- .../BaseConstructorTests.fs | 68 +++++++++++++++++ src/Fantomas.Core.Tests/ClassTests.fs | 8 +- .../Fantomas.Core.Tests.fsproj | 1 + ...ineBlockBracketsOnSameColumnRecordTests.fs | 56 +++++++++++++- src/Fantomas.Core/CodePrinter.fs | 76 +++++++------------ src/Fantomas.Core/SourceParser.fs | 17 ++++- 7 files changed, 174 insertions(+), 58 deletions(-) create mode 100644 src/Fantomas.Core.Tests/BaseConstructorTests.fs diff --git a/CHANGELOG.md b/CHANGELOG.md index e4c215194a..fee9e0fff7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,16 @@ # Changelog -## [Unreleased] +## [5.1.0-alpha-005] - 2022-10-07 ### Changed * Control space in pattern by `fsharp_space_before_lowercase_invocation` and `fsharp_space_before_uppercase_invocation`. [fslang-design/issues/712](https://github.com/fsharp/fslang-design/issues/712) +* Style of base constructor calls. [fsharp/fslang-design#693](https://github.com/fsharp/fslang-design/issues/693) +* Style of multiline type annotations/ [fsharp/fslang-design#708](https://github.com/fsharp/fslang-design/issues/708) ### Fixed * Comments in SynArgPats.NamePatPairs are lost. [#2541](https://github.com/fsprojects/fantomas/issues/2541) +* Vanity alignment used inside base ctor call. [#2111](https://github.com/fsprojects/fantomas/issues/2111) +* Add line break before start of argument list. [#2335](https://github.com/fsprojects/fantomas/issues/2335) ## [5.1.0-alpha-004] - 2022-10-07 diff --git a/src/Fantomas.Core.Tests/BaseConstructorTests.fs b/src/Fantomas.Core.Tests/BaseConstructorTests.fs new file mode 100644 index 0000000000..5a9dab2de0 --- /dev/null +++ b/src/Fantomas.Core.Tests/BaseConstructorTests.fs @@ -0,0 +1,68 @@ +module Fantomas.Core.Tests.BaseConstructorTests + +open NUnit.Framework +open FsUnit +open Fantomas.Core.Tests.TestHelper + +[] +let ``multiple base constructors in record, 2111`` () = + formatSourceString + false + """ +type UnhandledWebException = + inherit Exception + + new(status: WebExceptionStatus, innerException: Exception) = + { inherit Exception(SPrintF1 + "Backend not prepared for this WebException with Status[%i]" + (int status), + innerException) } + + new(info: SerializationInfo, context: StreamingContext) = + { inherit Exception(info, context) } +""" + { config with MaxLineLength = 100 } + |> prepend newline + |> should + equal + """ +type UnhandledWebException = + inherit Exception + + new(status: WebExceptionStatus, innerException: Exception) = + { inherit + Exception( + SPrintF1 "Backend not prepared for this WebException with Status[%i]" (int status), + innerException + ) } + + new(info: SerializationInfo, context: StreamingContext) = { inherit Exception(info, context) } +""" + +[] +let ``single multiline base constructor, 2335`` () = + formatSourceString + false + """ +type FieldNotFoundException<'T>(obj:'T, field:string, specLink:string) = + inherit SwaggerSchemaParseException( + sprintf "Object MUST contain field `%s` (See %s for more details).\nObject:%A" + field specLink obj) +""" + { config with + SpaceBeforeClassConstructor = true + MaxLineLength = 90 } + |> prepend newline + |> should + equal + """ +type FieldNotFoundException<'T> (obj: 'T, field: string, specLink: string) = + inherit + SwaggerSchemaParseException ( + sprintf + "Object MUST contain field `%s` (See %s for more details).\nObject:%A" + field + specLink + obj + ) +""" diff --git a/src/Fantomas.Core.Tests/ClassTests.fs b/src/Fantomas.Core.Tests/ClassTests.fs index 7615d9ce86..add95b86d1 100644 --- a/src/Fantomas.Core.Tests/ClassTests.fs +++ b/src/Fantomas.Core.Tests/ClassTests.fs @@ -954,8 +954,8 @@ type public DerivedExceptionWithLongNaaaaaaaaameException originalRequest: string, originalResponse: string ) = - inherit BaseExceptionWithLongNaaaameException - ( + inherit + BaseExceptionWithLongNaaaameException( message, code, originalRequest, @@ -991,8 +991,8 @@ type public DerivedExceptionWithLongNaaaaaaaaameException originalRequest: string, originalResponse: string ) = - inherit BaseExceptionWithLongNaaaameException - ( + inherit + BaseExceptionWithLongNaaaameException( message, code, originalRequest, diff --git a/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj b/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj index 50fb391e4c..d8662c0d9d 100644 --- a/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj +++ b/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj @@ -119,6 +119,7 @@ + diff --git a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs index a3e13457bb..ceb877e7ab 100644 --- a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs +++ b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs @@ -151,13 +151,21 @@ let ``record instance with inherit keyword and no fields`` () = """let a = { inherit ProjectPropertiesBase<_>(projectTypeGuids, factoryGuid, targetFrameworkIds, dotNetCoreSDK) } """ - config + { config with MaxLineLength = 80 } |> prepend newline |> should equal """ let a = - { inherit ProjectPropertiesBase<_>(projectTypeGuids, factoryGuid, targetFrameworkIds, dotNetCoreSDK) } + { + inherit + ProjectPropertiesBase<_>( + projectTypeGuids, + factoryGuid, + targetFrameworkIds, + dotNetCoreSDK + ) + } """ [] @@ -177,7 +185,10 @@ let ``type with record instance with inherit keyword`` () = type ServerCannotBeResolvedException = inherit CommunicationUnsuccessfulException - new(message) = { inherit CommunicationUnsuccessfulException(message) } + new(message) = + { + inherit CommunicationUnsuccessfulException(message) + } """ [] @@ -1270,3 +1281,42 @@ let ``comment after equals in anonymous record field`` () = B |} """ + +[] +let ``multiple base constructors in record`` () = + formatSourceString + false + """ +type UnhandledWebException = + inherit Exception + + new(status: WebExceptionStatus, innerException: Exception) = + { inherit Exception(SPrintF1 + "Backend not prepared for this WebException with Status[%i]" + (int status), + innerException) } + + new(info: SerializationInfo, context: StreamingContext) = + { inherit Exception(info, context) } +""" + { config with MaxLineLength = 100 } + |> prepend newline + |> should + equal + """ +type UnhandledWebException = + inherit Exception + + new(status : WebExceptionStatus, innerException : Exception) = + { + inherit + Exception( + SPrintF1 + "Backend not prepared for this WebException with Status[%i]" + (int status), + innerException + ) + } + + new(info : SerializationInfo, context : StreamingContext) = { inherit Exception(info, context) } +""" diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index 72019aa606..084f763bf8 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -1086,11 +1086,9 @@ and genExpr astContext synExpr ctx = let smallRecordExpr = genTriviaFor SynExpr_Record_OpeningBrace openingBrace sepOpenS +> optSingle - (fun (inheritType, inheritExpr) -> + (fun inheritCtor -> !- "inherit " - +> genType astContext inheritType - +> addSpaceBeforeClassConstructor inheritExpr - +> genExpr astContext inheritExpr + +> genInheritConstructor astContext inheritCtor +> onlyIf (List.isNotEmpty xs) sepSemi) inheritOpt +> optSingle (fun e -> genExpr astContext e +> !- " with ") eo @@ -2503,13 +2501,11 @@ and genMultilineRecordInstance let expr = match inheritOpt with - | Some (t, e) -> + | Some inheritCtor -> genTriviaFor SynExpr_Record_OpeningBrace openingBrace sepOpenS +> atCurrentColumn ( !- "inherit " - +> genType astContext t - +> addSpaceBeforeClassConstructor e - +> genExpr astContext e + +> autoIndentAndNlnIfExpressionExceedsPageWidth (genInheritConstructor astContext inheritCtor) +> onlyIf (List.isNotEmpty xs) sepNln +> fieldsExpr +> genTriviaFor SynExpr_Record_ClosingBrace closingBrace sepCloseS @@ -2568,22 +2564,16 @@ and genMultilineRecordInstanceAlignBrackets let hasFields = List.isNotEmpty xs match inheritOpt, eo with - | Some (inheritType, inheritExpr), None -> - genTriviaFor SynExpr_Record_OpeningBrace openingBrace sepOpenS - +> ifElse hasFields (indent +> sepNln) sepNone - +> !- "inherit " - +> genType astContext inheritType - +> addSpaceBeforeClassConstructor inheritExpr - +> genExpr astContext inheritExpr - +> ifElse - hasFields - (sepNln - +> fieldsExpr - +> unindent - +> sepNln - +> genTriviaFor SynExpr_Record_ClosingBrace closingBrace sepCloseSFixed) - (sepSpace +> genTriviaFor SynExpr_Record_ClosingBrace closingBrace sepCloseSFixed) - + | Some inheritCtor, None -> + genTriviaFor SynExpr_Record_OpeningBrace openingBrace sepOpenSFixed + +> indentSepNlnUnindent ( + !- "inherit " + +> autoIndentAndNlnIfExpressionExceedsPageWidth (genInheritConstructor astContext inheritCtor) + +> onlyIf hasFields sepNln + +> fieldsExpr + ) + +> sepNln + +> genTriviaFor SynExpr_Record_ClosingBrace closingBrace sepCloseSFixed | None, Some e -> genTriviaFor SynExpr_Record_OpeningBrace openingBrace sepOpenS +> atCurrentColumnIndent (genExpr astContext e) @@ -2603,6 +2593,18 @@ and genMultilineRecordInstanceAlignBrackets +> ifElseCtx lastWriteEventIsNewline sepNone sepNln +> genTriviaFor SynExpr_Record_ClosingBrace closingBrace sepCloseSFixed) +and genInheritConstructor (astContext: ASTContext) (inheritCtor: SynType * SynExpr) = + match inheritCtor with + | TypeOnlyInheritConstructor t -> genType astContext t + | UnitInheritConstructor t -> genType astContext t +> sepSpaceBeforeClassConstructor +> sepOpenT +> sepCloseT + | ParenInheritConstructor (t, px) -> + genType astContext t + +> sepSpaceBeforeClassConstructor + +> expressionFitsOnRestOfLine (genExpr astContext px) (genMultilineFunctionApplicationArguments astContext px) + | OtherInheritConstructor (t, e) -> + genType astContext t + +> sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidth (genExpr astContext e) + and genMultilineAnonRecord (isStruct: bool) fields copyInfo (astContext: ASTContext) = let recordExpr = match copyInfo with @@ -4222,32 +4224,8 @@ and genMemberDefn astContext node = | MDOpen lid -> !- "open " +> genSynLongIdent false lid // What is the role of so | MDImplicitInherit (t, e, _) -> - let genBasecall = - let shortExpr = genExpr astContext e - - let longExpr = - match e with - | Paren (lpr, Tuple (es, tr), rpr, pr) -> - indent - +> sepNln - +> indent - +> sepOpenTFor lpr - +> sepNln - +> (col (sepComma +> sepNln) es (genExpr astContext) - |> genTriviaFor SynExpr_Tuple tr) - +> unindent - +> sepNln - +> unindent - +> sepCloseTFor rpr - |> genTriviaFor SynExpr_Paren pr - | _ -> genExpr astContext e - - expressionFitsOnRestOfLine shortExpr longExpr - !- "inherit " - +> genType astContext t - +> addSpaceBeforeClassConstructor e - +> genBasecall + +> autoIndentAndNlnIfExpressionExceedsPageWidth (genInheritConstructor astContext (t, e)) | MDInherit (t, _) -> !- "inherit " +> genType astContext t | MDValField f -> genField astContext "val " f diff --git a/src/Fantomas.Core/SourceParser.fs b/src/Fantomas.Core/SourceParser.fs index a6fb450a05..3c5845eb87 100644 --- a/src/Fantomas.Core/SourceParser.fs +++ b/src/Fantomas.Core/SourceParser.fs @@ -1042,10 +1042,25 @@ let rec (|ElIf|_|) = Some([ (None, trivia.IfKeyword, trivia.IsElif, e1, trivia.ThenKeyword, e2) ], elseInfo, range) | _ -> None +let (|TypeOnlyInheritConstructor|UnitInheritConstructor|ParenInheritConstructor|OtherInheritConstructor|) + ( + t: SynType, + e: SynExpr + ) = + match e with + | SynExpr.Const (constant = SynConst.Unit; range = unitRange) -> + // The unit expression could have been added artificially. + if unitRange.StartColumn + 2 = unitRange.EndColumn then + UnitInheritConstructor(t) + else + TypeOnlyInheritConstructor t + | SynExpr.Paren _ as px -> ParenInheritConstructor(t, px) + | _ -> OtherInheritConstructor(t, e) + let (|Record|_|) = function | SynExpr.Record (inheritOpt, eo, xs, StartEndRange 1 (openingBrace, _, closingBrace)) -> - let inheritOpt = inheritOpt |> Option.map (fun (typ, expr, _, _, _) -> (typ, expr)) + let inheritOpt = inheritOpt |> Option.map (fun (t, e, _, _, _) -> t, e) Some(openingBrace, inheritOpt, xs, Option.map fst eo, closingBrace) | _ -> None