Skip to content

Commit 1c450e4

Browse files
T-GroT-Gro
committed
Fixup #1: fix missing [<Theory>], remove dead code, deduplicate expand logic, strengthen assertions
1 parent 1464402 commit 1c450e4

3 files changed

Lines changed: 41 additions & 87 deletions

File tree

‎src/Compiler/Symbols/XmlDocInheritance.fs‎

Lines changed: 27 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -113,62 +113,35 @@ and private expandInheritDocText
113113
if directives.IsEmpty then
114114
xmlText
115115
else
116+
let resolveAndReplace (directive: InheritDocDirective) (cref: string) (implicitCrefForRecursion: string option) =
117+
if visited.Contains(cref) then
118+
warning (Error(FSComp.SR.xmlDocInheritDocError ($"Circular reference detected for '{cref}'"), m))
119+
else
120+
match resolveCref cref with
121+
| Some inheritedXml ->
122+
let expandedInheritedXml =
123+
expandInheritedDoc resolveCref implicitCrefForRecursion m visited cref inheritedXml
124+
125+
let contentToInherit =
126+
match directive.Path with
127+
| Some xpath ->
128+
applyXPathFilter m xpath expandedInheritedXml
129+
|> Option.defaultValue expandedInheritedXml
130+
| None -> expandedInheritedXml
131+
132+
try
133+
let newContent = XElement.Parse("<temp>" + contentToInherit + "</temp>")
134+
directive.Element.ReplaceWith(newContent.Nodes())
135+
with ex ->
136+
warning (Error(FSComp.SR.xmlDocInheritDocError ($"Failed to process inheritdoc: {ex.Message}"), m))
137+
| None -> warning (Error(FSComp.SR.xmlDocInheritDocError ($"Cannot resolve cref '{cref}'"), m))
138+
116139
for directive in directives do
117140
match directive.Cref with
118-
| Some cref ->
119-
if visited.Contains(cref) then
120-
warning (Error(FSComp.SR.xmlDocInheritDocError ($"Circular reference detected for '{cref}'"), m))
121-
else
122-
match resolveCref cref with
123-
| Some inheritedXml ->
124-
let expandedInheritedXml =
125-
expandInheritedDoc resolveCref implicitTargetCrefOpt m visited cref inheritedXml
126-
127-
let contentToInherit =
128-
match directive.Path with
129-
| Some xpath ->
130-
applyXPathFilter m xpath expandedInheritedXml
131-
|> Option.defaultValue expandedInheritedXml
132-
| None -> expandedInheritedXml
133-
134-
try
135-
let newContent = XElement.Parse("<temp>" + contentToInherit + "</temp>")
136-
directive.Element.ReplaceWith(newContent.Nodes())
137-
with ex ->
138-
warning (Error(FSComp.SR.xmlDocInheritDocError ($"Failed to process inheritdoc: {ex.Message}"), m))
139-
| None -> warning (Error(FSComp.SR.xmlDocInheritDocError ($"Cannot resolve cref '{cref}'"), m))
141+
| Some cref -> resolveAndReplace directive cref implicitTargetCrefOpt
140142
| None ->
141143
match implicitTargetCrefOpt with
142-
| Some implicitCref ->
143-
if visited.Contains(implicitCref) then
144-
warning (
145-
Error(
146-
FSComp.SR.xmlDocInheritDocError (
147-
$"Circular reference detected for implicit target '{implicitCref}'"
148-
),
149-
m
150-
)
151-
)
152-
else
153-
match resolveCref implicitCref with
154-
| Some inheritedXml ->
155-
let expandedInheritedXml =
156-
expandInheritedDoc resolveCref None m visited implicitCref inheritedXml
157-
158-
let contentToInherit =
159-
match directive.Path with
160-
| Some xpath ->
161-
applyXPathFilter m xpath expandedInheritedXml
162-
|> Option.defaultValue expandedInheritedXml
163-
| None -> expandedInheritedXml
164-
165-
try
166-
let newContent = XElement.Parse("<temp>" + contentToInherit + "</temp>")
167-
directive.Element.ReplaceWith(newContent.Nodes())
168-
with ex ->
169-
warning (Error(FSComp.SR.xmlDocInheritDocError ($"Failed to process inheritdoc: {ex.Message}"), m))
170-
| None ->
171-
warning (Error(FSComp.SR.xmlDocInheritDocError ($"Cannot resolve implicit target '{implicitCref}'"), m))
144+
| Some implicitCref -> resolveAndReplace directive implicitCref None
172145
| None ->
173146
warning (
174147
Error(
@@ -186,32 +159,11 @@ and private expandInheritDocText
186159
with :? System.Xml.XmlException ->
187160
xmlText
188161

189-
/// Expands `<inheritdoc>` elements in XML documentation.
162+
/// Expands `<inheritdoc>` elements in XML documentation text.
190163
/// The caller provides a `resolveCref` function to look up documentation by cref string.
191164
/// Takes an optional implicit target cref for resolving <inheritdoc/> without cref attribute.
192165
/// Tracks visited signatures to prevent infinite recursion.
193-
let expandInheritDoc
194-
(resolveCref: string -> string option)
195-
(implicitTargetCrefOpt: string option)
196-
(m: range)
197-
(visited: Set<string>)
198-
(doc: XmlDoc)
199-
: XmlDoc =
200-
if doc.IsEmpty then
201-
doc
202-
else
203-
let xmlText = doc.GetXmlText()
204-
205-
let expandedText =
206-
expandInheritDocText resolveCref implicitTargetCrefOpt m visited xmlText
207-
208-
if obj.ReferenceEquals(xmlText, expandedText) || xmlText = expandedText then
209-
doc
210-
else
211-
XmlDoc([| expandedText |], m)
212-
213-
/// Like expandInheritDoc but takes a pre-computed xmlText string, avoiding an extra GetXmlText() call.
214-
/// Use when the caller has already obtained the XML text (e.g. to check for <inheritdoc> presence).
166+
/// Takes a pre-computed xmlText string, avoiding an extra GetXmlText() call.
215167
let expandInheritDocFromXmlText
216168
(resolveCref: string -> string option)
217169
(implicitTargetCrefOpt: string option)

‎src/Compiler/Symbols/XmlDocInheritance.fsi‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,12 @@
33
module internal FSharp.Compiler.XmlDocInheritance
44

55
open FSharp.Compiler.Text
6-
open FSharp.Compiler.Xml
76

8-
/// Expands `<inheritdoc>` elements in XML documentation.
7+
/// Expands `<inheritdoc>` elements in XML documentation text.
98
/// The caller provides a `resolveCref` function to look up documentation by cref string.
109
/// Takes an optional implicit target cref for resolving <inheritdoc/> without cref attribute.
1110
/// Takes a set of visited signatures to prevent cycles.
12-
val expandInheritDoc:
13-
resolveCref: (string -> string option) ->
14-
implicitTargetCrefOpt: string option ->
15-
m: range ->
16-
visited: Set<string> ->
17-
doc: XmlDoc ->
18-
XmlDoc
19-
20-
/// Like expandInheritDoc but takes a pre-computed xmlText string, avoiding an extra GetXmlText() call.
21-
/// Use when the caller has already obtained the XML text (e.g. to check for <inheritdoc> presence).
11+
/// Takes a pre-computed xmlText string, avoiding an extra GetXmlText() call.
2212
val expandInheritDocFromXmlText:
2313
resolveCref: (string -> string option) ->
2414
implicitTargetCrefOpt: string option ->

‎tests/FSharp.Compiler.Service.Tests/XmlDocTests.fs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1730,6 +1730,7 @@ type Derived() = class end
17301730
"""
17311731
let xmlText = getEntityXmlText code "Derived"
17321732
Assert.Contains("GrandBase documentation", xmlText)
1733+
Assert.DoesNotContain("inheritdoc", xmlText)
17331734

17341735
[<Fact>]
17351736
let ``inheritdoc circular reference should not crash tooltip``() =
@@ -1787,6 +1788,7 @@ type ServiceImpl() =
17871788
"""
17881789
let xmlText = getEntityXmlText code symbolName
17891790
Assert.Contains(expectedText, xmlText)
1791+
Assert.DoesNotContain("inheritdoc", xmlText)
17901792

17911793
[<Fact>]
17921794
let ``inheritdoc from same module nested type``() =
@@ -1803,6 +1805,7 @@ type DerivedFromOuter() = class end
18031805
"""
18041806
let xmlText = getEntityXmlText code "DerivedFromOuter"
18051807
Assert.Contains("Outer container documentation", xmlText)
1808+
Assert.DoesNotContain("inheritdoc", xmlText)
18061809

18071810
[<Theory>]
18081811
[<InlineData("DerivedInSecond", "Type in first module")>]
@@ -1822,6 +1825,7 @@ type DerivedInSecond() = class end
18221825
"""
18231826
let xmlText = getEntityXmlText code symbolName
18241827
Assert.Contains(expectedText, xmlText)
1828+
Assert.DoesNotContain("inheritdoc", xmlText)
18251829

18261830
[<Fact>]
18271831
let ``inheritdoc from System type via IL``() =
@@ -1900,6 +1904,7 @@ type DerivedRecord = { Id: int; Data: string }
19001904
"""
19011905
let xmlText = getEntityXmlText code symbolName
19021906
Assert.Contains(expectedText, xmlText)
1907+
Assert.DoesNotContain("inheritdoc", xmlText)
19031908

19041909
[<Theory>]
19051910
[<InlineData("DerivedUnion", "Base union type")>]
@@ -1921,6 +1926,7 @@ type DerivedUnion =
19211926
"""
19221927
let xmlText = getEntityXmlText code symbolName
19231928
Assert.Contains(expectedText, xmlText)
1929+
Assert.DoesNotContain("inheritdoc", xmlText)
19241930

19251931
[<Fact>]
19261932
let ``inheritdoc implicit without cref on interface impl should resolve``() =
@@ -1939,6 +1945,8 @@ type ServiceImpl() =
19391945
let xmlText = getMemberXmlText code "ServiceImpl" "DoWork"
19401946
Assert.Contains("Service method", xmlText)
19411947
Assert.DoesNotContain("<inheritdoc", xmlText)
1948+
1949+
[<Theory>]
19421950
[<InlineData("DerivedClass", "Base class documentation")>]
19431951
[<InlineData("DerivedClass", "Base remarks")>]
19441952
let ``implicit inheritdoc should resolve from base class for type`` (symbolName: string, expectedText: string) =
@@ -1955,6 +1963,7 @@ type DerivedClass() =
19551963
"""
19561964
let xmlText = getEntityXmlText code symbolName
19571965
Assert.Contains(expectedText, xmlText)
1966+
Assert.DoesNotContain("inheritdoc", xmlText)
19581967

19591968
[<Theory>]
19601969
[<InlineData("MyImpl", "Interface documentation")>]
@@ -1975,6 +1984,7 @@ type MyImpl() =
19751984
"""
19761985
let xmlText = getEntityXmlText code symbolName
19771986
Assert.Contains(expectedText, xmlText)
1987+
Assert.DoesNotContain("inheritdoc", xmlText)
19781988

19791989
// ===========================================
19801990
// IMPLICIT INHERITDOC ON METHODS AND PROPERTIES
@@ -2096,6 +2106,7 @@ type Box<'T> = { Item: 'T }
20962106
"""
20972107
let xmlText = getEntityXmlText code "Box`1"
20982108
Assert.Contains("A generic container", xmlText)
2109+
Assert.DoesNotContain("inheritdoc", xmlText)
20992110

21002111
[<Fact>]
21012112
let ``nested type cref should resolve``() =
@@ -2111,6 +2122,7 @@ type Other = { Y: int }
21112122
"""
21122123
let xmlText = getEntityXmlText code "Other"
21132124
Assert.Contains("Inner type docs", xmlText)
2125+
Assert.DoesNotContain("inheritdoc", xmlText)
21142126

21152127
[<Fact>]
21162128
let ``tooling-time resolution removes all inheritdoc elements``() =

0 commit comments

Comments
 (0)