From 96557f7ed0df6843ae5b6cf62d59624133e2a874 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Wed, 14 Jun 2023 23:39:45 -0700 Subject: [PATCH] When creation of a syntax node using string interpolation failed, log the diagnostics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- ...yStringInterpolationConformancesFile.swift | 33 ---------- Package.swift | 18 ++++-- Sources/SwiftSyntaxBuilder/CMakeLists.txt | 6 ++ ...ble+ExpressibleByStringInterpolation.swift | 61 +++++++++++++++++++ ...bleByStringInterpolationConformances.swift | 25 -------- Tests/SwiftParserTest/LinkageTests.swift | 43 ++++++++++++- ...gLiteralRepresentedLiteralValueTests.swift | 6 +- .../ExpandEditorPlaceholderTests.swift | 4 +- .../ExpandEditorPlaceholdersTests.swift | 6 +- .../StringInterpolationTests.swift | 15 +++-- .../StringLiteralExprSyntaxTests.swift | 4 +- 11 files changed, 144 insertions(+), 77 deletions(-) create mode 100644 Sources/SwiftSyntaxBuilder/SyntaxParsable+ExpressibleByStringInterpolation.swift diff --git a/CodeGeneration/Sources/generate-swiftsyntax/templates/swiftsyntaxbuilder/SyntaxExpressibleByStringInterpolationConformancesFile.swift b/CodeGeneration/Sources/generate-swiftsyntax/templates/swiftsyntaxbuilder/SyntaxExpressibleByStringInterpolationConformancesFile.swift index 8d4016f3cb1..5e0e77dfd79 100644 --- a/CodeGeneration/Sources/generate-swiftsyntax/templates/swiftsyntaxbuilder/SyntaxExpressibleByStringInterpolationConformancesFile.swift +++ b/CodeGeneration/Sources/generate-swiftsyntax/templates/swiftsyntaxbuilder/SyntaxExpressibleByStringInterpolationConformancesFile.swift @@ -17,41 +17,8 @@ import Utils let syntaxExpressibleByStringInterpolationConformancesFile = SourceFileSyntax(leadingTrivia: copyrightHeader) { DeclSyntax("import SwiftSyntax") - DeclSyntax("import SwiftParser") - DeclSyntax("import SwiftParserDiagnostics") - - try! ExtensionDeclSyntax("extension SyntaxParseable") { - DeclSyntax("public typealias StringInterpolation = SyntaxStringInterpolation") - - DeclSyntax( - """ - public init(stringInterpolation: SyntaxStringInterpolation) { - self = performParse(source: stringInterpolation.sourceText, parse: { parser in - return Self.parse(from: &parser) - }) - } - """ - ) - } for node in SYNTAX_NODES where node.parserFunction != nil { DeclSyntax("extension \(node.kind.syntaxType): SyntaxExpressibleByStringInterpolation {}") } - - DeclSyntax( - """ - // TODO: This should be inlined in SyntaxParseable.init(stringInterpolation:), - // but is currently used in `ConvenienceInitializers.swift`. - // See the corresponding TODO there. - func performParse(source: [UInt8], parse: (inout Parser) -> SyntaxType) -> SyntaxType { - return source.withUnsafeBufferPointer { buffer in - var parser = Parser(buffer) - // FIXME: When the parser supports incremental parsing, put the - // interpolatedSyntaxNodes in so we don't have to parse them again. - let result = parse(&parser) - return result - } - } - """ - ) } diff --git a/Package.swift b/Package.swift index 060924b1ee0..cd39f251cf8 100644 --- a/Package.swift +++ b/Package.swift @@ -3,9 +3,12 @@ import PackageDescription import Foundation +var swiftSyntaxSwiftSettings: [SwiftSetting] = [] +var swiftSyntaxBuilderSwiftSettings: [SwiftSetting] = [] +var swiftParserSwiftSettings: [SwiftSetting] = [] + /// If we are in a controlled CI environment, we can use internal compiler flags /// to speed up the build or improve it. -var swiftSyntaxSwiftSettings: [SwiftSetting] = [] if ProcessInfo.processInfo.environment["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] != nil { let groupFile = URL(fileURLWithPath: #file) .deletingLastPathComponent() @@ -21,14 +24,20 @@ if ProcessInfo.processInfo.environment["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] != nil "-enforce-exclusivity=unchecked", ]), ] + swiftSyntaxBuilderSwiftSettings += [ + .define("SWIFTSYNTAX_NO_OSLOG_DEPENDENCY") + ] + swiftParserSwiftSettings += [ + .define("SWIFTSYNTAX_NO_OSLOG_DEPENDENCY") + ] } + if ProcessInfo.processInfo.environment["SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION"] != nil { swiftSyntaxSwiftSettings += [ .define("SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION") ] } -var swiftParserSwiftSettings: [SwiftSetting] = [] if ProcessInfo.processInfo.environment["SWIFTPARSER_ENABLE_ALTERNATE_TOKEN_INTROSPECTION"] != nil { swiftParserSwiftSettings += [ .define("SWIFTPARSER_ENABLE_ALTERNATE_TOKEN_INTROSPECTION") @@ -162,8 +171,9 @@ let package = Package( .target( name: "SwiftSyntaxBuilder", - dependencies: ["SwiftBasicFormat", "SwiftParser", "SwiftParserDiagnostics", "SwiftSyntax"], - exclude: ["CMakeLists.txt"] + dependencies: ["SwiftBasicFormat", "SwiftParser", "SwiftDiagnostics", "SwiftParserDiagnostics", "SwiftSyntax"], + exclude: ["CMakeLists.txt"], + swiftSettings: swiftSyntaxBuilderSwiftSettings ), .testTarget( diff --git a/Sources/SwiftSyntaxBuilder/CMakeLists.txt b/Sources/SwiftSyntaxBuilder/CMakeLists.txt index 67b4be8a75e..a4451258d5a 100644 --- a/Sources/SwiftSyntaxBuilder/CMakeLists.txt +++ b/Sources/SwiftSyntaxBuilder/CMakeLists.txt @@ -12,6 +12,7 @@ add_swift_host_library(SwiftSyntaxBuilder ResultBuilderExtensions.swift Syntax+StringInterpolation.swift SyntaxNodeWithBody.swift + SyntaxParsable+ExpressibleByStringInterpolation.swift ValidatingSyntaxNodes.swift WithTrailingCommaSyntax+EnsuringTrailingComma.swift @@ -22,6 +23,11 @@ add_swift_host_library(SwiftSyntaxBuilder generated/SyntaxExpressibleByStringInterpolationConformances.swift ) +# Don't depend on OSLog when we are building for the compiler. +# In that case we don't want any dependencies on the SDK. +target_compile_options(SwiftSyntaxBuilder PRIVATE + $<$:-D;SWIFTSYNTAX_NO_OSLOG_DEPENDENCY>) + target_link_libraries(SwiftSyntaxBuilder PUBLIC SwiftBasicFormat SwiftParser diff --git a/Sources/SwiftSyntaxBuilder/SyntaxParsable+ExpressibleByStringInterpolation.swift b/Sources/SwiftSyntaxBuilder/SyntaxParsable+ExpressibleByStringInterpolation.swift new file mode 100644 index 00000000000..f3174d92dc0 --- /dev/null +++ b/Sources/SwiftSyntaxBuilder/SyntaxParsable+ExpressibleByStringInterpolation.swift @@ -0,0 +1,61 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import SwiftDiagnostics +import SwiftSyntax +import SwiftParser +import SwiftParserDiagnostics +// Don't introduce a dependency on OSLog when building SwiftSyntax using CMake +// for the compiler. +#if canImport(OSLog) && !SWIFTSYNTAX_NO_OSLOG_DEPENDENCY +import OSLog +#endif + +extension SyntaxParseable { + public typealias StringInterpolation = SyntaxStringInterpolation + + /// Assuming that this node contains a syntax error, log it using OSLog if we + /// are on a platform that supports OSLog, otherwise don't do anything. + private func logStringInterpolationParsingError() { + #if canImport(OSLog) && !SWIFTSYNTAX_NO_OSLOG_DEPENDENCY + if #available(macOS 11.0, *) { + let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: self) + let formattedDiagnostics = DiagnosticsFormatter().annotatedSource(tree: self, diags: diagnostics) + Logger(subsystem: "SwiftSyntax", category: "ParseError").fault( + """ + Parsing a `\(Self.self)` node from string interpolation produced the following parsing errors. + Set a breakpoint in `SyntaxParseable.logStringInterpolationParsingError()` to debug the failure. + \(formattedDiagnostics) + """ + ) + } + #endif + } + + /// Initialize the syntax node from a string interpolation. + /// + /// - Important: This asssumes that the string interpolation produces a valid + /// syntax tree. If the syntax tree is not valid, a fault will + /// be logged using OSLog on Darwin platforms. + public init(stringInterpolation: SyntaxStringInterpolation) { + self = stringInterpolation.sourceText.withUnsafeBufferPointer { buffer in + var parser = Parser(buffer) + // FIXME: When the parser supports incremental parsing, put the + // interpolatedSyntaxNodes in so we don't have to parse them again. + let result = Self.parse(from: &parser) + return result + } + if self.hasError { + self.logStringInterpolationParsingError() + } + } +} diff --git a/Sources/SwiftSyntaxBuilder/generated/SyntaxExpressibleByStringInterpolationConformances.swift b/Sources/SwiftSyntaxBuilder/generated/SyntaxExpressibleByStringInterpolationConformances.swift index f5c2c9cb5d2..d8061b91838 100644 --- a/Sources/SwiftSyntaxBuilder/generated/SyntaxExpressibleByStringInterpolationConformances.swift +++ b/Sources/SwiftSyntaxBuilder/generated/SyntaxExpressibleByStringInterpolationConformances.swift @@ -13,18 +13,6 @@ //===----------------------------------------------------------------------===// import SwiftSyntax -import SwiftParser -import SwiftParserDiagnostics - -extension SyntaxParseable { - public typealias StringInterpolation = SyntaxStringInterpolation - - public init(stringInterpolation: SyntaxStringInterpolation) { - self = performParse(source: stringInterpolation.sourceText, parse: { parser in - return Self.parse(from: &parser) - }) - } -} extension AccessorDeclSyntax: SyntaxExpressibleByStringInterpolation {} @@ -57,16 +45,3 @@ extension StmtSyntax: SyntaxExpressibleByStringInterpolation {} extension SwitchCaseSyntax: SyntaxExpressibleByStringInterpolation {} extension TypeSyntax: SyntaxExpressibleByStringInterpolation {} - -// TODO: This should be inlined in SyntaxParseable.init(stringInterpolation:), -// but is currently used in `ConvenienceInitializers.swift`. -// See the corresponding TODO there. -func performParse(source: [UInt8], parse: (inout Parser) -> SyntaxType) -> SyntaxType { - return source.withUnsafeBufferPointer { buffer in - var parser = Parser(buffer) - // FIXME: When the parser supports incremental parsing, put the - // interpolatedSyntaxNodes in so we don't have to parse them again. - let result = parse(&parser) - return result - } -} diff --git a/Tests/SwiftParserTest/LinkageTests.swift b/Tests/SwiftParserTest/LinkageTests.swift index 41029884179..c13d0ed4fc6 100644 --- a/Tests/SwiftParserTest/LinkageTests.swift +++ b/Tests/SwiftParserTest/LinkageTests.swift @@ -69,9 +69,27 @@ final class LinkageTest: XCTestCase { .library("-lswiftCompatibilityConcurrency"), .library("-lswiftCompatibilityPacks", condition: .mayBeAbsent("Only in newer compilers")), .library("-lswiftCore"), + .library("-lswiftCoreFoundation", condition: .when(compilationCondition: .osLogDependency)), + .library("-lswiftDarwin", condition: .when(compilationCondition: .osLogDependency)), + .library("-lswiftDispatch", condition: .when(compilationCondition: .osLogDependency)), + .library("-lswiftFoundation", condition: .when(compilationCondition: .osLogDependency)), + .library("-lswiftIOKit", condition: .when(compilationCondition: .osLogDependency)), + .library("-lswiftOSLog", condition: .when(compilationCondition: .osLogDependency)), + .library("-lswiftObjectiveC", condition: .when(compilationCondition: .osLogDependency)), .library("-lswiftSwiftOnoneSupport", condition: .when(configuration: .debug)), + .library("-lswiftXPC", condition: .when(compilationCondition: .osLogDependency)), .library("-lswift_Concurrency"), .library("-lswift_StringProcessing", condition: .mayBeAbsent("Starting in Xcode 14 this library is not always autolinked")), + .library("-lswiftos", condition: .when(compilationCondition: .osLogDependency)), + .framework("CFNetwork", condition: .when(compilationCondition: .osLogDependency)), + .framework("Combine", condition: .when(compilationCondition: .osLogDependency)), + .framework("CoreFoundation", condition: .when(compilationCondition: .osLogDependency)), + .framework("CoreServices", condition: .when(compilationCondition: .osLogDependency)), + .framework("DiskArbitration", condition: .when(compilationCondition: .osLogDependency)), + .framework("Foundation", condition: .when(compilationCondition: .osLogDependency)), + .framework("IOKit", condition: .when(compilationCondition: .osLogDependency)), + .framework("OSLog", condition: .when(compilationCondition: .osLogDependency)), + .framework("Security", condition: .when(compilationCondition: .osLogDependency)), ] ) @@ -120,7 +138,9 @@ extension LinkageTest { private func assertLinkage( of library: String, in bundle: EnclosingTestBundle, - assertions: [Linkage.Assertion] + assertions: [Linkage.Assertion], + file: StaticString = #file, + line: UInt = #line ) throws { let linkages = try bundle.objectFiles(for: library).reduce(into: []) { acc, next in acc += try self.extractAutolinkingHints(in: next) @@ -175,7 +195,11 @@ extension LinkageTest { } for superfluousLinkage in sortedLinkages[expectedLinkagesIdx.. Condition { return .swiftVersionAtLeast(versionBound: version) } @@ -336,6 +365,10 @@ extension Linkage.Assertion { return .configuration(configuration) } + fileprivate static func when(compilationCondition: CompilationCondition) -> Condition { + return .compilationCondition(compilationCondition) + } + fileprivate static func mayBeAbsent(_ reason: StaticString) -> Condition { return .flaky } @@ -360,6 +393,12 @@ extension Linkage.Assertion { let config: ProductConfiguration = .release #endif return config == expectation + case .compilationCondition(.osLogDependency): + #if SWIFTSYNTAX_NO_OSLOG_DEPENDENCY + return false + #else + return true + #endif case .flaky: return true } diff --git a/Tests/SwiftParserTest/StringLiteralRepresentedLiteralValueTests.swift b/Tests/SwiftParserTest/StringLiteralRepresentedLiteralValueTests.swift index d0605583237..52d692825e2 100644 --- a/Tests/SwiftParserTest/StringLiteralRepresentedLiteralValueTests.swift +++ b/Tests/SwiftParserTest/StringLiteralRepresentedLiteralValueTests.swift @@ -250,7 +250,8 @@ public class StringLiteralRepresentedLiteralValueTests: XCTestCase { // MARK: literal value not available func testMissingQuoteStringLiteral() throws { - let stringLiteral = StringLiteralExprSyntax(#""a"# as ExprSyntax)! + var parser = Parser(#""a"#) + let stringLiteral = StringLiteralExprSyntax(ExprSyntax.parse(from: &parser))! XCTAssertNil(stringLiteral.representedLiteralValue, "only fully parsed string literals should produce a literal value") } @@ -260,7 +261,8 @@ public class StringLiteralRepresentedLiteralValueTests: XCTestCase { } func testMalformedMultiLineStringLiteral() throws { - let stringLiteral = StringLiteralExprSyntax(#""""a""""# as ExprSyntax)! + var parser = Parser(#""""a""""#) + let stringLiteral = StringLiteralExprSyntax(ExprSyntax.parse(from: &parser))! XCTAssertNil(stringLiteral.representedLiteralValue, "missing newline in multiline string literal cannot produce a literal value") } diff --git a/Tests/SwiftRefactorTest/ExpandEditorPlaceholderTests.swift b/Tests/SwiftRefactorTest/ExpandEditorPlaceholderTests.swift index 0bab9f931e3..d53ae47d6a2 100644 --- a/Tests/SwiftRefactorTest/ExpandEditorPlaceholderTests.swift +++ b/Tests/SwiftRefactorTest/ExpandEditorPlaceholderTests.swift @@ -12,6 +12,7 @@ import SwiftBasicFormat import SwiftRefactor +import SwiftParser import SwiftSyntax import SwiftSyntaxBuilder import XCTest @@ -100,7 +101,8 @@ fileprivate func assertRefactorPlaceholder( if wrap { token = "\(raw: ExpandEditorPlaceholder.wrapInPlaceholder(placeholder))" } else { - let expr: ExprSyntax = "\(raw: placeholder)" + var parser = Parser(placeholder) + let expr = ExprSyntax.parse(from: &parser) token = try XCTUnwrap(expr.as(EditorPlaceholderExprSyntax.self)?.identifier, file: file, line: line) } diff --git a/Tests/SwiftRefactorTest/ExpandEditorPlaceholdersTests.swift b/Tests/SwiftRefactorTest/ExpandEditorPlaceholdersTests.swift index 2f11dbaf5ca..6e16edf42df 100644 --- a/Tests/SwiftRefactorTest/ExpandEditorPlaceholdersTests.swift +++ b/Tests/SwiftRefactorTest/ExpandEditorPlaceholdersTests.swift @@ -109,7 +109,8 @@ fileprivate func assertRefactorPlaceholderCall( file: StaticString = #file, line: UInt = #line ) throws { - let call = try XCTUnwrap(ExprSyntax("\(raw: expr)").as(FunctionCallExprSyntax.self), file: file, line: line) + var parser = Parser(expr) + let call = try XCTUnwrap(ExprSyntax.parse(from: &parser).as(FunctionCallExprSyntax.self), file: file, line: line) let arg = try XCTUnwrap(call.argumentList[placeholder].as(TupleExprElementSyntax.self), file: file, line: line) let token: TokenSyntax = try XCTUnwrap(arg.expression.as(EditorPlaceholderExprSyntax.self), file: file, line: line).identifier @@ -123,7 +124,8 @@ fileprivate func assertRefactorPlaceholderToken( file: StaticString = #file, line: UInt = #line ) throws { - let call = try XCTUnwrap(ExprSyntax("\(raw: expr)").as(FunctionCallExprSyntax.self), file: file, line: line) + var parser = Parser(expr) + let call = try XCTUnwrap(ExprSyntax.parse(from: &parser).as(FunctionCallExprSyntax.self), file: file, line: line) let arg = try XCTUnwrap(call.argumentList[placeholder].as(TupleExprElementSyntax.self), file: file, line: line) let token: TokenSyntax = try XCTUnwrap(arg.expression.as(EditorPlaceholderExprSyntax.self), file: file, line: line).identifier diff --git a/Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift b/Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift index 520fd6fcde4..1efc1177c7d 100644 --- a/Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift +++ b/Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift @@ -446,10 +446,11 @@ final class StringInterpolationTests: XCTestCase { } func testInvalidSyntax() { - let invalid = DeclSyntax("return 1") + var parser = Parser("return 1") + let invalid = DeclSyntax.parse(from: &parser) XCTAssert(invalid.hasError) - XCTAssertThrowsError(try DeclSyntax(validating: "return 1")) { error in + XCTAssertThrowsError(try DeclSyntax(validating: invalid)) { error in assertStringsEqualWithDiff( String(describing: error), """ @@ -464,10 +465,11 @@ final class StringInterpolationTests: XCTestCase { } func testInvalidSyntax2() { - let invalid = StmtSyntax("struct Foo {}") + var parser = Parser("struct Foo {}") + let invalid = StmtSyntax.parse(from: &parser) XCTAssert(invalid.hasError) - XCTAssertThrowsError(try StmtSyntax(validating: "struct Foo {}")) { error in + XCTAssertThrowsError(try StmtSyntax(validating: invalid)) { error in assertStringsEqualWithDiff( String(describing: error), """ @@ -482,10 +484,11 @@ final class StringInterpolationTests: XCTestCase { } func testInvalidSyntax3() { - let invalid: CodeBlockItemSyntax = " " + var parser = Parser(" ") + let invalid = CodeBlockItemSyntax.parse(from: &parser) XCTAssert(invalid.hasError) - XCTAssertThrowsError(try CodeBlockItemSyntax(validating: " ")) { error in + XCTAssertThrowsError(try CodeBlockItemSyntax(validating: invalid)) { error in assertStringsEqualWithDiff( String(describing: error), """ diff --git a/Tests/SwiftSyntaxBuilderTest/StringLiteralExprSyntaxTests.swift b/Tests/SwiftSyntaxBuilderTest/StringLiteralExprSyntaxTests.swift index 7cc43d887c1..6eac48a42ff 100644 --- a/Tests/SwiftSyntaxBuilderTest/StringLiteralExprSyntaxTests.swift +++ b/Tests/SwiftSyntaxBuilderTest/StringLiteralExprSyntaxTests.swift @@ -232,7 +232,7 @@ final class StringLiteralExprSyntaxTests: XCTestCase { StringSegmentSyntax(content: .stringSegment(#"Validation failures:"#), trailingTrivia: .newline) ExpressionSegmentSyntax( expressions: TupleExprElementListSyntax { - TupleExprElementSyntax(expression: ExprSyntax(#"nonNilErrors.map({ "- \($0.description)" }).joined(separator: "\n"))"#)) + TupleExprElementSyntax(expression: ExprSyntax(#"nonNilErrors.map({ "- \($0.description)" }).joined(separator: "\n")"#)) } ) }, @@ -246,7 +246,7 @@ final class StringLiteralExprSyntaxTests: XCTestCase { Error validating child at index \(index) of \(nodeKind): Node did not satisfy any node choice requirement. Validation failures: - \(nonNilErrors.map({ "- \($0.description)" }).joined(separator: "\n"))) + \(nonNilErrors.map({ "- \($0.description)" }).joined(separator: "\n")) """ """# )