Skip to content

Commit 70db9ce

Browse files
authored
Don't use #_sourceLocation in the fallback event handler. (#1609)
We can't use our own macros in the Testing target because they aren't available when we build the toolchain. Update the expansion of `#_sourceLocation` to detect it's being used in one of our own library targets and emit an error. Resolves rdar://171774246. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
1 parent 5284c70 commit 70db9ce

File tree

7 files changed

+132
-33
lines changed

7 files changed

+132
-33
lines changed

Package.swift

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,8 @@ let package = Package(
140140
"TestingMacros",
141141
],
142142
exclude: ["CMakeLists.txt", "Testing.swiftcrossimport"],
143-
cxxSettings: .packageSettings,
144-
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("Testing"),
143+
cxxSettings: .packageSettings(),
144+
swiftSettings: .packageSettings() + .enableLibraryEvolution() + .moduleABIName("Testing"),
145145
linkerSettings: [
146146
.linkedLibrary("execinfo", .when(platforms: [.custom("freebsd"), .openbsd])),
147147
.linkedLibrary("_TestingInterop"),
@@ -159,7 +159,7 @@ let package = Package(
159159
"_Testing_WinSDK",
160160
"MemorySafeTestingTests",
161161
],
162-
swiftSettings: .packageSettings,
162+
swiftSettings: .packageSettings(isTestTarget: true),
163163
linkerSettings: [
164164
.linkedLibrary("util", .when(platforms: [.openbsd]))
165165
]
@@ -176,7 +176,7 @@ let package = Package(
176176
"Testing",
177177
],
178178
path: "Tests/_MemorySafeTestingTests",
179-
swiftSettings: .packageSettings + [.strictMemorySafety()]
179+
swiftSettings: .packageSettings(isTestTarget: true) + [.strictMemorySafety()]
180180
),
181181

182182
.macro(
@@ -190,7 +190,7 @@ let package = Package(
190190
.product(name: "SwiftCompilerPlugin", package: "swift-syntax"),
191191
],
192192
exclude: ["CMakeLists.txt"],
193-
swiftSettings: .packageSettings + [
193+
swiftSettings: .packageSettings() + [
194194
// The only target which needs the ability to import this macro
195195
// implementation target's module is its unit test target. Users of the
196196
// macros this target implements use them via their declarations in the
@@ -206,14 +206,14 @@ let package = Package(
206206
.target(
207207
name: "_TestingInternals",
208208
exclude: ["CMakeLists.txt"],
209-
cxxSettings: .packageSettings
209+
cxxSettings: .packageSettings()
210210
),
211211
.target(
212212
name: "_TestDiscovery",
213213
dependencies: ["_TestingInternals",],
214214
exclude: ["CMakeLists.txt"],
215-
cxxSettings: .packageSettings,
216-
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("_TestDiscovery")
215+
cxxSettings: .packageSettings(),
216+
swiftSettings: .packageSettings() + .enableLibraryEvolution() + .moduleABIName("_TestDiscovery")
217217
),
218218
.target(
219219
// Build _TestingInterop for debugging/testing purposes only. It is
@@ -222,8 +222,8 @@ let package = Package(
222222
dependencies: ["_TestingInternals",],
223223
path: "Sources/_TestingInterop",
224224
exclude: ["CMakeLists.txt"],
225-
cxxSettings: .packageSettings,
226-
swiftSettings: .packageSettings + .moduleABIName("_TestingInterop")
225+
cxxSettings: .packageSettings(),
226+
swiftSettings: .packageSettings() + .moduleABIName("_TestingInterop")
227227
),
228228

229229
// Cross-import overlays (not supported by Swift Package Manager)
@@ -235,7 +235,7 @@ let package = Package(
235235
],
236236
path: "Sources/Overlays/_Testing_AppKit",
237237
exclude: ["CMakeLists.txt"],
238-
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("Testing")
238+
swiftSettings: .packageSettings() + .enableLibraryEvolution() + .moduleABIName("Testing")
239239
),
240240
.target(
241241
name: "_Testing_CoreGraphics",
@@ -244,7 +244,7 @@ let package = Package(
244244
],
245245
path: "Sources/Overlays/_Testing_CoreGraphics",
246246
exclude: ["CMakeLists.txt"],
247-
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("_Testing_CoreGraphics")
247+
swiftSettings: .packageSettings() + .enableLibraryEvolution() + .moduleABIName("_Testing_CoreGraphics")
248248
),
249249
.target(
250250
name: "_Testing_CoreImage",
@@ -254,7 +254,7 @@ let package = Package(
254254
],
255255
path: "Sources/Overlays/_Testing_CoreImage",
256256
exclude: ["CMakeLists.txt"],
257-
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("_Testing_CoreImage")
257+
swiftSettings: .packageSettings() + .enableLibraryEvolution() + .moduleABIName("_Testing_CoreImage")
258258
),
259259
.target(
260260
name: "_Testing_Foundation",
@@ -267,7 +267,7 @@ let package = Package(
267267
// The Foundation module only has Library Evolution enabled on Apple
268268
// platforms, and since this target's module publicly imports Foundation,
269269
// it can only enable Library Evolution itself on those platforms.
270-
swiftSettings: .packageSettings + .enableLibraryEvolution(.whenApple()) + .moduleABIName("_Testing_Foundation")
270+
swiftSettings: .packageSettings() + .enableLibraryEvolution(.whenApple()) + .moduleABIName("_Testing_Foundation")
271271
),
272272
.target(
273273
name: "_Testing_UIKit",
@@ -278,7 +278,7 @@ let package = Package(
278278
],
279279
path: "Sources/Overlays/_Testing_UIKit",
280280
exclude: ["CMakeLists.txt"],
281-
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("_Testing_UIKit")
281+
swiftSettings: .packageSettings() + .enableLibraryEvolution() + .moduleABIName("_Testing_UIKit")
282282
),
283283
.target(
284284
name: "_Testing_WinSDK",
@@ -287,7 +287,7 @@ let package = Package(
287287
],
288288
path: "Sources/Overlays/_Testing_WinSDK",
289289
exclude: ["CMakeLists.txt"],
290-
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("_Testing_WinSDK")
290+
swiftSettings: .packageSettings() + .enableLibraryEvolution() + .moduleABIName("_Testing_WinSDK")
291291
),
292292

293293
// Utility targets: These are utilities intended for use when developing
@@ -297,7 +297,7 @@ let package = Package(
297297
dependencies: [
298298
"Testing",
299299
],
300-
swiftSettings: .packageSettings
300+
swiftSettings: .packageSettings()
301301
),
302302
],
303303

@@ -313,7 +313,7 @@ package.targets.append(contentsOf: [
313313
"Testing",
314314
"TestingMacros",
315315
],
316-
swiftSettings: .packageSettings
316+
swiftSettings: .packageSettings(isTestTarget: true)
317317
)
318318
])
319319
#endif
@@ -362,7 +362,7 @@ extension BuildSettingCondition {
362362
extension Array where Element == PackageDescription.SwiftSetting {
363363
/// Settings intended to be applied to every Swift target in this package.
364364
/// Analogous to project-level build settings in an Xcode project.
365-
static var packageSettings: Self {
365+
static func packageSettings(isTestTarget: Bool = false) -> Self {
366366
var result = availabilityMacroSettings
367367

368368
// treatWarning(..., as: .warning) cannot be used in packages which are
@@ -376,6 +376,14 @@ extension Array where Element == PackageDescription.SwiftSetting {
376376
result.append(.enableExperimentalFeature("Embedded"))
377377
}
378378

379+
// Define a compiler condition so we can discover at macro expansion time if
380+
// we're accidentally expanding our own macros in Swift Testing.
381+
if !isTestTarget {
382+
result += [
383+
.define("SWT_BUILDING_SWIFT_TESTING_CONTENT"),
384+
]
385+
}
386+
379387
result += [
380388
.enableUpcomingFeature("ExistentialAny"),
381389

@@ -474,9 +482,17 @@ extension Array where Element == PackageDescription.SwiftSetting {
474482
extension Array where Element == PackageDescription.CXXSetting {
475483
/// Settings intended to be applied to every C++ target in this package.
476484
/// Analogous to project-level build settings in an Xcode project.
477-
static var packageSettings: Self {
485+
static func packageSettings(isTestTarget: Bool = false) -> Self {
478486
var result = Self()
479487

488+
// Define a compiler condition so we can discover at macro expansion time if
489+
// we're accidentally expanding our own macros in Swift Testing.
490+
if !isTestTarget {
491+
result += [
492+
.define("SWT_BUILDING_SWIFT_TESTING_CONTENT"),
493+
]
494+
}
495+
480496
result += [
481497
.define("SWT_NO_EXIT_TESTS", .whenEmbedded(or: .when(platforms: [.iOS, .watchOS, .tvOS, .visionOS, .wasi, .android]))),
482498
.define("SWT_NO_PROCESS_SPAWNING", .whenEmbedded(or: .when(platforms: [.iOS, .watchOS, .tvOS, .visionOS, .wasi, .android]))),

Sources/Testing/Events/Event+FallbackEventHandler.swift

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -108,22 +108,41 @@ extension Event {
108108
warnForXCTestUsageIssue.record()
109109
}
110110

111+
/// Get the best available source location to use when diagnosing an issue
112+
/// decoding a bad record JSON blob.
113+
///
114+
/// - Parameters:
115+
/// - recordJSON: The undecodable JSON.
116+
///
117+
/// - Returns: A source location to use when reporting an issue about
118+
/// `recordJSON`.
119+
private static func _bestAvailableSourceLocation(forInvalidRecordJSON recordJSON: UnsafeRawBufferPointer) -> SourceLocation {
120+
// TODO: try to actually extract a source location from arbitrary JSON?
121+
122+
// If there's a test associated with the current task, it should have a
123+
// source location associated with it.
124+
if let test = Test.current {
125+
return test.sourceLocation
126+
}
127+
128+
return SourceLocation(fileID: "<unknown>/<unknown>", filePath: "<unknown>", line: 1, column: 1)
129+
}
130+
111131
#if !SWT_NO_INTEROP
112132
/// The fallback event handler to install when Swift Testing is the active
113133
/// testing library.
114134
private static let _ourFallbackEventHandler: SWTFallbackEventHandler = {
115135
recordJSONSchemaVersionNumber, recordJSONBaseAddress, recordJSONByteCount, _ in
116136
let version = String(validatingCString: recordJSONSchemaVersionNumber)
117137
.flatMap(VersionNumber.init)
118-
.flatMap { ABI.version(forVersionNumber: $0) }
119-
if let version {
120-
let recordJSON = UnsafeRawBufferPointer(
121-
start: recordJSONBaseAddress, count: recordJSONByteCount)
122-
do {
123-
try Self.handle(recordJSON, encodedWith: version)
124-
} catch {
125-
// Surface otherwise "unhandleable" records instead of dropping them silently
126-
let errorContext: Comment = """
138+
.flatMap { ABI.version(forVersionNumber: $0) } ?? ABI.v0.self
139+
let recordJSON = UnsafeRawBufferPointer(
140+
start: recordJSONBaseAddress, count: recordJSONByteCount)
141+
do {
142+
try Self.handle(recordJSON, encodedWith: version)
143+
} catch {
144+
// Surface otherwise "unhandleable" records instead of dropping them silently
145+
let errorContext: Comment = """
127146
Another test library reported a test event that Swift Testing could not decode. Inspect the payload to determine if this was a test assertion failure.
128147
129148
Error:
@@ -132,8 +151,19 @@ extension Event {
132151
Raw payload:
133152
\(recordJSON)
134153
"""
135-
Issue.record(errorContext)
136-
}
154+
155+
// Try to figure out a reasonable source context for this issue.
156+
let sourceContext = SourceContext(
157+
backtrace: .current(),
158+
sourceLocation: _bestAvailableSourceLocation(forInvalidRecordJSON: recordJSON)
159+
)
160+
161+
// Record the issue.
162+
Issue(
163+
kind: .system,
164+
comments: [errorContext],
165+
sourceContext: sourceContext
166+
).record()
137167
}
138168
}
139169
#endif

Sources/TestingMacros/SourceLocationMacro.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ public struct SourceLocationMacro: ExpressionMacro, Sendable {
2020
of macro: some FreestandingMacroExpansionSyntax,
2121
in context: some MacroExpansionContext
2222
) throws -> ExprSyntax {
23-
createSourceLocationExpr(of: macro, context: context)
23+
diagnoseExpansionInLibraryTarget(of: macro, in: context)
24+
return createSourceLocationExpr(of: macro, context: context)
2425
}
2526

2627
public static var formatMode: FormatMode {

Sources/TestingMacros/Support/DiagnosticMessage+Diagnosing.swift

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
//
1010

1111
import SwiftDiagnostics
12+
import SwiftIfConfig
1213
import SwiftParser
1314
import SwiftSyntax
1415
import SwiftSyntaxBuilder
@@ -314,3 +315,33 @@ func declarationInheritsFromXCTestClass(_ decl: some DeclSyntaxProtocol) -> Bool
314315
// We couldn't tell either way.
315316
return nil
316317
}
318+
319+
// MARK: -
320+
321+
/// Check if the given macro is being expanded in one of the testing library's
322+
/// production targets.
323+
///
324+
/// - Parameters:
325+
/// - macro: The macro being expanded.
326+
/// - context: The macro context in which the expression is being parsed.
327+
///
328+
/// If `macro` is being expanded in one of our production targets (as opposed to
329+
/// test targets), a diagnostic is emitted.
330+
///
331+
/// This function has no effect in release builds.
332+
func diagnoseExpansionInLibraryTarget(of macro: some FreestandingMacroExpansionSyntax, in context: some MacroExpansionContext) {
333+
#if DEBUG
334+
guard let buildConfiguration = context.buildConfiguration,
335+
let isBuildingSwiftTestingContent = try? buildConfiguration.isCustomConditionSet(name: "SWT_BUILDING_SWIFT_TESTING_CONTENT"),
336+
isBuildingSwiftTestingContent else {
337+
return
338+
}
339+
340+
var targetName = "<unknown>"
341+
if let fileID = context.location(of: macro, at: .afterLeadingTrivia, filePathMode: .fileID)?.file.trimmedDescription,
342+
let slashIndex = fileID.firstIndex(of: "/") {
343+
targetName = String(fileID[..<slashIndex])
344+
}
345+
context.diagnose(.macroExpansionNotAllowed(macro, inLibraryTargetNamed: targetName))
346+
#endif
347+
}

Sources/TestingMacros/Support/DiagnosticMessage.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,24 @@ import SwiftSyntaxMacroExpansion
1818
/// A type describing diagnostic messages emitted by this module's macro during
1919
/// evaluation.
2020
struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage {
21+
#if DEBUG
22+
/// Create a diagnostic message for the given macro stating it cannot be used
23+
/// when building the testing library itself.
24+
///
25+
/// - Parameters:
26+
/// - macro: The macro that cannot be used.
27+
/// - targetName: The name of the target being built.
28+
///
29+
/// - Returns: A diagnostic message.
30+
static func macroExpansionNotAllowed(_ macro: some FreestandingMacroExpansionSyntax, inLibraryTargetNamed targetName: String) -> Self {
31+
Self(
32+
syntax: Syntax(macro),
33+
message: "\(_macroName(macro)) cannot be used within the target '\(targetName)' because it will not be available when building the Swift toolchain",
34+
severity: .error
35+
)
36+
}
37+
#endif
38+
2139
/// Create a diagnostic message for the macro with the specified name
2240
/// stating that its condition will always pass or fail.
2341
///

Tests/TestingTests/EventHandlingInteropTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ struct EventHandlingInteropTests {
176176

177177
// Assert that we record an issue with a helpful debug message
178178
let expectedPrefix =
179-
"Issue recorded (error): Another test library reported a test event that Swift Testing could not decode. Inspect the payload to determine if this was a test assertion failure."
179+
"A system failure occurred (error): Another test library reported a test event that Swift Testing could not decode. Inspect the payload to determine if this was a test assertion failure."
180180
let actualMessages = issues.rawValue.map { $0.description }
181181
#expect(actualMessages.count == 1)
182182
#expect(

cmake/modules/shared/CompilerSettings.cmake

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ add_compile_options(
2323
"SHELL:$<$<COMPILE_LANGUAGE:Swift>:-Xfrontend -enable-upcoming-feature -Xfrontend MemberImportVisibility>"
2424
"SHELL:$<$<COMPILE_LANGUAGE:Swift>:-Xfrontend -enable-upcoming-feature -Xfrontend InferIsolatedConformances>")
2525

26+
# We don't build test targets with CMake, so everything is content.
27+
add_compile_definitions("SWT_BUILDING_SWIFT_TESTING_CONTENT")
28+
2629
# Platform-specific definitions.
2730
if(APPLE)
2831
add_compile_definitions("SWT_TARGET_OS_APPLE")

0 commit comments

Comments
 (0)