Skip to content

Dependencies: validate Swift imports against MODULE_DEPENDENCIES and provide fix-its (rdar://154967562) #641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: release/6.2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Sources/SWBBuildSystem/DependencyCycleFormatter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ struct DependencyCycleFormatter {
message = "Target '\(previousTargetName)' has an explicit dependency on Target '\(targetName)'"
case let .implicitBuildPhaseLinkage(filename, _, buildPhase)?:
message = "Target '\(previousTargetName)' has an implicit dependency on Target '\(targetName)' because '\(previousTargetName)' references the file '\(filename)' in the build phase '\(buildPhase)'"
case let .implicitBuildSettingLinkage(settingName, options)?:
case let .implicitBuildSetting(settingName, options)?:
message = "Target '\(previousTargetName)' has an implicit dependency on Target '\(targetName)' because '\(previousTargetName)' defines the option '\(options.joined(separator: " "))' in the build setting '\(settingName)'"
case let .impliedByTransitiveDependencyViaRemovedTargets(intermediateTargetName: intermediateTargetName):
message = "Target '\(previousTargetName)' has a dependency on Target '\(targetName)' via its transitive dependency through '\(intermediateTargetName)'"
Expand Down Expand Up @@ -501,7 +501,7 @@ struct DependencyCycleFormatter {
suffix = " via the “Target Dependencies“ build phase"
case let .implicitBuildPhaseLinkage(filename, _, buildPhase)?:
suffix = " because the scheme has implicit dependencies enabled and the Target '\(lastTargetsName)' references the file '\(filename)' in the build phase '\(buildPhase)'"
case let .implicitBuildSettingLinkage(settingName, options)?:
case let .implicitBuildSetting(settingName, options)?:
suffix = " because the scheme has implicit dependencies enabled and the Target '\(lastTargetsName)' defines the options '\(options.joined(separator: " "))' in the build setting '\(settingName)'"
case let .impliedByTransitiveDependencyViaRemovedTargets(intermediateTargetName: intermediateTargetName):
suffix = " via its transitive dependency through '\(intermediateTargetName)'"
Expand Down
1 change: 1 addition & 0 deletions Sources/SWBCore/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ add_library(SWBCore
ConfiguredTarget.swift
Core.swift
CustomTaskTypeDescription.swift
Dependencies.swift
DependencyInfoEditPayload.swift
DependencyResolution.swift
DiagnosticSupport.swift
Expand Down
184 changes: 184 additions & 0 deletions Sources/SWBCore/Dependencies.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift open source project
//
// Copyright (c) 2025 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See http://swift.org/LICENSE.txt for license information
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

public import SWBUtil
import SWBMacro

public struct ModuleDependency: Hashable, Sendable, SerializableCodable {
public let name: String
public let accessLevel: AccessLevel

public enum AccessLevel: String, Hashable, Sendable, CaseIterable, Codable, Serializable {
case Private = "private"
case Package = "package"
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 ModuleDependenciesContext: Sendable, SerializableCodable {
var validate: BooleanWarningLevel
var moduleDependencies: [ModuleDependency]
var fixItContext: FixItContext?

init(validate: BooleanWarningLevel, moduleDependencies: [ModuleDependency], fixItContext: FixItContext? = nil) {
self.validate = validate
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 fixItContext = ModuleDependenciesContext.FixItContext(settings: settings)
self.init(validate: validate, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext)
}

/// Nil `imports` means the current toolchain doesn't have the features to gather imports. This is temporarily required to support running against older toolchains.
public func makeDiagnostics(imports: [(ModuleDependency, importLocations: [Diagnostic.Location])]?) -> [Diagnostic] {
guard validate != .no else { return [] }
guard let imports else {
return [Diagnostic(
behavior: .error,
location: .unknown,
data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES.name)"))]
}

let missingDeps = 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 }

// 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 }
return true
}

guard !missingDeps.isEmpty else { return [] }

let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning

let fixIt = fixItContext?.makeFixIt(newModules: missingDeps.map { $0.0 })
let fixIts = fixIt.map { [$0] } ?? []

let importDiags: [Diagnostic] = missingDeps
.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 message = "Missing entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(missingDeps.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)
} ?? Diagnostic.Location.buildSetting(BuiltinMacros.MODULE_DEPENDENCIES)

return [Diagnostic(
behavior: behavior,
location: location,
data: DiagnosticData(message),
fixIts: fixIts,
childDiagnostics: importDiags)]
}

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.MODULE_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: 0, startColumn: 0, endLine: 0, endColumn: 0), modificationStyle: .insertNewAssignment(targetNameCondition: nil))
}
else if let path = settings.constructionComponents.projectXcconfigPath {
self.init(sourceRange: .init(path: path, startLine: 0, startColumn: 0, endLine: 0, endColumn: 0), modificationStyle: .insertNewAssignment(targetNameCondition: target.name))
}
else {
return nil
}
}

enum ModificationStyle: Sendable, SerializableCodable, Hashable {
case appendToExistingAssignment
case insertNewAssignment(targetNameCondition: String?)
}

func makeFixIt(newModules: [ModuleDependency]) -> Diagnostic.FixIt {
let stringValue = newModules.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.MODULE_DEPENDENCIES.name)\(targetCondition) = $(inherited) \(stringValue)\n"
}

return Diagnostic.FixIt(sourceRange: sourceRange, newText: newText)
}
}
}
44 changes: 44 additions & 0 deletions Sources/SWBCore/LibSwiftDriver/LibSwiftDriver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,24 @@ public final class SwiftModuleDependencyGraph: SwiftGlobalExplicitDependencyGrap
return fileDependencies
}

func mainModule(for key: String) async throws -> SwiftDriver.ModuleInfo? {
let graph = try await registryQueue.sync {
guard let driver = self.registry[key] else {
throw StubError.error("Unable to find jobs for key \(key). Be sure to plan the build ahead of fetching results.")
}
return driver.intermoduleDependencyGraph
}
guard let graph else { return nil }
return graph.mainModule
}

/// Nil result means the current toolchain / libSwiftScan does not support importInfos
public func mainModuleImportModuleDependencies(for key: String) async throws -> [(ModuleDependency, importLocations: [SWBUtil.Diagnostic.Location])]? {
try await mainModule(for: key)?.importInfos?.map {
(ModuleDependency($0), $0.sourceLocations.map { Diagnostic.Location($0) })
}
}

public func queryTransitiveDependencyModuleNames(for key: String) async throws -> [String] {
let graph = try await registryQueue.sync {
guard let driver = self.registry[key] else {
Expand Down Expand Up @@ -849,3 +867,29 @@ extension SWBUtil.Diagnostic.Behavior {
}
}
}

extension SWBUtil.Diagnostic.Location {
init(_ loc: ScannerDiagnosticSourceLocation) {
self = .path(Path(loc.bufferIdentifier), line: loc.lineNumber, column: loc.columnNumber)
}
}

extension ModuleDependency.AccessLevel {
init(_ accessLevel: ImportInfo.ImportAccessLevel) {
switch accessLevel {
case .Private, .FilePrivate, .Internal:
self = .Private
case .Package:
self = .Package
case .Public:
self = .Public
}
}
}

extension ModuleDependency {
init(_ importInfo: ImportInfo) {
self.name = importInfo.importIdentifier
self.accessLevel = .init(importInfo.accessLevel)
}
}
Loading
Loading