Correct inline parameter hints for named optional arguments (#14498)

dotnet · Dec 21, 2022 · 81252e0 · 81252e0
1 parent c8acf2a
commit 81252e0
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 7 deletions.
diff --git a/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs b/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs
@@ -4,6 +4,7 @@ namespace Microsoft.VisualStudio.FSharp.Editor.Hints
 
 open Microsoft.VisualStudio.FSharp.Editor
 open FSharp.Compiler.CodeAnalysis
+open FSharp.Compiler.EditorServices
 open FSharp.Compiler.Symbols
 open FSharp.Compiler.Text
 open Hints
@@ -30,7 +31,7 @@ module InlineParameterNameHints =
     let private doesFieldNameExist (field: FSharpField) = 
         not field.IsNameGenerated
 
-    let private getTupleRanges
+    let private getArgumentLocations
         (symbolUse: FSharpSymbolUse)
         (parseResults: FSharpParseFileResults) =
 
@@ -40,9 +41,11 @@ module InlineParameterNameHints =
 
         parseResults.FindParameterLocations position
         |> Option.map (fun locations -> locations.ArgumentLocations)
-        |> Option.map (Seq.map (fun location -> location.ArgumentRange))
-        |> Option.defaultValue []
-        |> Seq.toList
+        |> Option.defaultValue [||]
+
+    let private getTupleRanges =
+        Seq.map (fun location -> location.ArgumentRange)
+        >> Seq.toList
 
     let private getCurryRanges 
         (symbolUse: FSharpSymbolUse) 
@@ -51,7 +54,15 @@ module InlineParameterNameHints =
         parseResults.GetAllArgumentsForFunctionApplicationAtPosition symbolUse.Range.Start
         |> Option.defaultValue []
 
-    let isMemberOrFunctionOrValueValidForHint (symbol: FSharpMemberOrFunctionOrValue) (symbolUse: FSharpSymbolUse) =
+    let private isNamedArgument range =
+        Seq.filter (fun location -> location.IsNamedArgument)
+        >> Seq.map (fun location -> location.ArgumentRange) 
+        >> Seq.contains range
+
+    let isMemberOrFunctionOrValueValidForHint 
+        (symbol: FSharpMemberOrFunctionOrValue) 
+        (symbolUse: FSharpSymbolUse) =
+
         if symbolUse.IsFromUse then
             let isNotBuiltInOperator = 
                 symbol.DeclaringEntity 
@@ -73,10 +84,14 @@ module InlineParameterNameHints =
         (symbolUse: FSharpSymbolUse) =
 
         let parameters = symbol.CurriedParameterGroups |> Seq.concat
+        let argumentLocations = getArgumentLocations symbolUse parseResults
 
-        let tupleRanges = parseResults |> getTupleRanges symbolUse
+        let tupleRanges = argumentLocations |> getTupleRanges
         let curryRanges = parseResults |> getCurryRanges symbolUse
-        let ranges = if tupleRanges |> (not << Seq.isEmpty) then tupleRanges else curryRanges
+
+        let ranges = 
+            if tupleRanges |> (not << Seq.isEmpty) then tupleRanges else curryRanges
+            |> Seq.filter (fun range -> argumentLocations |> (not << isNamedArgument range))
 
         parameters
         |> Seq.zip ranges // Seq.zip is important as List.zip requires equal lengths

diff --git a/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs
@@ -406,3 +406,45 @@ let x = "test".Split("").[0].Split("");
         let actual = getParameterNameHints document
 
         Assert.AreEqual(expected, actual)
+
+    [<Test>]
+    let ``Hints are not shown for optional parameters with specified names`` () =
+        let code =
+            """
+type MyType() =
+
+    member _.MyMethod(?beep: int, ?bap: int, ?boop: int) = ()
+
+    member this.Foo = this.MyMethod(3, boop = 4)
+"""
+
+        let document = getFsDocument code
+
+        let expected =
+            [
+                {
+                    Content = "beep = "
+                    Location = (5, 37)
+                }
+            ]
+
+        let actual = getParameterNameHints document
+
+        Assert.AreEqual(expected, actual)
+
+    [<Test>]
+    let ``Hints are not shown when all optional parameters are named`` () =
+        let code =
+            """
+type MyType() =
+
+    member _.MyMethod(?beep: int, ?bap : int, ?boop : int) = ()
+
+    member this.Foo = this.MyMethod(bap = 3, beep = 4)
+"""
+
+        let document = getFsDocument code
+
+        let actual = getParameterNameHints document
+
+        Assert.IsEmpty(actual)