Lower severity of source-symbol-not-found diagnostic (#966)

* Elaborate on diagnostic message when the source of a symbol graph relationship is missing

* Lower severity of the diagnostic when the "source" of a symbol graph relationship is missing.

Since the solution is for the tool that created the symbol graph file to not emit that relationship, DocC can safely skip it and proceed with the build. This issue isn't severe enough to fail the build.

* Further reduce the severity to a debug-only assertion.

Author: d-ronnqvist

Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphRelationshipsBuilder.swift

@@ -12,15 +12,37 @@ import SymbolKit
 
 /// A set of functions that add relationship information to a topic graph.
 struct SymbolGraphRelationshipsBuilder {
-    /// A namespace for creation of topic-graph related problems.
-    enum NodeProblem {
-        /// Returns a problem about a node with the given precise identifier not being found.
-        static func notFound(_ identifier: String) -> Problem {
-            return Problem(diagnostic: Diagnostic(source: nil, severity: .error, range: nil, identifier: "org.swift.docc.SymbolNodeNotFound", summary: "Symbol with identifier \(identifier.singleQuoted) couldn't be found"), possibleSolutions: [])
+    /// A namespace for debug assert messages for data-correctness issues in the symbol graph data.
+    private enum AssertionMessages {
+        static func sourceNotFound(_ relationship: SymbolGraph.Relationship) -> String {
+            """
+            Source symbol \(relationship.source.singleQuoted) not found locally, from \(relationship.kind.rawValue.singleQuoted) relationship to \(relationship.target.singleQuoted).
+            
+            The "source" of a symbol graph relationship should always refer to a symbol in the same symbol graph file.
+            If it doesn't, then the tool that created the symbol graph file should move the relationship to the symbol graph file that defines the "source" symbol \
+            or remove the relationship if none of the created symbol graph file defines the "source" symbol.
+            
+            The "target" may refer to a symbol in another module.
+            For example, if local symbol conforms to a protocol from another module, \
+            there will be a "{ source: local-symbol-ID, kind: conformsTo, target: protocol-in-other-module-ID }" relationship.
+            
+            A symbol graph relationship with a non-local "source" symbol is a bug in the tool that created the symbol graph file.
+            """
         }
-        /// Returns a problem about a node with the given reference not being found.
-        static func invalidReference(_ reference: String) -> Problem {
-            return Problem(diagnostic: Diagnostic(source: nil, severity: .error, range: nil, identifier: "org.swift.docc.InvalidSymbolIdentifier", summary: "Relationship symbol path \(reference.singleQuoted) isn't valid"), possibleSolutions: [])
+        
+        static func overloadGroupNotFound(_ relationship: SymbolGraph.Relationship) -> String {
+            """
+            Overload group \(relationship.source.singleQuoted) not found locally, from \(relationship.kind.rawValue.singleQuoted) relationship of \(relationship.source.singleQuoted).
+            
+            Both the "source" and "target" of an \(relationship.kind.rawValue.singleQuoted) symbol graph relationships with should always refer to symbols in the same symbol graph file.
+            A \(relationship.kind.rawValue.singleQuoted) symbol graph relationship with a non-local "target" symbol is a bug in the tool that created the symbol graph file.
+            """
+        }
+        
+        static func invalidSymbolReference(_ reference: SymbolReference) -> String {
+            return """
+            Failed to create an unresolved reference for \(reference.path.singleQuoted). It contains characters that are not allowed in the "path" of a RFC 3986 URL.
+            """
         }
     }
 
@@ -47,7 +69,7 @@ struct SymbolGraphRelationshipsBuilder {
               let implementorSymbol = implementorNode.semantic as? Symbol
         else {
             // The source node for implementation relationship not found.
-            engine.emit(NodeProblem.notFound(edge.source))
+            assertionFailure(AssertionMessages.sourceNotFound(edge))
             return
         }
 
@@ -63,7 +85,7 @@ struct SymbolGraphRelationshipsBuilder {
             let symbolReference = SymbolReference(edge.target, interfaceLanguage: language, symbol: localCache[edge.target]?.symbol)
             guard let unresolved = UnresolvedTopicReference(symbolReference: symbolReference, bundle: bundle) else {
                 // The symbol reference format is invalid.
-                engine.emit(NodeProblem.invalidReference(symbolReference.path))
+                assertionFailure(AssertionMessages.invalidSymbolReference(symbolReference))
                 return
             }
 
@@ -97,7 +119,7 @@ struct SymbolGraphRelationshipsBuilder {
             // Make the implementation a child of the requirement
             guard let childReference = localCache.reference(symbolID: edge.source) else {
                 // The child wasn't found, invalid reference in relationship.
-                engine.emit(SymbolGraphRelationshipsBuilder.NodeProblem.notFound(edge.source))
+                assertionFailure(SymbolGraphRelationshipsBuilder.AssertionMessages.sourceNotFound(edge))
                 return
             }
 
@@ -132,7 +154,7 @@ struct SymbolGraphRelationshipsBuilder {
               let conformingSymbol = conformingNode.semantic as? Symbol
         else {
             // The source node for conformance relationship not found.
-            engine.emit(NodeProblem.notFound(edge.source))
+            assertionFailure(AssertionMessages.sourceNotFound(edge))
             return
         }
 
@@ -153,7 +175,7 @@ struct SymbolGraphRelationshipsBuilder {
             let symbolReference = SymbolReference(edge.target, interfaceLanguage: language, symbol: localCache[edge.target]?.symbol)
             guard let unresolved = UnresolvedTopicReference(symbolReference: symbolReference, bundle: bundle) else {
                 // The symbol reference format is invalid.
-                engine.emit(NodeProblem.invalidReference(symbolReference.path))
+                assertionFailure(AssertionMessages.invalidSymbolReference(symbolReference))
                 return
             }
             conformanceNodeReference = .unresolved(unresolved)
@@ -221,7 +243,7 @@ struct SymbolGraphRelationshipsBuilder {
               let childSymbol = childNode.semantic as? Symbol
         else {
             // The source node for inheritance relationship not found.
-            engine.emit(NodeProblem.notFound(edge.source))
+            assertionFailure(AssertionMessages.sourceNotFound(edge))
             return
         }
 
@@ -240,7 +262,7 @@ struct SymbolGraphRelationshipsBuilder {
             let symbolReference = SymbolReference(edge.target, interfaceLanguage: language, symbol: nil)
             guard let unresolved = UnresolvedTopicReference(symbolReference: symbolReference, bundle: bundle) else {
                 // The symbol reference format is invalid.
-                engine.emit(NodeProblem.invalidReference(symbolReference.path))
+                assertionFailure(AssertionMessages.invalidSymbolReference(symbolReference))
                 return
             }
             parentNodeReference = .unresolved(unresolved)
@@ -321,7 +343,7 @@ struct SymbolGraphRelationshipsBuilder {
               let requiredSymbol = requiredNode.semantic as? Symbol
         else {
             // The source node for requirement relationship not found.
-            engine.emit(NodeProblem.notFound(edge.source))
+            assertionFailure(AssertionMessages.sourceNotFound(edge))
             return
         }
         requiredSymbol.isRequired = required
@@ -438,11 +460,11 @@ struct SymbolGraphRelationshipsBuilder {
         engine: DiagnosticEngine
     ) {
         guard let overloadNode = localCache[edge.source] else {
-            engine.emit(NodeProblem.notFound(edge.source))
+            assertionFailure(AssertionMessages.sourceNotFound(edge))
             return
         }
         guard let overloadGroupNode = localCache[edge.target] else {
-            engine.emit(NodeProblem.notFound(edge.target))
+            assertionFailure(AssertionMessages.overloadGroupNotFound(edge))
             return
         }
 

Tests/SwiftDocCUtilitiesTests/ConvertActionTests.swift

@@ -36,42 +36,6 @@ class ConvertActionTests: XCTestCase {
         forResource: "TestBundle", withExtension: "docc", subdirectory: "Test Bundles")!
         .appendingPathComponent("project.zip")
 
-    /// A symbol graph file that has missing symbols.
-    let incompleteSymbolGraphFile = TextFile(name: "TechnologyX.symbols.json", utf8Content: """
-        {
-          "metadata": {
-              "formatVersion" : {
-                  "major" : 1
-              },
-              "generator" : "app/1.0"
-          },
-          "module" : {
-            "name" : "MyKit",
-            "platform" : {
-              "architecture" : "x86_64",
-              "vendor" : "apple",
-              "operatingSystem" : {
-                "name" : "ios",
-                "minimumVersion" : {
-                  "major" : 13,
-                  "minor" : 0,
-                  "patch" : 0
-                }
-              }
-            }
-          },
-          "symbols" : [],
-          "relationships" : [
-            {
-              "source" : "s:5MyKit0A5ProtocolP",
-              "target" : "s:5Foundation0A5EarhartP",
-              "kind" : "conformsTo"
-            }
-          ]
-        }
-        """
-    )
-    
     func testCopyingImageAssets() throws {
         XCTAssert(FileManager.default.fileExists(atPath: imageFile.path))
         let testImageName = "TestImage.png"
@@ -667,7 +631,6 @@ class ConvertActionTests: XCTestCase {
         expectedOutput.assertExist(at: result.outputs[0], fileManager: testDataProvider)
     }
 
-    /// Ensures that we always generate diagnosticJSON when there are errors. (rdar://72345373)
     func testOutputFolderContainsDiagnosticJSONWhenThereAreErrorsAndNoTemplate() throws {
         // Documentation bundle that contains an image
         let bundle = Folder(name: "unit-test.docc", content: [
@@ -809,7 +772,6 @@ class ConvertActionTests: XCTestCase {
         expectedOutput.assertExist(at: result.outputs[0], fileManager: testDataProvider)
     }
 
-    /// Ensures we never delete an existing build folder if conversion fails (rdar://72339050)
     func testOutputFolderIsNotRemovedWhenThereAreErrors() throws {
         let tutorialsFile = TextFile(name: "TechnologyX.tutorial", utf8Content: """
             @Tutorials(name: "Technology Z") {
@@ -847,11 +809,6 @@ class ConvertActionTests: XCTestCase {
             bundleInfoPlist,
         ])
 
-        let badBundle = Folder(name: "unit-test.docc", content: [
-            incompleteSymbolGraphFile,
-            bundleInfoPlist,
-        ])
-        
         let testDataProvider = try TestFileSystem(folders: [goodBundle, Folder.emptyHTMLTemplateDirectory])
         let targetDirectory = URL(fileURLWithPath: testDataProvider.currentDirectoryPath)
             .appendingPathComponent("target", isDirectory: true)
@@ -883,76 +840,6 @@ class ConvertActionTests: XCTestCase {
             ])
             expectedOutput.assertExist(at: targetDirectory, fileManager: testDataProvider)
         }
-        
-        try testDataProvider.updateDocumentationBundles(withFolders: [badBundle])
-        
-        do {
-            var action = try ConvertAction(
-                documentationBundleURL: badBundle.absoluteURL,
-                outOfProcessResolver: nil,
-                analyze: false,
-                targetDirectory: targetDirectory,
-                htmlTemplateDirectory: Folder.emptyHTMLTemplateDirectory.absoluteURL,
-                emitDigest: false,
-                currentPlatforms: nil,
-                dataProvider: testDataProvider,
-                fileManager: testDataProvider,
-                temporaryDirectory: testDataProvider.uniqueTemporaryDirectory())
-            let result = try action.perform(logHandle: .none)
-
-            XCTAssert(
-                result.didEncounterError,
-                "We expect errors to occur during during conversion of the bad test bundle."
-            )
-
-            // Verify that the build output folder from the former successful conversion
-            // still exists after this failure.
-            let expectedOutput = Folder(name: ".docc-build", content: [
-                Folder(name: "data", content: [
-                ]),
-            ])
-            expectedOutput.assertExist(at: targetDirectory, fileManager: testDataProvider)
-        }
-    }
-    
-    func testOutputFolderContainsDiagnosticJSONWhenThereAreErrors() throws {
-        // Documentation bundle that contains an image
-        let bundle = Folder(name: "unit-test.docc", content: [
-            incompleteSymbolGraphFile,
-            InfoPlist(displayName: "TestBundle", identifier: "com.test.example"),
-        ])
-
-        let testDataProvider = try TestFileSystem(folders: [bundle, Folder.emptyHTMLTemplateDirectory])
-        let targetDirectory = URL(fileURLWithPath: testDataProvider.currentDirectoryPath)
-            .appendingPathComponent("target", isDirectory: true)
-
-        var action = try ConvertAction(
-            documentationBundleURL: bundle.absoluteURL,
-            outOfProcessResolver: nil,
-            analyze: false,
-            targetDirectory: targetDirectory,
-            htmlTemplateDirectory: Folder.emptyHTMLTemplateDirectory.absoluteURL,
-            emitDigest: true,
-            currentPlatforms: nil,
-            dataProvider: testDataProvider,
-            fileManager: testDataProvider,
-            temporaryDirectory: testDataProvider.uniqueTemporaryDirectory())
-        let result = try action.perform(logHandle: .none)
-
-        // Verify that the following files and folder exist at the output location
-        let expectedOutput = Folder(name: ".docc-build", content: [
-            JSONFile(name: "diagnostics.json", content: [
-                Digest.Diagnostic(
-                    start: nil,
-                    source: nil,
-                    severity: .error,
-                    summary: "Symbol with identifier 's:5MyKit0A5ProtocolP' couldn't be found",
-                    explanation: nil,
-                    notes: []
-                ),
-            ]),
-        ])
-        expectedOutput.assertExist(at: result.outputs[0], fileManager: testDataProvider)
     }
 
     /// Verifies that digest is correctly emitted for API documentation topics
@@ -1274,7 +1161,7 @@ class ConvertActionTests: XCTestCase {
         ])
     }
 
-    func testDownloadMetadataIsWritenToOutputFolder() throws {
+    func testDownloadMetadataIsWrittenToOutputFolder() throws {
         let bundle = Folder(name: "unit-test.docc", content: [
             CopyOfFile(original: projectZipFile),
             CopyOfFile(original: imageFile, newName: "referenced-tutorials-image.png"),
@@ -2387,7 +2274,6 @@ class ConvertActionTests: XCTestCase {
             This article has a malformed title and can't be analyzed, so it
             produces one warning.
             """),
-            incompleteSymbolGraphFile,
         ])
 
         let testDataProvider = try TestFileSystem(folders: [bundle, Folder.emptyHTMLTemplateDirectory])
@@ -2411,8 +2297,8 @@ class ConvertActionTests: XCTestCase {
         )
         let result = try action.perform(logHandle: .none)
 
-        XCTAssertEqual(engine.problems.count, 1, "\(ConvertAction.self) didn't filter out diagnostics above the 'error' level.")
-        XCTAssert(result.didEncounterError)
+        XCTAssertEqual(engine.problems.count, 0, "\(ConvertAction.self) didn't filter out diagnostics at-or-above the 'error' level.")
+        XCTAssertFalse(result.didEncounterError, "The issues with this test bundle are not severe enough to fail the build.")
     }
 
     func testDiagnosticLevelIgnoredWhenAnalyzeIsPresent() throws {
@@ -2425,7 +2311,6 @@ class ConvertActionTests: XCTestCase {
             This article has a malformed title and can't be analyzed, so it
             produces one warning.
             """),
-            incompleteSymbolGraphFile,
         ])
 
         let testDataProvider = try TestFileSystem(folders: [bundle, Folder.emptyHTMLTemplateDirectory])
@@ -2449,9 +2334,9 @@ class ConvertActionTests: XCTestCase {
         )
         let result = try action.perform(logHandle: .none)
 
-        XCTAssertEqual(engine.problems.count, 2, "\(ConvertAction.self) shouldn't filter out diagnostics when the '--analyze' flag is passed")
-        XCTAssertEqual(engine.problems.map { $0.diagnostic.identifier }, ["org.swift.docc.Article.Title.NotFound", "org.swift.docc.SymbolNodeNotFound"])
-        XCTAssert(result.didEncounterError)
+        XCTAssertEqual(engine.problems.count, 1, "\(ConvertAction.self) shouldn't filter out diagnostics when the '--analyze' flag is passed")
+        XCTAssertEqual(engine.problems.map { $0.diagnostic.identifier }, ["org.swift.docc.Article.Title.NotFound"])
+        XCTAssertFalse(result.didEncounterError, "The issues with this test bundle are not severe enough to fail the build.")
         XCTAssert(engine.problems.contains(where: { $0.diagnostic.severity == .warning }))
     }
 
@@ -2465,7 +2350,6 @@ class ConvertActionTests: XCTestCase {
             This article has a malformed title and can't be analyzed, so it
             produces one warning.
             """),
-            incompleteSymbolGraphFile,
         ])
 
         let testDataProvider = try TestFileSystem(folders: [bundle, Folder.emptyHTMLTemplateDirectory])
@@ -2485,9 +2369,7 @@ class ConvertActionTests: XCTestCase {
             temporaryDirectory: testDataProvider.uniqueTemporaryDirectory(),
             diagnosticLevel: "error"
         )
-        XCTAssertThrowsError(try action.performAndHandleResult(logHandle: .none)) { error in
-            XCTAssert(error is ErrorsEncountered, "Unexpected error type thrown by \(ConvertAction.self)")
-        }
+        XCTAssertNoThrow(try action.performAndHandleResult(logHandle: .none))
     }
 
     func testWritesDiagnosticFileWhenThrowingError() throws {
@@ -2500,7 +2382,6 @@ class ConvertActionTests: XCTestCase {
             This article has a malformed title and can't be analyzed, so it
             produces one warning.
             """),
-            incompleteSymbolGraphFile,
         ])
 
         let testDataProvider = try TestFileSystem(folders: [bundle, Folder.emptyHTMLTemplateDirectory])
@@ -2527,9 +2408,7 @@ class ConvertActionTests: XCTestCase {
 
         // TODO: Support TestFileSystem in DiagnosticFileWriter
         XCTAssertFalse(FileManager.default.fileExists(atPath: diagnosticFile.path), "Diagnostic file doesn't exist before")
-        XCTAssertThrowsError(try action.performAndHandleResult(logHandle: .none)) { error in
-            XCTAssert(error is ErrorsEncountered, "Unexpected error type thrown by \(ConvertAction.self)")
-        }
+        XCTAssertNoThrow(try action.performAndHandleResult(logHandle: .none))
         XCTAssertTrue(FileManager.default.fileExists(atPath: diagnosticFile.path), "Diagnostic file exist after")
     }
 
@@ -2580,7 +2459,6 @@ class ConvertActionTests: XCTestCase {
         let bundle = Folder(name: "unit-test.docc", content: [
             InfoPlist(displayName: "TestBundle", identifier: "com.test.example"),
             CopyOfFile(original: symbolGraphFile, newName: "MyKit.symbols.json"),
-            incompleteSymbolGraphFile,
         ])
 
         let testDataProvider = try TestFileSystem(folders: [bundle, Folder.emptyHTMLTemplateDirectory])
@@ -2602,13 +2480,12 @@ class ConvertActionTests: XCTestCase {
             temporaryDirectory: testDataProvider.uniqueTemporaryDirectory()
         )
 
-        XCTAssertThrowsError(try action.performAndHandleResult(logHandle: .none), "The test bundle should have thrown an error about an incomplete symbol graph file")
-        XCTAssert(testDataProvider.fileExists(atPath: digestFileURL.path), "The digest file should have been written even though compilation errors occurred")
+        XCTAssertNoThrow(try action.performAndHandleResult(logHandle: .none))
+        XCTAssert(testDataProvider.fileExists(atPath: digestFileURL.path))
 
         let data = try testDataProvider.contentsOfURL(digestFileURL)
         let diagnostics = try RenderJSONDecoder.makeDecoder().decode([Digest.Diagnostic].self, from: data)
-        XCTAssertEqual(diagnostics.count, 1)
-
+        XCTAssertEqual(diagnostics.count, 0)
     }
 
     func testRenderIndexJSONGeneration() throws {