Skip to content

Commit b90a04e

Browse files
author
Anthony Eid
committed
Improve PR based on feedback (#780)
- Fix extra "test" added in diagnostic warning for curating modules - Use SourceLocations test bundles instead of creating one - Change test article descriptions to add more context/information - Verify that relation is created between root and module in ancestor test - Change `hasTechnologyRoot` to not rely on subtle behavior
1 parent fa153a8 commit b90a04e

File tree

5 files changed

+33
-775
lines changed

5 files changed

+33
-775
lines changed

Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -235,20 +235,20 @@ struct DocumentationCurator {
235235
// be an error messsage. This is an if statement instead of a guard because
236236
// when there's no warning we still curate the node
237237
if childDocumentationNode.kind == .module {
238-
239-
var hasTechnologyRoot = documentationNode.kind.isSymbol == false
240-
241-
hasTechnologyRoot = hasTechnologyRoot || context.pathsTo(nodeReference).contains { path in
242-
if path.count == 0 {
243-
return false
244-
}
245-
246-
let node = context.topicGraph.nodeWithReference(path[0])
247-
return node?.kind == .module && node?.kind.isSymbol == false
238+
239+
func isTechnologyRoot(_ reference: ResolvedTopicReference) -> Bool {
240+
guard let node = context.topicGraph.nodeWithReference(reference) else {return false}
241+
return node.kind == .module && documentationNode.kind.isSymbol == false
242+
}
243+
244+
245+
let hasTechnologyRoot = isTechnologyRoot(nodeReference) || context.pathsTo(nodeReference).contains { path in
246+
guard let root = path.first else {return false}
247+
return isTechnologyRoot(root)
248248
}
249249

250250
if !hasTechnologyRoot {
251-
problems.append(Problem(diagnostic: Diagnostic(source: source(), severity: .warning, range: range(), identifier: "org.swift.docc.ModuleCuration", summary: "Linking to \((link.destination ?? "").singleQuoted) from a Topics group in \(nodeReference.absoluteString.singleQuoted) isn't allowed", explanation: "The former is a module test, and modules only exist at the root"), possibleSolutions: []))
251+
problems.append(Problem(diagnostic: Diagnostic(source: source(), severity: .warning, range: range(), identifier: "org.swift.docc.ModuleCuration", summary: "Linking to \((link.destination ?? "").singleQuoted) from a Topics group in \(nodeReference.absoluteString.singleQuoted) isn't allowed", explanation: "The former is a module, and modules only exist at the root"), possibleSolutions: []))
252252
continue
253253
}
254254

Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -146,24 +146,19 @@ class DocumentationCuratorTests: XCTestCase {
146146
}
147147

148148
func testModuleUnderTechnologyRoot() throws {
149-
let (_, bundle, context) = try testBundleAndContext(copying: "ModuleUnderTechnologyRoot") { url in
149+
let (_, bundle, context) = try testBundleAndContext(copying: "SourceLocations") { url in
150150
try """
151-
# Another Awesome Project
152-
153-
Technology Root
154-
151+
# Root curating a module
152+
155153
@Metadata {
156154
@TechnologyRoot
157155
}
158156
159-
## Overview
160-
161-
This shouldn't create a warning because it's a technologyRoot
157+
Curating a module from a technology root should not generated any warnings.
162158
163159
## Topics
164160
165-
### Code Reference
166-
- ``MyAwesomeApp``
161+
- ``SourceLocations``
167162
168163
""".write(to: url.appendingPathComponent("Root.md"), atomically: true, encoding: .utf8)
169164
}
@@ -180,58 +175,48 @@ class DocumentationCuratorTests: XCTestCase {
180175
}
181176

182177
func testModuleUnderAncestorOfTechnologyRoot() throws {
183-
let (_, bundle, context) = try testBundleAndContext(copying: "ModuleUnderTechnologyRoot") { url in
178+
let (_, bundle, context) = try testBundleAndContext(copying: "SourceLocations") { url in
184179
try """
185-
# Another Awesome Project
180+
# Root with ancestor curatings a module
186181
187-
Technology Root
182+
This is a root article that enables testing the behavior of it's ancestors.
188183
189184
@Metadata {
190185
@TechnologyRoot
191186
}
192187
193-
## Overview
194-
195-
This shouldn't create a warning because it's a technologyRoot
196-
197188
## Topics
198189
- <doc:Ancestor>
199190
200-
### Code Reference
201191
202192
""".write(to: url.appendingPathComponent("Root.md"), atomically: true, encoding: .utf8)
203193

204194
try """
205-
# MyAwesomeApp Project
195+
# Ancestor of root
206196
207-
Random summary
208-
209-
## Overview
210-
211-
This is a project to test that no errors are created when a technology root refers to a
212-
module.
213-
214-
This should create a warning because it links to a module without being a technology root
197+
Linking to a module shouldn't raise errors due to this article being an ancestor of a technology root.
215198
216199
## Topics
217-
218-
219-
### Code Reference
220-
221-
- ``MyAwesomeApp``
200+
- ``SourceLocations``
222201
223202
""".write(to: url.appendingPathComponent("Ancestor.md"), atomically: true, encoding: .utf8)
224203
}
225204

226-
var crawler = DocumentationCurator.init(in: context, bundle: bundle)
205+
let crawler = DocumentationCurator.init(in: context, bundle: bundle)
227206
XCTAssert(context.problems.isEmpty, "Expected no problems. Found: \(context.problems.map(\.diagnostic.summary))")
228207

229-
for node in context.rootModules {
230-
XCTAssertNoThrow(try crawler.crawlChildren(of: node, prepareForCuration: { _ in }, relateNodes: { _, _ in }))
208+
209+
210+
211+
guard let moduleNode = context.nodeWithSymbolIdentifier("SourceLocations"),
212+
let pathToRoot = context.pathsTo(moduleNode.reference).first,
213+
let root = pathToRoot.first else {
214+
215+
XCTFail("Module doesn't have technology root as a predecessor in its path")
216+
return
231217
}
232218

233-
234-
XCTAssertEqual(crawler.problems.count, 0)
219+
XCTAssertEqual(root.path, "/documentation/Root")
235220
}
236221

237222

0 commit comments

Comments
 (0)