fsprojects / fantomas

FSharp source code formatter
https://fsprojects.github.io/fantomas
Other
770 stars 191 forks source link

Idempotency problem when reformatting type declaration with large when clause #2896

Closed etareduction closed 1 year ago

etareduction commented 1 year ago

Issue created from fantomas-online

Input code

let inline func
    (arg:
        'a when 'a: (member a: int) and 'a: (member b: int) and 'a: (member c: int) and 'a: (member d: int) and 'a: (member e: int))
    = 0

Formatted code

let inline func
    (arg:
        'a

                when 'a: (member a: int)
                and 'a: (member b: int)
                and 'a: (member c: int)
                and 'a: (member d: int)
                and 'a: (member e: int))
    =
    0

Reformatted code

let inline func
    (arg:
        'a

                when

                'a: (member a: int)
                and 'a: (member b: int)
                and 'a: (member c: int)
                and 'a: (member d: int)
                and 'a: (member e: int))
    =
    0

Reformatted again

let inline func
    (arg:
        'a

                when

                'a: (member a: int)
                and 'a: (member b: int)
                and 'a: (member c: int)
                and 'a: (member d: int)
                and 'a: (member e: int))
    =
    0

Problem description

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

Extra information

Options

Fantomas v6.1 branch at 1/1/1990

Default Fantomas configuration

Did you know that you can ignore files when formatting by using a .fantomasignore file? PS: It's unlikely that someone else will solve your specific issue, as it's something that you have a personal stake in.

nojaf commented 1 year ago

Hello, thank you for reporting this issue.

I think: https://github.com/fsprojects/fantomas/blob/f95faa9845031b1c49fd7d5ab5deb80dda33efe1/src/Fantomas.Core/CodePrinter.fs#L3065-L3071

can probably be simplified to:

    | Type.WithGlobalConstraints node ->
        genType node.Type +> genTypeConstraints node.TypeConstraints
        |> genNode node

genTypeConstraints already seems to deal with the multilineness of things.

Are you interested in submitting a PR?

etareduction commented 1 year ago

Are you interested in submitting a PR?

I can look into it in a few hours, but I'm not familiar with the codebase.

nojaf commented 1 year ago

We have quite some documentation on the codebase at https://fsprojects.github.io/fantomas/docs/contributors/Index.html

I think in this case, you can follow the pull request ground rules, add a test in src/Fantomas.Core.Tests/FunctionDefinitionTests.fs and see if the suggestion works out. Maybe it does and that is all there is to it.

etareduction commented 1 year ago

Maybe it does and that is all there is to it.

No, apparently it doesn't. And i can't even run or debug it in Rider, only in the CLI with this build.fsx script which has a lot of extra output and strips off most of the provided/expected strings in test output.

Edit: Also if i just run dotnet test without the aforementioned buildscript, it says source file src\Fantomas.FCS\obj\Debug\netstandard2.0\FSComp.fsi not found

nojaf commented 1 year ago

I'm guessing you don't have the right preview SDK on your machine. Can you run dotnet --version please? If that indeed is 7.0.400-preview.23272.51 or higher, did dotnet fsi build.fsx -p Init work for you? If so you should be able to build the solution.

etareduction commented 1 year ago

I'm guessing you don't have the right preview SDK on your machine. Can you run dotnet --version please? If that indeed is 7.0.400-preview.23272.51 or higher, did dotnet fsi build.fsx -p Init work for you? If so you should be able to build the solution.

I've installed the preview sdk according to instructions in the docs you've linked to me. The thing i missed is dotnet fsi build.fsx -p Init. It seems to be missing in the getting started, and only exists in the last chapter of contributors guide. I'll try to run it later today.

etareduction commented 1 year ago

If so you should be able to build the solution.

Just tried running dotnet fsi build.fsx -p Init. It runs successfully, but doesn't change anything. dotnet --version is 7.0.400-preview.23305.1

dawedawe commented 1 year ago

Mmh, that's strange. The dotnet restore should take care of that iircc.

I'm guessing you don't have the right preview SDK on your machine. Can you run dotnet --version please? If that indeed is 7.0.400-preview.23272.51 or higher, did dotnet fsi build.fsx -p Init work for you? If so you should be able to build the solution.

I've installed the preview sdk according to instructions in the docs you've linked to me. The thing i missed is dotnet fsi build.fsx -p Init. It seems to be missing in the getting started, and only exists in the last chapter of contributors guide. I'll try to run it later today.

Mmh, that's strange. The dotnet restore should take care of that iircc.

dawedawe commented 1 year ago

If so you should be able to build the solution.

Just tried running dotnet fsi build.fsx -p Init. It runs successfully, but doesn't change anything. dotnet --version is 7.0.400-preview.23305.1

What change did you expect? Did you do a normal build afterwards? dotnet fsi build.fsx

etareduction commented 1 year ago

@dawedawe

What change did you expect? Did you do a normal build afterwards? dotnet fsi build.fsx

Yeah, i did. The change I'm expecting is that dotnet test and test runner in Rider start working. Currently they still spit out the same error I've mentioned above.

dawedawe commented 1 year ago

Okay, maybe let's start all over again. The following steps work for me:

mkdir fantomastest
cd .\fantomastest\
gh repo clone fsprojects/fantomas .
dotnet tool restore
dotnet restore
dotnet fsi .\build.fsx

After that, I'm also able to do a dotnet test in the root folder and the test support in Rider works.

etareduction commented 1 year ago

After that, I'm also able to do a dotnet test in the root folder and the test support in Rider works.

Yeah, I've just figured out that running build script and then dotnet clean followed by another dotnet restore fixes the problem.

etareduction commented 1 year ago

@nojaf When i was able to look into the output string more closely it became clear that suggested change fixes the problem quite well. I'll open up a PR with that.