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

Invalid location of comma when using named parameters for a class #2865

Closed
2 of 3 tasks
MNie opened this issue Apr 24, 2023 · 2 comments · Fixed by #3095
Closed
2 of 3 tasks

Invalid location of comma when using named parameters for a class #2865

MNie opened this issue Apr 24, 2023 · 2 comments · Fixed by #3095

Comments

@MNie
Copy link

MNie commented Apr 24, 2023

Issue created from fantomas-online

Code

type MyCSharpClass() =
    member val Prop = 0.0m with get, set
    member val Prop2 = 0.0m with get, set

type FSharpType = {
    SomeValue: string option
}

let v = { SomeValue = Some "str" }

let instance = 
    MyClass(
        Prop = 
            match value.SomeValue with 
            | Some _ -> 0.0m
            | None -> 0.0m
        ,
        Prop2 =
            match value.SomeValue with
            | Some _ -> 0.0m
            | None -> 0.0m
    )

Result

type MyCSharpClass() =
    member val Prop = 0.0m with get, set
    member val Prop2 = 0.0m with get, set

type FSharpType = { SomeValue: string option }

let v = { SomeValue = Some "str" }

let instance =
    MyClass(
        Prop =
            match value.SomeValue with
            | Some _ -> 0.0m
            | None -> 0.0m,
        Prop2 =
            match value.SomeValue with
            | Some _ -> 0.0m
            | None -> 0.0m
    )

Problem description

It seems that when we have multiple match statements passed to named parameters for a class it places the , in the wrong place. That results in a dotnet fantomas error.

Known workarounds are:

  1. surround the match statement with (),
let instance = 
    MyClass(
        (Prop = 
            match value.SomeValue with 
            | Some _ -> 0.0m
            | None -> 0.0m)
        ,
        (Prop2 =
            match value.SomeValue with
            | Some _ -> 0.0m
            | None -> 0.0m)
    )
  1. extract the match statement to an explicit variable.
let instance = 
    let v1 =
            match value.SomeValue with 
            | Some _ -> 0.0m
            | None -> 0.0m
    let v2 =
            match value.SomeValue with 
            | Some _ -> 0.0m
            | None -> 0.0m
    MyClass(
        Prop = v1,
        Prop2 = v2
    )

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 main branch at 2023-04-20T16:04:28Z - 4ed1e9d

Default Fantomas configuration

  • Fantomas v6,
  • .NET6
@nojaf
Copy link
Contributor

nojaf commented Apr 24, 2023

Hello,

Thank you for reporting this issue!
I think you could extend this check to include your use-case:

let containsLambdaOrMatchExpr =
// If the any items (expect the last) is a match/lambda
node.Items
|> List.chunkBySize 2
|> List.exists (fun pair ->
match pair with
| [ Choice1Of2 e; Choice2Of2 _ ] ->
match e with
| Expr.Match _
| Expr.Lambda _ -> true
| Expr.InfixApp node ->
match node.RightHandSide with
| Expr.Lambda _ -> true
| _ -> false
| Expr.SameInfixApps node ->
match List.last node.SubsequentExpressions with
| _, Expr.Lambda _ -> true
| _ -> false
| _ -> false
| _ -> false)

Are you interested in submitting a PR for this?

@MNie
Copy link
Author

MNie commented Apr 24, 2023

@nojaf will try to do so at the end of this week : )

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.

2 participants