Skip to content

Commit c4396b3

Browse files
tomerdtomerd
authored
improve error message when running into an unknown target dependency (#6787)
motivation: help users reason about erros when encountering an unknown dependency changes: * include the dependency location in the error string * refactor tests to cover more use cases rdar://113411527
1 parent 7aa80df commit c4396b3

File tree

3 files changed

+114
-30
lines changed

3 files changed

+114
-30
lines changed

‎Sources/PackageLoading/ManifestLoader+Validation.swift

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ public struct ManifestValidator {
193193
diagnostics.append(.unknownTargetPackageDependency(
194194
packageName: packageName ?? "unknown package name",
195195
targetName: target.name,
196-
validPackages: self.manifest.dependencies.map { $0.nameForTargetDependencyResolutionOnly }
196+
validPackages: self.manifest.dependencies
197197
))
198198
}
199199
case .byName(let name, _):
@@ -205,7 +205,7 @@ public struct ManifestValidator {
205205
diagnostics.append(.unknownTargetDependency(
206206
dependency: name,
207207
targetName: target.name,
208-
validDependencies: self.manifest.dependencies.map { $0.nameForTargetDependencyResolutionOnly }
208+
validDependencies: self.manifest.dependencies
209209
))
210210
}
211211
}
@@ -268,20 +268,13 @@ extension Basics.Diagnostic {
268268
.error("invalid type for binary product '\(productName)'; products referencing only binary targets must be executable or automatic library products")
269269
}
270270

271-
/*static func duplicateDependency(dependencyIdentity: PackageIdentity) -> Self {
272-
.error("duplicate dependency '\(dependencyIdentity)'")
273-
}
274-
275-
static func duplicateDependencyName(dependencyName: String) -> Self {
276-
.error("duplicate dependency named '\(dependencyName)'; consider differentiating them using the 'name' argument")
277-
}*/
271+
static func unknownTargetDependency(dependency: String, targetName: String, validDependencies: [PackageDependency]) -> Self {
278272

279-
static func unknownTargetDependency(dependency: String, targetName: String, validDependencies: [String]) -> Self {
280-
.error("unknown dependency '\(dependency)' in target '\(targetName)'; valid dependencies are: '\(validDependencies.joined(separator: "', '"))'")
273+
.error("unknown dependency '\(dependency)' in target '\(targetName)'; valid dependencies are: \(validDependencies.map{ "\($0.descriptionForValidation)" }.joined(separator: ", "))")
281274
}
282275

283-
static func unknownTargetPackageDependency(packageName: String, targetName: String, validPackages: [String]) -> Self {
284-
.error("unknown package '\(packageName)' in dependencies of target '\(targetName)'; valid packages are: '\(validPackages.joined(separator: "', '"))'")
276+
static func unknownTargetPackageDependency(packageName: String, targetName: String, validPackages: [PackageDependency]) -> Self {
277+
.error("unknown package '\(packageName)' in dependencies of target '\(targetName)'; valid packages are: \(validPackages.map{ "\($0.descriptionForValidation)" }.joined(separator: ", "))")
285278
}
286279

287280
static func invalidBinaryLocation(targetName: String) -> Self {
@@ -328,3 +321,29 @@ extension TargetDescription {
328321
fileprivate var isRemote: Bool { url != nil }
329322
fileprivate var isLocal: Bool { path != nil }
330323
}
324+
325+
extension PackageDependency {
326+
fileprivate var descriptionForValidation: String {
327+
var description = "'\(self.nameForTargetDependencyResolutionOnly)'"
328+
329+
if let locationsString = {
330+
switch self {
331+
case .fileSystem(let settings):
332+
return "at '\(settings.path.pathString)'"
333+
case .sourceControl(let settings):
334+
switch settings.location {
335+
case .local(let path):
336+
return "at '\(path.pathString)'"
337+
case .remote(let url):
338+
return "from '\(url.absoluteString)'"
339+
}
340+
case .registry:
341+
return .none
342+
}
343+
}() {
344+
description += " (\(locationsString))"
345+
}
346+
347+
return description
348+
}
349+
}

‎Tests/PackageLoadingTests/PD_5_2_LoadingTests.swift

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
109109
name: "Trivial",
110110
products: [],
111111
dependencies: [
112-
.package(name: "Foo", url: "/foo1", from: "1.0.0"),
113-
.package(name: "Bar", url: "/bar1", from: "2.0.0"),
112+
.package(url: "http://scm.com/org/foo", from: "1.0.0"),
113+
.package(url: "http://scm.com/org/bar", from: "2.0.0"),
114114
],
115115
targets: [
116116
.target(
@@ -127,8 +127,8 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
127127
let (_, validationDiagnostics) = try loadAndValidateManifest(content, observabilityScope: observability.topScope)
128128
XCTAssertNoDiagnostics(observability.diagnostics)
129129
testDiagnostics(validationDiagnostics) { result in
130-
result.checkUnordered(diagnostic: "unknown package 'foo1' in dependencies of target 'Target1'; valid packages are: 'Foo', 'Bar'", severity: .error)
131-
result.checkUnordered(diagnostic: "unknown dependency 'foos' in target 'Target2'; valid dependencies are: 'Foo', 'Bar'", severity: .error)
130+
result.checkUnordered(diagnostic: "unknown package 'foo1' in dependencies of target 'Target1'; valid packages are: 'foo' (from 'http://scm.com/org/foo'), 'bar' (from 'http://scm.com/org/bar')", severity: .error)
131+
result.checkUnordered(diagnostic: "unknown dependency 'foos' in target 'Target2'; valid dependencies are: 'foo' (from 'http://scm.com/org/foo'), 'bar' (from 'http://scm.com/org/bar')", severity: .error)
132132
}
133133
}
134134

@@ -139,7 +139,8 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
139139
name: "Trivial",
140140
products: [],
141141
dependencies: [
142-
.package(name: "Foo", url: "/foo1", from: "1.0.0"),
142+
.package(name: "Foo", url: "http://scm.com/org/foo", from: "1.0.0"),
143+
.package(name: "Bar", url: "http://scm.com/org/bar", from: "2.0.0"),
143144
],
144145
targets: [
145146
.target(
@@ -152,23 +153,25 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
152153
)
153154
"""
154155

155-
// note: root has special rules in this case
156156
let observability = ObservabilitySystem.makeForTesting()
157-
let (_, validationDiagnostics) = try loadAndValidateManifest(content, packageKind: .root(.root), observabilityScope: observability.topScope)
157+
let (_, validationDiagnostics) = try loadAndValidateManifest(content, observabilityScope: observability.topScope)
158158
XCTAssertNoDiagnostics(observability.diagnostics)
159159
testDiagnostics(validationDiagnostics) { result in
160-
result.checkUnordered(diagnostic: "unknown package 'foo1' in dependencies of target 'Target1'; valid packages are: 'Foo'", severity: .error)
160+
result.checkUnordered(diagnostic: "unknown package 'foo1' in dependencies of target 'Target1'; valid packages are: 'Foo' (from 'http://scm.com/org/foo'), 'Bar' (from 'http://scm.com/org/bar')", severity: .error)
161+
result.checkUnordered(diagnostic: "unknown dependency 'foos' in target 'Target2'; valid dependencies are: 'Foo' (from 'http://scm.com/org/foo'), 'Bar' (from 'http://scm.com/org/bar')", severity: .error)
161162
}
162163
}
163164

165+
// packageKind == root has special rules in this case
166+
164167
do {
165168
let content = """
166169
import PackageDescription
167170
let package = Package(
168171
name: "Trivial",
169172
products: [],
170173
dependencies: [
171-
.package(path: "/foo2"),
174+
.package(name: "Foo", url: "http://scm.com/org/foo1", from: "1.0.0"),
172175
],
173176
targets: [
174177
.target(
@@ -181,42 +184,72 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
181184
)
182185
"""
183186

184-
// note: root has special rules in this case
185187
let observability = ObservabilitySystem.makeForTesting()
186188
let (_, validationDiagnostics) = try loadAndValidateManifest(content, packageKind: .root(.root), observabilityScope: observability.topScope)
187189
XCTAssertNoDiagnostics(observability.diagnostics)
188190
testDiagnostics(validationDiagnostics) { result in
189-
result.checkUnordered(diagnostic: "unknown package 'foo1' in dependencies of target 'Target1'; valid packages are: 'foo2'", severity: .error)
191+
result.check(diagnostic: "unknown package 'foo1' in dependencies of target 'Target1'; valid packages are: 'Foo' (from 'http://scm.com/org/foo1')", severity: .error)
190192
}
191193
}
192194

195+
193196
do {
194197
let content = """
195198
import PackageDescription
196199
let package = Package(
197200
name: "Trivial",
198201
products: [],
199202
dependencies: [
200-
.package(url: "/foo1", from: "1.0.0"),
201-
.package(url: "/foo2", from: "1.0.0"),
203+
.package(path: "/foo"),
204+
.package(path: "/bar"),
202205
],
203206
targets: [
204207
.target(
205208
name: "Target1",
206-
dependencies: [.product(name: "product", package: "foo3")]),
209+
dependencies: [.product(name: "product", package: "foo1")]),
207210
.target(
208211
name: "Target2",
209212
dependencies: ["foos"]),
210213
]
211214
)
212215
"""
213216

214-
// note: root has special rules in this case
215217
let observability = ObservabilitySystem.makeForTesting()
216-
let (_, validationDiagnostics) = try loadAndValidateManifest(content, packageKind: .root(.root), observabilityScope: observability.topScope)
218+
let (_, validationDiagnostics) = try loadAndValidateManifest(content, observabilityScope: observability.topScope)
219+
XCTAssertNoDiagnostics(observability.diagnostics)
220+
testDiagnostics(validationDiagnostics) { result in
221+
result.checkUnordered(diagnostic: "unknown package 'foo1' in dependencies of target 'Target1'; valid packages are: 'foo' (at '/foo'), 'bar' (at '/bar')", severity: .error)
222+
result.checkUnordered(diagnostic: "unknown dependency 'foos' in target 'Target2'; valid dependencies are: 'foo' (at '/foo'), 'bar' (at '/bar')", severity: .error)
223+
}
224+
}
225+
226+
do {
227+
let content = """
228+
import PackageDescription
229+
let package = Package(
230+
name: "Trivial",
231+
products: [],
232+
dependencies: [
233+
.package(name: "Foo", path: "/foo1"),
234+
.package(name: "Bar", path: "/bar1"),
235+
],
236+
targets: [
237+
.target(
238+
name: "Target1",
239+
dependencies: [.product(name: "product", package: "foo1")]),
240+
.target(
241+
name: "Target2",
242+
dependencies: ["foos"]),
243+
]
244+
)
245+
"""
246+
247+
let observability = ObservabilitySystem.makeForTesting()
248+
let (_, validationDiagnostics) = try loadAndValidateManifest(content, observabilityScope: observability.topScope)
217249
XCTAssertNoDiagnostics(observability.diagnostics)
218250
testDiagnostics(validationDiagnostics) { result in
219-
result.checkUnordered(diagnostic: "unknown package 'foo3' in dependencies of target 'Target1'; valid packages are: 'foo1', 'foo2'", severity: .error)
251+
result.checkUnordered(diagnostic: "unknown package 'foo1' in dependencies of target 'Target1'; valid packages are: 'Foo' (at '/foo1'), 'Bar' (at '/bar1')", severity: .error)
252+
result.checkUnordered(diagnostic: "unknown dependency 'foos' in target 'Target2'; valid dependencies are: 'Foo' (at '/foo1'), 'Bar' (at '/bar1')", severity: .error)
220253
}
221254
}
222255
}

‎Tests/PackageLoadingTests/PD_5_7_LoadingTests.swift

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,4 +189,36 @@ class PackageDescription5_7LoadingTests: PackageDescriptionLoadingTests {
189189
}
190190
}
191191
}
192+
193+
func testTargetDependencyProductInvalidPackage() throws {
194+
do {
195+
let content = """
196+
import PackageDescription
197+
let package = Package(
198+
name: "Trivial",
199+
products: [],
200+
dependencies: [
201+
.package(id: "org.foo", from: "1.0.0"),
202+
.package(id: "org.bar", from: "1.0.0"),
203+
],
204+
targets: [
205+
.target(
206+
name: "Target1",
207+
dependencies: [.product(name: "product", package: "org.baz")]),
208+
.target(
209+
name: "Target2",
210+
dependencies: ["foos"]),
211+
]
212+
)
213+
"""
214+
215+
let observability = ObservabilitySystem.makeForTesting()
216+
let (_, validationDiagnostics) = try loadAndValidateManifest(content, observabilityScope: observability.topScope)
217+
XCTAssertNoDiagnostics(observability.diagnostics)
218+
testDiagnostics(validationDiagnostics) { result in
219+
result.checkUnordered(diagnostic: "unknown package 'org.baz' in dependencies of target 'Target1'; valid packages are: 'org.foo', 'org.bar'", severity: .error)
220+
result.checkUnordered(diagnostic: "unknown dependency 'foos' in target 'Target2'; valid dependencies are: 'org.foo', 'org.bar'", severity: .error)
221+
}
222+
}
223+
}
192224
}

0 commit comments

Comments
 (0)