Skip to content

Commit 6aff18f

Browse files
elshelsh
authored
Add diagnostics for unsupported plugin dependency (#5831)
Add diagnostics for unsupported plugin dependency Resolves rdar://95117424
1 parent 0cff858 commit 6aff18f

File tree

3 files changed

+201
-3
lines changed

3 files changed

+201
-3
lines changed

‎Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,8 +792,10 @@ private final class ResolvedTargetBuilder: ResolvedBuilder<ResolvedTarget> {
792792
let dependencies = try self.dependencies.map { dependency -> ResolvedTarget.Dependency in
793793
switch dependency {
794794
case .target(let targetBuilder, let conditions):
795+
try self.target.validateDependency(target: targetBuilder.target)
795796
return .target(try targetBuilder.construct(), conditions: conditions)
796797
case .product(let productBuilder, let conditions):
798+
try self.target.validateDependency(product: productBuilder.product, productPackage: productBuilder.packageBuilder.package.identity)
797799
let product = try productBuilder.construct()
798800
if !productBuilder.packageBuilder.isAllowedToVendUnsafeProducts {
799801
try self.diagnoseInvalidUseOfUnsafeFlags(product)
@@ -811,6 +813,19 @@ private final class ResolvedTargetBuilder: ResolvedBuilder<ResolvedTarget> {
811813
}
812814
}
813815

816+
extension Target {
817+
818+
func validateDependency(target: Target) throws {
819+
if self.type == .plugin && target.type == .library {
820+
throw PackageGraphError.unsupportedPluginDependency(targetName: self.name, dependencyName: target.name, dependencyType: target.type.rawValue, dependencyPackage: nil)
821+
}
822+
}
823+
func validateDependency(product: Product, productPackage: PackageIdentity) throws {
824+
if self.type == .plugin && product.type.isLibrary {
825+
throw PackageGraphError.unsupportedPluginDependency(targetName: self.name, dependencyName: product.name, dependencyType: product.type.description, dependencyPackage: productPackage.description)
826+
}
827+
}
828+
}
814829
/// Builder for resolved package.
815830
private final class ResolvedPackageBuilder: ResolvedBuilder<ResolvedPackage> {
816831

‎Sources/PackageGraph/PackageGraph.swift

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ enum PackageGraphError: Swift.Error {
3636
targetName: String,
3737
packageIdentifier: String
3838
)
39-
39+
/// Dependency between a plugin and a dependent target/product of a given type is unsupported
40+
case unsupportedPluginDependency(targetName: String, dependencyName: String, dependencyType: String, dependencyPackage: String?)
4041
/// A product was found in multiple packages.
4142
case duplicateProduct(product: String, packages: [String])
4243

@@ -254,6 +255,12 @@ extension PackageGraphError: CustomStringConvertible {
254255
return "multiple aliases: ['\(aliases.joined(separator: "', '"))'] found for target '\(target)' in product '\(product)' from package '\(package)'"
255256
case .invalidSourcesForModuleAliasing(let target, let product, let package):
256257
return "module aliasing can only be used for Swift based targets; non-Swift sources found in target '\(target)' for product '\(product)' from package '\(package)'"
258+
case .unsupportedPluginDependency(let targetName, let dependencyName, let dependencyType, let dependencyPackage):
259+
var trailingMsg = ""
260+
if let depPkg = dependencyPackage {
261+
trailingMsg = " from package '\(depPkg)'"
262+
}
263+
return "plugin '\(targetName)' cannot depend on '\(dependencyName)' of type '\(dependencyType)'\(trailingMsg); this dependency is unsupported"
257264
}
258265
}
259266
}

‎Tests/SPMBuildCoreTests/PluginInvocationTests.swift

Lines changed: 178 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
import Basics
14-
import PackageGraph
14+
@testable import PackageGraph
1515
import PackageLoading
1616
import PackageModel
1717
@testable import SPMBuildCore
@@ -584,6 +584,183 @@ class PluginInvocationTests: XCTestCase {
584584
}
585585
}
586586

587+
func testUnsupportedDependencyProduct() throws {
588+
// Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require).
589+
try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency")
590+
591+
try testWithTemporaryDirectory { tmpPath in
592+
// Create a sample package with a library product and a plugin.
593+
let packageDir = tmpPath.appending(components: "MyPackage")
594+
try localFileSystem.createDirectory(packageDir, recursive: true)
595+
try localFileSystem.writeFileContents(packageDir.appending(component: "Package.swift"), string: """
596+
// swift-tools-version: 5.7
597+
import PackageDescription
598+
let package = Package(
599+
name: "MyPackage",
600+
dependencies: [
601+
.package(path: "../FooPackage"),
602+
],
603+
targets: [
604+
.plugin(
605+
name: "MyPlugin",
606+
capability: .buildTool(),
607+
dependencies: [
608+
.product(name: "FooLib", package: "FooPackage"),
609+
]
610+
),
611+
]
612+
)
613+
""")
614+
615+
let myPluginTargetDir = packageDir.appending(components: "Plugins", "MyPlugin")
616+
try localFileSystem.createDirectory(myPluginTargetDir, recursive: true)
617+
try localFileSystem.writeFileContents(myPluginTargetDir.appending(component: "plugin.swift"), string: """
618+
import PackagePlugin
619+
import Foo
620+
@main struct MyBuildToolPlugin: BuildToolPlugin {
621+
func createBuildCommands(
622+
context: PluginContext,
623+
target: Target
624+
) throws -> [Command] { }
625+
}
626+
""")
627+
628+
let fooPkgDir = tmpPath.appending(components: "FooPackage")
629+
try localFileSystem.createDirectory(fooPkgDir, recursive: true)
630+
try localFileSystem.writeFileContents(fooPkgDir.appending(component: "Package.swift"), string: """
631+
// swift-tools-version: 5.7
632+
import PackageDescription
633+
let package = Package(
634+
name: "FooPackage",
635+
products: [
636+
.library(name: "FooLib",
637+
targets: ["Foo"]),
638+
],
639+
targets: [
640+
.target(
641+
name: "Foo",
642+
dependencies: []
643+
),
644+
]
645+
)
646+
""")
647+
let fooTargetDir = fooPkgDir.appending(components: "Sources", "Foo")
648+
try localFileSystem.createDirectory(fooTargetDir, recursive: true)
649+
try localFileSystem.writeFileContents(fooTargetDir.appending(component: "file.swift"), string: """
650+
public func foo() { }
651+
""")
652+
653+
// Load a workspace from the package.
654+
let observability = ObservabilitySystem.makeForTesting()
655+
let workspace = try Workspace(
656+
fileSystem: localFileSystem,
657+
forRootPackage: packageDir,
658+
customManifestLoader: ManifestLoader(toolchain: UserToolchain.default),
659+
delegate: MockWorkspaceDelegate()
660+
)
661+
662+
// Load the root manifest.
663+
let rootInput = PackageGraphRootInput(packages: [packageDir], dependencies: [])
664+
let rootManifests = try tsc_await {
665+
workspace.loadRootManifests(
666+
packages: rootInput.packages,
667+
observabilityScope: observability.topScope,
668+
completion: $0
669+
)
670+
}
671+
XCTAssert(rootManifests.count == 1, "\(rootManifests)")
672+
673+
// Load the package graph.
674+
XCTAssertThrowsError(try workspace.loadPackageGraph(rootInput: rootInput, observabilityScope: observability.topScope)) { error in
675+
var diagnosed = false
676+
if let realError = error as? PackageGraphError,
677+
realError.description == "plugin 'MyPlugin' cannot depend on 'FooLib' of type 'library' from package 'foopackage'; this dependency is unsupported" {
678+
diagnosed = true
679+
}
680+
XCTAssertTrue(diagnosed)
681+
}
682+
}
683+
}
684+
685+
func testUnsupportedDependencyTarget() throws {
686+
// Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require).
687+
try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency")
688+
689+
try testWithTemporaryDirectory { tmpPath in
690+
// Create a sample package with a library target and a plugin.
691+
let packageDir = tmpPath.appending(components: "MyPackage")
692+
try localFileSystem.createDirectory(packageDir, recursive: true)
693+
try localFileSystem.writeFileContents(packageDir.appending(component: "Package.swift"), string: """
694+
// swift-tools-version: 5.7
695+
import PackageDescription
696+
let package = Package(
697+
name: "MyPackage",
698+
targets: [
699+
.target(
700+
name: "MyLibrary",
701+
dependencies: []
702+
),
703+
.plugin(
704+
name: "MyPlugin",
705+
capability: .buildTool(),
706+
dependencies: [
707+
"MyLibrary"
708+
]
709+
),
710+
]
711+
)
712+
""")
713+
714+
let myLibraryTargetDir = packageDir.appending(components: "Sources", "MyLibrary")
715+
try localFileSystem.createDirectory(myLibraryTargetDir, recursive: true)
716+
try localFileSystem.writeFileContents(myLibraryTargetDir.appending(component: "library.swift"), string: """
717+
public func hello() { }
718+
""")
719+
let myPluginTargetDir = packageDir.appending(components: "Plugins", "MyPlugin")
720+
try localFileSystem.createDirectory(myPluginTargetDir, recursive: true)
721+
try localFileSystem.writeFileContents(myPluginTargetDir.appending(component: "plugin.swift"), string: """
722+
import PackagePlugin
723+
import MyLibrary
724+
@main struct MyBuildToolPlugin: BuildToolPlugin {
725+
func createBuildCommands(
726+
context: PluginContext,
727+
target: Target
728+
) throws -> [Command] { }
729+
}
730+
""")
731+
732+
// Load a workspace from the package.
733+
let observability = ObservabilitySystem.makeForTesting()
734+
let workspace = try Workspace(
735+
fileSystem: localFileSystem,
736+
forRootPackage: packageDir,
737+
customManifestLoader: ManifestLoader(toolchain: UserToolchain.default),
738+
delegate: MockWorkspaceDelegate()
739+
)
740+
741+
// Load the root manifest.
742+
let rootInput = PackageGraphRootInput(packages: [packageDir], dependencies: [])
743+
let rootManifests = try tsc_await {
744+
workspace.loadRootManifests(
745+
packages: rootInput.packages,
746+
observabilityScope: observability.topScope,
747+
completion: $0
748+
)
749+
}
750+
XCTAssert(rootManifests.count == 1, "\(rootManifests)")
751+
752+
// Load the package graph.
753+
XCTAssertThrowsError(try workspace.loadPackageGraph(rootInput: rootInput, observabilityScope: observability.topScope)) { error in
754+
var diagnosed = false
755+
if let realError = error as? PackageGraphError,
756+
realError.description == "plugin 'MyPlugin' cannot depend on 'MyLibrary' of type 'library'; this dependency is unsupported" {
757+
diagnosed = true
758+
}
759+
XCTAssertTrue(diagnosed)
760+
}
761+
}
762+
}
763+
587764
func testPrebuildPluginShouldNotUseExecTarget() throws {
588765
// Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require).
589766
try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency")
@@ -659,7 +836,6 @@ class PluginInvocationTests: XCTestCase {
659836
)
660837
]
661838
}
662-
663839
}
664840
""")
665841

0 commit comments

Comments
 (0)