remove changes relevant to absolute paths and shadowing

Author: theMomax

Sources/SwiftDocC/Infrastructure/Link Resolution/DocumentationCacheBasedLinkResolver.swift

@@ -78,17 +78,8 @@ final class DocumentationCacheBasedLinkResolver {
     func referenceFor(absoluteSymbolPath path: String, parent: ResolvedTopicReference) -> ResolvedTopicReference? {
         // Check if `destination` is a known absolute reference URL.
         if let match = referencesIndex[path] { return match }
-
-        // Check if `destination` is a known absolute symbol path...
-        if !path.hasPrefix("/") && parent.pathComponents.count > 2 {
-            // ...in the parent's module
-            let parentModule = parent.pathComponents[2]
-            let referenceURLString = "doc://\(parent.bundleIdentifier)/documentation/\(parentModule)/\(path)"
-            if let reference = referencesIndex[referenceURLString] {
-                return reference
-            }
-        }
-         // ...globally
+        
+        // Check if `destination` is a known absolute symbol path.
         let referenceURLString = "doc://\(parent.bundleIdentifier)/documentation/\(path.hasPrefix("/") ? String(path.dropFirst()) : path)"
         return referencesIndex[referenceURLString]
     }
@@ -308,6 +299,8 @@ final class DocumentationCacheBasedLinkResolver {
     }
 
 
+    // MARK: Symbol reference creation
+    
     /// Returns a map between symbol identifiers and topic references.
     ///
     /// - Parameters:

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift

@@ -305,36 +305,17 @@ struct PathHierarchy {
     }
 
     private func findNode(path rawPath: String, parent: ResolvedIdentifier?, onlyFindSymbols: Bool) throws -> Node {
+        // The search for a documentation element can be though of as 3 steps:
         // First, parse the path into structured path components.
         let (path, isAbsolute) = Self.parse(path: rawPath)
         guard !path.isEmpty else {
             throw Error.notFound(availableChildren: [])
         }
 
-        // Second, we try to the node matching the path. This is done by first finding
-        // the root of the (possibly relative) path and then searching for the child
-        // from that root.
-        // A relative path could have multiple root candidates (where the first
-        // component is a match). We start searching at the `parent`, working
-        // our way up the tree, trying to find a child for each root candidate.
-        // This function reports all errors found on the way and - if successful - the
-        // matching node.
-        let (node, errors) = try searchForChildOnAllPossibleRoots(parentID: parent, path: path, isAbsolute: isAbsolute, onlyFindSymbols: onlyFindSymbols)
-        
-        if let node = node {
-            return node
-        }
-        
-        // Currently, we only report the first error, which corresponds to
-        // the root candidate closest to the `parent`. Aggregating errors could
-        // help giving more precise suggestions in the future.
-        throw errors.first!
-    }
-    
-    /// Tries to find a node in the subtree of `node` where `remaining` is the relative path from `node` to the child.
-    private func findChild(of node: Node, remaining: ArraySlice<PathComponent>) throws -> Node {
-        var node = node
-        var remaining = remaining
+        // Second, find the node to start the search relative to.
+        // This may consume or or more path components. See implementation for details.
+        var remaining = path[...]
+        var node = try findRoot(parentID: parent, remaining: &remaining, isAbsolute: isAbsolute, onlyFindSymbols: onlyFindSymbols)
 
         // Third, search for the match relative to the start node.
         if remaining.isEmpty {
@@ -458,13 +439,11 @@ struct PathHierarchy {
     ///
     /// - Parameters:
     ///   - parentID: An optional ID of the node to start the search relative to.
-    ///   - path: The parsed path components.
+    ///   - remaining: The parsed path components.
     ///   - isAbsolute: If the parsed path represent an absolute documentation link.
     ///   - onlyFindSymbols: If symbol results are required.
     /// - Returns: The node to start the relative search relative to.
-    private func searchForChildOnAllPossibleRoots(parentID: ResolvedIdentifier?, path: [PathComponent], isAbsolute: Bool, onlyFindSymbols: Bool) throws -> (Node?, [Error]) {
-        var remaining = path[...]
-        
+    private func findRoot(parentID: ResolvedIdentifier?, remaining: inout ArraySlice<PathComponent>, isAbsolute: Bool, onlyFindSymbols: Bool) throws -> Node {
         // If the first path component is "tutorials" or "documentation" then that
         let isKnownTutorialPath = remaining.first!.full == "tutorials"
         let isKnownDocumentationPath = remaining.first!.full == "documentation"
@@ -487,34 +466,29 @@ struct PathHierarchy {
                         }
                     }
                     remaining = remaining.dropFirst()
-                    return (try findChild(of: articlesContainer, remaining: remaining) , [])
+                    return articlesContainer
                 } else if articlesContainer.children.keys.contains(component.name) || articlesContainer.children.keys.contains(component.full)  {
-                    return (try findChild(of: articlesContainer, remaining: remaining) , [])
+                    return articlesContainer
                 }
             }
             if !isKnownDocumentationPath {
                 if tutorialContainer.name == component.name || tutorialContainer.name == component.full {
                     remaining = remaining.dropFirst()
-                    return (try findChild(of: tutorialContainer, remaining: remaining) , [])
+                    return tutorialContainer
                 } else if tutorialContainer.children.keys.contains(component.name) || tutorialContainer.children.keys.contains(component.full)  {
-                    return (try findChild(of: tutorialContainer, remaining: remaining) , [])
+                    return tutorialContainer
                 }
                 // The parent for tutorial overviews / technologies is "tutorials" which has already been removed above, so no need to check against that name.
                 else if tutorialOverviewContainer.children.keys.contains(component.name) || tutorialOverviewContainer.children.keys.contains(component.full)  {
-                    return (try findChild(of: tutorialOverviewContainer, remaining: remaining) , [])
+                    return tutorialOverviewContainer
                 }
             }
-        }
-        
-        if !isKnownTutorialPath && isAbsolute {
-            // If this is an absolute non-tutorial link, then the first component will be a module name.
-            if let matched = modules[component.name] ?? modules[component.full] {
-                remaining = remaining.dropFirst()
-                return (try findChild(of: matched, remaining: remaining) , [])
-            } else {
-                // This is an absolute path that doesn't start with a valid module. Don't continue the search
-                // in relative mode.
-                throw Error.notFound(availableChildren: Array(modules.keys))
+            if !isKnownTutorialPath && isAbsolute {
+                // If this is an absolute non-tutorial link, then the first component will be a module name.
+                if let matched = modules[component.name] ?? modules[component.full] {
+                    remaining = remaining.dropFirst()
+                    return matched
+                }
             }
         }
 
@@ -529,73 +503,37 @@ struct PathHierarchy {
         }
 
         if let parentID = parentID {
-            // We're dealing with a relative path, so search will be a bit more complicated.
-            // Starting from the parent, we ascend in the tree trying to find a node that matches
-            // our search path's first component. If we find one, we try to obtain the descendant
-            // matching the remainder of the search path using `findChild(of:remaining:)`. If that
-            // fails, we continue the search up the tree, after we've saved the error to be returned
-            // later.
-            
-            // Errors collected during the process
-            var errors: [Error] = []
-            
             // If a parent ID was provided, start at that node and continue up the hierarchy until that node has a child that matches the first path components name.
             var parentNode = lookup[parentID]!
             let firstComponent = remaining.first!
             if matches(node: parentNode, component: firstComponent) {
                 remaining = remaining.dropFirst()
-                do {
-                    return (try findChild(of: parentNode, remaining: remaining), errors)
-                } catch let error as Error {
-                    errors.append(error)
-                }
+                return parentNode
             }
-            while true {
-                if parentNode.children.keys.contains(firstComponent.name) || parentNode.children.keys.contains(firstComponent.full) {
-                    do {
-                        return (try findChild(of: parentNode, remaining: remaining), errors)
-                    } catch let error as Error {
-                        errors.append(error)
-                    }
-                }
-                
+            while !parentNode.children.keys.contains(firstComponent.name) && !parentNode.children.keys.contains(firstComponent.full) {
                 guard let parent = parentNode.parent else {
                     if matches(node: parentNode, component: firstComponent){
                         remaining = remaining.dropFirst()
-                        do {
-                            return (try findChild(of: parentNode, remaining: remaining), errors)
-                        } catch let error as Error {
-                            errors.append(error)
-                        }
+                        return parentNode
                     }
                     if let matched = modules[component.name] ?? modules[component.full] {
                         remaining = remaining.dropFirst()
-                        do {
-                            return (try findChild(of: matched, remaining: remaining), errors)
-                        } catch let error as Error {
-                            errors.append(error)
-                        }
+                        return matched
                     }
-                    
                     // No node up the hierarchy from the provided parent has a child that matches the first path component.
                     // Go back to the provided parent node for diagnostic information about its available children.
                     parentNode = lookup[parentID]!
-                    
-                    // We've reached the top of the tree...we return all the errors we obtained in the process along with
-                    // the final error providing the partial result.
-                    
-                    errors.append(Error.partialResult(partialResult: parentNode, remainingSubpath: remaining.map({ $0.full }).joined(separator: "/"), availableChildren: parentNode.children.keys.sorted(by: availableChildNameIsBefore)))
-                    return (nil, errors)
+                    throw Error.partialResult(partialResult: parentNode, remainingSubpath: remaining.map({ $0.full }).joined(separator: "/"), availableChildren: parentNode.children.keys.sorted(by: availableChildNameIsBefore))
                 }
-                
                 parentNode = parent
             }
+            return parentNode
         }
 
         // If no parent ID was provided, check if the first path component is a module name.
         if let matched = modules[component.name] ?? modules[component.full] {
             remaining = remaining.dropFirst()
-            return (try findChild(of: matched, remaining: remaining) , [])
+            return matched
         }
 
         // No place to start the search from could be found.

Sources/SwiftDocC/SwiftDocC.docc/SwiftDocC/LinkResolution.md

@@ -22,25 +22,11 @@ doc://com.example/path/to/documentation/page#optional-heading
       bundle ID     path in docs hierarchy    heading name 
 ```
 
-Both types of links can be used in a relative or absolute way. Absolute symbol links have a leading slash (`/`) and must start with the module they are referring to, for example ` ``/MyModule/MyClass/myProperty`` `.
-
 ## Resolving a Documentation Link
 
-To make authored documentation links easier to write and easier to read in plain text format all authored documentation links can be written as relative links. The symbol links in documentation extension headers are written relative to the scope of modules. All other authored documentation links are written relative to the page where the link is written.
-
-These relative documentation links can specify path components from higher up in the documentation hierarchy to reference container symbols or container pages. If a higher-up container page is shadowed by one of its descendants because they share the same name, the higher-up container page must be linked to using an absolute link or a sufficiently unambigious relative link.
-
-```swift
-struct Container {
-    struct Container {
-        /// ``Container`` links to `Container.Container`
-        /// ``MyModule/Container`` links to the outer `Container`
-        func foo() { }
-    }
-}
-```
+To make authored documentation links easier to write and easier to read in plain text format all authored documentation links are relative links. The symbol links in documentation extension headers are written relative to the scope of modules. All other authored documentation links are written relative to the page where the link is written. 
 
-> Note: If `MyModule` were to be named `Container` too, only the absolute link `/Container/Container` could be used to refer to the outer `Container`.
+These relative documentation links can specify path components from higher up in the documentation hierarchy to reference container symbols or container pages.
 
 ### Handling Ambiguous Links
 

Tests/SwiftDocCTests/Infrastructure/ReferenceResolverTests.swift

@@ -344,37 +344,67 @@ class ReferenceResolverTests: XCTestCase {
         XCTAssertEqual(referencingFileDiagnostics.filter({ $0.identifier == "org.swift.docc.unresolvedTopicReference" }).count, 1)
     }
 
-    func testAbsoluteAndRelativeReferencesToExternalAndExtensionSymbols() throws {
-        let (bundleURL, bundle, context) = try testBundleAndContext(copying: "BundleWithRelativePathAmbiguity")
+    func testRelativeReferencesToExtensionSymbols() throws {
+        let (bundleURL, bundle, context) = try testBundleAndContext(copying: "BundleWithRelativePathAmbiguity") { root in
+            // We don't want the external target to be part of the archive as that is not
+            // officially supported yet.
+            try FileManager.default.removeItem(at: root.appendingPathComponent("Dependency.symbols.json"))
+            
+            try """
+            # ``BundleWithRelativePathAmbiguity/Dependency``
+
+            ## Overview
+            
+            ### Module Scope Links
+            
+            - ``BundleWithRelativePathAmbiguity/Dependency``
+            - ``BundleWithRelativePathAmbiguity/Dependency/AmbiguousType``
+            - ``BundleWithRelativePathAmbiguity/Dependency/AmbiguousType/foo()``
+            
+            ### Extended Module Scope Links
+            
+            - ``Dependency``
+            - ``Dependency/AmbiguousType``
+            - ``Dependency/AmbiguousType/foo()``
+            
+            ### Local Scope Links
+            
+            - ``Dependency``
+            - ``AmbiguousType``
+            - ``AmbiguousType/foo()``
+            """.write(to: root.appendingPathComponent("Article.md"), atomically: true, encoding: .utf8)
+        }
 
         defer { try? FileManager.default.removeItem(at: bundleURL) }
 
         // Get a translated render node
-        let node = try context.entity(with: ResolvedTopicReference(bundleIdentifier: "org.swift.docc.example", path: "/documentation/BundleWithRelativePathAmbiguity", sourceLanguage: .swift))
+        let node = try context.entity(with: ResolvedTopicReference(bundleIdentifier: "org.swift.docc.example", path: "/documentation/BundleWithRelativePathAmbiguity/Dependency", sourceLanguage: .swift))
         var translator = RenderNodeTranslator(context: context, bundle: bundle, identifier: node.reference, source: nil)
         let renderNode = translator.visit(node.semantic as! Symbol) as! RenderNode
 
         let content = try XCTUnwrap(renderNode.primaryContentSections.first as? ContentRenderSection).content
 
-        func assertListedReferencesInSectionMatchHeading(_ absoluteShorthandReference: String) throws {
-            let headingString = "`\(absoluteShorthandReference)`"
-            let absoluteReferenceString = "doc://org.swift.docc.example/documentation\(absoluteShorthandReference)"
+        let expectedReferences = [
+            "doc://org.swift.docc.example/documentation/BundleWithRelativePathAmbiguity/Dependency",
+            "doc://org.swift.docc.example/documentation/BundleWithRelativePathAmbiguity/Dependency/AmbiguousType",
+            "doc://org.swift.docc.example/documentation/BundleWithRelativePathAmbiguity/Dependency/AmbiguousType/foo()",
+        ]
+        
+        let sectionContents = [
+            content.contents(of: "Module Scope Links"),
+            content.contents(of: "Extended Module Scope Links"),
+            content.contents(of: "Local Scope Links"),
+        ]
+        
+        let sectionReferences = try sectionContents.map { sectionContent in
+            try sectionContent.listItems().map { item in try XCTUnwrap(item.firstReference(), "found no reference for \(item)") }
+        }
 
-            for listItem in content.contents(of: headingString).listItems() {
-                let reference = try XCTUnwrap(listItem.firstReference(), "found no reference for \(listItem)")
-                XCTAssertEqual(reference.identifier, absoluteReferenceString, "found mismatch for \(listItem)")
+        for resolvedReferencesOfSection in sectionReferences {
+            zip(resolvedReferencesOfSection, expectedReferences).forEach { resolved, expected in
+                XCTAssertEqual(resolved.identifier, expected)
             }
         }
-        
-        try assertListedReferencesInSectionMatchHeading("/BundleWithRelativePathAmbiguity")
-        try assertListedReferencesInSectionMatchHeading("/Dependency")
-        try assertListedReferencesInSectionMatchHeading("/BundleWithRelativePathAmbiguity/Dependency")
-        try assertListedReferencesInSectionMatchHeading("/Dependency/AmbiguousType")
-        try assertListedReferencesInSectionMatchHeading("/Dependency/AmbiguousProtocol")
-        try assertListedReferencesInSectionMatchHeading("/Dependency/UnambiguousType")
-        try assertListedReferencesInSectionMatchHeading("/BundleWithRelativePathAmbiguity/Dependency/AmbiguousType")
-        try assertListedReferencesInSectionMatchHeading("/BundleWithRelativePathAmbiguity/Dependency/AmbiguousProtocol")
-        try assertListedReferencesInSectionMatchHeading("/Dependency/AmbiguousType/unambiguousFunction()")
     }
 
     struct TestExternalReferenceResolver: ExternalReferenceResolver {