unnecessary parentheses analyzer trigger in chained function call

[joprice]

Version

v0.77.2

Dotnet Info

.NET SDK:
Version: 8.0.403
Commit: c64aa40a71
Workload version: 8.0.400-manifests.e0880c5d
MSBuild version: 17.11.9+a69bbaaf5

Runtime Environment:
OS Name: Mac OS X
OS Version: 15.0
OS Platform: Darwin
RID: osx-arm64
Base Path: /usr/local/share/dotnet/sdk/8.0.403/

.NET workloads installed:
Configured to use loose manifests when installing new manifests.
There are no installed workloads to display.

Host:
Version: 9.0.0
Architecture: arm64
Commit: 9d5a6a9aa4

.NET SDKs installed:
6.0.427 [/usr/local/share/dotnet/sdk]
7.0.410 [/usr/local/share/dotnet/sdk]
8.0.303 [/usr/local/share/dotnet/sdk]
8.0.403 [/usr/local/share/dotnet/sdk]
9.0.101 [/usr/local/share/dotnet/sdk]

.NET runtimes installed:
Microsoft.AspNetCore.App 6.0.35 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.20 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 8.0.7 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 8.0.10 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 9.0.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.NETCore.App 6.0.31 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.32 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.35 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.20 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 8.0.6 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 8.0.7 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 8.0.10 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 9.0.0-preview.6.24327.7 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 9.0.0-rc.2.24473.5 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 9.0.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

Steps to reproduce

Call a member function that takes at least one argument with parens that is followed by another function:

 bg.lighten(0.2).hexa ()

The Parentheses can be removed warning is triggered for the (0.2) argument, even thought applying the quick fix will result in the invalid expression

bg.lighten 0.2.hexa ()

Details

• Expected behavior: on warning should be triggered when a function application in a chain of calls requires parens to disambiguate it
• Actual behavior: a warning is issues for a valid expression, and an invalid quick fix is applied

Logs

No response

Checklist

• I have looked through existing issues to make sure that this bug has not been reported before
• I have provided a descriptive title for this issue
• I have made sure that that this bug is reproducible on the latest version of the package
• I have provided all the information needed to reproduce this bug as efficiently as possible
• I or my company would be willing to contribute this fix

[TheAngryByrd]

cc @brianrourkeboll

[brianrourkeboll]

Hmm, this is a pretty obvious bug in hindsight: this case needs to come after this one 🤦‍♂️.

// Parens are never required around suffixed or infixed numeric literals, e.g.,
//
//     (1l).ToString()
//     (1uy).ToString()
//     (0b1).ToString()
//     (1e10).ToString()
//     (1.0).ToString()
| DotSafeNumericLiteral, _ -> false

// We can't remove parens when they're required for fluent calls:
//
//     x.M(y).N z
//     x.M(y).[z]
//     _.M(x)
//     (f x)[z]
//     (f(x))[z]
//     x.M(y)[z]
//     M(x).N <- y
| SynExpr.App _, SyntaxNode.SynExpr(SynExpr.App(argExpr = SynExpr.ArrayOrListComputed(isArray = false))) :: _ -> true

| _, SyntaxNode.SynExpr(SynExpr.App _) :: path
| _, SyntaxNode.SynExpr(OuterBinaryExpr expr (Dot, _)) :: SyntaxNode.SynExpr(SynExpr.App _) :: path when
    let rec appChainDependsOnDotOrPseudoDotPrecedence path =
        match path with
        | SyntaxNode.SynExpr(SynExpr.DotGet _) :: _
        | SyntaxNode.SynExpr(SynExpr.DotLambda _) :: _
        | SyntaxNode.SynExpr(SynExpr.DotIndexedGet _) :: _
        | SyntaxNode.SynExpr(SynExpr.Set _) :: _
        | SyntaxNode.SynExpr(SynExpr.DotSet _) :: _
        | SyntaxNode.SynExpr(SynExpr.DotIndexedSet _) :: _
        | SyntaxNode.SynExpr(SynExpr.DotNamedIndexedPropertySet _) :: _
        | SyntaxNode.SynExpr(SynExpr.App(argExpr = SynExpr.ArrayOrListComputed(isArray = false))) :: _ -> true
        | SyntaxNode.SynExpr(SynExpr.App _) :: path -> appChainDependsOnDotOrPseudoDotPrecedence path
        | _ -> false

    appChainDependsOnDotOrPseudoDotPrecedence path
    ->
    true

So I think this bug should technically only happen for fluent calls where the parenthesized argument is a suffixed numeric literal or a numeric literal with a decimal point.

I.e.:

bg.lighten(0.2).hexa () // Wrongly suggests removal.

but

let x = 0.2
bg.lighten(x).hexa () // Won't suggest removal.

[brianrourkeboll]

Fixed in dotnet/fsharp#18350. Unfortunately, it will take a while for the fix to make it into the released SDK.