diff --git a/Sources/SWBCore/Dependencies.swift b/Sources/SWBCore/Dependencies.swift index a1cfb927..92c30391 100644 --- a/Sources/SWBCore/Dependencies.swift +++ b/Sources/SWBCore/Dependencies.swift @@ -16,6 +16,7 @@ import SWBMacro public struct ModuleDependency: Hashable, Sendable, SerializableCodable { public let name: String public let accessLevel: AccessLevel + public let optional: Bool public enum AccessLevel: String, Hashable, Sendable, CaseIterable, Codable, Serializable, Comparable { case Private = "private" @@ -43,29 +44,25 @@ public struct ModuleDependency: Hashable, Sendable, SerializableCodable { } } - public init(name: String, accessLevel: AccessLevel) { + public init(name: String, accessLevel: AccessLevel, optional: Bool) { self.name = name self.accessLevel = accessLevel + self.optional = optional } 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)") + let re = #/((?private|package|public)\s+)?(?\w+)(?\?)?/# + guard let match = entry.wholeMatch(of: re) else { + throw StubError.error("Invalid module dependency format: \(entry), expected [private|package|public] [?]") } + + self.name = String(match.output.name) + self.accessLevel = try match.output.accessLevel.map { try AccessLevel(String($0)) } ?? .Private + self.optional = match.output.optional != nil } public var asBuildSettingEntry: String { - "\(accessLevel == .Private ? "" : "\(accessLevel.rawValue) ")\(name)" + "\(accessLevel == .Private ? "" : "\(accessLevel.rawValue) ")\(name)\(optional ? "?" : "")" } public var asBuildSettingEntryQuotedIfNeeded: String { @@ -76,35 +73,39 @@ public struct ModuleDependency: Hashable, Sendable, SerializableCodable { public struct ModuleDependenciesContext: Sendable, SerializableCodable { public var validate: BooleanWarningLevel + public var validateUnused: BooleanWarningLevel var moduleDependencies: [ModuleDependency] var fixItContext: FixItContext? - init(validate: BooleanWarningLevel, moduleDependencies: [ModuleDependency], fixItContext: FixItContext? = nil) { + init(validate: BooleanWarningLevel, validateUnused: BooleanWarningLevel, moduleDependencies: [ModuleDependency], fixItContext: FixItContext? = nil) { self.validate = validate + self.validateUnused = validateUnused self.moduleDependencies = moduleDependencies self.fixItContext = fixItContext } public init?(settings: Settings) { let validate = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES) - guard validate != .no else { return nil } + let validateUnused = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_UNUSED_MODULE_DEPENDENCIES) + guard validate != .no || validateUnused != .no else { return nil } let downgrade = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_DEPENDENCIES_DOWNGRADE_ERRORS) - let fixItContext = ModuleDependenciesContext.FixItContext(settings: settings) - self.init(validate: downgrade ? .yes : validate, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext) + let fixItContext = validate != .no ? ModuleDependenciesContext.FixItContext(settings: settings) : nil + self.init(validate: downgrade ? .yes : validate, validateUnused: validateUnused, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext) + } + + public func makeUnsupportedToolchainDiagnostic() -> Diagnostic { + Diagnostic( + behavior: .warning, + location: .unknown, + data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES.name)")) } /// 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])]?, + imports: [(ModuleDependency, importLocations: [Diagnostic.Location])], fromSwift: Bool - ) -> [(ModuleDependency, importLocations: [Diagnostic.Location])]? { + ) -> [(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 fromSwift && $0.importLocations.isEmpty { return false } @@ -115,45 +116,63 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable { } } - /// 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 !missingDependencies.isEmpty else { return [] } + public func computeUnusedDependencies(usedModuleNames: Set) -> [ModuleDependency] { + guard validateUnused != .no else { return [] } + return moduleDependencies.filter { !$0.optional && !usedModuleNames.contains($0.name) } + } - let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning + /// Make diagnostics for missing module dependencies. + public func makeDiagnostics(missingDependencies: [(ModuleDependency, importLocations: [Diagnostic.Location])], unusedDependencies: [ModuleDependency]) -> [Diagnostic] { + let missingDiagnostics: [Diagnostic] + if !missingDependencies.isEmpty { + let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning + + let fixIt = fixItContext?.makeFixIt(newModules: missingDependencies.map { $0.0 }) + let fixIts = fixIt.map { [$0] } ?? [] + + let importDiags: [Diagnostic] = missingDependencies + .flatMap { dep in + dep.1.map { + return Diagnostic( + behavior: behavior, + location: $0, + data: DiagnosticData("Missing entry in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(dep.0.asBuildSettingEntryQuotedIfNeeded)"), + fixIts: fixIts) + } + } - let fixIt = fixItContext?.makeFixIt(newModules: missingDependencies.map { $0.0 }) - let fixIts = fixIt.map { [$0] } ?? [] + let message = "Missing entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(missingDependencies.map { $0.0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))" - let importDiags: [Diagnostic] = missingDependencies - .flatMap { dep in - dep.1.map { - return Diagnostic( - behavior: behavior, - location: $0, - data: DiagnosticData("Missing entry in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(dep.0.asBuildSettingEntryQuotedIfNeeded)"), - fixIts: fixIts) - } - } + 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) - let message = "Missing entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(missingDependencies.map { $0.0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))" + missingDiagnostics = [Diagnostic( + behavior: behavior, + location: location, + data: DiagnosticData(message), + fixIts: fixIts, + childDiagnostics: importDiags)] + } + else { + missingDiagnostics = [] + } - 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) + let unusedDiagnostics: [Diagnostic] + if !unusedDependencies.isEmpty { + let message = "Unused entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(unusedDependencies.map { $0.name }.sorted().joined(separator: " "))" + // TODO location & fixit + unusedDiagnostics = [Diagnostic( + behavior: validateUnused == .yesError ? .error : .warning, + location: .unknown, + data: DiagnosticData(message), + fixIts: [])] + } + else { + unusedDiagnostics = [] + } - return [Diagnostic( - behavior: behavior, - location: location, - data: DiagnosticData(message), - fixIts: fixIts, - childDiagnostics: importDiags)] + return missingDiagnostics + unusedDiagnostics } struct FixItContext: Sendable, SerializableCodable { @@ -169,6 +188,7 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable { guard let target = settings.target else { return nil } let thisTargetCondition = MacroCondition(parameter: BuiltinMacros.targetNameCondition, valuePattern: target.name) + // TODO: if you have an assignment in a project-xcconfig and another assignment in target-settings, this would find the project-xcconfig assignment, but updating that might have no effect depending on the target-settings assignment if let assignment = (settings.globalScope.table.lookupMacro(BuiltinMacros.MODULE_DEPENDENCIES)?.sequence.first { $0.location != nil && ($0.conditions?.conditions == [thisTargetCondition] || ($0.conditions?.conditions.isEmpty ?? true)) }), diff --git a/Sources/SWBCore/LibSwiftDriver/LibSwiftDriver.swift b/Sources/SWBCore/LibSwiftDriver/LibSwiftDriver.swift index c875d9da..c3ef9d63 100644 --- a/Sources/SWBCore/LibSwiftDriver/LibSwiftDriver.swift +++ b/Sources/SWBCore/LibSwiftDriver/LibSwiftDriver.swift @@ -891,5 +891,6 @@ extension ModuleDependency { init(_ importInfo: ImportInfo) { self.name = importInfo.importIdentifier self.accessLevel = .init(importInfo.accessLevel) + self.optional = false } } diff --git a/Sources/SWBCore/Settings/BuiltinMacros.swift b/Sources/SWBCore/Settings/BuiltinMacros.swift index cfdd6309..25021fef 100644 --- a/Sources/SWBCore/Settings/BuiltinMacros.swift +++ b/Sources/SWBCore/Settings/BuiltinMacros.swift @@ -1150,6 +1150,7 @@ public final class BuiltinMacros { public static let VALIDATE_DEVELOPMENT_ASSET_PATHS = BuiltinMacros.declareEnumMacro("VALIDATE_DEVELOPMENT_ASSET_PATHS") as EnumMacroDeclaration public static let VALIDATE_HEADER_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_HEADER_DEPENDENCIES") as EnumMacroDeclaration public static let VALIDATE_MODULE_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_MODULE_DEPENDENCIES") as EnumMacroDeclaration + public static let VALIDATE_UNUSED_MODULE_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_UNUSED_MODULE_DEPENDENCIES") as EnumMacroDeclaration public static let VECTOR_SUFFIX = BuiltinMacros.declareStringMacro("VECTOR_SUFFIX") public static let VERBOSE_PBXCP = BuiltinMacros.declareBooleanMacro("VERBOSE_PBXCP") public static let VERSIONING_STUB = BuiltinMacros.declareStringMacro("VERSIONING_STUB") @@ -2382,6 +2383,7 @@ public final class BuiltinMacros { VALIDATE_DEVELOPMENT_ASSET_PATHS, VALIDATE_HEADER_DEPENDENCIES, VALIDATE_MODULE_DEPENDENCIES, + VALIDATE_UNUSED_MODULE_DEPENDENCIES, VALID_ARCHS, VECTOR_SUFFIX, VERBOSE_PBXCP, diff --git a/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift index 10941936..1b7c8cf3 100644 --- a/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift @@ -375,7 +375,7 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA includeFiles.append(file) } } - let moduleDependencies = moduleNames.map { ModuleDependency(name: $0, accessLevel: .Private) } + let moduleDependencies = moduleNames.map { ModuleDependency(name: $0, accessLevel: .Private, optional: false) } let moduleImports = moduleDependencies.map { DependencyValidationInfo.Import(dependency: $0, importLocations: []) } return (moduleImports, includeFiles) } diff --git a/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift index 9d463104..9844389c 100644 --- a/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift @@ -66,32 +66,34 @@ public final class ValidateDependenciesTaskAction: TaskAction { if let moduleContext = payload.moduleDependenciesContext { if unsupported { - diagnostics.append(contentsOf: moduleContext.makeDiagnostics(missingDependencies: nil)) + diagnostics.append(moduleContext.makeUnsupportedToolchainDiagnostic()) } else { let clangMissingDeps = moduleContext.computeMissingDependencies(imports: allClangImports.map { ($0.dependency, $0.importLocations) }, fromSwift: false) let swiftMissingDeps = moduleContext.computeMissingDependencies(imports: allSwiftImports.map { ($0.dependency, $0.importLocations) }, fromSwift: true) // Update Swift dependencies with information from Clang dependencies on the same module. var clangMissingDepsByName = [String: (ModuleDependency, importLocations: [Diagnostic.Location])]() - clangMissingDeps?.forEach { + clangMissingDeps.forEach { clangMissingDepsByName[$0.0.name] = $0 } - let updatedSwiftMissingDeps: [(ModuleDependency, importLocations: [Diagnostic.Location])] = swiftMissingDeps?.map { + 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)), + ModuleDependency(name: $0.0.name, accessLevel: max($0.0.accessLevel, clangMissingDep.0.accessLevel), optional: $0.0.optional), $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) } ?? [] + let uniqueClangMissingDeps = clangMissingDeps.filter { !swiftImports.contains($0.0.name) } + + let unusedDeps = moduleContext.computeUnusedDependencies(usedModuleNames: Set(allClangImports.map { $0.dependency.name } + allSwiftImports.map { $0.dependency.name })) - diagnostics.append(contentsOf: moduleContext.makeDiagnostics(missingDependencies: uniqueClangMissingDeps + updatedSwiftMissingDeps)) + diagnostics.append(contentsOf: moduleContext.makeDiagnostics(missingDependencies: uniqueClangMissingDeps + updatedSwiftMissingDeps, unusedDependencies: unusedDeps)) } } if let headerContext = payload.headerDependenciesContext { diff --git a/Tests/SWBBuildSystemTests/DependencyValidationTests.swift b/Tests/SWBBuildSystemTests/DependencyValidationTests.swift index 7d5e16b7..7b57cd75 100644 --- a/Tests/SWBBuildSystemTests/DependencyValidationTests.swift +++ b/Tests/SWBBuildSystemTests/DependencyValidationTests.swift @@ -690,4 +690,93 @@ fileprivate struct DependencyValidationTests: CoreBasedTests { } } } + + @Test(.requireSDKs(.host), .requireClangFeatures(.printHeadersDirectPerFile), .skipHostOS(.windows, "toolchain too old"), .skipHostOS(.linux, "toolchain too old")) + func validateUnusedModuleDependencies() async throws { + try await withTemporaryDirectory { tmpDir in + let testWorkspace = try await TestWorkspace( + "Test", + sourceRoot: tmpDir.join("Test"), + projects: [ + TestProject( + "Project", + groupTree: TestGroup( + "Sources", + children: [ + TestFile("Swift.swift"), + TestFile("ObjC.m"), + TestFile("Project.xcconfig"), + ]), + buildConfigurations: [TestBuildConfiguration( + "Debug", + baseConfig: "Project.xcconfig", + 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, + "DEFINES_MODULE": "YES", + "DSTROOT": tmpDir.join("dstroot").str, + "VALIDATE_MODULE_DEPENDENCIES": "YES_ERROR", + "VALIDATE_UNUSED_MODULE_DEPENDENCIES": "YES", + "SDKROOT": "$(HOST_PLATFORM)", + "SUPPORTED_PLATFORMS": "$(HOST_PLATFORM)", + + // 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( + "TargetA", + type: .framework, + buildPhases: [ + TestSourcesBuildPhase(["Swift.swift", "ObjC.m"]), + ]), + ]), + ]) + + let tester = try await BuildOperationTester(getCore(), testWorkspace, simulated: false) + + let swiftSourcePath = testWorkspace.sourceRoot.join("Project/Swift.swift") + try await tester.fs.writeFileContents(swiftSourcePath) { stream in + stream <<< + """ + import Foundation + """ + } + + let objcSourcePath = testWorkspace.sourceRoot.join("Project/ObjC.m") + try await tester.fs.writeFileContents(objcSourcePath) { stream in + stream <<< + """ + #include + + void objcFunction(void) { } + """ + } + + let projectXCConfigPath = testWorkspace.sourceRoot.join("Project/Project.xcconfig") + try await tester.fs.writeFileContents(projectXCConfigPath) { stream in + stream <<< + """ + MODULE_DEPENDENCIES[target=TargetA] = CoreFoundation Foundation AppKit UIKit? + """ + } + + let target = try #require(tester.workspace.projects.only?.targets.first { $0.name == "TargetA" }) + let parameters = BuildParameters(configuration: "Debug") + let buildRequest = BuildRequest(parameters: parameters, buildTargets: [BuildRequest.BuildTargetInfo(parameters: parameters, target: target)], continueBuildingAfterErrors: false, useParallelTargets: true, useImplicitDependencies: true, useDryRun: false) + + try await tester.checkBuild(runDestination: .host, buildRequest: buildRequest, persistent: true) { results in + guard !results.checkWarning(.prefix("The current toolchain does not support VALIDATE_MODULE_DEPENDENCIES"), failIfNotFound: false) else { return } + + // This diagnostic should only report AppKit because UIKit is marked optional. + results.checkWarning(.equal("Unused entries in MODULE_DEPENDENCIES: AppKit (for task: [\"ValidateDependencies\"])")) + } + } + } }