Skip to content
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
22 changes: 22 additions & 0 deletions Sources/PackageLoading/ManifestLoader+Validation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,24 @@ public struct ManifestValidator {
}
}

// Check against target setting condition traits.
let targets = self.manifest.targets

for target in targets {
let settings = target.settings
for setting in settings {
guard let traits = setting.condition?.traits else {
continue
}

for trait in traits {
if !traitKeys.contains(trait) {
diagnostics.append(.invalidTraitInSettingsCondition(trait: trait, target: target))
}
}
}
}

return diagnostics
}

Expand Down Expand Up @@ -372,6 +390,10 @@ extension Basics.Diagnostic {
.error("Trait \(enablerTrait) enables \(trait) which is not defined in the package")
}

public static func invalidTraitInSettingsCondition(trait: String, target: TargetDescription) -> Self {
.error("Trait '\(trait)' referenced in the build settings condition for target '\(target.name)' is not defined in the package")
}

static func invalidDefaultTrait(defaultTrait: String) -> Self {
.error("Default trait \(defaultTrait) is not defined in the package")
}
Expand Down
51 changes: 51 additions & 0 deletions Tests/PackageLoadingTests/TraitLoadingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -315,4 +315,55 @@ final class TraitLoadingTests: PackageDescriptionLoadingTests {
)
)
}

func testTargetBuildSettingsConditionTraits() async throws {
let content = """
import PackageDescription
let package = Package(
name: "Foo",
targets: [
.target(
name: "Target",
cSettings: [
.headerSearchPath("path/to/headers", .when(traits: ["UndefinedTrait2"])),
],
cxxSettings: [
.define("CXX_FLAG", .when(traits: ["UndefinedTrait3"])),
],
swiftSettings: [
.define("DEFINE1", .when(traits: ["Trait1"])),
],
linkerSettings: [
.linkedFramework("_Framework", .when(traits: ["UndefinedTrait1"]))
],
)
]
)
"""

let observability = ObservabilitySystem.makeForTesting()
let (_, validationDiagnostics) = try await loadAndValidateManifest(content, observabilityScope: observability.topScope)
XCTAssertNoDiagnostics(observability.diagnostics)
let firstDiagnostic = try XCTUnwrap(validationDiagnostics[0])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, why don't we emit the diagnostics via the observability diagnostics property?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question -- it seems like there is a pre-existing practice involved with the Manifest validation step here (see ManifestLoader+Validation.swift), so I continued to follow that for this change since I make the checks for these traits within those same validation methods.

let secondDiagnostic = try XCTUnwrap(validationDiagnostics[1])
let thirdDiagnostic = try XCTUnwrap(validationDiagnostics[2])
let fourthDiagnostic = try XCTUnwrap(validationDiagnostics[3])

func createDiagnostic(_ traitName: String) throws -> Basics.Diagnostic {
let target = try XCTUnwrap(TargetDescription(name: "Target"))
return Basics.Diagnostic.invalidTraitInSettingsCondition(trait: traitName, target: target)
}

let diagnostic1 = try XCTUnwrap(createDiagnostic("UndefinedTrait2"))
XCTAssertEqual(firstDiagnostic.description, diagnostic1.description)

let diagnostic2 = try XCTUnwrap(createDiagnostic("UndefinedTrait3"))
XCTAssertEqual(secondDiagnostic.description, diagnostic2.description)

let diagnostic3 = try XCTUnwrap(createDiagnostic("Trait1"))
XCTAssertEqual(thirdDiagnostic.description, diagnostic3.description)

let diagnostic4 = try XCTUnwrap(createDiagnostic("UndefinedTrait1"))
XCTAssertEqual(fourthDiagnostic.description, diagnostic4.description)
}
}
Loading