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

Line of code including a backward composition operator causes error #1998

Closed
thomasorton-i2o opened this issue Dec 21, 2021 · 5 comments
Closed
Labels
bug (soundness) good first issue Long hanging fruit: easy issue to get your feet wet!

Comments

@thomasorton-i2o
Copy link

The following line within a block of code causes Fantomas to throw an error:

(SomeModule.doSomething << SomeModule.doSomethingElse) (fun x -> x)

The error starts with "System.Exception: cannot determine if synExpr Paren". I have reproduced it in the in the Fantomas online tool.

(I tried to use the "Create an issue" button but I got a GitHub "Your request URL is too long. " error.)

@nojaf nojaf added bug (soundness) good first issue Long hanging fruit: easy issue to get your feet wet! labels Dec 21, 2021
@nojaf
Copy link
Contributor

nojaf commented Dec 21, 2021

Hello, thank you for reporting this issue.

I believe the problem is occurring around:

// functionName arg1 arg2 (fun x y z -> ...)
| AppWithLambda (e, es, lpr, lambda, rpr, pr) ->
let sepSpaceAfterFunctionName ctx =
match List.tryHead es with
| Some (SimpleExpr _) -> sepSpace ctx
| _ ->
match e with
| UppercaseSynExpr -> onlyIf ctx.Config.SpaceBeforeUppercaseInvocation sepSpace ctx
| LowercaseSynExpr -> onlyIf ctx.Config.SpaceBeforeLowercaseInvocation sepSpace ctx

We try to see if the (fun x -> x) is uppercase or lowercase. I think in this case, we should just add a space between both the function expression and the argument expression have parenthesis.

A potential fix might be something like:

            let sepSpaceAfterFunctionName ctx =
                match e with
                | Paren _ -> sepSpace ctx
                | _ ->
                    match List.tryHead es with
                    | Some (SimpleExpr _) -> sepSpace ctx
                    | _ ->
                        match e with
                        | UppercaseSynExpr -> onlyIf ctx.Config.SpaceBeforeUppercaseInvocation sepSpace ctx
                        | LowercaseSynExpr -> onlyIf ctx.Config.SpaceBeforeLowercaseInvocation sepSpace ctx

Are you interested in submitting a PR for this?

@thomasorton-i2o
Copy link
Author

Hello,

No problem. I would be happy to submit a PR, I'm just a bit busy at the moment. I will try to submit one tomorrow if that is OK.

@nojaf
Copy link
Contributor

nojaf commented Dec 23, 2021

Sure thing, take your time. Thanks!
Please review our Contribution Guidelines and don't hesitate to ask any questions you might have.
Good luck!

@nojaf
Copy link
Contributor

nojaf commented Jan 25, 2022

I believe this issue was resolved by #2036.
To close this issue, we can add a regression test to be sure.

@knocte
Copy link
Contributor

knocte commented Feb 3, 2022

This can be closed now after the merge of #2060 FYI

@nojaf nojaf closed this as completed Feb 3, 2022
jindraivanek pushed a commit to jindraivanek/fantomas that referenced this issue Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug (soundness) good first issue Long hanging fruit: easy issue to get your feet wet!
Projects
None yet
Development

No branches or pull requests

3 participants