Skip to content

Commit 96557f7

Browse files
committed
When creation of a syntax node using string interpolation failed, log the diagnostics
From the first feedback of developers starting to build macros, the number one mistake was to build incorrect syntax nodes, e.g. build an `ExprSyntax` for a declaration. Up until recently, these errors weren’t diagnosed at all. By now we at least issue an XCTest assertion if the expanded macro code contains syntax errors. But I think the issues are a lot easier to debug if we can emit an error at the location where the incorrect syntax node is being created. Since we can’t throw an error from string interpolation, use OSLog to log the parsing error to Xcode or OS console. When OSLog is not available or we are building using CMake (and thus don’t want to introduce any dependency on the SDK), silently accept the error as we do right now. Logging to stderr might be too fragile in case some application is expecting to populate stderr itself.
1 parent 338a626 commit 96557f7

11 files changed

+144
-77
lines changed

CodeGeneration/Sources/generate-swiftsyntax/templates/swiftsyntaxbuilder/SyntaxExpressibleByStringInterpolationConformancesFile.swift

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,41 +17,8 @@ import Utils
1717

1818
let syntaxExpressibleByStringInterpolationConformancesFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {
1919
DeclSyntax("import SwiftSyntax")
20-
DeclSyntax("import SwiftParser")
21-
DeclSyntax("import SwiftParserDiagnostics")
22-
23-
try! ExtensionDeclSyntax("extension SyntaxParseable") {
24-
DeclSyntax("public typealias StringInterpolation = SyntaxStringInterpolation")
25-
26-
DeclSyntax(
27-
"""
28-
public init(stringInterpolation: SyntaxStringInterpolation) {
29-
self = performParse(source: stringInterpolation.sourceText, parse: { parser in
30-
return Self.parse(from: &parser)
31-
})
32-
}
33-
"""
34-
)
35-
}
3620

3721
for node in SYNTAX_NODES where node.parserFunction != nil {
3822
DeclSyntax("extension \(node.kind.syntaxType): SyntaxExpressibleByStringInterpolation {}")
3923
}
40-
41-
DeclSyntax(
42-
"""
43-
// TODO: This should be inlined in SyntaxParseable.init(stringInterpolation:),
44-
// but is currently used in `ConvenienceInitializers.swift`.
45-
// See the corresponding TODO there.
46-
func performParse<SyntaxType: SyntaxProtocol>(source: [UInt8], parse: (inout Parser) -> SyntaxType) -> SyntaxType {
47-
return source.withUnsafeBufferPointer { buffer in
48-
var parser = Parser(buffer)
49-
// FIXME: When the parser supports incremental parsing, put the
50-
// interpolatedSyntaxNodes in so we don't have to parse them again.
51-
let result = parse(&parser)
52-
return result
53-
}
54-
}
55-
"""
56-
)
5724
}

Package.swift

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@
33
import PackageDescription
44
import Foundation
55

6+
var swiftSyntaxSwiftSettings: [SwiftSetting] = []
7+
var swiftSyntaxBuilderSwiftSettings: [SwiftSetting] = []
8+
var swiftParserSwiftSettings: [SwiftSetting] = []
9+
610
/// If we are in a controlled CI environment, we can use internal compiler flags
711
/// to speed up the build or improve it.
8-
var swiftSyntaxSwiftSettings: [SwiftSetting] = []
912
if ProcessInfo.processInfo.environment["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] != nil {
1013
let groupFile = URL(fileURLWithPath: #file)
1114
.deletingLastPathComponent()
@@ -21,14 +24,20 @@ if ProcessInfo.processInfo.environment["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] != nil
2124
"-enforce-exclusivity=unchecked",
2225
]),
2326
]
27+
swiftSyntaxBuilderSwiftSettings += [
28+
.define("SWIFTSYNTAX_NO_OSLOG_DEPENDENCY")
29+
]
30+
swiftParserSwiftSettings += [
31+
.define("SWIFTSYNTAX_NO_OSLOG_DEPENDENCY")
32+
]
2433
}
34+
2535
if ProcessInfo.processInfo.environment["SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION"] != nil {
2636
swiftSyntaxSwiftSettings += [
2737
.define("SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION")
2838
]
2939
}
3040

31-
var swiftParserSwiftSettings: [SwiftSetting] = []
3241
if ProcessInfo.processInfo.environment["SWIFTPARSER_ENABLE_ALTERNATE_TOKEN_INTROSPECTION"] != nil {
3342
swiftParserSwiftSettings += [
3443
.define("SWIFTPARSER_ENABLE_ALTERNATE_TOKEN_INTROSPECTION")
@@ -162,8 +171,9 @@ let package = Package(
162171

163172
.target(
164173
name: "SwiftSyntaxBuilder",
165-
dependencies: ["SwiftBasicFormat", "SwiftParser", "SwiftParserDiagnostics", "SwiftSyntax"],
166-
exclude: ["CMakeLists.txt"]
174+
dependencies: ["SwiftBasicFormat", "SwiftParser", "SwiftDiagnostics", "SwiftParserDiagnostics", "SwiftSyntax"],
175+
exclude: ["CMakeLists.txt"],
176+
swiftSettings: swiftSyntaxBuilderSwiftSettings
167177
),
168178

169179
.testTarget(

Sources/SwiftSyntaxBuilder/CMakeLists.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ add_swift_host_library(SwiftSyntaxBuilder
1212
ResultBuilderExtensions.swift
1313
Syntax+StringInterpolation.swift
1414
SyntaxNodeWithBody.swift
15+
SyntaxParsable+ExpressibleByStringInterpolation.swift
1516
ValidatingSyntaxNodes.swift
1617
WithTrailingCommaSyntax+EnsuringTrailingComma.swift
1718

@@ -22,6 +23,11 @@ add_swift_host_library(SwiftSyntaxBuilder
2223
generated/SyntaxExpressibleByStringInterpolationConformances.swift
2324
)
2425

26+
# Don't depend on OSLog when we are building for the compiler.
27+
# In that case we don't want any dependencies on the SDK.
28+
target_compile_options(SwiftSyntaxBuilder PRIVATE
29+
$<$<COMPILE_LANGUAGE:Swift>:-D;SWIFTSYNTAX_NO_OSLOG_DEPENDENCY>)
30+
2531
target_link_libraries(SwiftSyntaxBuilder PUBLIC
2632
SwiftBasicFormat
2733
SwiftParser
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import SwiftDiagnostics
14+
import SwiftSyntax
15+
import SwiftParser
16+
import SwiftParserDiagnostics
17+
// Don't introduce a dependency on OSLog when building SwiftSyntax using CMake
18+
// for the compiler.
19+
#if canImport(OSLog) && !SWIFTSYNTAX_NO_OSLOG_DEPENDENCY
20+
import OSLog
21+
#endif
22+
23+
extension SyntaxParseable {
24+
public typealias StringInterpolation = SyntaxStringInterpolation
25+
26+
/// Assuming that this node contains a syntax error, log it using OSLog if we
27+
/// are on a platform that supports OSLog, otherwise don't do anything.
28+
private func logStringInterpolationParsingError() {
29+
#if canImport(OSLog) && !SWIFTSYNTAX_NO_OSLOG_DEPENDENCY
30+
if #available(macOS 11.0, *) {
31+
let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: self)
32+
let formattedDiagnostics = DiagnosticsFormatter().annotatedSource(tree: self, diags: diagnostics)
33+
Logger(subsystem: "SwiftSyntax", category: "ParseError").fault(
34+
"""
35+
Parsing a `\(Self.self)` node from string interpolation produced the following parsing errors.
36+
Set a breakpoint in `SyntaxParseable.logStringInterpolationParsingError()` to debug the failure.
37+
\(formattedDiagnostics)
38+
"""
39+
)
40+
}
41+
#endif
42+
}
43+
44+
/// Initialize the syntax node from a string interpolation.
45+
///
46+
/// - Important: This asssumes that the string interpolation produces a valid
47+
/// syntax tree. If the syntax tree is not valid, a fault will
48+
/// be logged using OSLog on Darwin platforms.
49+
public init(stringInterpolation: SyntaxStringInterpolation) {
50+
self = stringInterpolation.sourceText.withUnsafeBufferPointer { buffer in
51+
var parser = Parser(buffer)
52+
// FIXME: When the parser supports incremental parsing, put the
53+
// interpolatedSyntaxNodes in so we don't have to parse them again.
54+
let result = Self.parse(from: &parser)
55+
return result
56+
}
57+
if self.hasError {
58+
self.logStringInterpolationParsingError()
59+
}
60+
}
61+
}

Sources/SwiftSyntaxBuilder/generated/SyntaxExpressibleByStringInterpolationConformances.swift

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,6 @@
1313
//===----------------------------------------------------------------------===//
1414

1515
import SwiftSyntax
16-
import SwiftParser
17-
import SwiftParserDiagnostics
18-
19-
extension SyntaxParseable {
20-
public typealias StringInterpolation = SyntaxStringInterpolation
21-
22-
public init(stringInterpolation: SyntaxStringInterpolation) {
23-
self = performParse(source: stringInterpolation.sourceText, parse: { parser in
24-
return Self.parse(from: &parser)
25-
})
26-
}
27-
}
2816

2917
extension AccessorDeclSyntax: SyntaxExpressibleByStringInterpolation {}
3018

@@ -57,16 +45,3 @@ extension StmtSyntax: SyntaxExpressibleByStringInterpolation {}
5745
extension SwitchCaseSyntax: SyntaxExpressibleByStringInterpolation {}
5846

5947
extension TypeSyntax: SyntaxExpressibleByStringInterpolation {}
60-
61-
// TODO: This should be inlined in SyntaxParseable.init(stringInterpolation:),
62-
// but is currently used in `ConvenienceInitializers.swift`.
63-
// See the corresponding TODO there.
64-
func performParse<SyntaxType: SyntaxProtocol>(source: [UInt8], parse: (inout Parser) -> SyntaxType) -> SyntaxType {
65-
return source.withUnsafeBufferPointer { buffer in
66-
var parser = Parser(buffer)
67-
// FIXME: When the parser supports incremental parsing, put the
68-
// interpolatedSyntaxNodes in so we don't have to parse them again.
69-
let result = parse(&parser)
70-
return result
71-
}
72-
}

Tests/SwiftParserTest/LinkageTests.swift

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,27 @@ final class LinkageTest: XCTestCase {
6969
.library("-lswiftCompatibilityConcurrency"),
7070
.library("-lswiftCompatibilityPacks", condition: .mayBeAbsent("Only in newer compilers")),
7171
.library("-lswiftCore"),
72+
.library("-lswiftCoreFoundation", condition: .when(compilationCondition: .osLogDependency)),
73+
.library("-lswiftDarwin", condition: .when(compilationCondition: .osLogDependency)),
74+
.library("-lswiftDispatch", condition: .when(compilationCondition: .osLogDependency)),
75+
.library("-lswiftFoundation", condition: .when(compilationCondition: .osLogDependency)),
76+
.library("-lswiftIOKit", condition: .when(compilationCondition: .osLogDependency)),
77+
.library("-lswiftOSLog", condition: .when(compilationCondition: .osLogDependency)),
78+
.library("-lswiftObjectiveC", condition: .when(compilationCondition: .osLogDependency)),
7279
.library("-lswiftSwiftOnoneSupport", condition: .when(configuration: .debug)),
80+
.library("-lswiftXPC", condition: .when(compilationCondition: .osLogDependency)),
7381
.library("-lswift_Concurrency"),
7482
.library("-lswift_StringProcessing", condition: .mayBeAbsent("Starting in Xcode 14 this library is not always autolinked")),
83+
.library("-lswiftos", condition: .when(compilationCondition: .osLogDependency)),
84+
.framework("CFNetwork", condition: .when(compilationCondition: .osLogDependency)),
85+
.framework("Combine", condition: .when(compilationCondition: .osLogDependency)),
86+
.framework("CoreFoundation", condition: .when(compilationCondition: .osLogDependency)),
87+
.framework("CoreServices", condition: .when(compilationCondition: .osLogDependency)),
88+
.framework("DiskArbitration", condition: .when(compilationCondition: .osLogDependency)),
89+
.framework("Foundation", condition: .when(compilationCondition: .osLogDependency)),
90+
.framework("IOKit", condition: .when(compilationCondition: .osLogDependency)),
91+
.framework("OSLog", condition: .when(compilationCondition: .osLogDependency)),
92+
.framework("Security", condition: .when(compilationCondition: .osLogDependency)),
7593
]
7694
)
7795

@@ -120,7 +138,9 @@ extension LinkageTest {
120138
private func assertLinkage(
121139
of library: String,
122140
in bundle: EnclosingTestBundle,
123-
assertions: [Linkage.Assertion]
141+
assertions: [Linkage.Assertion],
142+
file: StaticString = #file,
143+
line: UInt = #line
124144
) throws {
125145
let linkages = try bundle.objectFiles(for: library).reduce(into: []) { acc, next in
126146
acc += try self.extractAutolinkingHints(in: next)
@@ -175,7 +195,11 @@ extension LinkageTest {
175195
}
176196

177197
for superfluousLinkage in sortedLinkages[expectedLinkagesIdx..<sortedLinkages.endIndex] {
178-
XCTFail("Found unasserted link-time dependency: \(superfluousLinkage.linkage)")
198+
XCTFail(
199+
"Found unasserted link-time dependency: \(superfluousLinkage.linkage)",
200+
file: file,
201+
line: line
202+
)
179203
}
180204
}
181205

@@ -314,6 +338,7 @@ extension Linkage.Assertion {
314338
fileprivate enum Condition {
315339
case swiftVersionAtLeast(versionBound: SwiftVersion)
316340
case configuration(ProductConfiguration)
341+
case compilationCondition(CompilationCondition)
317342
case flaky
318343

319344
enum SwiftVersion: Comparable {
@@ -328,6 +353,10 @@ extension Linkage.Assertion {
328353
case release
329354
}
330355

356+
enum CompilationCondition {
357+
case osLogDependency
358+
}
359+
331360
fileprivate static func when(swiftVersionAtLeast version: SwiftVersion) -> Condition {
332361
return .swiftVersionAtLeast(versionBound: version)
333362
}
@@ -336,6 +365,10 @@ extension Linkage.Assertion {
336365
return .configuration(configuration)
337366
}
338367

368+
fileprivate static func when(compilationCondition: CompilationCondition) -> Condition {
369+
return .compilationCondition(compilationCondition)
370+
}
371+
339372
fileprivate static func mayBeAbsent(_ reason: StaticString) -> Condition {
340373
return .flaky
341374
}
@@ -360,6 +393,12 @@ extension Linkage.Assertion {
360393
let config: ProductConfiguration = .release
361394
#endif
362395
return config == expectation
396+
case .compilationCondition(.osLogDependency):
397+
#if SWIFTSYNTAX_NO_OSLOG_DEPENDENCY
398+
return false
399+
#else
400+
return true
401+
#endif
363402
case .flaky:
364403
return true
365404
}

Tests/SwiftParserTest/StringLiteralRepresentedLiteralValueTests.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ public class StringLiteralRepresentedLiteralValueTests: XCTestCase {
250250
// MARK: literal value not available
251251

252252
func testMissingQuoteStringLiteral() throws {
253-
let stringLiteral = StringLiteralExprSyntax(#""a"# as ExprSyntax)!
253+
var parser = Parser(#""a"#)
254+
let stringLiteral = StringLiteralExprSyntax(ExprSyntax.parse(from: &parser))!
254255
XCTAssertNil(stringLiteral.representedLiteralValue, "only fully parsed string literals should produce a literal value")
255256
}
256257

@@ -260,7 +261,8 @@ public class StringLiteralRepresentedLiteralValueTests: XCTestCase {
260261
}
261262

262263
func testMalformedMultiLineStringLiteral() throws {
263-
let stringLiteral = StringLiteralExprSyntax(#""""a""""# as ExprSyntax)!
264+
var parser = Parser(#""""a""""#)
265+
let stringLiteral = StringLiteralExprSyntax(ExprSyntax.parse(from: &parser))!
264266
XCTAssertNil(stringLiteral.representedLiteralValue, "missing newline in multiline string literal cannot produce a literal value")
265267
}
266268

Tests/SwiftRefactorTest/ExpandEditorPlaceholderTests.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import SwiftBasicFormat
1414
import SwiftRefactor
15+
import SwiftParser
1516
import SwiftSyntax
1617
import SwiftSyntaxBuilder
1718
import XCTest
@@ -100,7 +101,8 @@ fileprivate func assertRefactorPlaceholder(
100101
if wrap {
101102
token = "\(raw: ExpandEditorPlaceholder.wrapInPlaceholder(placeholder))"
102103
} else {
103-
let expr: ExprSyntax = "\(raw: placeholder)"
104+
var parser = Parser(placeholder)
105+
let expr = ExprSyntax.parse(from: &parser)
104106
token = try XCTUnwrap(expr.as(EditorPlaceholderExprSyntax.self)?.identifier, file: file, line: line)
105107
}
106108

Tests/SwiftRefactorTest/ExpandEditorPlaceholdersTests.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ fileprivate func assertRefactorPlaceholderCall(
109109
file: StaticString = #file,
110110
line: UInt = #line
111111
) throws {
112-
let call = try XCTUnwrap(ExprSyntax("\(raw: expr)").as(FunctionCallExprSyntax.self), file: file, line: line)
112+
var parser = Parser(expr)
113+
let call = try XCTUnwrap(ExprSyntax.parse(from: &parser).as(FunctionCallExprSyntax.self), file: file, line: line)
113114
let arg = try XCTUnwrap(call.argumentList[placeholder].as(TupleExprElementSyntax.self), file: file, line: line)
114115
let token: TokenSyntax = try XCTUnwrap(arg.expression.as(EditorPlaceholderExprSyntax.self), file: file, line: line).identifier
115116

@@ -123,7 +124,8 @@ fileprivate func assertRefactorPlaceholderToken(
123124
file: StaticString = #file,
124125
line: UInt = #line
125126
) throws {
126-
let call = try XCTUnwrap(ExprSyntax("\(raw: expr)").as(FunctionCallExprSyntax.self), file: file, line: line)
127+
var parser = Parser(expr)
128+
let call = try XCTUnwrap(ExprSyntax.parse(from: &parser).as(FunctionCallExprSyntax.self), file: file, line: line)
127129
let arg = try XCTUnwrap(call.argumentList[placeholder].as(TupleExprElementSyntax.self), file: file, line: line)
128130
let token: TokenSyntax = try XCTUnwrap(arg.expression.as(EditorPlaceholderExprSyntax.self), file: file, line: line).identifier
129131

0 commit comments

Comments
 (0)