Bugfix : make sure nullness does not break XmlDoc info import for methods and types (#17741)

* Remove nullness signal in string-based type encoding of a symbol (since it is used for xmldoc lookup)

* release notes

* Discard unused values

Author: T-Gro

docs/release-notes/.FSharp.Compiler.Service/9.0.100.md

@@ -16,6 +16,7 @@
 * Add missing byte chars notations, enforce limits in decimal notation in byte char & string (Issues [#15867](https://github.com/dotnet/fsharp/issues/15867), [#15868](https://github.com/dotnet/fsharp/issues/15868), [#15869](https://github.com/dotnet/fsharp/issues/15869), [PR #15898](https://github.com/dotnet/fsharp/pull/15898))
 * Parentheses analysis: keep extra parentheses around unit & tuples in method definitions. ([PR #17618](https://github.com/dotnet/fsharp/pull/17618))
 * Fix IsUnionCaseTester throwing for non-methods/properties [#17301](https://github.com/dotnet/fsharp/pull/17634)
+* Fix xmlc doc tooltip display for nullable types [#17741](https://github.com/dotnet/fsharp/pull/17741)
 * Consider `open type` used when the type is an enum and any of the enum cases is used unqualified. ([PR #17628](https://github.com/dotnet/fsharp/pull/17628))
 * Guard for possible StackOverflowException when typechecking non recursive modules and namespaces ([PR #17654](https://github.com/dotnet/fsharp/pull/17654))
 * Nullable - fix for processing System.Nullable types with nesting ([PR #17736](https://github.com/dotnet/fsharp/pull/17736))

src/Compiler/TypedTree/TcGlobals.fs

@@ -193,9 +193,7 @@ type TcGlobals(
     realsig: bool) =
 
   let v_langFeatureNullness = langVersion.SupportsFeature LanguageFeature.NullnessChecking
-
-  let v_renderNullness = checkNullness && v_langFeatureNullness
-
+  
   let v_knownWithNull =
       if v_langFeatureNullness then KnownWithNull else KnownAmbivalentToNull
 
@@ -1113,8 +1111,6 @@ type TcGlobals(
 
   member _.langFeatureNullness = v_langFeatureNullness
 
-  member _.renderNullnessAnnotations = v_renderNullness
-
   member _.knownWithNull = v_knownWithNull
 
   member _.knownWithoutNull = v_knownWithoutNull

src/Compiler/TypedTree/TcGlobals.fsi

@@ -1055,8 +1055,6 @@ type internal TcGlobals =
 
     member reference_equality_inner_vref: FSharp.Compiler.TypedTree.ValRef
 
-    member renderNullnessAnnotations: bool
-
     member reraise_info: IntrinsicValRef
 
     member reraise_vref: FSharp.Compiler.TypedTree.ValRef

src/Compiler/TypedTree/TypedTreeOps.fs

@@ -8926,10 +8926,7 @@ let typarEnc _g (gtpsType, gtpsMethod) typar =
             warning(InternalError("Typar not found during XmlDoc generation", typar.Range))
             "``0"
 
-let nullnessEnc (g:TcGlobals) (nullness:Nullness) = 
-    if g.renderNullnessAnnotations then nullness.ToFsharpCodeString() else ""
-
-let rec typeEnc g (gtpsType, gtpsMethod) ty = 
+let rec typeEnc g (gtpsType, gtpsMethod) ty =
     let stripped = stripTyEqnsAndMeasureEqns g ty
     match stripped with 
     | TType_forall _ -> 
@@ -8943,26 +8940,26 @@ let rec typeEnc g (gtpsType, gtpsMethod) ty =
         let ety = destNativePtrTy g ty
         typeEnc g (gtpsType, gtpsMethod) ety + "*"
 
-    | TType_app (_, _, nullness) when isArrayTy g ty -> 
+    | TType_app (_, _, _nullness) when isArrayTy g ty -> 
         let tcref, tinst = destAppTy g ty        
         let rank = rankOfArrayTyconRef g tcref
         let arraySuffix = "[" + String.concat ", " (List.replicate (rank-1) "0:") + "]"
-        typeEnc g (gtpsType, gtpsMethod) (List.head tinst) + arraySuffix + nullnessEnc g nullness
+        typeEnc g (gtpsType, gtpsMethod) (List.head tinst) + arraySuffix
 
     | TType_ucase (_, tinst)   
     | TType_app (_, tinst, _) -> 
-        let tyName,nullness = 
+        let tyName = 
             let ty = stripTyEqnsAndMeasureEqns g ty
             match ty with
-            | TType_app (tcref, _tinst, nullness) -> 
+            | TType_app (tcref, _tinst, _nullness) -> 
                 // Generic type names are (name + "`" + digits) where name does not contain "`".
                 // In XML doc, when used in type instances, these do not use the ticks.
                 let path = Array.toList (fullMangledPathToTyconRef tcref) @ [tcref.CompiledName]
-                textOfPath (List.map DemangleGenericTypeName path),nullness
+                textOfPath (List.map DemangleGenericTypeName path)
             | _ ->
                 assert false
                 failwith "impossible"
-        tyName + tyargsEnc g (gtpsType, gtpsMethod) tinst + nullnessEnc g nullness
+        tyName + tyargsEnc g (gtpsType, gtpsMethod) tinst
 
     | TType_anon (anonInfo, tinst) -> 
         sprintf "%s%s" anonInfo.ILTypeRef.FullName (tyargsEnc g (gtpsType, gtpsMethod) tinst)
@@ -8973,11 +8970,11 @@ let rec typeEnc g (gtpsType, gtpsMethod) ty =
         else 
             sprintf "System.Tuple%s"(tyargsEnc g (gtpsType, gtpsMethod) tys)
 
-    | TType_fun (domainTy, rangeTy, nullness) -> 
-        "Microsoft.FSharp.Core.FSharpFunc" + tyargsEnc g (gtpsType, gtpsMethod) [domainTy; rangeTy] + nullnessEnc g nullness
+    | TType_fun (domainTy, rangeTy, _nullness) -> 
+        "Microsoft.FSharp.Core.FSharpFunc" + tyargsEnc g (gtpsType, gtpsMethod) [domainTy; rangeTy]
 
-    | TType_var (typar, nullness) -> 
-        typarEnc g (gtpsType, gtpsMethod) typar + nullnessEnc g nullness
+    | TType_var (typar, _nullness) -> 
+        typarEnc g (gtpsType, gtpsMethod) typar
 
     | TType_measure _ -> "?"
 

tests/FSharp.Compiler.Service.Tests/TooltipTests.fs

@@ -9,6 +9,7 @@ open FSharp.Compiler.Text
 open FSharp.Compiler.Tokenization
 open FSharp.Compiler.EditorServices
 open FSharp.Compiler.Symbols
+open FSharp.Compiler.Xml
 open FSharp.Test
 open Xunit
 
@@ -398,16 +399,24 @@ let getCheckResults source options =
     let _, checkResults = parseAndCheckFile fileName source options
     checkResults
 
-let assertAndGetSingleToolTipText (ToolTipText(items)) =
+
+let taggedTextsToString (t: TaggedText array) =
+    t
+    |> Array.map (fun taggedText -> taggedText.Text)
+    |> String.concat ""
+let assertAndExtractTooltip (ToolTipText(items)) =
     Assert.Equal(1,items.Length)
     match items.[0] with
-    | ToolTipElement.Group [ { MainDescription = description } ] ->
+    | ToolTipElement.Group [ singleElement ] ->
         let toolTipText =
-            description
-            |> Array.map (fun taggedText -> taggedText.Text)
-            |> String.concat ""
-        toolTipText
+            singleElement.MainDescription
+            |> taggedTextsToString
+        toolTipText, singleElement.XmlDoc, singleElement.Remarks |> Option.map taggedTextsToString
     | _ -> failwith $"Expected group, got {items.[0]}"
+    
+let assertAndGetSingleToolTipText items =
+    let text,_xml,_remarks = assertAndExtractTooltip items
+    text
 
 let normalize (s:string) = s.Replace("\r\n", "\n").Replace("\n\n", "\n")
 
@@ -436,6 +445,42 @@ let exists() = System.IO.Path.Exists(null:string)
     checkResults.GetToolTip(2, 36, "let exists() = System.IO.Path.Exists(null:string)", [ "Exists" ], FSharpTokenTag.Identifier)
     |> assertAndGetSingleToolTipText
     |> Assert.shouldBeEquivalentTo "System.IO.Path.Exists([<NotNullWhenAttribute (true)>] path: string | null) : bool"
+    
+[<FactForNETCOREAPP>]
+let ``Should display xml doc on a nullable BLC method`` () =
+    
+    let source = """module Foo
+let exists() = System.IO.Path.Exists(null:string)
+"""
+    let checkResults = getCheckResults source [|"--checknulls+";"--langversion:preview"|]
+    checkResults.GetToolTip(2, 36, "let exists() = System.IO.Path.Exists(null:string)", [ "Exists" ], FSharpTokenTag.Identifier)
+    |> assertAndExtractTooltip
+    |> fun (text,xml,remarks) ->
+            text |> Assert.shouldBeEquivalentTo "System.IO.Path.Exists([<NotNullWhenAttribute (true)>] path: string | null) : bool"
+            match xml with
+            | FSharpXmlDoc.FromXmlFile (_dll,sigPath) -> sigPath |> Assert.shouldBeEquivalentTo "M:System.IO.Path.Exists(System.String)"
+            | _ -> failwith $"Xml wrong type %A{xml}"
+
+            
+[<FactForNETCOREAPP>]
+let ``Should display xml doc on fsharp hosted nullable function`` () =
+    
+    let source = """module Foo
+/// This is a xml doc above myFunc
+let myFunc(x:string|null) : string | null = x
+
+let exists() = myFunc(null)
+"""
+    let checkResults = getCheckResults source [|"--checknulls+";"--langversion:preview"|]
+    checkResults.GetToolTip(5, 21, "let exists() = myFunc(null)", [ "myFunc" ], FSharpTokenTag.Identifier)
+    |> assertAndExtractTooltip
+    |> fun (text,xml,remarks) ->
+            match xml with
+            | FSharpXmlDoc.FromXmlText t ->
+                 t.UnprocessedLines |> Assert.shouldBeEquivalentTo [|" This is a xml doc above myFunc"|]
+            | _ -> failwith $"xml was %A{xml}"
+            text |> Assert.shouldBeEquivalentTo "val myFunc: x: string | null -> string | null"            
+            remarks |> Assert.shouldBeEquivalentTo (Some "Full name: Foo.myFunc")
 
 
 [<FactForNETCOREAPP>]