Function tooltip shows documentation for its parameter (#9117)

* Function tooltip shows documentation for its parameter

* Missed array reversal

* Update test - debugging via CI...

* Update MultiProjectAnalysisTests.fs

* Removed supposedly redundent filtering to fix up tests

* Add additional grab point so that argument doc comments aren't included in binding comment.

* Review comments on tests.

src/fsharp/XmlDoc.fs

@@ -77,18 +77,9 @@ type XmlDocCollector() =
                 let prevGrabPointPos = grabPoints.[grabPointIndex-1]
                 Array.findFirstIndexWhereTrue lines (fun (_, pos) -> posGeq pos prevGrabPointPos)
 
-        let lines = lines.[firstLineIndexAfterPrevGrabPoint..firstLineIndexAfterGrabPoint-1] |> Array.rev
-        if lines.Length = 0 then
-            [| |]
-        else
-            let firstLineNumber = (snd lines.[0]).Line
-            lines
-            |> Array.mapi (fun i x -> firstLineNumber - i, x)
-            |> Array.takeWhile (fun (sequencedLineNumber, (_, pos)) -> sequencedLineNumber = pos.Line)
-            |> Array.map (fun (_, (lineStr, _)) -> lineStr)
-            |> Array.rev
+        let lines = lines.[firstLineIndexAfterPrevGrabPoint..firstLineIndexAfterGrabPoint-1]
+        lines |> Array.map fst
       with e ->
-        //printfn "unexpected error in LinesBefore:\n%s" (e.ToString())
         [| |]
 
 /// Represents the XmlDoc fragments as collected from the lexer during parsing

src/fsharp/pars.fsy

@@ -2876,9 +2876,14 @@ constant:
     { SynConst.Measure($1, $3) }
 
 bindingPattern:
-  | headBindingPattern   
-      {  let xmlDoc = grabXmlDoc(parseState, 1)
-         mkSynBinding (xmlDoc, $1), rhs parseState 1 }
+  | headBindingPattern
+     
+      { // Adds a grab point at the start of the binding, so as not to include XML doc comments on the arguments
+        let xmlDoc = LexbufLocalXmlDocStore.GrabXmlDocBeforeMarker(parseState.LexBuffer, (lhs parseState).StartRange)
+        // Adds grab point at the end of the binding head, so subsequent types don't erroneously include argument doc comemnts
+        grabXmlDoc(parseState, 1) |> ignore
+     
+        mkSynBinding (xmlDoc, $1), rhs parseState 1 }
 
 // Subset of patterns allowed to be used in implicit ctors.
 // For a better error recovery we could replace these rules with the actual SynPat parsing

tests/service/MultiProjectAnalysisTests.fs

@@ -14,9 +14,11 @@ open FSharp.Compiler.SourceCodeServices
 open NUnit.Framework
 open FsUnit
 open System.IO
+open System.Collections.Generic
 
 open FSharp.Compiler.Service.Tests.Common
 
+let toIList (x: _ array) = x :> IList<_>
 let numProjectsForStressTest = 100
 let internal checker = FSharpChecker.Create(projectCacheSize=numProjectsForStressTest + 10)
 
@@ -43,6 +45,13 @@ let x1 = C.M(arg1 = 3, arg2 = 4, arg3 = 5)
 /// This is x2
 let x2 = C.M(arg1 = 3, arg2 = 4, ?arg3 = Some 5)
 
+/// This is
+/// x3
+let x3 (
+          /// This is not x3
+          p: int
+      ) = ()
+
 /// This is type U
 type U = 
 
@@ -195,10 +204,13 @@ let ``Test multi project 1 xmldoc`` () =
     let p1B = checker.ParseAndCheckProject(Project1B.options) |> Async.RunSynchronously
     let mp = checker.ParseAndCheckProject(MultiProject1.options) |> Async.RunSynchronously
 
-    let x1FromProject1A = 
+    let symbolFromProject1A sym = 
         [ for s in p1A.GetAllUsesOfAllSymbols() |> Async.RunSynchronously do
-             if  s.Symbol.DisplayName = "x1" then 
+             if  s.Symbol.DisplayName = sym then 
                  yield s.Symbol ]   |> List.head
+        
+    let x1FromProject1A = symbolFromProject1A "x1"
+    let x3FromProject1A = symbolFromProject1A "x3"
 
     let x1FromProjectMultiProject = 
         [ for s in mp.GetAllUsesOfAllSymbols() |> Async.RunSynchronously do
@@ -219,6 +231,10 @@ let ``Test multi project 1 xmldoc`` () =
     match x1FromProject1A with 
     | :? FSharpMemberOrFunctionOrValue as v -> v.XmlDoc.Count |> shouldEqual 1
     | _ -> failwith "odd symbol!"
+    
+    match x3FromProject1A with 
+    | :? FSharpMemberOrFunctionOrValue as v -> v.XmlDoc |> shouldEqual ([|" This is"; " x3"|] |> toIList)
+    | _ -> failwith "odd symbol!"
 
     match x1FromProjectMultiProject with 
     | :? FSharpMemberOrFunctionOrValue as v -> v.XmlDoc.Count |> shouldEqual 1

vsintegration/tests/UnitTests/LegacyLanguageService/Tests.LanguageService.QuickInfo.fs

@@ -2942,7 +2942,8 @@ query."
                              type bar() =
                                  /// <summary> Test for members</summary>
                                  /// <param name="x1">x1 param!</param>
-                                 member this.foo (x1:int)=
+                                 member this.foo
+                                     (x1:int)=
                                      System.Console.WriteLine(x1.ToString())
                              
                              type Uni1 = 
@@ -2956,7 +2957,7 @@ query."
                              exception Ex1 of value: string
 
                              // Methods
-                             let f1 = (new bar()).foo(x1(*Marker1*) = 10)
+                             let f1 = (new bar()).foo(*Marker0*)(x1(*Marker1*) = 10)
                              let f2 = System.String.Concat(1, arg1(*Marker2*) = "") 
                              
                              //Unions
@@ -2976,6 +2977,7 @@ query."
                              type provType = N1.T<Param1(*Marker7*)="hello", ParamIgnored(*Marker8*)=10>
                              """
 
+        this.AssertQuickInfoContainsAtStartOfMarker (fileContent, "(*Marker0*)", "Test for members")
         this.AssertQuickInfoContainsAtStartOfMarker (fileContent, "(*Marker1*)", "x1 param!")
         this.AssertQuickInfoContainsAtStartOfMarker (fileContent, "(*Marker2*)", "[ParamName: arg1]")
         this.AssertQuickInfoContainsAtStartOfMarker (fileContent, "(*Marker3*)", "str of case1")