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

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

Closed
1 of 4 tasks
etareduction opened this issue Jun 5, 2023 · 13 comments · Fixed by #2897
Closed
1 of 4 tasks

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

etareduction opened this issue Jun 5, 2023 · 13 comments · Fixed by #2897

Comments

@etareduction
Copy link
Contributor

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

  • The formatted result breaks my code.
  • The formatted result gives compiler warnings.
  • I or my company would be willing to help fix this.
  • I would like a release if this problem is solved.

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
Copy link
Contributor

nojaf commented Jun 5, 2023

Hello, thank you for reporting this issue.

I think:

| Type.WithGlobalConstraints node ->
leadingExpressionIsMultiline (genType node.Type) (fun isMultiline ->
if isMultiline then
indentSepNlnUnindent (genTypeConstraints node.TypeConstraints)
else
sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidth (genTypeConstraints node.TypeConstraints))
|> genNode node

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
Copy link
Contributor Author

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
Copy link
Contributor

nojaf commented Jun 5, 2023

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
Copy link
Contributor Author

etareduction commented Jun 5, 2023

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
Copy link
Contributor

nojaf commented Jun 5, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

dawedawe commented Jun 6, 2023

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
Copy link
Member

dawedawe commented Jun 6, 2023

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
Copy link
Contributor Author

@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
Copy link
Member

dawedawe commented Jun 6, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

@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.

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