Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiline AbstractSlot without constraints introduces newline #2175

Closed
3 tasks
MangelMaxime opened this issue Mar 31, 2022 · 3 comments · Fixed by #2183
Closed
3 tasks

Multiline AbstractSlot without constraints introduces newline #2175

MangelMaxime opened this issue Mar 31, 2022 · 3 comments · Fixed by #2183

Comments

@MangelMaxime
Copy link

When trying to format some code using Fantomas I got an Idempotency warning.

I was able to narrow it to these code:

The code can looks strange but that's because it is a Fable binding

Issue created from fantomas-online

Formatted code

// ts2fable 0.8.0
module rec Glutinum.Fuse

#nowarn "3390" // disable warnings for invalid XML comments

open System
open Fable.Core
open Fable.Core.JS

module Fuse =

    [<AllowNullLiteral>]
    type FuseSortFunctionItem =
        [<EmitIndexer>]
        abstract Item: key: string -> U2<{| ``$``: string |}, ResizeArray<{| ``$``: string; idx: float |}>> with get, set


    [<AllowNullLiteral>]
    [<Global>]
    type FuseOptionKeyObject [<ParamObject; Emit("$0")>] (name: U2<string, ResizeArray<string>>, weight: float) =
        member val name: U2<string, ResizeArray<string>> = jsNative with get, set
        member val weight: float = jsNative with get, set

Reformatted code

// ts2fable 0.8.0
module rec Glutinum.Fuse

#nowarn "3390" // disable warnings for invalid XML comments

open System
open Fable.Core
open Fable.Core.JS

module Fuse =

    [<AllowNullLiteral>]
    type FuseSortFunctionItem =
        [<EmitIndexer>]
        abstract Item: key: string -> U2<{| ``$``: string |}, ResizeArray<{| ``$``: string; idx: float |}>> with get, set



    [<AllowNullLiteral>]
    [<Global>]
    type FuseOptionKeyObject [<ParamObject; Emit("$0")>] (name: U2<string, ResizeArray<string>>, weight: float) =
        member val name: U2<string, ResizeArray<string>> = jsNative with get, set
        member val weight: float = jsNative with get, set

Problem description

Fantomas was not able to produce the same code after reformatting the result.

Extra information

  • The formatted result breaks by code.
  • The formatted result gives compiler warnings.
  • I or my company would be willing to help fix this.

Options

Fantomas master branch at 2022-03-29T09:12:14Z - 8f2ab14

    { config with
                MaxArrayOrListWidth = 5
                MultilineBlockBracketsOnSameColumn = true }

Did you know that you can ignore files when formatting from fantomas-tool or the FAKE targets by using a .fantomasignore file?

@baronfel
Copy link
Contributor

This took me a minute to spot, but the warning is coming from the addition of another newline in between the definitions of FuseSortFunctionItem and FuseOptionKeyObject.

@nojaf nojaf changed the title Idempotency problem reported by Fantomas Newline trivia not properly restored between two SynModuleDecl_Types Apr 1, 2022
@nojaf nojaf changed the title Newline trivia not properly restored between two SynModuleDecl_Types Multiline AbstractSlot without constraints introduces newline Apr 1, 2022
@nojaf
Copy link
Contributor

nojaf commented Apr 1, 2022

Hello Maxime, thank you for reporting this issue.
I was able to reduce the problem to

[<Test>]
let ``multiline abstract member without constraints, 2175`` () =
    formatSourceString
        false
        """
    type FuseSortFunctionItem =
        abstract Item: key: string -> U2<{| ``$``: string |}, ResizeArray<{| ``$``: string; idx: float |}>> with get, set
        abstract X : int
"""
        { config with MaxLineLength = 60 }
    |> prepend newline
    |> should
        equal
        """
type FuseSortFunctionItem =
    abstract Item:
        key: string ->
            U2<{| ``$``: string |}, ResizeArray<{| ``$``: string
                                                   idx: float |}>> with get, set

    abstract X: int
"""

The AbstractSlot does not have any constraints but is already crossing the max line length.
This will trigger:

genPreXmlDoc px
+> genAttributes astContext ats
+> opt sepSpace ao genAccess
+> genMemberFlags mf
+> !-s
+> genTypeParamPostfix astContext tds
+> ifElse hasGenerics sepColonWithSpacesFixed sepColon
+> autoIndentAndNlnIfExpressionExceedsPageWidth (genTypeList astContext namedArgs)
-- genPropertyKind (not isFunctionProperty) mf.MemberKind
+> autoIndentAndNlnIfExpressionExceedsPageWidth (genConstraints astContext t vi)

to add a new line.

It can be resolved by adding an additional check like:

        +> onlyIf
            (match t with
             | TWithGlobalConstraints _ -> true
             | _ -> false)
            autoIndentAndNlnIfExpressionExceedsPageWidth
            (genConstraints astContext t vi)

Are you interested in submitting a PR for this?

@MangelMaxime
Copy link
Author

Hello @nojaf,

thank you for the clear explanations.

Are you interested in submitting a PR for this?

I don't think I will have the time to look into the issue as I have a lot of stuff on my side to handle already. And learning about Fantomas internal, will probably takes me some times ^^ sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants