No pages warning (#664)

* Warning when build resulted in no pages (#553)

* Emit warning when converting/previewing article-only catalogs with no technology root

* Emit diagnostics originated in the Convert Action `perform` process

rdar://112462434

* Emit coverage if no errors occurred

* Changed diagnostic message

* DiagnosticConsumer finalize() deprecated

* Marked `finalized()` as deprecated

* Protocol extension for default implementation of `flush()`

* Message for article-only warning `needs` noun corrected

rdar://108011754

Author: sofiaromorales

Sources/SwiftDocC/Infrastructure/Diagnostics/DiagnosticConsumer.swift

@@ -17,11 +17,23 @@ public protocol DiagnosticConsumer: AnyObject {
     func receive(_ problems: [Problem])
 
     /// Inform the consumer that the engine has sent all diagnostics for this build.
+    @available(*, deprecated, renamed: "flush()")
     func finalize() throws
+    
+    /// Inform the consumer that the engine has sent all diagnostics in a given context.
+    func flush() throws
 }
 
 /// A type that can format received diagnostics in way that's suitable for writing to a destination such as a file or `TextOutputStream`.
 public protocol DiagnosticFormattingConsumer: DiagnosticConsumer {
     /// Options for how problems should be formatted if written to output.
     var formattingOptions: DiagnosticFormattingOptions { get set }
 }
+
+public extension DiagnosticConsumer {
+    // Deprecated for supressing the warning emitted when calling `finalize()`
+    @available(*, deprecated)
+    func flush() throws {
+        try finalize()
+    }
+}

Sources/SwiftDocC/Infrastructure/Diagnostics/DiagnosticEngine.swift

@@ -96,10 +96,15 @@ public final class DiagnosticEngine {
         }
     }
 
+    @available(*, deprecated, renamed: "flush()")
     public func finalize() {
+        flush()
+    }
+    
+    public func flush() {
         workQueue.sync {
             for consumer in self.consumers.sync({ $0.values }) {
-                try? consumer.finalize()
+                try? consumer.flush()
             }
         }
     }

Sources/SwiftDocC/Infrastructure/DocumentationConverter.swift

@@ -178,7 +178,7 @@ public struct DocumentationConverter: DocumentationConverterProtocol {
         outputConsumer: OutputConsumer
     ) throws -> (analysisProblems: [Problem], conversionProblems: [Problem]) {
         defer {
-            diagnosticEngine.finalize()
+            diagnosticEngine.flush()
         }
 
         // Unregister the current file data provider and all its bundles

Sources/SwiftDocCUtilities/Action/Actions/Convert/ConvertAction.swift

@@ -357,6 +357,10 @@ public struct ConvertAction: Action, RecreatingContext {
     /// Converts each eligible file from the source documentation bundle,
     /// saves the results in the given output alongside the template files.
     mutating public func perform(logHandle: LogHandle) throws -> ActionResult {
+        
+        // The converter has already emitted its problems to the diagnostic engine. 
+        // Track additional problems separately to avoid repeating the converter's problems.
+        var postConversionProblems: [Problem] = []
         let totalTimeMetric = benchmark(begin: Benchmark.Duration(id: "convert-total-time"))
 
         // While running this method keep the `isPerforming` flag up.
@@ -365,6 +369,7 @@ public struct ConvertAction: Action, RecreatingContext {
         defer {
             didPerformFuture?()
             isPerforming.sync({ $0 = false })
+            diagnosticEngine.flush()
         }
 
         if let outOfProcessResolver = outOfProcessResolver {
@@ -451,11 +456,18 @@ public struct ConvertAction: Action, RecreatingContext {
             throw error
         }
 
-        var allProblems = analysisProblems + conversionProblems
-
-        if allProblems.containsErrors == false {
-            let coverageResults = try coverageAction.perform(logHandle: logHandle)
-            allProblems.append(contentsOf: coverageResults.problems)
+        var didEncounterError = analysisProblems.containsErrors || conversionProblems.containsErrors
+        if try context.renderRootModules.isEmpty {
+            postConversionProblems.append(
+                Problem(
+                    diagnostic: Diagnostic(
+                        severity: .warning,
+                        identifier: "org.swift.docc.MissingTechnologyRoot",
+                         summary: "No TechnologyRoot to organize article-only documentation.",
+                         explanation: "Article-only documentation needs a TechnologyRoot page (indicated by a `TechnologyRoot` directive within a `Metadata` directive) to define the root of the documentation hierarchy."
+                     )
+                )
+            )
         }
 
         // If we're building a navigation index, finalize the process and collect encountered problems.
@@ -468,17 +480,27 @@ public struct ConvertAction: Action, RecreatingContext {
             // Always emit a JSON representation of the index but only emit the LMDB
             // index if the user has explicitly opted in with the `--emit-lmdb-index` flag.
             let indexerProblems = indexer.finalize(emitJSON: true, emitLMDB: buildLMDBIndex)
-            allProblems.append(contentsOf: indexerProblems)
+            postConversionProblems.append(contentsOf: indexerProblems)
         }
+        
+        // Output to the user the problems encountered during the convert process
+        diagnosticEngine.emit(postConversionProblems)
 
         // Stop the "total time" metric here. The moveOutput time isn't very interesting to include in the benchmark.
         // New tasks and computations should be added above this line so that they're included in the benchmark.
         benchmark(end: totalTimeMetric)
 
+        if !didEncounterError {
+            let coverageResults = try coverageAction.perform(logHandle: logHandle)
+            postConversionProblems.append(contentsOf: coverageResults.problems)
+        }
+        
+        didEncounterError = didEncounterError || postConversionProblems.containsErrors
+        
         // We should generally only replace the current build output if we didn't encounter errors
         // during conversion. However, if the `emitDigest` flag is true,
         // we should replace the current output with our digest of problems.
-        if !allProblems.containsErrors || emitDigest {
+        if !didEncounterError || emitDigest {
             try moveOutput(from: temporaryFolder, to: targetDirectory)
         }
 
@@ -516,12 +538,7 @@ public struct ConvertAction: Action, RecreatingContext {
             try outputConsumer.consume(benchmarks: Benchmark.main)
         }
 
-        // If the user didn't provide the `analyze` flag, filter based on diagnostic level
-        if !analyze {
-            allProblems.removeAll(where: { $0.diagnostic.severity.rawValue >  diagnosticLevel.rawValue })
-        }
-
-        return ActionResult(didEncounterError: allProblems.containsErrors, outputs: [targetDirectory])
+        return ActionResult(didEncounterError: didEncounterError, outputs: [targetDirectory])
     }
 
     func createTempFolder(with templateURL: URL?) throws -> URL {

Sources/SwiftDocCUtilities/Action/Actions/PreviewAction.swift

@@ -264,11 +264,19 @@ extension PreviewAction {
 #endif
 
 extension DocumentationContext {
+    
+    /// A collection of non-implicit root modules
+    var renderRootModules: [ResolvedTopicReference] {
+        get throws {
+            try rootModules.filter({ try !entity(with: $0).isVirtual })
+        }
+    }
+    
     /// Finds the module and technology pages in the context and returns their paths.
     func previewPaths() throws -> [String] {
         let urlGenerator = PresentationURLGenerator(context: self, baseURL: URL(string: "/")!)
 
-        let rootModules = try rootModules.filter { try !entity(with: $0).isVirtual }
+        let rootModules = try renderRootModules
 
         return (rootModules + rootTechnologies).map { page in
             urlGenerator.presentationURLForReference(page).absoluteString

Tests/SwiftDocCUtilitiesTests/ConvertActionTests.swift

@@ -3026,6 +3026,35 @@ class ConvertActionTests: XCTestCase {
         }) .map { DiagnosticConsoleWriter.formattedDescription(for: $0.diagnostic) }.sorted().joined(separator: "\n")
     }
 
+    // Tests that when converting a catalog with no technology root a warning is raised (r93371988)
+    func testConvertWithNoTechnologyRoot() throws {
+        let bundle = Folder(name: "unit-test.docc", content: [
+            InfoPlist(displayName: "TestBundle", identifier: "com.test.example"),
+            TextFile(name: "Documentation.md", utf8Content: "")
+        ])
+        let testDataProvider = try TestFileSystem(folders: [bundle, Folder.emptyHTMLTemplateDirectory])
+        let targetDirectory = URL(fileURLWithPath: testDataProvider.currentDirectoryPath)
+            .appendingPathComponent("target", isDirectory: true)
+        let engine = DiagnosticEngine()
+        var action = try ConvertAction(
+            documentationBundleURL: bundle.absoluteURL,
+            outOfProcessResolver: nil,
+            analyze: true,
+            targetDirectory: targetDirectory,
+            htmlTemplateDirectory: Folder.emptyHTMLTemplateDirectory.absoluteURL,
+            emitDigest: false,
+            currentPlatforms: nil,
+            dataProvider: testDataProvider,
+            fileManager: testDataProvider,
+            temporaryDirectory: createTemporaryDirectory(),
+            diagnosticEngine: engine
+        )
+        let _ = try action.perform(logHandle: .standardOutput)
+        XCTAssertEqual(engine.problems.count, 1)
+        XCTAssertEqual(engine.problems.map { $0.diagnostic.identifier }, ["org.swift.docc.MissingTechnologyRoot"])
+        XCTAssert(engine.problems.contains(where: { $0.diagnostic.severity == .warning }))
+    }
+    
     #endif
 }