Skip to content

Commit 9051756

Browse files
authored
Add more context for validation errors for target properties. (#8612)
Add more context for validation errors for target properties. ### Motivation: I came across a post https://forums.swift.org/t/getting-error-on-package-resolve/75419 which reports the error message to be not clear. I thought I could clarify the message. ``` error: 'xerr': target 'Penny' contains a value for disallowed property 'settings' ``` ### Modifications: I've updated the message for the error, so it includes more details to narrow down what is the culprit: - highlight that the property depends on the target type - show the value of the property, so that there's a hint what line in the Package.swift lead to the error (for example, linkerSettings or cxxSettings) ### Result: Before: ``` error: 'xerr': target 'Penny' contains a value for disallowed property 'settings' ``` After: ``` error: 'xerr': target 'Penny' is assigned a property 'settings' which is not accepted for the binary target type. The current value of the property has the following representation: [PackageModel.TargetBuildSettingDescription.Setting( tool: PackageModel.TargetBuildSettingDescription.Tool.linker, kind: PackageModel.TargetBuildSettingDescription.Kind.linkedFramework("AVFoundation"), condition: nil)]. ```
1 parent a4cd504 commit 9051756

File tree

3 files changed

+290
-44
lines changed

3 files changed

+290
-44
lines changed

Sources/PackageModel/Manifest/TargetDescription.swift

Lines changed: 234 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -212,51 +212,237 @@ public struct TargetDescription: Hashable, Encodable, Sendable {
212212
checksum: String? = nil,
213213
pluginUsages: [PluginUsage]? = nil
214214
) throws {
215+
let targetType = String(describing: type)
215216
switch type {
216217
case .regular, .executable, .test:
217-
if url != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "url") }
218-
if pkgConfig != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pkgConfig") }
219-
if providers != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "providers") }
220-
if pluginCapability != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pluginCapability") }
221-
if checksum != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "checksum") }
218+
if url != nil { throw Error.disallowedPropertyInTarget(
219+
targetName: name,
220+
targetType: targetType,
221+
propertyName: "url",
222+
value: url ?? "<nil>"
223+
) }
224+
if pkgConfig != nil { throw Error.disallowedPropertyInTarget(
225+
targetName: name,
226+
targetType: targetType,
227+
propertyName: "pkgConfig",
228+
value: pkgConfig ?? "<nil>"
229+
) }
230+
if providers != nil { throw Error.disallowedPropertyInTarget(
231+
targetName: name,
232+
targetType: targetType,
233+
propertyName: "providers",
234+
value: String(describing: providers!)
235+
) }
236+
if pluginCapability != nil { throw Error.disallowedPropertyInTarget(
237+
targetName: name,
238+
targetType: targetType,
239+
propertyName: "pluginCapability",
240+
value: String(describing: pluginCapability!)
241+
) }
242+
if checksum != nil { throw Error.disallowedPropertyInTarget(
243+
targetName: name,
244+
targetType: targetType,
245+
propertyName: "checksum",
246+
value: checksum ?? "<nil>"
247+
) }
222248
case .system:
223-
if !dependencies.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "dependencies") }
224-
if !exclude.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "exclude") }
225-
if sources != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "sources") }
226-
if !resources.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "resources") }
227-
if publicHeadersPath != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "publicHeadersPath") }
228-
if pluginCapability != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pluginCapability") }
229-
if !settings.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "settings") }
230-
if checksum != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "checksum") }
231-
if pluginUsages != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pluginUsages") }
249+
if !dependencies.isEmpty { throw Error.disallowedPropertyInTarget(
250+
targetName: name,
251+
targetType: targetType,
252+
propertyName: "dependencies",
253+
value: String(describing: dependencies)
254+
) }
255+
if !exclude.isEmpty { throw Error.disallowedPropertyInTarget(
256+
targetName: name,
257+
targetType: targetType,
258+
propertyName: "exclude",
259+
value: String(describing: exclude)
260+
) }
261+
if sources != nil { throw Error.disallowedPropertyInTarget(
262+
targetName: name,
263+
targetType: targetType,
264+
propertyName: "sources",
265+
value: String(describing: sources!)
266+
) }
267+
if !resources.isEmpty { throw Error.disallowedPropertyInTarget(
268+
targetName: name,
269+
targetType: targetType,
270+
propertyName: "resources",
271+
value: String(describing: resources)
272+
) }
273+
if publicHeadersPath != nil { throw Error.disallowedPropertyInTarget(
274+
targetName: name,
275+
targetType: targetType,
276+
propertyName: "publicHeadersPath",
277+
value: publicHeadersPath ?? "<nil>"
278+
) }
279+
if pluginCapability != nil { throw Error.disallowedPropertyInTarget(
280+
targetName: name,
281+
targetType: targetType,
282+
propertyName: "pluginCapability",
283+
value: String(describing: pluginCapability!)
284+
) }
285+
if !settings.isEmpty { throw Error.disallowedPropertyInTarget(
286+
targetName: name,
287+
targetType: targetType,
288+
propertyName: "settings",
289+
value: String(describing: settings)
290+
) }
291+
if checksum != nil { throw Error.disallowedPropertyInTarget(
292+
targetName: name,
293+
targetType: targetType,
294+
propertyName: "checksum",
295+
value: checksum ?? "<nil>"
296+
) }
297+
if pluginUsages != nil { throw Error.disallowedPropertyInTarget(
298+
targetName: name,
299+
targetType: targetType,
300+
propertyName: "pluginUsages",
301+
value: String(describing: pluginUsages!)
302+
) }
232303
case .binary:
233304
if path == nil && url == nil { throw Error.binaryTargetRequiresEitherPathOrURL(targetName: name) }
234-
if !dependencies.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "dependencies") }
235-
if !exclude.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "exclude") }
236-
if sources != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "sources") }
237-
if !resources.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "resources") }
238-
if publicHeadersPath != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "publicHeadersPath") }
239-
if pkgConfig != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pkgConfig") }
240-
if providers != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "providers") }
241-
if pluginCapability != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pluginCapability") }
242-
if !settings.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "settings") }
243-
if pluginUsages != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pluginUsages") }
305+
if !dependencies.isEmpty { throw Error.disallowedPropertyInTarget(
306+
targetName: name,
307+
targetType: targetType,
308+
propertyName: "dependencies",
309+
value: String(describing: dependencies)
310+
) }
311+
if !exclude.isEmpty { throw Error.disallowedPropertyInTarget(
312+
targetName: name,
313+
targetType: targetType,
314+
propertyName: "exclude",
315+
value: String(describing: exclude)
316+
) }
317+
if sources != nil { throw Error.disallowedPropertyInTarget(
318+
targetName: name,
319+
targetType: targetType,
320+
propertyName: "sources",
321+
value: String(describing: sources!)
322+
) }
323+
if !resources.isEmpty { throw Error.disallowedPropertyInTarget(
324+
targetName: name,
325+
targetType: targetType,
326+
propertyName: "resources",
327+
value: String(describing: resources)
328+
) }
329+
if publicHeadersPath != nil { throw Error.disallowedPropertyInTarget(
330+
targetName: name,
331+
targetType: targetType,
332+
propertyName: "publicHeadersPath",
333+
value: publicHeadersPath ?? "<nil>"
334+
) }
335+
if pkgConfig != nil { throw Error.disallowedPropertyInTarget(
336+
targetName: name,
337+
targetType: targetType,
338+
propertyName: "pkgConfig",
339+
value: pkgConfig ?? "<nil>"
340+
) }
341+
if providers != nil { throw Error.disallowedPropertyInTarget(
342+
targetName: name,
343+
targetType: targetType,
344+
propertyName: "providers",
345+
value: String(describing: providers!)
346+
) }
347+
if pluginCapability != nil { throw Error.disallowedPropertyInTarget(
348+
targetName: name,
349+
targetType: targetType,
350+
propertyName: "pluginCapability",
351+
value: String(describing: pluginCapability!)
352+
) }
353+
if !settings.isEmpty { throw Error.disallowedPropertyInTarget(
354+
targetName: name,
355+
targetType: targetType,
356+
propertyName: "settings",
357+
value: String(describing: settings)
358+
) }
359+
if pluginUsages != nil { throw Error.disallowedPropertyInTarget(
360+
targetName: name,
361+
targetType: targetType,
362+
propertyName: "pluginUsages",
363+
value: String(describing: pluginUsages!)
364+
) }
244365
case .plugin:
245-
if url != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "url") }
246-
if !resources.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "resources") }
247-
if publicHeadersPath != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "publicHeadersPath") }
248-
if pkgConfig != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pkgConfig") }
249-
if providers != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "providers") }
250-
if pluginCapability == nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pluginCapability") }
251-
if !settings.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "settings") }
252-
if pluginUsages != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pluginUsages") }
366+
if pluginCapability == nil { throw Error.pluginTargetRequiresPluginCapability(targetName: name) }
367+
if url != nil { throw Error.disallowedPropertyInTarget(
368+
targetName: name,
369+
targetType: targetType,
370+
propertyName: "url",
371+
value: url ?? "<nil>"
372+
) }
373+
if !resources.isEmpty { throw Error.disallowedPropertyInTarget(
374+
targetName: name,
375+
targetType: targetType,
376+
propertyName: "resources",
377+
value: String(describing: resources)
378+
) }
379+
if publicHeadersPath != nil { throw Error.disallowedPropertyInTarget(
380+
targetName: name,
381+
targetType: targetType,
382+
propertyName: "publicHeadersPath",
383+
value: publicHeadersPath ?? "<nil>"
384+
) }
385+
if pkgConfig != nil { throw Error.disallowedPropertyInTarget(
386+
targetName: name,
387+
targetType: targetType,
388+
propertyName: "pkgConfig",
389+
value: pkgConfig ?? "<nil>"
390+
) }
391+
if providers != nil { throw Error.disallowedPropertyInTarget(
392+
targetName: name,
393+
targetType: targetType,
394+
propertyName: "providers",
395+
value: String(describing: providers!)
396+
) }
397+
if !settings.isEmpty { throw Error.disallowedPropertyInTarget(
398+
targetName: name,
399+
targetType: targetType,
400+
propertyName: "settings",
401+
value: String(describing: settings)
402+
) }
403+
if pluginUsages != nil { throw Error.disallowedPropertyInTarget(
404+
targetName: name,
405+
targetType: targetType,
406+
propertyName: "pluginUsages",
407+
value: String(describing: pluginUsages!)
408+
) }
253409
case .macro:
254-
if url != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "url") }
255-
if !resources.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "resources") }
256-
if publicHeadersPath != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "publicHeadersPath") }
257-
if pkgConfig != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pkgConfig") }
258-
if providers != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "providers") }
259-
if pluginCapability != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pluginCapability") }
410+
if url != nil { throw Error.disallowedPropertyInTarget(
411+
targetName: name,
412+
targetType: targetType,
413+
propertyName: "url",
414+
value: url ?? "<nil>"
415+
) }
416+
if !resources.isEmpty { throw Error.disallowedPropertyInTarget(
417+
targetName: name,
418+
targetType: targetType,
419+
propertyName: "resources",
420+
value: String(describing: resources)
421+
) }
422+
if publicHeadersPath != nil { throw Error.disallowedPropertyInTarget(
423+
targetName: name,
424+
targetType: targetType,
425+
propertyName: "publicHeadersPath",
426+
value: publicHeadersPath ?? "<nil>"
427+
) }
428+
if pkgConfig != nil { throw Error.disallowedPropertyInTarget(
429+
targetName: name,
430+
targetType: targetType,
431+
propertyName: "pkgConfig",
432+
value: pkgConfig ?? "<nil>"
433+
) }
434+
if providers != nil { throw Error.disallowedPropertyInTarget(
435+
targetName: name,
436+
targetType: targetType,
437+
propertyName: "providers",
438+
value: String(describing: providers!)
439+
) }
440+
if pluginCapability != nil { throw Error.disallowedPropertyInTarget(
441+
targetName: name,
442+
targetType: targetType,
443+
propertyName: "pluginCapability",
444+
value: String(describing: pluginCapability!)
445+
) }
260446
}
261447

262448
self.name = name
@@ -404,14 +590,19 @@ import protocol Foundation.LocalizedError
404590

405591
private enum Error: LocalizedError, Equatable {
406592
case binaryTargetRequiresEitherPathOrURL(targetName: String)
407-
case disallowedPropertyInTarget(targetName: String, propertyName: String)
408-
593+
case pluginTargetRequiresPluginCapability(targetName: String)
594+
case disallowedPropertyInTarget(targetName: String, targetType: String, propertyName: String, value: String)
595+
409596
var errorDescription: String? {
410597
switch self {
411598
case .binaryTargetRequiresEitherPathOrURL(let targetName):
412-
return "binary target '\(targetName)' neither defines neither path nor URL for its artifacts"
413-
case .disallowedPropertyInTarget(let targetName, let propertyName):
414-
return "target '\(targetName)' contains a value for disallowed property '\(propertyName)'"
599+
"binary target '\(targetName)' must define either path or URL for its artifacts"
600+
case .pluginTargetRequiresPluginCapability(let targetName):
601+
"plugin target '\(targetName)' must define a plugin capability"
602+
case .disallowedPropertyInTarget(let targetName, let targetType, let propertyName, let value):
603+
"target '\(targetName)' is assigned a property '\(propertyName)' which is not accepted " +
604+
"for the \(targetType) target type. The current property value has " +
605+
"the following representation: \(value)."
415606
}
416607
}
417608
}

Tests/PackageLoadingTests/PD_5_3_LoadingTests.swift

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,38 @@ final class PackageDescription5_3LoadingTests: PackageDescriptionLoadingTests {
158158

159159
let observability = ObservabilitySystem.makeForTesting()
160160
await XCTAssertAsyncThrowsError(try await loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
161-
XCTAssertEqual(error.localizedDescription, "target 'Foo' contains a value for disallowed property 'settings'")
161+
XCTAssertEqual(error.localizedDescription,
162+
"target 'Foo' is assigned a property 'settings' which is not accepted for the binary target type. " +
163+
"The current property value has the following representation: " +
164+
"[PackageModel.TargetBuildSettingDescription.Setting(" +
165+
"tool: PackageModel.TargetBuildSettingDescription.Tool.linker, " +
166+
"kind: PackageModel.TargetBuildSettingDescription.Kind.linkedFramework(\"AVFoundation\"), " +
167+
"condition: nil)].")
168+
}
169+
}
170+
171+
func testBinaryTargetRequiresPathOrUrl() async throws {
172+
let content = """
173+
import PackageDescription
174+
var fwBinaryTarget = Target.binaryTarget(
175+
name: "nickel",
176+
url: "https://example.com/foo.git",
177+
checksum: "bee"
178+
)
179+
fwBinaryTarget.url = nil
180+
let package = Package(name: "foo", targets: [fwBinaryTarget])
181+
"""
182+
183+
let observability = ObservabilitySystem.makeForTesting()
184+
await XCTAssertAsyncThrowsError(
185+
try await loadAndValidateManifest(
186+
content, observabilityScope: observability.topScope
187+
), "expected error"
188+
) { error in
189+
XCTAssertEqual(
190+
error.localizedDescription,
191+
"binary target 'nickel' must define either path or URL for its artifacts"
192+
)
162193
}
163194
}
164195

Tests/PackageLoadingTests/PD_5_6_LoadingTests.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,30 @@ final class PackageDescription5_6LoadingTests: PackageDescriptionLoadingTests {
9898
XCTAssertEqual(manifest.targets[0].pluginCapability, .buildTool)
9999
}
100100

101+
func testPluginTargetRequiresPluginCapability() async throws {
102+
let content = """
103+
import PackageDescription
104+
var fwPluginTarget = Target.plugin(
105+
name: "quarter",
106+
capability: .buildTool
107+
)
108+
fwPluginTarget.pluginCapability = nil
109+
let package = Package(name: "foo", targets: [fwPluginTarget])
110+
"""
111+
112+
let observability = ObservabilitySystem.makeForTesting()
113+
await XCTAssertAsyncThrowsError(
114+
try await loadAndValidateManifest(
115+
content, observabilityScope: observability.topScope
116+
), "expected error"
117+
) { error in
118+
XCTAssertEqual(
119+
error.localizedDescription,
120+
"plugin target 'quarter' must define a plugin capability"
121+
)
122+
}
123+
}
124+
101125
func testPluginTargetCustomization() async throws {
102126
let content = """
103127
import PackageDescription

0 commit comments

Comments
 (0)