diff --git a/Sources/SWBCore/Dependencies.swift b/Sources/SWBCore/Dependencies.swift index a05f33e3..0675792a 100644 --- a/Sources/SWBCore/Dependencies.swift +++ b/Sources/SWBCore/Dependencies.swift @@ -17,7 +17,7 @@ public struct ModuleDependency: Hashable, Sendable, SerializableCodable { public let name: String public let accessLevel: AccessLevel - public enum AccessLevel: String, Hashable, Sendable, CaseIterable, Codable, Serializable { + public enum AccessLevel: String, Hashable, Sendable, CaseIterable, Codable, Serializable, Comparable { case Private = "private" case Package = "package" case Public = "public" @@ -29,6 +29,18 @@ public struct ModuleDependency: Hashable, Sendable, SerializableCodable { self = accessLevel } + + // This allows easy merging of different access levels to always end up with the broadest one needed for a target. + public static func < (lhs: Self, rhs: Self) -> Bool { + switch lhs { + case .Private: + return true + case .Public: + return false + case .Package: + return rhs == .Public + } + } } public init(name: String, accessLevel: AccessLevel) { @@ -80,18 +92,15 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable { self.init(validate: validate, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext) } - /// Make diagnostics for missing module dependencies from Clang imports. + /// 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 makeDiagnostics(files: [Path]?) -> [Diagnostic] { + public func computeMissingDependencies(files: [Path]?) -> [(ModuleDependency, importLocations: [Diagnostic.Location])]? { guard validate != .no else { return [] } guard let files else { - return [Diagnostic( - behavior: .warning, - location: .unknown, - data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES.name)"))] + return nil } // The following is a provisional/incomplete mechanism for resolving a module dependency from a file path. @@ -111,39 +120,19 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable { ModuleDependency(name: $0, accessLevel: .Private) }) - guard !missingDeps.isEmpty else { return [] } - - let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning - - let fixIt = fixItContext?.makeFixIt(newModules: Array(missingDeps)) - let fixIts = fixIt.map { [$0] } ?? [] - - let message = "Missing entries in \(BuiltinMacros.MODULE_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.MODULE_DEPENDENCIES) - - return [Diagnostic( - behavior: behavior, - location: location, - data: DiagnosticData(message), - fixIts: fixIts)] + return missingDeps.map { ($0, []) } } - /// Make diagnostics for missing module dependencies from Swift imports. + /// Compute missing module dependencies from Swift imports. /// /// If `imports` is nil, the current toolchain does not support the features to gather imports. - public func makeDiagnostics(imports: [(ModuleDependency, importLocations: [Diagnostic.Location])]?) -> [Diagnostic] { + public func computeMissingDependencies(imports: [(ModuleDependency, importLocations: [Diagnostic.Location])]?) -> [(ModuleDependency, importLocations: [Diagnostic.Location])]? { guard validate != .no else { return [] } guard let imports else { - return [Diagnostic( - behavior: .warning, - location: .unknown, - data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES.name)"))] + return nil } - let missingDeps = imports.filter { + 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 } @@ -151,15 +140,25 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable { if moduleDependencies.contains($0.0) { return false } return true } + } + + /// Make diagnostics for missing module dependencies. + public func makeDiagnostics(missingDependencies: [(ModuleDependency, importLocations: [Diagnostic.Location])]?) -> [Diagnostic] { + guard let missingDependencies else { + return [Diagnostic( + behavior: .warning, + location: .unknown, + data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES.name)"))] + } - guard !missingDeps.isEmpty else { return [] } + guard !missingDependencies.isEmpty else { return [] } let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning - let fixIt = fixItContext?.makeFixIt(newModules: missingDeps.map { $0.0 }) + let fixIt = fixItContext?.makeFixIt(newModules: missingDependencies.map { $0.0 }) let fixIts = fixIt.map { [$0] } ?? [] - let importDiags: [Diagnostic] = missingDeps + let importDiags: [Diagnostic] = missingDependencies .flatMap { dep in dep.1.map { return Diagnostic( @@ -170,7 +169,7 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable { } } - let message = "Missing entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(missingDeps.map { $0.0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))" + let message = "Missing entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(missingDependencies.map { $0.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) diff --git a/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift index 313262ab..06f67771 100644 --- a/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift @@ -36,8 +36,8 @@ public final class ValidateDependenciesTaskAction: TaskAction { } do { - var allFiles = Set() - var allImports = Set() + var allClangFiles = Set() + var allSwiftImports = Set() var unsupported = false for inputPath in task.inputPaths { @@ -47,11 +47,11 @@ public final class ValidateDependenciesTaskAction: TaskAction { switch info.payload { case .clangDependencies(let files): files.forEach { - allFiles.insert($0) + allClangFiles.insert($0) } case .swiftDependencies(let imports): imports.forEach { - allImports.insert($0) + allSwiftImports.insert($0) } case .unsupported: unsupported = true @@ -61,10 +61,32 @@ public final class ValidateDependenciesTaskAction: TaskAction { var diagnostics: [Diagnostic] = [] if unsupported { - diagnostics.append(contentsOf: context.makeDiagnostics(files: nil)) + diagnostics.append(contentsOf: context.makeDiagnostics(missingDependencies: nil)) } else { - diagnostics.append(contentsOf: context.makeDiagnostics(files: allFiles.map { Path($0) })) - diagnostics.append(contentsOf: context.makeDiagnostics(imports: allImports.map { ($0.dependency, $0.importLocations) })) + let clangMissingDeps = context.computeMissingDependencies(files: allClangFiles.map { Path($0) }) + let swiftMissingDeps = context.computeMissingDependencies(imports: allSwiftImports.map { ($0.dependency, $0.importLocations) }) + + // Update Swift dependencies with information from Clang dependencies on the same module. + var clangMissingDepsByName = [String: (ModuleDependency, importLocations: [Diagnostic.Location])]() + clangMissingDeps?.forEach { + clangMissingDepsByName[$0.0.name] = $0 + } + let updatedSwiftMissingDeps: [(ModuleDependency, importLocations: [Diagnostic.Location])] = swiftMissingDeps?.map { + if let clangMissingDep = clangMissingDepsByName[$0.0.name] { + return ( + ModuleDependency(name: $0.0.name, accessLevel: max($0.0.accessLevel, clangMissingDep.0.accessLevel)), + $0.importLocations + clangMissingDep.importLocations + ) + } else { + return $0 + } + } ?? [] + + // Filter missing C dependencies by known Swift dependencies to avoid duplicate diagnostics. + let swiftImports = Set(allSwiftImports.map { $0.dependency.name }) + let uniqueClangMissingDeps = clangMissingDeps?.filter { !swiftImports.contains($0.0.name) } ?? [] + + diagnostics.append(contentsOf: context.makeDiagnostics(missingDependencies: uniqueClangMissingDeps + updatedSwiftMissingDeps)) } for diagnostic in diagnostics { diff --git a/Sources/SWBTestSupport/CoreBasedTests.swift b/Sources/SWBTestSupport/CoreBasedTests.swift index b6b0e837..1ac926fd 100644 --- a/Sources/SWBTestSupport/CoreBasedTests.swift +++ b/Sources/SWBTestSupport/CoreBasedTests.swift @@ -155,6 +155,14 @@ extension CoreBasedTests { } } + /// The path to clang in the default toolchain. + public var defaultClangPath: Path { + get async throws { + let clangInfo = try await clangInfo + return clangInfo.toolPath + } + } + /// The path to the libClang.dylib compiler in the default toolchain. package var libClangPath: Path { get async throws { diff --git a/Tests/SWBBuildSystemTests/DependencyValidationTests.swift b/Tests/SWBBuildSystemTests/DependencyValidationTests.swift index 8c156951..3fd4b2cd 100644 --- a/Tests/SWBBuildSystemTests/DependencyValidationTests.swift +++ b/Tests/SWBBuildSystemTests/DependencyValidationTests.swift @@ -19,6 +19,23 @@ import SWBMacro @Suite fileprivate struct DependencyValidationTests: CoreBasedTests { + @Test + func moduleDependencyAccessLevelComparable() async throws { + #expect(ModuleDependency.AccessLevel.Private == .Private) + #expect(ModuleDependency.AccessLevel.Private < .Package) + #expect(ModuleDependency.AccessLevel.Private < .Public) + + #expect(ModuleDependency.AccessLevel.Package == .Package) + #expect(ModuleDependency.AccessLevel.Package < .Public) + #expect(ModuleDependency.AccessLevel.Package > .Private) + + #expect(ModuleDependency.AccessLevel.Public == .Public) + #expect(ModuleDependency.AccessLevel.Public > .Private) + #expect(ModuleDependency.AccessLevel.Public > .Package) + + #expect(max(ModuleDependency.AccessLevel.Public, ModuleDependency.AccessLevel.Private) == .Public) + } + @Test(.requireSDKs(.macOS)) func dependencyValidation() async throws { try await testDependencyValidation(BuildParameters(configuration: "Debug")) @@ -529,4 +546,78 @@ fileprivate struct DependencyValidationTests: CoreBasedTests { } } + @Test(.requireSDKs(.host), .requireClangFeatures(.printHeadersDirectPerFile), .skipHostOS(.windows, "toolchain too old"), .skipHostOS(.linux, "toolchain too old")) + func validateModuleDependenciesMixedSource() async throws { + try await withTemporaryDirectory { tmpDir async throws -> Void in + let testWorkspace = try await TestWorkspace( + "Test", + sourceRoot: tmpDir.join("Test"), + projects: [ + TestProject( + "aProject", + groupTree: TestGroup( + "Sources", path: "Sources", + children: [ + TestFile("CoreFoo.m"), + TestFile("Swift.swift"), + ]), + buildConfigurations: [ + TestBuildConfiguration( + "Debug", + buildSettings: [ + "PRODUCT_NAME": "$(TARGET_NAME)", + "CLANG_ENABLE_MODULES": "YES", + "CLANG_ENABLE_EXPLICIT_MODULES": "YES", + "SWIFT_ENABLE_EXPLICIT_MODULES": "YES", + "SWIFT_UPCOMING_FEATURE_INTERNAL_IMPORTS_BY_DEFAULT": "YES", + "SWIFT_VERSION": swiftVersion, + "GENERATE_INFOPLIST_FILE": "YES", + "VALIDATE_MODULE_DEPENDENCIES": "YES_ERROR", + "SDKROOT": "$(HOST_PLATFORM)", + "SUPPORTED_PLATFORMS": "$(HOST_PLATFORM)", + "DSTROOT": tmpDir.join("dstroot").str, + + // Temporarily override to use the latest toolchain in CI because we depend on swift and swift-driver changes which aren't in the baseline tools yet + "TOOLCHAINS": "swift", + // We still want to use the default clang since that is used to gate the test + "CC": defaultClangPath.str, + ] + ) + ], + targets: [ + TestStandardTarget( + "CoreFoo", type: .framework, + buildPhases: [ + TestSourcesBuildPhase(["CoreFoo.m", "Swift.swift"]), + TestFrameworksBuildPhase() + ]) + ]) + ] + ) + + let tester = try await BuildOperationTester(getCore(), testWorkspace, simulated: false) + let SRCROOT = testWorkspace.sourceRoot.join("aProject") + + try await tester.fs.writeFileContents(SRCROOT.join("Sources/Swift.swift")) { stream in + stream <<< """ + import Foundation + import AppKit + """ + } + + try await tester.fs.writeFileContents(SRCROOT.join("Sources/CoreFoo.m")) { contents in + contents <<< """ + #include + #include + + void f(void) { }; + """ + } + + try await tester.checkBuild(parameters: BuildParameters(configuration: "Debug"), runDestination: .host, persistent: true) { results in + results.checkError(.contains("Missing entries in MODULE_DEPENDENCIES: Accelerate AppKit Foundation")) + } + } + } + }