convert the PackageLoading module to the new diagnostics apis (#3754)

motivation: adapt new diagnostics apis

changes: refactor the PackageLoading module to use new diagnostics api

Author: tomerd

Sources/PackageLoading/ModuleMapGenerator.swift

@@ -8,9 +8,10 @@
  See http://swift.org/CONTRIBUTORS.txt for Swift project authors
  */
 
-import TSCBasic
-import PackageModel
+import Basics
 import Foundation
+import PackageModel
+import TSCBasic
 
 /// Name of the module map file recognized by the Clang and Swift compilers.
 public let moduleMapFilename = "module.modulemap"
@@ -86,8 +87,14 @@ public struct ModuleMapGenerator {
     }
 
     /// Inspects the file system at the public-headers directory with which the module map generator was instantiated, and returns the type of module map that applies to that directory.  This function contains all of the heuristics that implement module map policy for package targets; other functions just use the results of this determination.
-    public func determineModuleMapType(diagnostics: DiagnosticsEngine) -> ModuleMapType {
+    public func determineModuleMapType(observabilityScope: ObservabilityScope) -> ModuleMapType {
         // The following rules are documented at https://github.com/apple/swift-package-manager/blob/master/Documentation/Usage.md#creating-c-language-targets.  To avoid breaking existing packages, do not change the semantics here without making any change conditional on the tools version of the package that defines the target.
+
+        let diagnosticsEmitter = observabilityScope.makeDiagnosticsEmitter {
+            var metadata = ObservabilityMetadata()
+            metadata.targetName = self.targetName
+            return metadata
+        }
 
         // First check for a custom module map.
         let customModuleMapFile = publicHeadersDir.appending(component: moduleMapFilename)
@@ -97,7 +104,7 @@ public struct ModuleMapGenerator {
 
         // Warn if the public-headers directory is missing.  For backward compatibility reasons, this is not an error, we just won't generate a module map in that case.
         guard fileSystem.exists(publicHeadersDir) else {
-            diagnostics.emit(.missingPublicHeadersDirectory(targetName: targetName, publicHeadersDir: publicHeadersDir))
+            diagnosticsEmitter.emit(.missingPublicHeadersDirectory(targetName: targetName, publicHeadersDir: publicHeadersDir))
             return .none
         }
 
@@ -108,7 +115,7 @@ public struct ModuleMapGenerator {
         }
         catch {
             // This might fail because of a file system error, etc.
-            diagnostics.emit(.inaccessiblePublicHeadersDirectory(targetName: targetName, publicHeadersDir: publicHeadersDir, fileSystemError: error))
+            diagnosticsEmitter.emit(.inaccessiblePublicHeadersDirectory(targetName: targetName, publicHeadersDir: publicHeadersDir, fileSystemError: error))
             return .none
         }
 
@@ -122,7 +129,7 @@ public struct ModuleMapGenerator {
         if fileSystem.isFile(umbrellaHeader) {
             // In this case, 'PublicHeadersDir' is expected to contain no subdirectories.
             if directories.count != 0 {
-                diagnostics.emit(.umbrellaHeaderHasSiblingDirectories(targetName: targetName, umbrellaHeader: umbrellaHeader, siblingDirs: directories))
+                diagnosticsEmitter.emit(.umbrellaHeaderHasSiblingDirectories(targetName: targetName, umbrellaHeader: umbrellaHeader, siblingDirs: directories))
                 return .none
             }
             return .umbrellaHeader(umbrellaHeader)
@@ -131,20 +138,20 @@ public struct ModuleMapGenerator {
         /// Check for the common mistake of naming the umbrella header 'TargetName.h' instead of 'ModuleName.h'.
         let misnamedUmbrellaHeader = publicHeadersDir.appending(component: targetName + ".h")
         if fileSystem.isFile(misnamedUmbrellaHeader) {
-            diagnostics.emit(.misnamedUmbrellaHeader(misnamedUmbrellaHeader: misnamedUmbrellaHeader, umbrellaHeader: umbrellaHeader))
+            diagnosticsEmitter.emit(.misnamedUmbrellaHeader(misnamedUmbrellaHeader: misnamedUmbrellaHeader, umbrellaHeader: umbrellaHeader))
         }
 
         // If 'PublicHeadersDir/ModuleName/ModuleName.h' exists, then use it as the umbrella header.
         let nestedUmbrellaHeader = publicHeadersDir.appending(components: moduleName, moduleName + ".h")
         if fileSystem.isFile(nestedUmbrellaHeader) {
             // In this case, 'PublicHeadersDir' is expected to contain no subdirectories other than 'ModuleName'.
             if directories.count != 1 {
-                diagnostics.emit(.umbrellaHeaderParentDirHasSiblingDirectories(targetName: targetName, umbrellaHeader: nestedUmbrellaHeader, siblingDirs: directories.filter{ $0.basename != moduleName }))
+                diagnosticsEmitter.emit(.umbrellaHeaderParentDirHasSiblingDirectories(targetName: targetName, umbrellaHeader: nestedUmbrellaHeader, siblingDirs: directories.filter{ $0.basename != moduleName }))
                 return .none
             }
             // In this case, 'PublicHeadersDir' is also expected to contain no header files.
             if headers.count != 0 {
-                diagnostics.emit(.umbrellaHeaderParentDirHasSiblingHeaders(targetName: targetName, umbrellaHeader: nestedUmbrellaHeader, siblingHeaders: headers))
+                diagnosticsEmitter.emit(.umbrellaHeaderParentDirHasSiblingHeaders(targetName: targetName, umbrellaHeader: nestedUmbrellaHeader, siblingHeaders: headers))
                 return .none
             }
             return .umbrellaHeader(nestedUmbrellaHeader)
@@ -153,7 +160,7 @@ public struct ModuleMapGenerator {
         /// Check for the common mistake of naming the nested umbrella header 'TargetName.h' instead of 'ModuleName.h'.
         let misnamedNestedUmbrellaHeader = publicHeadersDir.appending(components: moduleName, targetName + ".h")
         if fileSystem.isFile(misnamedNestedUmbrellaHeader) {
-            diagnostics.emit(.misnamedUmbrellaHeader(misnamedUmbrellaHeader: misnamedNestedUmbrellaHeader, umbrellaHeader: nestedUmbrellaHeader))
+            diagnosticsEmitter.emit(.misnamedUmbrellaHeader(misnamedUmbrellaHeader: misnamedNestedUmbrellaHeader, umbrellaHeader: nestedUmbrellaHeader))
         }
 
         // Otherwise, if 'PublicHeadersDir' contains only header files and no subdirectories, use it as the umbrella directory.
@@ -197,7 +204,6 @@ public enum GeneratedModuleMapType {
     case umbrellaDirectory(AbsolutePath)
 }
 
-
 public extension ModuleMapType {
     /// Returns the type of module map to generate for this kind of module map, or nil to not generate one at all.
     var generatedModuleMapType: GeneratedModuleMapType? {
@@ -209,36 +215,35 @@ public extension ModuleMapType {
     }
 }
 
-
-private extension Diagnostic.Message {
+private extension Basics.Diagnostic {
 
     /// Warning emitted if the public-headers directory is missing.
-    static func missingPublicHeadersDirectory(targetName: String, publicHeadersDir: AbsolutePath) -> Diagnostic.Message {
+    static func missingPublicHeadersDirectory(targetName: String, publicHeadersDir: AbsolutePath) -> Self {
         .warning("no include directory found for target '\(targetName)'; libraries cannot be imported without public headers")
     }
 
     /// Error emitted if the public-headers directory is inaccessible.
-    static func inaccessiblePublicHeadersDirectory(targetName: String, publicHeadersDir: AbsolutePath, fileSystemError: Error) -> Diagnostic.Message {
+    static func inaccessiblePublicHeadersDirectory(targetName: String, publicHeadersDir: AbsolutePath, fileSystemError: Error) -> Self {
         .error("cannot access public-headers directory for target '\(targetName)': \(String(describing: fileSystemError))")
     }
 
     /// Warning emitted if a misnamed umbrella header was found.
-    static func misnamedUmbrellaHeader(misnamedUmbrellaHeader: AbsolutePath, umbrellaHeader: AbsolutePath) -> Diagnostic.Message {
+    static func misnamedUmbrellaHeader(misnamedUmbrellaHeader: AbsolutePath, umbrellaHeader: AbsolutePath) -> Self {
         .warning("\(misnamedUmbrellaHeader) should be renamed to \(umbrellaHeader) to be used as an umbrella header")
     }
 
     /// Error emitted if there are directories next to a top-level umbrella header.
-    static func umbrellaHeaderHasSiblingDirectories(targetName: String, umbrellaHeader: AbsolutePath, siblingDirs: Set<AbsolutePath>) -> Diagnostic.Message {
+    static func umbrellaHeaderHasSiblingDirectories(targetName: String, umbrellaHeader: AbsolutePath, siblingDirs: Set<AbsolutePath>) -> Self {
         .error("target '\(targetName)' has invalid header layout: umbrella header found at '\(umbrellaHeader)', but directories exist next to it: \(siblingDirs.map({ String(describing: $0) }).sorted().joined(separator: ", ")); consider removing them")
     }
 
     /// Error emitted if there are other directories next to the parent directory of a nested umbrella header.
-    static func umbrellaHeaderParentDirHasSiblingDirectories(targetName: String, umbrellaHeader: AbsolutePath, siblingDirs: Set<AbsolutePath>) -> Diagnostic.Message {
+    static func umbrellaHeaderParentDirHasSiblingDirectories(targetName: String, umbrellaHeader: AbsolutePath, siblingDirs: Set<AbsolutePath>) -> Self {
         .error("target '\(targetName)' has invalid header layout: umbrella header found at '\(umbrellaHeader)', but more than one directory exists next to its parent directory: \(siblingDirs.map({ String(describing: $0) }).sorted().joined(separator: ", ")); consider reducing them to one")
     }
 
     /// Error emitted if there are other headers next to the parent directory of a nested umbrella header.
-    static func umbrellaHeaderParentDirHasSiblingHeaders(targetName: String, umbrellaHeader: AbsolutePath, siblingHeaders: Set<AbsolutePath>) -> Diagnostic.Message {
+    static func umbrellaHeaderParentDirHasSiblingHeaders(targetName: String, umbrellaHeader: AbsolutePath, siblingHeaders: Set<AbsolutePath>) -> Self {
         .error("target '\(targetName)' has invalid header layout: umbrella header found at '\(umbrellaHeader)', but additional header files exist: \((siblingHeaders.map({ String(describing: $0) }).sorted().joined(separator: ", "))); consider reducing them to one")
     }
 }

Sources/PackageLoading/PackageBuilder.swift

@@ -885,7 +885,7 @@ public final class PackageBuilder {
 
             if fileSystem.exists(publicHeadersPath) {
                 let moduleMapGenerator = ModuleMapGenerator(targetName: potentialModule.name, moduleName: potentialModule.name.spm_mangledToC99ExtendedIdentifier(), publicHeadersDir: publicHeadersPath, fileSystem: fileSystem)
-                moduleMapType = moduleMapGenerator.determineModuleMapType(diagnostics: ObservabilitySystem.topScope.makeDiagnosticsEngine())
+                moduleMapType = moduleMapGenerator.determineModuleMapType(observabilityScope: self.observabilityScope)
             } else if targetType == .library, manifest.toolsVersion >= .v5_5 {
                 // If this clang target is a library, it must contain "include" directory.
                 throw ModuleError.invalidPublicHeadersDirectory(potentialModule.name)

Tests/PackageLoadingTests/ModuleMapGenerationTests.swift

@@ -93,12 +93,17 @@ class ModuleMapGeneration: XCTestCase {
     }
 
     func testWarnings() throws {
-        var fs = InMemoryFileSystem(emptyFiles:
-            "/Foo.c")
+        var fs = InMemoryFileSystem(emptyFiles: "/Foo.c")
         ModuleMapTester("Foo", in: fs) { result in
             result.checkNotCreated()
             result.checkDiagnostics { result in
-                result.check(diagnostic: "no include directory found for target \'Foo\'; libraries cannot be imported without public headers", severity: .warning)
+                var expectedMetadata = ObservabilityMetadata()
+                expectedMetadata.targetName = "Foo"
+                result.check(
+                    diagnostic: "no include directory found for target \'Foo\'; libraries cannot be imported without public headers",
+                    severity: .warning,
+                    metadata: expectedMetadata
+                )
             }
         }
 
@@ -114,21 +119,31 @@ class ModuleMapGeneration: XCTestCase {
 
                 """)
             result.checkDiagnostics { result in
-                result.check(diagnostic: "/include/F-o-o.h should be renamed to /include/F_o_o.h to be used as an umbrella header", severity: .warning)
+                var expectedMetadata = ObservabilityMetadata()
+                expectedMetadata.targetName = "F-o-o"
+                result.check(
+                    diagnostic: "/include/F-o-o.h should be renamed to /include/F_o_o.h to be used as an umbrella header",
+                    severity: .warning,
+                    metadata: expectedMetadata
+                )
             }
         }
     }
 
     func testUnsupportedLayouts() throws {
-        var fs: InMemoryFileSystem
-
-        fs = InMemoryFileSystem(emptyFiles:
+        var fs = InMemoryFileSystem(emptyFiles:
             "/include/Foo/Foo.h",
             "/include/Bar/Foo.h")
         ModuleMapTester("Foo", in: fs) { result in
             result.checkNotCreated()
             result.checkDiagnostics { result in
-                result.check(diagnostic: "target 'Foo' has invalid header layout: umbrella header found at '/include/Foo/Foo.h', but more than one directory exists next to its parent directory: /include/Bar; consider reducing them to one", severity: .error)
+                var expectedMetadata = ObservabilityMetadata()
+                expectedMetadata.targetName = "Foo"
+                result.check(
+                    diagnostic: "target 'Foo' has invalid header layout: umbrella header found at '/include/Foo/Foo.h', but more than one directory exists next to its parent directory: /include/Bar; consider reducing them to one",
+                    severity: .error,
+                    metadata: expectedMetadata
+                )
             }
         }
 
@@ -138,7 +153,13 @@ class ModuleMapGeneration: XCTestCase {
         ModuleMapTester("Foo", in: fs) { result in
             result.checkNotCreated()
             result.checkDiagnostics { result in
-                result.check(diagnostic: "target 'Foo' has invalid header layout: umbrella header found at '/include/Foo.h', but directories exist next to it: /include/Bar; consider removing them", severity: .error)
+                var expectedMetadata = ObservabilityMetadata()
+                expectedMetadata.targetName = "Foo"
+                result.check(
+                    diagnostic: "target 'Foo' has invalid header layout: umbrella header found at '/include/Foo.h', but directories exist next to it: /include/Bar; consider removing them",
+                    severity: .error,
+                    metadata: expectedMetadata
+                )
             }
         }
     }
@@ -149,7 +170,7 @@ func ModuleMapTester(_ targetName: String, includeDir: String = "include", in fi
     let observability = ObservabilitySystem.bootstrapForTesting()
     // Create a module map generator, and determine the type of module map to use for the header directory.  This may emit diagnostics.
     let moduleMapGenerator = ModuleMapGenerator(targetName: targetName, moduleName: targetName.spm_mangledToC99ExtendedIdentifier(), publicHeadersDir: AbsolutePath.root.appending(component: includeDir), fileSystem: fileSystem)
-    let moduleMapType = moduleMapGenerator.determineModuleMapType(diagnostics: ObservabilitySystem.topScope.makeDiagnosticsEngine())
+    let moduleMapType = moduleMapGenerator.determineModuleMapType(observabilityScope: ObservabilitySystem.topScope)
 
     // Generate a module map and capture any emitted diagnostics.
     let generatedModuleMapPath = AbsolutePath.root.appending(components: "module.modulemap")