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

Use of A | B as c fails to format correctly #2289

Closed
3 tasks
dsyme opened this issue Jun 3, 2022 · 2 comments
Closed
3 tasks

Use of A | B as c fails to format correctly #2289

dsyme opened this issue Jun 3, 2022 · 2 comments
Labels
bug (stylistic) good first issue Long hanging fruit: easy issue to get your feet wet!

Comments

@dsyme
Copy link
Contributor

dsyme commented Jun 3, 2022

Found this while formatting the F# compiler. It's not a blocker, see workaround below

Issue created from fantomas-online

Code

type T = A | B

let f a  =
    match a with
    | (A | B as bi, x) ->
        1

Result

type T =
    | A
    | B

let f a =
    match a with
    | (A
       | B as bi,
       label1) -> 1

Problem description

Please describe here the Fantomas problem you encountered.
Check out our Contribution Guidelines.

Workaround

The workaround is to parenthesize, e.g.

type T = A | B

let f a  =
    match a with
    | ((A | B) as bi, x) ->
        1

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.

Options

Fantomas master branch at 2022-05-30T18:11:41Z - 5e92752

Default Fantomas configuration

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

@nojaf
Copy link
Contributor

nojaf commented Jun 7, 2022

Hello, this rings a bell.
If I recall correctly, the main problem is that we format each SynPat.Or inside a clause slightly differently using ASTContext.
ASTContext is evil, however:

/// This type consists of contextual information which is important for formatting
/// Please avoid using this record as it can be the cause of unexpected behavior when used incorrectly
type ASTContext =

I think this problem could be solved by removing that record field and trying to format SynPat.Or differently when encountered in the pattern of a clause handler.

Are you interested in submitting a PR for this?

@nojaf
Copy link
Contributor

nojaf commented Aug 8, 2022

I believe this problem is addressed due to the refactor of ASTContext in a7f5b3e.

@nojaf nojaf added the good first issue Long hanging fruit: easy issue to get your feet wet! label Aug 8, 2022
@nojaf nojaf closed this as completed Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug (stylistic) good first issue Long hanging fruit: easy issue to get your feet wet!
Projects
None yet
Development

No branches or pull requests

2 participants