Skip to content

Commit 632eb45

Browse files
mirza-garibovicmirza-garibovic
committed
Dependencies: validate unused module dependencies
1 parent 3ab87e8 commit 632eb45

File tree

4 files changed

+166
-51
lines changed

4 files changed

+166
-51
lines changed

‎Sources/SWBCore/Dependencies.swift‎

Lines changed: 68 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -76,35 +76,39 @@ public struct ModuleDependency: Hashable, Sendable, SerializableCodable {
7676

7777
public struct ModuleDependenciesContext: Sendable, SerializableCodable {
7878
public var validate: BooleanWarningLevel
79+
public var validateUnused: BooleanWarningLevel
7980
var moduleDependencies: [ModuleDependency]
8081
var fixItContext: FixItContext?
8182

82-
init(validate: BooleanWarningLevel, moduleDependencies: [ModuleDependency], fixItContext: FixItContext? = nil) {
83+
init(validate: BooleanWarningLevel, validateUnused: BooleanWarningLevel, moduleDependencies: [ModuleDependency], fixItContext: FixItContext? = nil) {
8384
self.validate = validate
85+
self.validateUnused = validateUnused
8486
self.moduleDependencies = moduleDependencies
8587
self.fixItContext = fixItContext
8688
}
8789

8890
public init?(settings: Settings) {
8991
let validate = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES)
90-
guard validate != .no else { return nil }
92+
let validateUnused = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_UNUSED_MODULE_DEPENDENCIES)
93+
guard validate != .no || validateUnused != .no else { return nil }
9194
let downgrade = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_DEPENDENCIES_DOWNGRADE_ERRORS)
92-
let fixItContext = ModuleDependenciesContext.FixItContext(settings: settings)
93-
self.init(validate: downgrade ? .yes : validate, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext)
95+
let fixItContext = validate != .no ? ModuleDependenciesContext.FixItContext(settings: settings) : nil
96+
self.init(validate: downgrade ? .yes : validate, validateUnused: validateUnused, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext)
97+
}
98+
99+
public func makeUnsupportedToolchainDiagnostic() -> Diagnostic {
100+
Diagnostic(
101+
behavior: .warning,
102+
location: .unknown,
103+
data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES.name)"))
94104
}
95105

96106
/// Compute missing module dependencies.
97-
///
98-
/// If `imports` is nil, the current toolchain does not support the features to gather imports.
99107
public func computeMissingDependencies(
100-
imports: [(ModuleDependency, importLocations: [Diagnostic.Location])]?,
108+
imports: [(ModuleDependency, importLocations: [Diagnostic.Location])],
101109
fromSwift: Bool
102-
) -> [(ModuleDependency, importLocations: [Diagnostic.Location])]? {
110+
) -> [(ModuleDependency, importLocations: [Diagnostic.Location])] {
103111
guard validate != .no else { return [] }
104-
guard let imports else {
105-
return nil
106-
}
107-
108112
return imports.filter {
109113
// 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
110114
if fromSwift && $0.importLocations.isEmpty { return false }
@@ -115,45 +119,63 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
115119
}
116120
}
117121

118-
/// Make diagnostics for missing module dependencies.
119-
public func makeDiagnostics(missingDependencies: [(ModuleDependency, importLocations: [Diagnostic.Location])]?) -> [Diagnostic] {
120-
guard let missingDependencies else {
121-
return [Diagnostic(
122-
behavior: .warning,
123-
location: .unknown,
124-
data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES.name)"))]
125-
}
126-
127-
guard !missingDependencies.isEmpty else { return [] }
122+
public func computeUnusedDependencies(usedModuleNames: Set<String>) -> [ModuleDependency] {
123+
guard validateUnused != .no else { return [] }
124+
return moduleDependencies.filter { !usedModuleNames.contains($0.name) }
125+
}
128126

129-
let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning
127+
/// Make diagnostics for missing module dependencies.
128+
public func makeDiagnostics(missingDependencies: [(ModuleDependency, importLocations: [Diagnostic.Location])], unusedDependencies: [ModuleDependency]) -> [Diagnostic] {
129+
let missingDiagnostics: [Diagnostic]
130+
if !missingDependencies.isEmpty {
131+
let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning
132+
133+
let fixIt = fixItContext?.makeFixIt(newModules: missingDependencies.map { $0.0 })
134+
let fixIts = fixIt.map { [$0] } ?? []
135+
136+
let importDiags: [Diagnostic] = missingDependencies
137+
.flatMap { dep in
138+
dep.1.map {
139+
return Diagnostic(
140+
behavior: behavior,
141+
location: $0,
142+
data: DiagnosticData("Missing entry in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(dep.0.asBuildSettingEntryQuotedIfNeeded)"),
143+
fixIts: fixIts)
144+
}
145+
}
130146

131-
let fixIt = fixItContext?.makeFixIt(newModules: missingDependencies.map { $0.0 })
132-
let fixIts = fixIt.map { [$0] } ?? []
147+
let message = "Missing entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(missingDependencies.map { $0.0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))"
133148

134-
let importDiags: [Diagnostic] = missingDependencies
135-
.flatMap { dep in
136-
dep.1.map {
137-
return Diagnostic(
138-
behavior: behavior,
139-
location: $0,
140-
data: DiagnosticData("Missing entry in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(dep.0.asBuildSettingEntryQuotedIfNeeded)"),
141-
fixIts: fixIts)
142-
}
143-
}
149+
let location: Diagnostic.Location = fixIt.map {
150+
Diagnostic.Location.path($0.sourceRange.path, line: $0.sourceRange.endLine, column: $0.sourceRange.endColumn)
151+
} ?? Diagnostic.Location.buildSetting(BuiltinMacros.MODULE_DEPENDENCIES)
144152

145-
let message = "Missing entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(missingDependencies.map { $0.0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))"
153+
missingDiagnostics = [Diagnostic(
154+
behavior: behavior,
155+
location: location,
156+
data: DiagnosticData(message),
157+
fixIts: fixIts,
158+
childDiagnostics: importDiags)]
159+
}
160+
else {
161+
missingDiagnostics = []
162+
}
146163

147-
let location: Diagnostic.Location = fixIt.map {
148-
Diagnostic.Location.path($0.sourceRange.path, line: $0.sourceRange.endLine, column: $0.sourceRange.endColumn)
149-
} ?? Diagnostic.Location.buildSetting(BuiltinMacros.MODULE_DEPENDENCIES)
164+
let unusedDiagnostics: [Diagnostic]
165+
if !unusedDependencies.isEmpty {
166+
let message = "Unused entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(unusedDependencies.map { $0.name }.sorted().joined(separator: " "))"
167+
// TODO location & fixit
168+
unusedDiagnostics = [Diagnostic(
169+
behavior: validateUnused == .yesError ? .error : .warning,
170+
location: .unknown,
171+
data: DiagnosticData(message),
172+
fixIts: [])]
173+
}
174+
else {
175+
unusedDiagnostics = []
176+
}
150177

151-
return [Diagnostic(
152-
behavior: behavior,
153-
location: location,
154-
data: DiagnosticData(message),
155-
fixIts: fixIts,
156-
childDiagnostics: importDiags)]
178+
return missingDiagnostics + unusedDiagnostics
157179
}
158180

159181
struct FixItContext: Sendable, SerializableCodable {
@@ -169,6 +191,7 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
169191
guard let target = settings.target else { return nil }
170192
let thisTargetCondition = MacroCondition(parameter: BuiltinMacros.targetNameCondition, valuePattern: target.name)
171193

194+
// 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
172195
if let assignment = (settings.globalScope.table.lookupMacro(BuiltinMacros.MODULE_DEPENDENCIES)?.sequence.first {
173196
$0.location != nil && ($0.conditions?.conditions == [thisTargetCondition] || ($0.conditions?.conditions.isEmpty ?? true))
174197
}),

‎Sources/SWBCore/Settings/BuiltinMacros.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,6 +1150,7 @@ public final class BuiltinMacros {
11501150
public static let VALIDATE_DEVELOPMENT_ASSET_PATHS = BuiltinMacros.declareEnumMacro("VALIDATE_DEVELOPMENT_ASSET_PATHS") as EnumMacroDeclaration<BooleanWarningLevel>
11511151
public static let VALIDATE_HEADER_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_HEADER_DEPENDENCIES") as EnumMacroDeclaration<BooleanWarningLevel>
11521152
public static let VALIDATE_MODULE_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_MODULE_DEPENDENCIES") as EnumMacroDeclaration<BooleanWarningLevel>
1153+
public static let VALIDATE_UNUSED_MODULE_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_UNUSED_MODULE_DEPENDENCIES") as EnumMacroDeclaration<BooleanWarningLevel>
11531154
public static let VECTOR_SUFFIX = BuiltinMacros.declareStringMacro("VECTOR_SUFFIX")
11541155
public static let VERBOSE_PBXCP = BuiltinMacros.declareBooleanMacro("VERBOSE_PBXCP")
11551156
public static let VERSIONING_STUB = BuiltinMacros.declareStringMacro("VERSIONING_STUB")
@@ -2382,6 +2383,7 @@ public final class BuiltinMacros {
23822383
VALIDATE_DEVELOPMENT_ASSET_PATHS,
23832384
VALIDATE_HEADER_DEPENDENCIES,
23842385
VALIDATE_MODULE_DEPENDENCIES,
2386+
VALIDATE_UNUSED_MODULE_DEPENDENCIES,
23852387
VALID_ARCHS,
23862388
VECTOR_SUFFIX,
23872389
VERBOSE_PBXCP,

‎Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,17 +66,17 @@ public final class ValidateDependenciesTaskAction: TaskAction {
6666

6767
if let moduleContext = payload.moduleDependenciesContext {
6868
if unsupported {
69-
diagnostics.append(contentsOf: moduleContext.makeDiagnostics(missingDependencies: nil))
69+
diagnostics.append(moduleContext.makeUnsupportedToolchainDiagnostic())
7070
} else {
7171
let clangMissingDeps = moduleContext.computeMissingDependencies(imports: allClangImports.map { ($0.dependency, $0.importLocations) }, fromSwift: false)
7272
let swiftMissingDeps = moduleContext.computeMissingDependencies(imports: allSwiftImports.map { ($0.dependency, $0.importLocations) }, fromSwift: true)
7373

7474
// Update Swift dependencies with information from Clang dependencies on the same module.
7575
var clangMissingDepsByName = [String: (ModuleDependency, importLocations: [Diagnostic.Location])]()
76-
clangMissingDeps?.forEach {
76+
clangMissingDeps.forEach {
7777
clangMissingDepsByName[$0.0.name] = $0
7878
}
79-
let updatedSwiftMissingDeps: [(ModuleDependency, importLocations: [Diagnostic.Location])] = swiftMissingDeps?.map {
79+
let updatedSwiftMissingDeps: [(ModuleDependency, importLocations: [Diagnostic.Location])] = swiftMissingDeps.map {
8080
if let clangMissingDep = clangMissingDepsByName[$0.0.name] {
8181
return (
8282
ModuleDependency(name: $0.0.name, accessLevel: max($0.0.accessLevel, clangMissingDep.0.accessLevel)),
@@ -85,13 +85,15 @@ public final class ValidateDependenciesTaskAction: TaskAction {
8585
} else {
8686
return $0
8787
}
88-
} ?? []
88+
}
8989

9090
// Filter missing C dependencies by known Swift dependencies to avoid duplicate diagnostics.
9191
let swiftImports = Set(allSwiftImports.map { $0.dependency.name })
92-
let uniqueClangMissingDeps = clangMissingDeps?.filter { !swiftImports.contains($0.0.name) } ?? []
92+
let uniqueClangMissingDeps = clangMissingDeps.filter { !swiftImports.contains($0.0.name) }
93+
94+
let unusedDeps = moduleContext.computeUnusedDependencies(usedModuleNames: Set(allClangImports.map { $0.dependency.name } + allSwiftImports.map { $0.dependency.name }))
9395

94-
diagnostics.append(contentsOf: moduleContext.makeDiagnostics(missingDependencies: uniqueClangMissingDeps + updatedSwiftMissingDeps))
96+
diagnostics.append(contentsOf: moduleContext.makeDiagnostics(missingDependencies: uniqueClangMissingDeps + updatedSwiftMissingDeps, unusedDependencies: unusedDeps))
9597
}
9698
}
9799
if let headerContext = payload.headerDependenciesContext {

‎Tests/SWBBuildSystemTests/DependencyValidationTests.swift‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,4 +690,92 @@ fileprivate struct DependencyValidationTests: CoreBasedTests {
690690
}
691691
}
692692
}
693+
694+
@Test(.requireSDKs(.host), .requireClangFeatures(.printHeadersDirectPerFile), .skipHostOS(.windows, "toolchain too old"), .skipHostOS(.linux, "toolchain too old"))
695+
func validateUnusedModuleDependencies() async throws {
696+
try await withTemporaryDirectory { tmpDir in
697+
let testWorkspace = try await TestWorkspace(
698+
"Test",
699+
sourceRoot: tmpDir.join("Test"),
700+
projects: [
701+
TestProject(
702+
"Project",
703+
groupTree: TestGroup(
704+
"Sources",
705+
children: [
706+
TestFile("Swift.swift"),
707+
TestFile("ObjC.m"),
708+
TestFile("Project.xcconfig"),
709+
]),
710+
buildConfigurations: [TestBuildConfiguration(
711+
"Debug",
712+
baseConfig: "Project.xcconfig",
713+
buildSettings: [
714+
"PRODUCT_NAME": "$(TARGET_NAME)",
715+
"CLANG_ENABLE_MODULES": "YES",
716+
"CLANG_ENABLE_EXPLICIT_MODULES": "YES",
717+
"SWIFT_ENABLE_EXPLICIT_MODULES": "YES",
718+
"SWIFT_UPCOMING_FEATURE_INTERNAL_IMPORTS_BY_DEFAULT": "YES",
719+
"SWIFT_VERSION": swiftVersion,
720+
"DEFINES_MODULE": "YES",
721+
"DSTROOT": tmpDir.join("dstroot").str,
722+
"VALIDATE_MODULE_DEPENDENCIES": "YES_ERROR",
723+
"VALIDATE_UNUSED_MODULE_DEPENDENCIES": "YES",
724+
"SDKROOT": "$(HOST_PLATFORM)",
725+
"SUPPORTED_PLATFORMS": "$(HOST_PLATFORM)",
726+
727+
// 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
728+
"TOOLCHAINS": "swift",
729+
// We still want to use the default clang since that is used to gate the test
730+
"CC": defaultClangPath.str,
731+
])],
732+
targets: [
733+
TestStandardTarget(
734+
"TargetA",
735+
type: .framework,
736+
buildPhases: [
737+
TestSourcesBuildPhase(["Swift.swift", "ObjC.m"]),
738+
]),
739+
]),
740+
])
741+
742+
let tester = try await BuildOperationTester(getCore(), testWorkspace, simulated: false)
743+
744+
let swiftSourcePath = testWorkspace.sourceRoot.join("Project/Swift.swift")
745+
try await tester.fs.writeFileContents(swiftSourcePath) { stream in
746+
stream <<<
747+
"""
748+
import Foundation
749+
"""
750+
}
751+
752+
let objcSourcePath = testWorkspace.sourceRoot.join("Project/ObjC.m")
753+
try await tester.fs.writeFileContents(objcSourcePath) { stream in
754+
stream <<<
755+
"""
756+
#include <CoreFoundation/CoreFoundation.h>
757+
758+
void objcFunction(void) { }
759+
"""
760+
}
761+
762+
let projectXCConfigPath = testWorkspace.sourceRoot.join("Project/Project.xcconfig")
763+
try await tester.fs.writeFileContents(projectXCConfigPath) { stream in
764+
stream <<<
765+
"""
766+
MODULE_DEPENDENCIES[target=TargetA] = CoreFoundation Foundation AppKit
767+
"""
768+
}
769+
770+
let target = try #require(tester.workspace.projects.only?.targets.first { $0.name == "TargetA" })
771+
let parameters = BuildParameters(configuration: "Debug")
772+
let buildRequest = BuildRequest(parameters: parameters, buildTargets: [BuildRequest.BuildTargetInfo(parameters: parameters, target: target)], continueBuildingAfterErrors: false, useParallelTargets: true, useImplicitDependencies: true, useDryRun: false)
773+
774+
try await tester.checkBuild(runDestination: .host, buildRequest: buildRequest, persistent: true) { results in
775+
guard !results.checkWarning(.prefix("The current toolchain does not support VALIDATE_MODULE_DEPENDENCIES"), failIfNotFound: false) else { return }
776+
777+
results.checkWarning(.contains("Unused entries in MODULE_DEPENDENCIES: AppKit"))
778+
}
779+
}
780+
}
693781
}

0 commit comments

Comments
 (0)