Merge pull request #1790 from ahoppen/ahoppen/log-string-interpolation-parsing-errors

When creation of a syntax node using string interpolation failed, log the diagnostics

Author: ahoppen

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<SyntaxType: SyntaxProtocol>(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
-      }
-    }
-    """
-  )
 }

Package.swift

@@ -3,21 +3,30 @@
 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 {
   swiftSyntaxSwiftSettings += [
     .define("SWIFTSYNTAX_ENABLE_ASSERTIONS")
   ]
+  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")
@@ -151,8 +160,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(

Sources/SwiftSyntaxBuilder/CMakeLists.txt

@@ -13,6 +13,7 @@ add_swift_host_library(SwiftSyntaxBuilder
   SwiftSyntaxBuilderCompatibility.swift
   Syntax+StringInterpolation.swift
   SyntaxNodeWithBody.swift
+  SyntaxParsable+ExpressibleByStringInterpolation.swift
   ValidatingSyntaxNodes.swift
   WithTrailingCommaSyntax+EnsuringTrailingComma.swift
 
@@ -24,6 +25,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
+  $<$<COMPILE_LANGUAGE:Swift>:-D;SWIFTSYNTAX_NO_OSLOG_DEPENDENCY>)
+
 target_link_libraries(SwiftSyntaxBuilder PUBLIC
   SwiftBasicFormat
   SwiftParser

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()
+    }
+  }
+}

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<SyntaxType: SyntaxProtocol>(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
-  }
-}

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..<sortedLinkages.endIndex] {
-      XCTFail("Found unasserted link-time dependency: \(superfluousLinkage.linkage)")
+      XCTFail(
+        "Found unasserted link-time dependency: \(superfluousLinkage.linkage)",
+        file: file,
+        line: line
+      )
     }
   }
 
@@ -314,6 +338,7 @@ extension Linkage.Assertion {
   fileprivate enum Condition {
     case swiftVersionAtLeast(versionBound: SwiftVersion)
     case configuration(ProductConfiguration)
+    case compilationCondition(CompilationCondition)
     case flaky
 
     enum SwiftVersion: Comparable {
@@ -328,6 +353,10 @@ extension Linkage.Assertion {
       case release
     }
 
+    enum CompilationCondition {
+      case osLogDependency
+    }
+
     fileprivate static func when(swiftVersionAtLeast version: SwiftVersion) -> 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
       }

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")
   }
 

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)
   }
 

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
 

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),
         """