Diagnose using line & col of formatted text in PrettyPrinter.

The PrettyPrinter was using `AbsolutePosition`s from the TokenStreamCreator when creating diagnostics. Those are based on a utf8 offset, which is invalidated when the PrettyPrinter adds/removes content from the text while printing it. Instead, the diagnostics from the PrettyPrinter use a line and column computed based on its output buffer.

I removed all instances of storing `AbsolutePosition` from TokenStreamCreator, since using `AbsolutePosition` in PrettyPrinter is inherently flawed.

Author: dylansturg

Sources/SwiftFormatPrettyPrint/Comment.swift

@@ -40,12 +40,10 @@ struct Comment {
 
   let kind: Kind
   var text: [String]
-  let position: AbsolutePosition?
   public var length: Int
 
-  init(kind: Kind, text: String, position: AbsolutePosition? = nil) {
+  init(kind: Kind, text: String) {
     self.kind = kind
-    self.position = position
 
     switch kind {
     case .line, .docLine:

Sources/SwiftFormatPrettyPrint/PrettyPrint.swift

@@ -435,7 +435,7 @@ public class PrettyPrinter {
       write(comment.print(indent: currentIndentation))
       if wasEndOfLine {
         if comment.length > spaceRemaining {
-          diagnose(.moveEndOfLineComment, at: comment.position)
+          diagnose(.moveEndOfLineComment)
         }
       } else {
         spaceRemaining -= comment.length
@@ -459,21 +459,22 @@ public class PrettyPrinter {
     case .commaDelimitedRegionStart:
       commaDelimitedRegionStack.append(openCloseBreakCompensatingLineNumber)
 
-    case .commaDelimitedRegionEnd(let hasTrailingComma, let position):
+    case .commaDelimitedRegionEnd(let hasTrailingComma):
       guard let startLineNumber = commaDelimitedRegionStack.popLast() else {
         fatalError("Found trailing comma end with no corresponding start.")
       }
 
       let shouldHaveTrailingComma = startLineNumber != openCloseBreakCompensatingLineNumber
       if shouldHaveTrailingComma && !hasTrailingComma {
-        diagnose(.addTrailingComma, at: position)
+        diagnose(.addTrailingComma)
       } else if !shouldHaveTrailingComma && hasTrailingComma {
-        diagnose(.removeTrailingComma, at: position)
+        diagnose(.removeTrailingComma)
       }
 
       let shouldWriteComma = whitespaceOnly ? hasTrailingComma : shouldHaveTrailingComma
       if shouldWriteComma {
         write(",")
+        spaceRemaining -= 1
       }
     }
   }
@@ -679,14 +680,13 @@ public class PrettyPrinter {
     }
   }
 
-  private func diagnose(_ message: Diagnostic.Message, at position: AbsolutePosition?) {
-    let location: SourceLocation?
-    if let position = position {
-      location
-        = SourceLocation(offset: position.utf8Offset, converter: context.sourceLocationConverter)
-    } else {
-      location = nil
-    }
+  /// Diagnoses the given message at the current location in `outputBuffer`.
+  private func diagnose(_ message: Diagnostic.Message) {
+    // Add 1 since columns uses 1-based indices.
+    let column = maxLineLength - spaceRemaining + 1
+    let offset = outputBuffer.utf8.count
+    let location =
+      SourceLocation(line: lineNumber, column: column, offset: offset, file: context.fileURL.path)
     context.diagnosticEngine?.diagnose(message, location: location)
   }
 }

Sources/SwiftFormatPrettyPrint/Token.swift

@@ -177,7 +177,7 @@ enum Token {
 
   /// Marks the end of a comma delimited collection, where a trailing comma should be inserted
   /// if and only if the collection spans multiple lines.
-  case commaDelimitedRegionEnd(hasTrailingComma: Bool, position: AbsolutePosition)
+  case commaDelimitedRegionEnd(hasTrailingComma: Bool)
 
   // Convenience overloads for the enum types
   static let open = Token.open(.inconsistent, 0)

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

@@ -714,8 +714,8 @@ private final class TokenStreamCreator: SyntaxVisitor {
     // a trailing comma to 1 element arrays.
     if let firstElement = node.first, let lastElement = node.last, firstElement != lastElement {
       before(firstElement.firstToken, tokens: .commaDelimitedRegionStart)
-      let endToken = Token.commaDelimitedRegionEnd(
-        hasTrailingComma: lastElement.trailingComma != nil, position: lastElement.endPosition)
+      let endToken =
+        Token.commaDelimitedRegionEnd(hasTrailingComma: lastElement.trailingComma != nil)
       after(lastElement.lastToken, tokens: endToken)
       if let existingTrailingComma = lastElement.trailingComma {
         ignoredTokens.insert(existingTrailingComma)
@@ -745,8 +745,8 @@ private final class TokenStreamCreator: SyntaxVisitor {
     // so we never add a trailing comma to 1 element dictionaries.
     if let firstElement = node.first, let lastElement = node.last, firstElement != lastElement {
       before(firstElement.firstToken, tokens: .commaDelimitedRegionStart)
-      let endToken = Token.commaDelimitedRegionEnd(
-        hasTrailingComma: lastElement.trailingComma != nil, position: lastElement.endPosition)
+      let endToken =
+        Token.commaDelimitedRegionEnd(hasTrailingComma: lastElement.trailingComma != nil)
       after(lastElement.lastToken, tokens: endToken)
       if let existingTrailingComma = lastElement.trailingComma {
         ignoredTokens.insert(existingTrailingComma)
@@ -2320,15 +2320,13 @@ private final class TokenStreamCreator: SyntaxVisitor {
       return (false, [])
     }
 
-    let position = token.endPosition
-
     switch firstPiece {
     case .lineComment(let text):
       return (
         true,
         [
           .space(size: 2, flexible: true),
-          .comment(Comment(kind: .line, text: text, position: position), wasEndOfLine: true),
+          .comment(Comment(kind: .line, text: text), wasEndOfLine: true),
           // There must be a break with a soft newline after the comment, but it's impossible to
           // know which kind of break must be used. Adding this newline is deferred until the
           // comment is added to the token stream.
@@ -2339,7 +2337,7 @@ private final class TokenStreamCreator: SyntaxVisitor {
         false,
         [
           .space(size: 1, flexible: true),
-          .comment(Comment(kind: .block, text: text, position: position), wasEndOfLine: false),
+          .comment(Comment(kind: .block, text: text), wasEndOfLine: false),
           // We place a size-0 break after the comment to allow a discretionary newline after
           // the comment if the user places one here but the comment is otherwise adjacent to a
           // text token.

Tests/SwiftFormatPrettyPrintTests/ArrayDeclTests.swift

@@ -109,7 +109,7 @@ public class ArrayDeclTests: PrettyPrintTestCase {
     assertPrettyPrintEqual(
       input: input, expected: input + "\n", linelength: 45, whitespaceOnly: true)
 
-    XCTAssertDiagnosed(Diagnostic.Message.removeTrailingComma, line: 1, column: 18)
+    XCTAssertDiagnosed(Diagnostic.Message.removeTrailingComma, line: 1, column: 17)
     XCTAssertDiagnosed(Diagnostic.Message.addTrailingComma, line: 4, column: 26)
   }
 }

Tests/SwiftFormatPrettyPrintTests/DictionaryDeclTests.swift

@@ -92,7 +92,7 @@ public class DictionaryDeclTests: PrettyPrintTestCase {
     assertPrettyPrintEqual(
       input: input, expected: input + "\n", linelength: 50, whitespaceOnly: true)
 
-    XCTAssertDiagnosed(Diagnostic.Message.removeTrailingComma, line: 1, column: 33)
+    XCTAssertDiagnosed(Diagnostic.Message.removeTrailingComma, line: 1, column: 32)
     XCTAssertDiagnosed(Diagnostic.Message.addTrailingComma, line: 4, column: 17)
   }
 }