From ef89c6922f20c2982ea84f0474a1228d71f5677b Mon Sep 17 00:00:00 2001
From: Petr <psfinaki@users.noreply.github.com>
Date: Thu, 20 Apr 2023 12:31:03 +0200
Subject: [PATCH] Fixing hints for custom ops (#15119)

---
 .../Hints/InlineParameterNameHints.fs         |  9 ++++---
 .../Hints/InlineParameterNameHintTests.fs     | 25 +++++++++++++++++--
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs b/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs
index c4da0889448..35601c2799b 100644
--- a/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs
+++ b/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs
@@ -54,6 +54,9 @@ type InlineParameterNameHints(parseResults: FSharpParseFileResults) =
         >> Seq.map (fun location -> location.ArgumentRange)
         >> Seq.contains range
 
+    let isCustomOperation (symbol: FSharpMemberOrFunctionOrValue) =
+        symbol.HasAttribute<CustomOperationAttribute>()
+
     let getSourceTextAtRange (sourceText: SourceText) (range: range) =
         (RoslynHelpers.FSharpRangeToTextSpan(sourceText, range) |> sourceText.GetSubText)
             .ToString()
@@ -65,11 +68,9 @@ type InlineParameterNameHints(parseResults: FSharpParseFileResults) =
                 symbol.DeclaringEntity
                 |> Option.exists (fun entity -> entity.CompiledName <> "Operators")
 
-            let isNotCustomOperation = not <| symbol.HasAttribute<CustomOperationAttribute>()
-
             (symbol.IsFunction && isNotBuiltInOperator) // arguably, hints for those would be rather useless
             || symbol.IsConstructor
-            || (symbol.IsMethod && isNotCustomOperation)
+            || symbol.IsMethod
         else
             false
 
@@ -100,8 +101,10 @@ type InlineParameterNameHints(parseResults: FSharpParseFileResults) =
                 |> Seq.filter (fun range -> argumentLocations |> (not << isNamedArgument range))
 
             let argumentNames = Seq.map (getSourceTextAtRange sourceText) ranges
+            let skipped = if symbol |> isCustomOperation then 1 else 0
 
             parameters
+            |> Seq.skip skipped
             |> Seq.zip ranges // Seq.zip is important as List.zip requires equal lengths
             |> Seq.where (snd >> parameterNameExists)
             |> Seq.zip argumentNames
diff --git a/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs
index 46805aea480..7d58b05bd81 100644
--- a/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs
+++ b/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs
@@ -481,7 +481,7 @@ let test sequences =
         Assert.Equal(expected, actual)
 
     [<Fact>]
-    let ``Hints are not shown when CustomOperation attribute is detected`` () =
+    let ``Hints are shown correctly for custom operations`` () =
         let code =
             """
 let q = query { for x in { 1 .. 10 } do select x }
@@ -489,9 +489,30 @@ let q = query { for x in { 1 .. 10 } do select x }
 
         let document = getFsDocument code
 
+        let expected =
+            [
+                {
+                    Content = "projection = "
+                    Location = (1, 48)
+                }
+            ]
+
+        let actual = getParameterNameHints document
+
+        Assert.Equal(expected, actual)
+
+    [<Fact>]
+    let ``Hints are not shown for custom operations with only 1 parameter`` () =
+        let code =
+            """
+let q = query { for _ in { 1 .. 10 } do count }
+"""
+
+        let document = getFsDocument code
+
         let actual = getParameterNameHints document
 
-        Assert.Empty actual
+        Assert.Empty(actual)
 
     [<Fact>]
     let ``Hints are not shown when parameter names coincide with variable names`` () =