Don’t fatalError when expressing syntax nodes by string literals

We should not crash the process if the source code that you use to create a syntax node using string interpolation is invalid. Instead we should:

- Not check for errors in the normal string interpolation case and just return a syntax tree that has the `hasError` flag set when there are syntax errors
- In either SwiftSyntaxBuilder offer `init(validating node: Self) throws` that check that `node` has no sytnax errors and throws otherwise. This allows us write e.g. `try wDeclSyntax(validating: "struct Foo {}")` to create a `DeclSyntax` that’s guaranteed to not have any syntax errors

rdar://104423126

Author: ahoppen

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

@@ -25,10 +25,12 @@ let syntaxExpressibleByStringInterpolationConformancesFile = SourceFileSyntax {
   ImportDeclSyntax("import SwiftParserDiagnostics")
 
   ExtensionDeclSyntax("extension SyntaxParseable") {
+    DeclSyntax("public typealias StringInterpolation = SyntaxStringInterpolation")
+
     InitializerDeclSyntax(
       """
-      public init(stringInterpolationOrThrow stringInterpolation: SyntaxStringInterpolation) throws {
-        self = try performParse(source: stringInterpolation.sourceText, parse: { parser in
+      public init(stringInterpolation: SyntaxStringInterpolation) {
+        self = performParse(source: stringInterpolation.sourceText, parse: { parser in
           return Self.parse(from: &parser)
         })
       }
@@ -41,19 +43,15 @@ let syntaxExpressibleByStringInterpolationConformancesFile = SourceFileSyntax {
 
   FunctionDeclSyntax(
     """
-    // TODO: This should be fileprivate, but is currently used in
-    // `ConvenienceInitializers.swift`. See the corresponding TODO there.
-    func performParse<SyntaxType: SyntaxProtocol>(source: [UInt8], parse: (inout Parser) throws -> SyntaxType) throws -> SyntaxType {
-      return try source.withUnsafeBufferPointer { buffer in
+    // 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 = try parse(&parser)
-        if result.hasError {
-          let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: result)
-          assert(!diagnostics.isEmpty)
-          throw SyntaxStringInterpolationError.diagnostics(diagnostics, tree: Syntax(result))
-        }
+        let result = parse(&parser)
         return result
       }
     }

Sources/SwiftParserDiagnostics/MissingNodesError.swift

@@ -334,8 +334,6 @@ extension ParseDiagnosticsGenerator {
           break
         }
       }
-    } else {
-      missingNodes = []
     }
 
     let changes = missingNodes.enumerated().map { (index, missingNode) -> FixIt.Changes in

Sources/SwiftSyntaxBuilder/CMakeLists.txt

@@ -12,6 +12,7 @@ add_swift_host_library(SwiftSyntaxBuilder
   ResultBuilderExtensions.swift
   Syntax+StringInterpolation.swift
   SyntaxNodeWithBody.swift
+  ValidatingSyntaxNodes.swift
   WithTrailingCommaSyntax+EnsuringTrailingComma.swift
 
 

Sources/SwiftSyntaxBuilder/ConvenienceInitializers.swift

@@ -176,7 +176,7 @@ extension FunctionParameterSyntax {
     _ source: String,
     for subject: Parser.ParameterSubject
   ) {
-    self = try! performParse(
+    self = performParse(
       source: Array(source.utf8),
       parse: {
         let raw = RawSyntax($0.parseFunctionParameter(for: subject))

Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift

@@ -143,7 +143,7 @@ extension SyntaxStringInterpolation: StringInterpolationProtocol {
 public protocol SyntaxExpressibleByStringInterpolation:
   ExpressibleByStringInterpolation
 where Self.StringInterpolation == SyntaxStringInterpolation {
-  init(stringInterpolationOrThrow stringInterpolation: SyntaxStringInterpolation) throws
+  init(stringInterpolation: SyntaxStringInterpolation)
 }
 
 enum SyntaxStringInterpolationError: Error, CustomStringConvertible {
@@ -221,34 +221,10 @@ public protocol ExpressibleByLiteralSyntax {
 }
 
 extension SyntaxExpressibleByStringInterpolation {
-  /// Initialize a syntax node by parsing the contents of the interpolation.
-  /// This function is marked `@_transparent` so that fatalErrors raised here
-  /// are reported at the string literal itself.
-  /// This makes debugging easier because Xcode will jump to the string literal
-  /// that had a parsing error instead of the initializer that raised the `fatalError`
-  @_transparent
-  public init(stringInterpolation: SyntaxStringInterpolation) {
-    do {
-      try self.init(stringInterpolationOrThrow: stringInterpolation)
-    } catch {
-      fatalError(String(describing: error))
-    }
-  }
-
-  @_transparent
   public init(stringLiteral value: String) {
-    do {
-      try self.init(stringLiteralOrThrow: value)
-    } catch {
-      fatalError(String(describing: error))
-    }
-  }
-
-  /// Initialize a syntax node from a string literal.
-  public init(stringLiteralOrThrow value: String) throws {
     var interpolation = SyntaxStringInterpolation()
     interpolation.appendLiteral(value)
-    try self.init(stringInterpolationOrThrow: interpolation)
+    self.init(stringInterpolation: interpolation)
   }
 }
 
@@ -445,7 +421,7 @@ extension Optional: ExpressibleByLiteralSyntax where Wrapped: ExpressibleByLiter
 }
 
 extension TokenSyntax: SyntaxExpressibleByStringInterpolation {
-  public init(stringInterpolationOrThrow stringInterpolation: SyntaxStringInterpolation) throws {
+  public init(stringInterpolation: SyntaxStringInterpolation) {
     let string = stringInterpolation.sourceText.withUnsafeBufferPointer { buf in
       return String(syntaxText: SyntaxText(buffer: buf))
     }
@@ -476,21 +452,7 @@ struct UnexpectedTrivia: DiagnosticMessage {
 }
 
 extension Trivia: ExpressibleByStringInterpolation {
-  /// Initialize a syntax node by parsing the contents of the interpolation.
-  /// This function is marked `@_transparent` so that fatalErrors raised here
-  /// are reported at the string literal itself.
-  /// This makes debugging easier because Xcode will jump to the string literal
-  /// that had a parsing error instead of the initializer that raised the `fatalError`
-  @_transparent
   public init(stringInterpolation: String.StringInterpolation) {
-    do {
-      try self.init(stringInterpolationOrThrow: stringInterpolation)
-    } catch {
-      fatalError(String(describing: error))
-    }
-  }
-
-  public init(stringInterpolationOrThrow stringInterpolation: String.StringInterpolation) throws {
     var text = String(stringInterpolation: stringInterpolation)
     let pieces = text.withUTF8 { (buf) -> [TriviaPiece] in
       // The leading trivia position is a little bit less restrictive (it allows a shebang), so let's use it.
@@ -499,40 +461,11 @@ extension Trivia: ExpressibleByStringInterpolation {
     }
 
     self.init(pieces: pieces)
-
-    if pieces.contains(where: { $0.isUnexpected }) {
-      var diagnostics: [Diagnostic] = []
-      let tree = SourceFileSyntax(statements: [], eofToken: .eof(leadingTrivia: self))
-      var offset = 0
-      for piece in pieces {
-        if case .unexpectedText(let contents) = piece {
-          diagnostics.append(
-            Diagnostic(
-              node: Syntax(tree),
-              position: tree.position.advanced(by: offset),
-              message: UnexpectedTrivia(triviaContents: contents)
-            )
-          )
-        }
-        offset += piece.sourceLength.utf8Length
-      }
-      throw SyntaxStringInterpolationError.diagnostics(diagnostics, tree: Syntax(tree))
-    }
   }
 
-  @_transparent
   public init(stringLiteral value: String) {
-    do {
-      try self.init(stringLiteralOrThrow: value)
-    } catch {
-      fatalError(String(describing: error))
-    }
-  }
-
-  /// Initialize a syntax node from a string literal.
-  public init(stringLiteralOrThrow value: String) throws {
     var interpolation = String.StringInterpolation(literalCapacity: 1, interpolationCount: 0)
     interpolation.appendLiteral(value)
-    try self.init(stringInterpolationOrThrow: interpolation)
+    self.init(stringInterpolation: interpolation)
   }
 }

Sources/SwiftSyntaxBuilder/SyntaxNodeWithBody.swift

@@ -23,7 +23,7 @@ import SwiftSyntax
 public struct PartialSyntaxNodeString: SyntaxExpressibleByStringInterpolation {
   let sourceText: [UInt8]
 
-  public init(stringInterpolationOrThrow stringInterpolation: SyntaxStringInterpolation) throws {
+  public init(stringInterpolation: SyntaxStringInterpolation) {
     self.sourceText = stringInterpolation.sourceText
   }
 }

Sources/SwiftSyntaxBuilder/ValidatingSyntaxNodes.swift

@@ -0,0 +1,58 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 SwiftSyntax
+import SwiftDiagnostics
+import SwiftParserDiagnostics
+
+extension SyntaxProtocol {
+  /// If `node` has contains no syntax errors, return `node`, otherwise
+  /// throw an error with diagnostics describing the syntax errors.
+  ///
+  /// This allows clients to e.g. write `try DeclSyntax(validating: "struct Foo {}")`
+  /// which constructs a `DeclSyntax` that's guaranteed to be free of syntax
+  /// errors.
+  public init(validating node: Self) throws {
+    if node.hasError {
+      let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: node)
+      assert(!diagnostics.isEmpty)
+      throw SyntaxStringInterpolationError.diagnostics(diagnostics, tree: Syntax(node))
+    }
+    self = node
+  }
+}
+
+extension Trivia {
+  /// If `trivia` has contains no unexpected trivia, return `trivia`, otherwise
+  /// throw an error with diagnostics describing the unexpected trivia.
+  public init(validating trivia: Trivia) throws {
+    self = trivia
+    if pieces.contains(where: { $0.isUnexpected }) {
+      var diagnostics: [Diagnostic] = []
+      let tree = SourceFileSyntax(statements: [], eofToken: .eof(leadingTrivia: self))
+      var offset = 0
+      for piece in pieces {
+        if case .unexpectedText(let contents) = piece {
+          diagnostics.append(
+            Diagnostic(
+              node: Syntax(tree),
+              position: tree.position.advanced(by: offset),
+              message: UnexpectedTrivia(triviaContents: contents)
+            )
+          )
+        }
+        offset += piece.sourceLength.utf8Length
+      }
+      throw SyntaxStringInterpolationError.diagnostics(diagnostics, tree: Syntax(tree))
+    }
+  }
+}

Sources/SwiftSyntaxBuilder/generated/SyntaxExpressibleByStringInterpolationConformances.swift

@@ -19,10 +19,12 @@ import SwiftParser
 import SwiftParserDiagnostics
 
 extension SyntaxParseable {
-  public init(stringInterpolationOrThrow stringInterpolation: SyntaxStringInterpolation) throws {
-    self = try performParse(source: stringInterpolation.sourceText, parse: { parser in 
-        return Self.parse(from: &parser)
-      })
+  public typealias StringInterpolation = SyntaxStringInterpolation
+  
+  public init(stringInterpolation: SyntaxStringInterpolation) {
+    self = performParse(source: stringInterpolation.sourceText, parse: { parser in
+      return Self.parse(from: &parser)
+    })
   }
 }
 
@@ -64,17 +66,12 @@ extension TypeSyntax: SyntaxExpressibleByStringInterpolation {
 
 // TODO: This should be fileprivate, but is currently used in
 // `ConvenienceInitializers.swift`. See the corresponding TODO there.
-func performParse<SyntaxType: SyntaxProtocol>(source: [UInt8], parse: (inout Parser) throws -> SyntaxType) throws -> SyntaxType {
-  return try source.withUnsafeBufferPointer { buffer in 
+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 = try parse(&parser)
-    if result.hasError {
-      let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: result)
-      assert(!diagnostics.isEmpty)
-      throw SyntaxStringInterpolationError.diagnostics(diagnostics, tree: Syntax(result))
-    }
+    let result = parse(&parser)
     return result
   }
 }

Tests/SwiftSyntaxBuilderTest/StringInterpolation.swift renamed to Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift

@@ -471,9 +471,10 @@ final class StringInterpolationTests: XCTestCase {
   }
 
   func testInvalidTrivia() {
-    var interpolation = String.StringInterpolation(literalCapacity: 1, interpolationCount: 0)
-    interpolation.appendLiteral("/*comment*/ invalid /*comm*/")
-    XCTAssertThrowsError(try Trivia(stringInterpolationOrThrow: interpolation)) { error in
+    let invalid = Trivia("/*comment*/ invalid /*comm*/")
+    XCTAssertEqual(invalid, [.blockComment("/*comment*/"), .spaces(1), .unexpectedText("invalid"), .spaces(1), .blockComment("/*comm*/")])
+
+    XCTAssertThrowsError(try Trivia(validating: "/*comment*/ invalid /*comm*/")) { error in
       AssertStringsEqualWithDiff(
         String(describing: error),
         """
@@ -485,4 +486,22 @@ final class StringInterpolationTests: XCTestCase {
       )
     }
   }
+
+  func testInvalidSyntax() {
+    let invalid = DeclSyntax("return 1")
+    XCTAssert(invalid.hasError)
+
+    XCTAssertThrowsError(try DeclSyntax(validating: "return 1")) { error in
+      AssertStringsEqualWithDiff(
+        String(describing: error),
+        """
+
+        1 │ return 1
+          ∣ │       ╰─ expected declaration
+          ∣ ╰─ unexpected code 'return 1' before declaration
+
+        """
+      )
+    }
+  }
 }