Change trivia attribution rule to make comments trailing trivia unless separated by a newline

This changes the trivia attribution rule in the new parser to consider all trivia up to the next newline as trailing trivia. Previously, comments were always considered trailing trivia.

This was discussed last year in https://forums.swift.org/t/changing-comment-trivia-attribution-from-trailing-trivia-to-leading-trivia/50773.

My idea is that right now is probably one of the best times to change this because switching from the old to the new parser is an active source change for all clients.

Resolves rdar://68234477
Resolves rdar://95639215
Fixes #438

Author: ahoppen

Sources/SwiftParser/Diagnostics/MissingNodesError.swift

@@ -28,14 +28,16 @@ fileprivate enum NodesDescriptionPart {
 
   func description(format: Bool) -> String? {
     switch self {
-    case .tokensWithDefaultText(let tokens):
-      let tokenContents: String
+    case .tokensWithDefaultText(var tokens):
       if format {
-        tokenContents = tokens.map({ BasicFormat().visit($0).description }).joined()
-      } else {
-        tokenContents = tokens.map(\.description).joined()
+        tokens = tokens.map({ BasicFormat().visit($0) })
+      }
+      if !tokens.isEmpty {
+        tokens[0] = tokens[0].withLeadingTrivia([])
+        tokens[tokens.count - 1] = tokens[tokens.count - 1].withTrailingTrivia([])
       }
-      return "'\(tokenContents.trimmingWhitespace())'"
+      let tokenContents = tokens.map(\.description).joined()
+      return "'\(tokenContents)'"
     case .tokenWithoutDefaultText(let token):
       if let childName = token.childNameInParent {
         return childName

Sources/SwiftParser/Lexer.swift

@@ -921,10 +921,6 @@ extension Lexer.Cursor {
         guard !self.isAtEndOfFile else {
           break
         }
-        if case .trailing = position {
-          // Don't lex comments as trailing trivia (for now).
-          break
-        }
 
         switch self.peek() {
         case UInt8(ascii: "/"):

Tests/SwiftParserTest/Assertions.swift

@@ -27,24 +27,30 @@ func AssertEqualTokens(_ actual: [Lexer.Lexeme], _ expected: [Lexer.Lexeme], fil
     }
 
     guard l.leadingTriviaText == r.leadingTriviaText else {
-      return XCTFail("""
-        Token at index \(idx) does not have matching leading trivia! \
-        \(l.leadingTriviaText.debugDescription) != \(r.leadingTriviaText.debugDescription)
-        """, file: file, line: line)
+      return FailStringsEqualWithDiff(
+        String(syntaxText: l.leadingTriviaText),
+        String(syntaxText: r.leadingTriviaText),
+        "Token at index \(idx) does not have matching leading trivia",
+        file: file, line: line
+      )
     }
 
-    guard l.tokenText == r.tokenText else {
-      return XCTFail("""
-        Text at index \(idx) does not have matching text! \
-        \(l.tokenText.debugDescription) != \(r.tokenText.debugDescription)"
-        """, file: file, line: line)
+    guard l.tokenText.debugDescription == r.tokenText.debugDescription else {
+      return FailStringsEqualWithDiff(
+        l.tokenText.debugDescription,
+        r.tokenText.debugDescription,
+        "Text at index \(idx) does not have matching text",
+        file: file, line: line
+      )
     }
 
     guard l.trailingTriviaText == r.trailingTriviaText else {
-      return XCTFail("""
-        Token at index \(idx) does not have matching trailing trivia! \
-        \(l.trailingTriviaText.debugDescription) != \(r.trailingTriviaText.debugDescription)
-        """, file: file, line: line)
+      return FailStringsEqualWithDiff(
+        String(syntaxText: l.trailingTriviaText),
+        String(syntaxText: r.trailingTriviaText),
+        "Token at index \(idx) does not have matching trailing trivia",
+        file: file, line: line
+      )
     }
   }
 }

Tests/SwiftParserTest/ClassificationTests.swift

@@ -5,7 +5,10 @@ import SwiftParser
 public class ClassificationTests: XCTestCase {
 
   public func testClassification() {
-    let source = "// blah.\nlet x/*yo*/ = 0"
+    let source = """
+    // blah.
+    let x/*yo*/ = 0
+    """
     let tree = Parser.parse(source: source)
     do {
       let classif = Array(tree.classifications)
@@ -56,28 +59,28 @@ public class ClassificationTests: XCTestCase {
         return
       }
       XCTAssertEqual(classif[0].kind, .none)
-      XCTAssertEqual(classif[0].range, ByteSourceRange(offset: 20, length: 3))
+      XCTAssertEqual(classif[0].range, ByteSourceRange(offset: 21, length: 2))
     }
     do {
-      let initializer = (tree.statements[0].item.as(VariableDeclSyntax.self)!).bindings[0].initializer!
-      XCTAssertEqual(initializer.description, "/*yo*/ = 0")
+      let pattern = (tree.statements[0].item.as(VariableDeclSyntax.self)!).bindings[0].pattern
+      XCTAssertEqual(pattern.description, "x/*yo*/ ")
       // Classify with a relative range inside this node.
-      let classif = Array(initializer.classifications(in: ByteSourceRange(offset: 5, length: 2)))
+      let classif = Array(pattern.classifications(in: ByteSourceRange(offset: 5, length: 2)))
       XCTAssertEqual(classif.count, 2)
       guard classif.count == 2 else {
         return
       }
       XCTAssertEqual(classif[0].kind, .blockComment)
       XCTAssertEqual(classif[0].range, ByteSourceRange(offset: 14, length: 6))
       XCTAssertEqual(classif[1].kind, .none)
-      XCTAssertEqual(classif[1].range, ByteSourceRange(offset: 20, length: 3))
+      XCTAssertEqual(classif[1].range, ByteSourceRange(offset: 20, length: 1))
 
       do {
-        let singleClassif = initializer.classification(at: 5)
+        let singleClassif = pattern.classification(at: 5)
         XCTAssertEqual(singleClassif, classif[0])
       }
       do {
-        let singleClassif = initializer.classification(at: AbsolutePosition(utf8Offset: 19))
+        let singleClassif = pattern.classification(at: AbsolutePosition(utf8Offset: 19))
         XCTAssertEqual(singleClassif, classif[0])
       }
     }