Skip to content

Commit

Permalink
Display signature xmldoc for implementation symbol when implementatio…
Browse files Browse the repository at this point in the history
…n xmldoc is missing. (#14711)
  • Loading branch information
nojaf authored Feb 7, 2023
1 parent 2cd8fb4 commit fd6eb11
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 2 deletions.
1 change: 1 addition & 0 deletions src/Compiler/Checking/SignatureConformance.fs
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ type Checker(g, amap, denv, remapInfo: SignatureRepackageInfo, checkingSig) =
// Propagate defn location information from implementation to signature .
sigVal.SetOtherRange (implVal.Range, true)
implVal.SetOtherRange (sigVal.Range, false)
implVal.SetOtherXmlDoc(sigVal.XmlDoc)

let mk_err denv f = ValueNotContained(denv, infoReader, implModRef, implVal, sigVal, f)
let err denv f = errorR(mk_err denv f); false
Expand Down
20 changes: 18 additions & 2 deletions src/Compiler/TypedTree/TypedTree.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2545,7 +2545,10 @@ type ValOptionalData =

/// XML documentation attached to a value.
/// MUTABILITY: for unpickle linkage
mutable val_xmldoc: XmlDoc
mutable val_xmldoc: XmlDoc

/// the signature xml doc for an item in an implementation file.
mutable val_other_xmldoc : XmlDoc option

/// Is the value actually an instance method/property/event that augments
/// a type, and if so what name does it take in the IL?
Expand Down Expand Up @@ -2601,6 +2604,7 @@ type Val =
val_repr_info_for_display = None
val_access = TAccess []
val_xmldoc = XmlDoc.Empty
val_other_xmldoc = None
val_member_info = None
val_declaring_entity = ParentNone
val_xmldocsig = String.Empty
Expand Down Expand Up @@ -2835,7 +2839,13 @@ type Val =
/// Get the declared documentation for the value
member x.XmlDoc =
match x.val_opt_data with
| Some optData -> optData.val_xmldoc
| Some optData ->
if not optData.val_xmldoc.IsEmpty then
optData.val_xmldoc
else
match optData.val_other_xmldoc with
| Some xmlDoc -> xmlDoc
| None -> XmlDoc.Empty
| _ -> XmlDoc.Empty

///Get the signature for the value's XML documentation
Expand Down Expand Up @@ -3065,6 +3075,11 @@ type Val =
| Some optData -> optData.val_other_range <- Some m
| _ -> x.val_opt_data <- Some { Val.NewEmptyValOptData() with val_other_range = Some m }

member x.SetOtherXmlDoc xmlDoc =
match x.val_opt_data with
| Some optData -> optData.val_other_xmldoc <- Some xmlDoc
| _ -> x.val_opt_data <- Some { Val.NewEmptyValOptData() with val_other_xmldoc = Some xmlDoc }

member x.SetDeclaringEntity parent =
match x.val_opt_data with
| Some optData -> optData.val_declaring_entity <- parent
Expand Down Expand Up @@ -3119,6 +3134,7 @@ type Val =
val_repr_info = tg.val_repr_info
val_access = tg.val_access
val_xmldoc = tg.val_xmldoc
val_other_xmldoc = tg.val_other_xmldoc
val_member_info = tg.val_member_info
val_declaring_entity = tg.val_declaring_entity
val_xmldocsig = tg.val_xmldocsig
Expand Down
5 changes: 5 additions & 0 deletions src/Compiler/TypedTree/TypedTree.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -1820,6 +1820,9 @@ type ValOptionalData =
/// MUTABILITY: for unpickle linkage
mutable val_xmldoc: XmlDoc

/// the signature xml doc for an item in an implementation file.
mutable val_other_xmldoc: XmlDoc option

/// Is the value actually an instance method/property/event that augments
/// a type, type if so what name does it take in the IL?
/// MUTABILITY: for unpickle linkage
Expand Down Expand Up @@ -1913,6 +1916,8 @@ type Val =

member SetOtherRange: m: (range * bool) -> unit

member SetOtherXmlDoc: xmlDoc: XmlDoc -> unit

member SetType: ty: TType -> unit

member SetValDefn: val_defn: Expr -> unit
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/TypedTree/TypedTreePickle.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2262,6 +2262,7 @@ and u_ValData st =
val_const = x14
val_access = x13
val_xmldoc = defaultArg x15 XmlDoc.Empty
val_other_xmldoc = None
val_member_info = x8
val_declaring_entity = x13b
val_xmldocsig = x12
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@
<Compile Include="..\service\PrettyNaming.fs">
<Link>PrettyNaming.fs</Link>
</Compile>
<Compile Include="TooltipTests.fs" />
<Compile Include="..\service\Program.fs">
<Link>Program.fs</Link>
</Compile>
Expand Down
68 changes: 68 additions & 0 deletions tests/FSharp.Compiler.Service.Tests/TooltipTests.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
module FSharp.Compiler.Service.Tests.TooltipTests

#nowarn "57"

open FSharp.Compiler.CodeAnalysis
open FSharp.Compiler.Service.Tests.Common
open FSharp.Compiler.Text
open FSharp.Compiler.Tokenization
open FSharp.Compiler.EditorServices
open FSharp.Compiler.Symbols
open NUnit.Framework

[<Test>]
let ``Display XML doc of signature file if implementation doesn't have one`` () =
let files =
Map.ofArray
[| "A.fsi",
SourceText.ofString
"""
module Foo
/// Great XML doc comment
val bar: a: int -> b: int -> int
"""

"A.fs",
SourceText.ofString
"""
module Foo
// No XML doc here because the signature file has one right?
let bar a b = a - b
""" |]

let documentSource fileName = Map.tryFind fileName files

let projectOptions =
let _, projectOptions = mkTestFileAndOptions "" Array.empty

{ projectOptions with
SourceFiles = [| "A.fsi"; "A.fs" |] }

let checker =
FSharpChecker.Create(documentSource = DocumentSource.Custom documentSource)

let checkResult =
checker.ParseAndCheckFileInProject("A.fs", 0, Map.find "A.fs" files, projectOptions)
|> Async.RunImmediate

match checkResult with
| _, FSharpCheckFileAnswer.Succeeded(checkResults) ->
let barSymbol = findSymbolByName "bar" checkResults

match barSymbol with
| :? FSharpMemberOrFunctionOrValue as mfv -> Assert.True mfv.HasSignatureFile
| _ -> Assert.Fail "Expected to find a symbol FSharpMemberOrFunctionOrValue that HasSignatureFile"

// Get the tooltip for `bar` in the implementation file
let (ToolTipText tooltipElements) =
checkResults.GetToolTip(4, 4, "let bar a b = a - b", [ "bar" ], FSharpTokenTag.Identifier)

match tooltipElements with
| [ ToolTipElement.Group [ element ] ] ->
match element.XmlDoc with
| FSharpXmlDoc.FromXmlText xmlDoc -> Assert.True xmlDoc.NonEmpty
| xmlDoc -> Assert.Fail $"Expected FSharpXmlDoc.FromXmlText, got {xmlDoc}"
| elements -> Assert.Fail $"Expected a single tooltip group element, got {elements}"
| _ -> Assert.Fail "Expected checking to succeed."

0 comments on commit fd6eb11

Please sign in to comment.