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

Unary operators: inconsistent formatting when arg is literal vs variable #3131

Open
4 tasks
pleaseletmesearch opened this issue Oct 28, 2024 · 2 comments · Fixed by #3134
Open
4 tasks

Unary operators: inconsistent formatting when arg is literal vs variable #3131

pleaseletmesearch opened this issue Oct 28, 2024 · 2 comments · Fixed by #3134
Assignees

Comments

@pleaseletmesearch
Copy link

Issue created from fantomas-online

(This is @Smaug123 but from the office.)

Code

let x = ~~1
let y = ~~x

Result

let x = ~~ 1
let y = ~~x

Problem description

The formatting is inconsistent between when the argument of a unary operator is a literal vs a variable. I'd prefer us to stick with ~~1 (removing the space, as we do with the variable case).

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 main branch at 2024-09-14T12:20:28Z - 06d86ad

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 nojaf self-assigned this Oct 28, 2024
@nojaf
Copy link
Contributor

nojaf commented Oct 28, 2024

Hi Patrick, thanks for bringing this up. I see that the style guide does indeed mention not to place a space. I will investigate whether there is a specific reason for this or if it's just a historical practice.

@brianrourkeboll
Copy link
Member

brianrourkeboll commented Oct 28, 2024

I will investigate whether there is a specific reason for this or if it's just a historical practice.

The space is needed (or parentheses are needed) if the numeric literal starts with a unary - or +, which is not currently reflected in the AST. (The unary - or + is, of course, reflected in the AST for non-literal expressions, e.g., -x.)

In the parens analyzer, I had to resort to looking at the source text:

https://github.com/dotnet/fsharp/blob/686032982194a6dd916478d3092aa4c98194bcdc/src/Compiler/Service/SynExpr.fs#L578-L603

// Matches if the given expression starts with a symbol, e.g., <@ … @>, $"…", @"…", +1, -1…
let (|StartsWithSymbol|_|) =
    let (|TextStartsWith|) (m: range) =
        let line = getSourceLineStr m.StartLine
        line[m.StartColumn]

    function
    | SynExpr.Quote _
    | SynExpr.InterpolatedString _
    | SynExpr.Const(SynConst.String(synStringKind = SynStringKind.Verbatim), _)
    | SynExpr.Const(SynConst.Byte _, TextStartsWith '+')
    | SynExpr.Const(SynConst.UInt16 _, TextStartsWith '+')
    | SynExpr.Const(SynConst.UInt32 _, TextStartsWith '+')
    | SynExpr.Const(SynConst.UInt64 _, TextStartsWith '+')
    | SynExpr.Const(SynConst.UIntPtr _, TextStartsWith '+')
    | SynExpr.Const(SynConst.SByte _, TextStartsWith('-' | '+'))
    | SynExpr.Const(SynConst.Int16 _, TextStartsWith('-' | '+'))
    | SynExpr.Const(SynConst.Int32 _, TextStartsWith('-' | '+'))
    | SynExpr.Const(SynConst.Int64 _, TextStartsWith('-' | '+'))
    | SynExpr.Const(SynConst.IntPtr _, TextStartsWith('-' | '+'))
    | SynExpr.Const(SynConst.Decimal _, TextStartsWith('-' | '+'))
    | SynExpr.Const(SynConst.Double _, TextStartsWith('-' | '+'))
    | SynExpr.Const(SynConst.Single _, TextStartsWith('-' | '+'))
    | SynExpr.Const(SynConst.Measure(_, TextStartsWith('-' | '+'), _, _), _)
    | SynExpr.Const(SynConst.UserNum(StartsWith('-' | '+'), _), _) -> Some StartsWithSymbol
    | _ -> None

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.

3 participants