Repository: apple/swift-syntax (swift)

Issue:
`SourceLocation` should not have an optional `line` and `column`
`SourceLocation` currently has optional `line` and `column` properties if it doesn’t have a `ComputedLocation`. We should remove `ComputedLocation` and make `line` and `column` non-optional.

Pull request:
Fixes #1525

Patch:
diff --git a/Sources/SwiftCompilerPluginMessageHandling/PluginMacroExpansionContext.swift b/Sources/SwiftCompilerPluginMessageHandling/PluginMacroExpansionContext.swift
index 5e751a9d2a1..96cffe6695d 100644
--- a/Sources/SwiftCompilerPluginMessageHandling/PluginMacroExpansionContext.swift
+++ b/Sources/SwiftCompilerPluginMessageHandling/PluginMacroExpansionContext.swift
@@ -160,8 +160,8 @@ class SourceManager {
     return SourceLocation(
       // NOTE: IUO because 'localLocation' is created by a location converter
       // which guarantees non-nil line/column.
-      line: localLocation.line! + lineOffset,
-      column: localLocation.column! + columnOffset,
+      line: localLocation.line + lineOffset,
+      column: localLocation.column + columnOffset,
       offset: localLocation.offset + positionOffset,
       file: file
     )
diff --git a/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift b/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift
index 35eb258f623..d1c7152eee3 100644
--- a/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift
+++ b/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift
@@ -112,32 +112,28 @@ public struct DiagnosticsFormatter {
         return nil
       }
 
-      let startLoc = highlight.startLocation(converter: slc, afterLeadingTrivia: true);
-      guard let startLine = startLoc.line else {
-        return nil
-      }
+      let startLoc = highlight.startLocation(converter: slc, afterLeadingTrivia: true)
+      let startLine = startLoc.line
 
       // Find the starting column.
       let startColumn: Int
       if startLine < lineNumber {
         startColumn = 1
-      } else if startLine == lineNumber, let column = startLoc.column {
-        startColumn = column
+      } else if startLine == lineNumber {
+        startColumn = startLoc.column
       } else {
         return nil
       }
 
       // Find the ending column.
       let endLoc = highlight.endLocation(converter: slc, afterTrailingTrivia: false)
-      guard let endLine = endLoc.line else {
-        return nil
-      }
+      let endLine = endLoc.line
 
       let endColumn: Int
       if endLine > lineNumber {
         endColumn = annotatedLine.sourceString.count
-      } else if endLine == lineNumber, let column = endLoc.column {
-        endColumn = column
+      } else if endLine == lineNumber {
+        endColumn = endLoc.column
       } else {
         return nil
       }
@@ -283,9 +279,9 @@ public struct DiagnosticsFormatter {
         annotatedSource.append("\n")
       }
 
-      let columnsWithDiagnostics = Set(annotatedLine.diagnostics.map { $0.location(converter: slc).column ?? 0 })
+      let columnsWithDiagnostics = Set(annotatedLine.diagnostics.map { $0.location(converter: slc).column })
       let diagsPerColumn = Dictionary(grouping: annotatedLine.diagnostics) { diag in
-        diag.location(converter: slc).column ?? 0
+        diag.location(converter: slc).column
       }.sorted { lhs, rhs in
         lhs.key > rhs.key
       }
diff --git a/Sources/SwiftSyntax/SourceLocation.swift b/Sources/SwiftSyntax/SourceLocation.swift
index 58f889e13cf..56c548902f1 100644
--- a/Sources/SwiftSyntax/SourceLocation.swift
+++ b/Sources/SwiftSyntax/SourceLocation.swift
@@ -10,83 +10,32 @@
 //
 //===----------------------------------------------------------------------===//
 
-/// Represent the user-facing part of SourceLocation that can be calculated
-/// on demand.
-struct ComputedLocation: Hashable, Codable, CustomDebugStringConvertible {
-  /// The line in the file where this location resides. 1-based.
-  let line: Int
-
-  /// The UTF-8 byte offset from the beginning of the line where this location
-  /// resides. 1-based.
-  let column: Int
-
-  /// The file in which this location resides.
-  let file: String
-
-  var debugDescription: String {
-    // Print file name?
-    return "\(line):\(column)"
-  }
-
-  init(line: Int, column: Int, file: String) {
-    self.line = line
-    self.column = column
-    self.file = file
-  }
-  init(offset: Int, using converter: SourceLocationConverter) {
-    let loc = converter.location(for: AbsolutePosition(utf8Offset: offset))
-    precondition(loc.offset == offset)
-    self.line = loc.line!
-    self.column = loc.column!
-    self.file = loc.file!
-  }
-}
-
 /// Represents a source location in a Swift file.
 public struct SourceLocation: Hashable, Codable, CustomDebugStringConvertible {
 
-  /// Line and column that can be computed on demand.
-  private var compLoc: ComputedLocation?
-
   /// The UTF-8 byte offset into the file where this location resides.
   public let offset: Int
 
   /// The line in the file where this location resides. 1-based.
-  public var line: Int? {
-    return compLoc?.line
-  }
+  public var line: Int
 
   /// The UTF-8 byte offset from the beginning of the line where this location
   /// resides. 1-based.
-  public var column: Int? {
-    return compLoc?.column
-  }
+  public let column: Int
 
   /// The file in which this location resides.
-  public var file: String? {
-    return compLoc?.file
-  }
+  public let file: String
 
   public var debugDescription: String {
-    guard let compLoc = compLoc else {
-      return "\(offset)"
-    }
-    return compLoc.debugDescription
+    // Print file name?
+    return "\(line):\(column)"
   }
 
   public init(line: Int, column: Int, offset: Int, file: String) {
+    self.line = line
     self.offset = offset
-    self.compLoc = ComputedLocation(line: line, column: column, file: file)
-  }
-
-  /// Initialize SourceLocation with a utf8 offset.
-  /// If a SourceLocationConverter is given, line, column and file will be populated;
-  /// otherwise they will be nil.
-  public init(offset: Int, converter: SourceLocationConverter? = nil) {
-    self.offset = offset
-    if let converter = converter {
-      self.compLoc = ComputedLocation(offset: offset, using: converter)
-    }
+    self.column = column
+    self.file = file
   }
 }
 
diff --git a/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift b/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift
index 6b68eb152a1..a7c84b25f9f 100644
--- a/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift
+++ b/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift
@@ -134,18 +134,14 @@ extension MacroExpansionContext {
     at position: PositionInSyntaxNode,
     filePathMode: SourceLocationFilePathMode
   ) -> AbstractSourceLocation? {
-    guard let sourceLoc: SourceLocation = location(of: node, at: position, filePathMode: filePathMode),
-      let file = sourceLoc.file,
-      let line = sourceLoc.line,
-      let column = sourceLoc.column
-    else {
+    guard let sourceLoc: SourceLocation = location(of: node, at: position, filePathMode: filePathMode) else {
       return nil
     }
 
     return AbstractSourceLocation(
-      file: "\(literal: file)",
-      line: "\(literal: line)",
-      column: "\(literal: column)"
+      file: "\(literal: sourceLoc.file)",
+      line: "\(literal: sourceLoc.line)",
+      column: "\(literal: sourceLoc.column)"
     )
   }
 


Test patch:
diff --git a/Tests/SwiftParserTest/Assertions.swift b/Tests/SwiftParserTest/Assertions.swift
index d55e9505aed..888f61c7386 100644
--- a/Tests/SwiftParserTest/Assertions.swift
+++ b/Tests/SwiftParserTest/Assertions.swift
@@ -336,16 +336,8 @@ func assertLocation<T: SyntaxProtocol>(
     let locationConverter = SourceLocationConverter(file: "", source: tree.description)
     let actualLocation = location
     let expectedLocation = locationConverter.location(for: AbsolutePosition(utf8Offset: markerLoc))
-    if let actualLine = actualLocation.line,
-      let actualColumn = actualLocation.column,
-      let expectedLine = expectedLocation.line,
-      let expectedColumn = expectedLocation.column
-    {
-      if actualLine != expectedLine || actualColumn != expectedColumn {
-        XCTFail("Expected location \(expectedLine):\(expectedColumn) but got \(actualLine):\(actualColumn)", file: file, line: line)
-      }
-    } else {
-      XCTFail("Failed to resolve diagnostic location to line/column", file: file, line: line)
+    if actualLocation.line != expectedLocation.line || actualLocation.column != expectedLocation.column {
+      XCTFail("Expected location \(expectedLocation.line):\(expectedLocation.column) but got \(actualLocation.line):\(actualLocation.column)", file: file, line: line)
     }
   } else {
     XCTFail("Did not find marker \(locationMarker) in the source code", file: file, line: line)
diff --git a/Tests/SwiftSyntaxTest/AbsolutePositionTests.swift b/Tests/SwiftSyntaxTest/AbsolutePositionTests.swift
index 518012d11b2..dbaa22781c5 100644
--- a/Tests/SwiftSyntaxTest/AbsolutePositionTests.swift
+++ b/Tests/SwiftSyntaxTest/AbsolutePositionTests.swift
@@ -155,7 +155,7 @@ public class AbsolutePositionTests: XCTestCase {
     XCTAssertEqual(startLoc.line, 8)
     XCTAssertEqual(startLoc.column, 1)
     XCTAssertEqual(
-      converter.position(ofLine: startLoc.line!, column: startLoc.column!),
+      converter.position(ofLine: startLoc.line, column: startLoc.column),
       secondReturnStmt.positionAfterSkippingLeadingTrivia
     )
 
@@ -167,7 +167,7 @@ public class AbsolutePositionTests: XCTestCase {
     XCTAssertEqual(startLocBeforeTrivia.line, 6)
     XCTAssertEqual(startLocBeforeTrivia.column, 1)
     XCTAssertEqual(
-      converter.position(ofLine: startLocBeforeTrivia.line!, column: startLocBeforeTrivia.column!),
+      converter.position(ofLine: startLocBeforeTrivia.line, column: startLocBeforeTrivia.column),
       secondReturnStmt.position
     )
 
@@ -183,9 +183,9 @@ public class AbsolutePositionTests: XCTestCase {
     XCTAssertEqual(endLocAfterTrivia.line, 11)
     XCTAssertEqual(endLocAfterTrivia.column, 1)
 
-    XCTAssertTrue(converter.isValid(line: startLoc.line!, column: startLoc.column!))
-    XCTAssertFalse(converter.isValid(line: startLoc.line!, column: startLoc.column! + 50))
-    XCTAssertFalse(converter.isValid(line: 0, column: startLoc.column!))
+    XCTAssertTrue(converter.isValid(line: startLoc.line, column: startLoc.column))
+    XCTAssertFalse(converter.isValid(line: startLoc.line, column: startLoc.column + 50))
+    XCTAssertFalse(converter.isValid(line: 0, column: startLoc.column))
     XCTAssertTrue(converter.isValid(position: secondReturnStmt.position))
     XCTAssertFalse(converter.isValid(position: secondReturnStmt.position + SourceLength(utf8Length: 100)))
   }