Merge pull request #706 from bob-wilson/bwilson/header-dependency-validation

Add HEADER_DEPENDENCIES build setting with validation

Author: bob-wilson

Sources/SWBCore/Dependencies.swift

@@ -92,49 +92,21 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
         self.init(validate: validate, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext)
     }
 
-    /// Compute missing module dependencies from Clang imports.
-    ///
-    /// The compiler tracing information does not provide the import locations or whether they are public imports
-    /// (which depends on whether the import is in an installed header file).
-    /// If `files` is nil, the current toolchain does support the feature to trace imports.
-    public func computeMissingDependencies(files: [Path]?) -> [(ModuleDependency, importLocations: [Diagnostic.Location])]? {
-        guard validate != .no else { return [] }
-        guard let files else {
-            return nil
-        }
-
-        // The following is a provisional/incomplete mechanism for resolving a module dependency from a file path.
-        // For now, just grab the framework name and assume there is a module with the same name.
-        func findFrameworkName(_ file: Path) -> String? {
-            if file.fileExtension == "framework" {
-                return file.basenameWithoutSuffix
-            }
-            return file.dirname.isEmpty || file.dirname.isRoot ? nil : findFrameworkName(file.dirname)
-        }
-
-        let moduleDependencyNames = moduleDependencies.map { $0.name }
-        let fileNames = files.compactMap { findFrameworkName($0) }
-        let missingDeps = Set(fileNames.filter {
-            return !moduleDependencyNames.contains($0)
-        }.map {
-            ModuleDependency(name: $0, accessLevel: .Private)
-        })
-
-        return missingDeps.map { ($0, []) }
-    }
-
-    /// Compute missing module dependencies from Swift imports.
+    /// Compute missing module dependencies.
     ///
     /// If `imports` is nil, the current toolchain does not support the features to gather imports.
-    public func computeMissingDependencies(imports: [(ModuleDependency, importLocations: [Diagnostic.Location])]?) -> [(ModuleDependency, importLocations: [Diagnostic.Location])]? {
+    public func computeMissingDependencies(
+        imports: [(ModuleDependency, importLocations: [Diagnostic.Location])]?,
+        fromSwift: Bool
+    ) -> [(ModuleDependency, importLocations: [Diagnostic.Location])]? {
         guard validate != .no else { return [] }
         guard let imports else {
             return nil
         }
 
         return imports.filter {
             // ignore module deps without source locations, these are inserted by swift / swift-build and we should treat them as implementation details which we can track without needing the user to declare them
-            if $0.importLocations.isEmpty { return false }
+            if fromSwift && $0.importLocations.isEmpty { return false }
 
             // TODO: if the difference is just the access modifier, we emit a new entry, but ultimately our fixit should update the existing entry or emit an error about a conflict
             if moduleDependencies.contains($0.0) { return false }
@@ -235,6 +207,166 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
     }
 }
 
+public struct HeaderDependency: Hashable, Sendable, SerializableCodable {
+    public let name: String
+    public let accessLevel: AccessLevel
+
+    public enum AccessLevel: String, Hashable, Sendable, CaseIterable, Codable, Serializable {
+        case Private = "private"
+        case Public = "public"
+
+        public init(_ string: String) throws {
+            guard let accessLevel = AccessLevel(rawValue: string) else {
+                throw StubError.error("unexpected access modifier '\(string)', expected one of: \(AccessLevel.allCases.map { $0.rawValue }.joined(separator: ", "))")
+            }
+
+            self = accessLevel
+        }
+    }
+
+    public init(name: String, accessLevel: AccessLevel) {
+        self.name = name
+        self.accessLevel = accessLevel
+    }
+
+    public init(entry: String) throws {
+        var it = entry.split(separator: " ").makeIterator()
+        switch (it.next(), it.next(), it.next()) {
+        case (let .some(name), nil, nil):
+            self.name = String(name)
+            self.accessLevel = .Private
+
+        case (let .some(accessLevel), let .some(name), nil):
+            self.name = String(name)
+            self.accessLevel = try AccessLevel(String(accessLevel))
+
+        default:
+            throw StubError.error("expected 1 or 2 space-separated components in: \(entry)")
+        }
+    }
+
+    public var asBuildSettingEntry: String {
+        "\(accessLevel == .Private ? "" : "\(accessLevel.rawValue) ")\(name)"
+    }
+
+    public var asBuildSettingEntryQuotedIfNeeded: String {
+        let e = asBuildSettingEntry
+        return e.contains(" ") ? "\"\(e)\"" : e
+    }
+}
+
+public struct HeaderDependenciesContext: Sendable, SerializableCodable {
+    public var validate: BooleanWarningLevel
+    var headerDependencies: [HeaderDependency]
+    var fixItContext: FixItContext?
+
+    init(validate: BooleanWarningLevel, headerDependencies: [HeaderDependency], fixItContext: FixItContext? = nil) {
+        self.validate = validate
+        self.headerDependencies = headerDependencies
+        self.fixItContext = fixItContext
+    }
+
+    public init?(settings: Settings) {
+        let validate = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_HEADER_DEPENDENCIES)
+        guard validate != .no else { return nil }
+        let fixItContext = HeaderDependenciesContext.FixItContext(settings: settings)
+        self.init(validate: validate, headerDependencies: settings.headerDependencies, fixItContext: fixItContext)
+    }
+
+    /// Make diagnostics for missing header dependencies.
+    ///
+    /// The compiler tracing information does not provide the include locations or whether they are public imports
+    /// (which depends on whether the import is in an installed header file).
+    /// If `includes` is nil, the current toolchain does support the feature to trace imports.
+    public func makeDiagnostics(includes: [Path]?) -> [Diagnostic] {
+        guard validate != .no else { return [] }
+        guard let includes else {
+            return [Diagnostic(
+                behavior: .warning,
+                location: .unknown,
+                data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_HEADER_DEPENDENCIES.name)"))]
+        }
+
+        let headerDependencyNames = headerDependencies.map { $0.name }
+        let missingDeps = includes.filter { file in
+            return !headerDependencyNames.contains(where: { file.ends(with: $0) })
+        }.map {
+            // TODO: What if the basename doesn't uniquely identify the header?
+            HeaderDependency(name: $0.basename, accessLevel: .Private)
+        }
+
+        guard !missingDeps.isEmpty else { return [] }
+
+        let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning
+
+        let fixIt = fixItContext?.makeFixIt(newHeaders: missingDeps)
+        let fixIts = fixIt.map { [$0] } ?? []
+
+        let message = "Missing entries in \(BuiltinMacros.HEADER_DEPENDENCIES.name): \(missingDeps.map { $0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))"
+
+        let location: Diagnostic.Location = fixIt.map {
+            Diagnostic.Location.path($0.sourceRange.path, line: $0.sourceRange.endLine, column: $0.sourceRange.endColumn)
+        } ?? Diagnostic.Location.buildSetting(BuiltinMacros.HEADER_DEPENDENCIES)
+
+        return [Diagnostic(
+            behavior: behavior,
+            location: location,
+            data: DiagnosticData(message),
+            fixIts: fixIts)]
+    }
+
+    struct FixItContext: Sendable, SerializableCodable {
+        var sourceRange: Diagnostic.SourceRange
+        var modificationStyle: ModificationStyle
+
+        init(sourceRange: Diagnostic.SourceRange, modificationStyle: ModificationStyle) {
+            self.sourceRange = sourceRange
+            self.modificationStyle = modificationStyle
+        }
+
+        init?(settings: Settings) {
+            guard let target = settings.target else { return nil }
+            let thisTargetCondition = MacroCondition(parameter: BuiltinMacros.targetNameCondition, valuePattern: target.name)
+
+            if let assignment = (settings.globalScope.table.lookupMacro(BuiltinMacros.HEADER_DEPENDENCIES)?.sequence.first {
+                   $0.location != nil && ($0.conditions?.conditions == [thisTargetCondition] || ($0.conditions?.conditions.isEmpty ?? true))
+               }),
+               let location = assignment.location
+            {
+                self.init(sourceRange: .init(path: location.path, startLine: location.endLine, startColumn: location.endColumn, endLine: location.endLine, endColumn: location.endColumn), modificationStyle: .appendToExistingAssignment)
+            }
+            else if let path = settings.constructionComponents.targetXcconfigPath {
+                self.init(sourceRange: .init(path: path, startLine: .max, startColumn: .max, endLine: .max, endColumn: .max), modificationStyle: .insertNewAssignment(targetNameCondition: nil))
+            }
+            else if let path = settings.constructionComponents.projectXcconfigPath {
+                self.init(sourceRange: .init(path: path, startLine: .max, startColumn: .max, endLine: .max, endColumn: .max), modificationStyle: .insertNewAssignment(targetNameCondition: target.name))
+            }
+            else {
+                return nil
+            }
+        }
+
+        enum ModificationStyle: Sendable, SerializableCodable, Hashable {
+            case appendToExistingAssignment
+            case insertNewAssignment(targetNameCondition: String?)
+        }
+
+        func makeFixIt(newHeaders: [HeaderDependency]) -> Diagnostic.FixIt {
+            let stringValue = newHeaders.map { $0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " ")
+            let newText: String
+            switch modificationStyle {
+            case .appendToExistingAssignment:
+                newText = " \(stringValue)"
+            case .insertNewAssignment(let targetNameCondition):
+                let targetCondition = targetNameCondition.map { "[target=\($0)]" } ?? ""
+                newText = "\n\(BuiltinMacros.HEADER_DEPENDENCIES.name)\(targetCondition) = $(inherited) \(stringValue)\n"
+            }
+
+            return Diagnostic.FixIt(sourceRange: sourceRange, newText: newText)
+        }
+    }
+}
+
 public struct DependencyValidationInfo: Hashable, Sendable, Codable {
     public struct Import: Hashable, Sendable, Codable {
         public let dependency: ModuleDependency
@@ -247,7 +379,7 @@ public struct DependencyValidationInfo: Hashable, Sendable, Codable {
     }
 
     public enum Payload: Hashable, Sendable, Codable {
-        case clangDependencies(files: [String])
+        case clangDependencies(imports: [Import], includes: [Path])
         case swiftDependencies(imports: [Import])
         case unsupported
     }

Sources/SWBCore/PlannedTaskAction.swift

@@ -337,6 +337,7 @@ public protocol TaskActionCreationDelegate
     func createValidateProductTaskAction() -> any PlannedTaskAction
     func createConstructStubExecutorInputFileListTaskAction() -> any PlannedTaskAction
     func createClangCompileTaskAction() -> any PlannedTaskAction
+    func createClangNonModularCompileTaskAction() -> any PlannedTaskAction
     func createClangScanTaskAction() -> any PlannedTaskAction
     func createSwiftDriverTaskAction() -> any PlannedTaskAction
     func createSwiftCompilationRequirementTaskAction() -> any PlannedTaskAction

Sources/SWBCore/Settings/BuiltinMacros.swift

@@ -732,6 +732,7 @@ public final class BuiltinMacros {
     public static let GLOBAL_CFLAGS = BuiltinMacros.declareStringListMacro("GLOBAL_CFLAGS")
     public static let HEADERMAP_INCLUDES_FRAMEWORK_ENTRIES_FOR_TARGETS_NOT_BEING_BUILT = BuiltinMacros.declareBooleanMacro("HEADERMAP_INCLUDES_FRAMEWORK_ENTRIES_FOR_TARGETS_NOT_BEING_BUILT")
     public static let HEADERMAP_USES_VFS = BuiltinMacros.declareBooleanMacro("HEADERMAP_USES_VFS")
+    public static let HEADER_DEPENDENCIES = BuiltinMacros.declareStringListMacro("HEADER_DEPENDENCIES")
     public static let HEADER_OUTPUT_DIR = BuiltinMacros.declareStringMacro("HEADER_OUTPUT_DIR")
     public static let HEADER_SEARCH_PATHS = BuiltinMacros.declarePathListMacro("HEADER_SEARCH_PATHS")
     public static let IBC_REGIONS_AND_STRINGS_FILES = BuiltinMacros.declareStringListMacro("IBC_REGIONS_AND_STRINGS_FILES")
@@ -1145,6 +1146,7 @@ public final class BuiltinMacros {
     public static let VALIDATE_PRODUCT = BuiltinMacros.declareBooleanMacro("VALIDATE_PRODUCT")
     public static let VALIDATE_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_DEPENDENCIES") as EnumMacroDeclaration<BooleanWarningLevel>
     public static let VALIDATE_DEVELOPMENT_ASSET_PATHS = BuiltinMacros.declareEnumMacro("VALIDATE_DEVELOPMENT_ASSET_PATHS") as EnumMacroDeclaration<BooleanWarningLevel>
+    public static let VALIDATE_HEADER_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_HEADER_DEPENDENCIES") as EnumMacroDeclaration<BooleanWarningLevel>
     public static let VALIDATE_MODULE_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_MODULE_DEPENDENCIES") as EnumMacroDeclaration<BooleanWarningLevel>
     public static let VECTOR_SUFFIX = BuiltinMacros.declareStringMacro("VECTOR_SUFFIX")
     public static let VERBOSE_PBXCP = BuiltinMacros.declareBooleanMacro("VERBOSE_PBXCP")
@@ -1798,6 +1800,7 @@ public final class BuiltinMacros {
         GROUP,
         HEADERMAP_INCLUDES_FRAMEWORK_ENTRIES_FOR_TARGETS_NOT_BEING_BUILT,
         HEADERMAP_USES_VFS,
+        HEADER_DEPENDENCIES,
         HEADER_SEARCH_PATHS,
         HEADER_OUTPUT_DIR,
         HOME,
@@ -2373,6 +2376,7 @@ public final class BuiltinMacros {
         VALIDATE_PRODUCT,
         VALIDATE_DEPENDENCIES,
         VALIDATE_DEVELOPMENT_ASSET_PATHS,
+        VALIDATE_HEADER_DEPENDENCIES,
         VALIDATE_MODULE_DEPENDENCIES,
         VALID_ARCHS,
         VECTOR_SUFFIX,

Sources/SWBCore/Settings/Settings.swift

@@ -756,6 +756,7 @@ public final class Settings: PlatformBuildContext, Sendable {
     }
 
     public let moduleDependencies: [ModuleDependency]
+    public let headerDependencies: [HeaderDependency]
 
     public static func supportsMacCatalyst(scope: MacroEvaluationScope, core: Core) -> Bool {
         @preconcurrency @PluginExtensionSystemActor func sdkVariantInfoExtensions() -> [any SDKVariantInfoExtensionPoint.ExtensionProtocol] {
@@ -904,6 +905,7 @@ public final class Settings: PlatformBuildContext, Sendable {
 
         self.supportedBuildVersionPlatforms = effectiveSupportedPlatforms(sdkRegistry: sdkRegistry)
         self.moduleDependencies = builder.moduleDependencies
+        self.headerDependencies = builder.headerDependencies
 
         self.constructionComponents = builder.constructionComponents
     }
@@ -1290,6 +1292,7 @@ private class SettingsBuilder {
     var signingSettings: Settings.SigningSettings? = nil
 
     var moduleDependencies: [ModuleDependency] = []
+    var headerDependencies: [HeaderDependency] = []
 
 
     // Mutable state of the builder as we're building up the settings table.
@@ -1631,6 +1634,12 @@ private class SettingsBuilder {
         catch {
             errors.append("Failed to parse \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(error)")
         }
+        do {
+            self.headerDependencies = try createScope(sdkToUse: boundProperties.sdk).evaluate(BuiltinMacros.HEADER_DEPENDENCIES).map { try HeaderDependency(entry: $0) }
+        }
+        catch {
+            errors.append("Failed to parse \(BuiltinMacros.HEADER_DEPENDENCIES.name): \(error)")
+        }
 
         // At this point settings construction is finished.
 

Sources/SWBCore/SpecImplementations/PropertyDomainSpec.swift

@@ -101,6 +101,7 @@ private final class EnumBuildOptionType : BuildOptionType {
             return try namespace.declareEnumMacro(name) as EnumMacroDeclaration<PackageResourceTargetKind>
         case "VALIDATE_DEPENDENCIES",
             "VALIDATE_MODULE_DEPENDENCIES",
+            "VALIDATE_HEADER_DEPENDENCIES",
             "VALIDATE_DEVELOPMENT_ASSET_PATHS":
             return try namespace.declareEnumMacro(name) as EnumMacroDeclaration<BooleanWarningLevel>
         case "STRIP_STYLE":