Skip to content

Commit 6f469cf

Browse files
authored
Improve check for unsafe flags (#2845)
We prohibit unsafe flags in package dependencies, but the check was only looking at targets which are direct dependencies of a product, not transitive ones. rdar://problem/66499615
1 parent 4d720d6 commit 6f469cf

File tree

4 files changed

+22
-6
lines changed

4 files changed

+22
-6
lines changed

Sources/PackageGraph/PackageGraphLoader.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ private final class ResolvedTargetBuilder: ResolvedBuilder<ResolvedTarget> {
571571

572572
func diagnoseInvalidUseOfUnsafeFlags(_ product: ResolvedProduct) {
573573
// Diagnose if any target in this product uses an unsafe flag.
574-
for target in product.targets {
574+
for target in product.recursiveTargetDependencies() {
575575
let declarations = target.underlyingTarget.buildSettings.assignments.keys
576576
for decl in declarations {
577577
if BuildSettings.Declaration.unsafeSettings.contains(decl) {

Sources/PackageModel/ResolvedModels.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,12 @@ public final class ResolvedProduct: ObjectIdentifierProtocol, CustomStringConver
235235
// contain Swift code we don't know about as part of this build).
236236
return targets.contains { $0.underlyingTarget is SwiftTarget }
237237
}
238+
239+
/// Returns the recursive target dependencies.
240+
public func recursiveTargetDependencies() -> [ResolvedTarget] {
241+
let recursiveDependencies = targets.lazy.flatMap { $0.recursiveTargetDependencies() }
242+
return Array(Set(targets).union(recursiveDependencies))
243+
}
238244
}
239245

240246
extension ResolvedTarget.Dependency: CustomStringConvertible {

Tests/PackageGraphTests/PackageGraphTests.swift

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -887,9 +887,11 @@ class PackageGraphTests: XCTestCase {
887887
func testUnsafeFlags() throws {
888888
let fs = InMemoryFileSystem(emptyFiles:
889889
"/Foo/Sources/Foo/foo.swift",
890+
"/Foo/Sources/Foo2/foo.swift",
890891
"/Bar/Sources/Bar/bar.swift",
891892
"/Bar/Sources/Bar2/bar.swift",
892893
"/Bar/Sources/Bar3/bar.swift",
894+
"/Bar/Sources/TransitiveBar/bar.swift",
893895
"<end>"
894896
)
895897

@@ -906,14 +908,16 @@ class PackageGraphTests: XCTestCase {
906908
],
907909
targets: [
908910
TargetDescription(name: "Foo", dependencies: ["Bar"]),
911+
TargetDescription(name: "Foo2", dependencies: ["TransitiveBar"]),
909912
]),
910913
Manifest.createV4Manifest(
911914
name: "Bar",
912915
path: "/Bar",
913916
url: "/Bar",
914917
packageKind: .local,
915918
products: [
916-
ProductDescription(name: "Bar", targets: ["Bar", "Bar2", "Bar3"])
919+
ProductDescription(name: "Bar", targets: ["Bar", "Bar2", "Bar3"]),
920+
ProductDescription(name: "TransitiveBar", targets: ["TransitiveBar"]),
917921
],
918922
targets: [
919923
TargetDescription(
@@ -933,14 +937,19 @@ class PackageGraphTests: XCTestCase {
933937
TargetDescription(
934938
name: "Bar3"
935939
),
940+
TargetDescription(
941+
name: "TransitiveBar",
942+
dependencies: ["Bar2"]
943+
),
936944
]),
937945
]
938946
)
939947

940-
XCTAssertEqual(diagnostics.diagnostics.count, 2)
948+
XCTAssertEqual(diagnostics.diagnostics.count, 3)
941949
DiagnosticsEngineTester(diagnostics, ignoreNotes: true) { result in
942-
result.check(diagnostic: .contains("the target 'Bar' in product 'Bar' contains unsafe build flags"), behavior: .error)
943-
result.check(diagnostic: .contains("the target 'Bar2' in product 'Bar' contains unsafe build flags"), behavior: .error)
950+
result.checkUnordered(diagnostic: .contains("the target 'Bar2' in product 'TransitiveBar' contains unsafe build flags"), behavior: .error)
951+
result.checkUnordered(diagnostic: .contains("the target 'Bar' in product 'Bar' contains unsafe build flags"), behavior: .error)
952+
result.checkUnordered(diagnostic: .contains("the target 'Bar2' in product 'Bar' contains unsafe build flags"), behavior: .error)
944953
}
945954
}
946955

Tests/WorkspaceTests/WorkspaceTests.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3843,7 +3843,8 @@ final class WorkspaceTests: XCTestCase {
38433843
// We should only see errors about use of unsafe flag in the version-based dependency.
38443844
workspace.checkPackageGraph(roots: ["Foo", "Bar"]) { (graph, diagnostics) in
38453845
DiagnosticsEngineTester(diagnostics, ignoreNotes: true) { result in
3846-
result.check(diagnostic: .equal("the target 'Baz' in product 'Baz' contains unsafe build flags"), behavior: .error)
3846+
result.checkUnordered(diagnostic: .equal("the target 'Baz' in product 'Baz' contains unsafe build flags"), behavior: .error)
3847+
result.checkUnordered(diagnostic: .equal("the target 'Bar' in product 'Baz' contains unsafe build flags"), behavior: .error)
38473848
}
38483849
}
38493850
}

0 commit comments

Comments
 (0)