Refactor selection support in response to feedback (for <#297>).

Make `Selection` and enum so that we don't need to use optionals everywhere. And use `Range<AbsolutePosition>` rather than a custom type (matching recent refactorings in swift-syntax).

Author: DaveEwing

Sources/SwiftFormat/API/Selection.swift

@@ -14,72 +14,63 @@ import Foundation
 import SwiftSyntax
 
 /// The selection as given on the command line - an array of offets and lengths
-public struct Selection {
-  // TODO: make this a typealias instead
-  public struct Range: Decodable {
-    public let offset: AbsolutePosition
-    public let length: Int
-    public var end: AbsolutePosition { offset.advanced(by: length) }
+public enum Selection {
+  case infinite
+  case ranges([Range<AbsolutePosition>])
 
-    public init(from decoder: any Decoder) throws {
-      let container = try decoder.container(keyedBy: CodingKeys.self)
-      offset = AbsolutePosition(utf8Offset: try container.decode(Int.self, forKey: .offset))
-      length = try container.decode(Int.self, forKey: .length)
-    }
-
-    public init(start: AbsolutePosition, end: AbsolutePosition) {
-      offset = start
-      length = end.utf8Offset - start.utf8Offset
-    }
-
-    public init(start: Int, end: Int) {
-      offset = AbsolutePosition(utf8Offset: start)
-      length = end - start
-    }
-
-    enum CodingKeys: CodingKey {
-      case offset
-      case length
+  public init(json: String) {
+    if let data = json.data(using: .utf8),
+       let ranges = try? JSONDecoder().decode([Range<AbsolutePosition>].self, from: data) {
+      self = .ranges(ranges)
+    } else {
+      self = .infinite
     }
   }
 
-  public let ranges: [Range]
-
-  public init?(json: String) {
-    guard let data = json.data(using: .utf8),
-          let ranges = try? JSONDecoder().decode([Range].self, from: data) else {
-      return nil
+  public init(ranges: [Range<AbsolutePosition>]) {
+    if ranges.isEmpty {
+      self = .infinite
+    } else {
+      self = .ranges(ranges)
     }
-    self.ranges = ranges
-  }
-
-  public init(ranges: [Selection.Range]) {
-    self.ranges = ranges
   }
 
   public func contains(_ position: AbsolutePosition) -> Bool {
-    return ranges.contains { return $0.offset <= position && position < $0.end }
+    switch self {
+    case .infinite:
+      return true
+    case .ranges(let ranges):
+      return ranges.contains { $0.contains(position) }
+    }
   }
 
-  public func overlaps(_ range: Range) -> Bool {
-    return ranges.contains {
-      let maxStart = max($0.offset, range.offset)
-      let minEnd = (min($0.end, range.end))
-      return maxStart <= minEnd
+  public func overlapsOrTouches(_ range: Range<AbsolutePosition>) -> Bool {
+    switch self {
+    case .infinite:
+      return true
+    case .ranges(let ranges):
+      return ranges.contains { $0.overlapsOrTouches(range) }
     }
   }
 }
 
 
 public extension Syntax {
-  /// return true if the node is completely inside any range in the selection (or if there's
-  /// no selection)
-  func isInsideSelection(_ selection: Selection?) -> Bool {
-    // no selection means we should process everything
-    guard let selection else { return true }
-
-    return selection.ranges.contains { range in
-      return range.offset <= position && endPosition <= range.end
+  /// return true if the node is _completely_ inside any range in the selection
+  func isInsideSelection(_ selection: Selection) -> Bool {
+    switch selection {
+    case .infinite:
+      return true
+    case .ranges(let ranges):
+      return ranges.contains { return $0.lowerBound <= position && endPosition <= $0.upperBound }
     }
   }
 }
+
+// FIXME: temporary until we convert argument parsing
+extension AbsolutePosition : Decodable {
+  public init(from decoder: any Decoder) throws {
+    var container = try decoder.unkeyedContainer()
+    self.init(utf8Offset: try container.decode(Int.self))
+  }
+}

Sources/SwiftFormat/API/SwiftFormatter.swift

@@ -22,7 +22,7 @@ public final class SwiftFormatter {
   public let configuration: Configuration
 
   /// the ranges of text to format
-  public var selection: Selection? = nil
+  public var selection: Selection = .infinite
 
   /// An optional callback that will be notified with any findings encountered during formatting.
   public let findingConsumer: ((Finding) -> Void)?
@@ -73,7 +73,7 @@ public final class SwiftFormatter {
     try format(
       source: String(contentsOf: url, encoding: .utf8),
       assumingFileURL: url,
-      selection: nil,
+      selection: .infinite,
       to: &outputStream,
       parsingDiagnosticHandler: parsingDiagnosticHandler)
   }
@@ -99,7 +99,7 @@ public final class SwiftFormatter {
   public func format<Output: TextOutputStream>(
     source: String,
     assumingFileURL url: URL?,
-    selection: Selection?,
+    selection: Selection,
     to outputStream: inout Output,
     parsingDiagnosticHandler: ((Diagnostic, SourceLocation) -> Void)? = nil
   ) throws {
@@ -141,7 +141,7 @@ public final class SwiftFormatter {
   /// - Throws: If an unrecoverable error occurs when formatting the code.
   public func format<Output: TextOutputStream>(
     syntax: SourceFileSyntax, source: String, operatorTable: OperatorTable,
-    assumingFileURL url: URL?, selection: Selection?, to outputStream: inout Output
+    assumingFileURL url: URL?, selection: Selection, to outputStream: inout Output
   ) throws {
     let assumedURL = url ?? URL(fileURLWithPath: "source")
     let context = Context(

Sources/SwiftFormat/Core/Context.swift

@@ -40,7 +40,7 @@ public final class Context {
   let configuration: Configuration
 
   /// The optional ranges to process
-  let selection: Selection?
+  let selection: Selection
 
   /// Defines the operators and their precedence relationships that were used during parsing.
   let operatorTable: OperatorTable
@@ -69,7 +69,7 @@ public final class Context {
     operatorTable: OperatorTable,
     findingConsumer: ((Finding) -> Void)?,
     fileURL: URL,
-    selection: Selection? = nil,
+    selection: Selection = .infinite,
     sourceFileSyntax: SourceFileSyntax,
     source: String? = nil,
     ruleNameCache: [ObjectIdentifier: String]

Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift

@@ -34,7 +34,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
   private let config: Configuration
   private let operatorTable: OperatorTable
   private let maxlinelength: Int
-  private let selection: Selection?
+  private let selection: Selection
 
   /// The index of the most recently appended break, or nil when no break has been appended.
   private var lastBreakIndex: Int? = nil
@@ -72,7 +72,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
   /// Tracks whether we last considered ourselves inside the selection
   private var isInsideSelection = true
 
-  init(configuration: Configuration, selection: Selection?, operatorTable: OperatorTable) {
+  init(configuration: Configuration, selection: Selection, operatorTable: OperatorTable) {
     self.config = configuration
     self.selection = selection
     self.operatorTable = operatorTable
@@ -82,7 +82,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
 
   func makeStream(from node: Syntax) -> [Token] {
     // if we have a selection, then we start outside of it
-    if selection != nil {
+    if case .ranges = selection {
       appendToken(.disableFormatting(AbsolutePosition(utf8Offset: 0)))
       isInsideSelection = false
     }
@@ -92,7 +92,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
     self.walk(node)
 
     // Make sure we output any trailing text after the last selection range
-    if selection != nil {
+    if case .ranges = selection {
       appendToken(.enableFormatting(nil))
     }
     defer { tokens = [] }
@@ -2735,9 +2735,9 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
     extractLeadingTrivia(token)
     closeScopeTokens.forEach(appendToken)
 
-    generateEnable(Selection.Range(
-      start: token.positionAfterSkippingLeadingTrivia,
-      end: token.endPositionBeforeTrailingTrivia))
+    generateEnable(
+      token.positionAfterSkippingLeadingTrivia ..< token.endPositionBeforeTrailingTrivia
+    )
 
     if !ignoredTokens.contains(token) {
       // Otherwise, it's just a regular token, so add the text as-is.
@@ -2753,16 +2753,16 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
     return .skipChildren
   }
 
-  func generateEnable(_ range: Selection.Range) {
-    guard let selection else { return }
-    if !isInsideSelection && selection.overlaps(range) {
-      appendToken(.enableFormatting(range.offset))
+  func generateEnable(_ range: Range<AbsolutePosition>) {
+    if case .infinite = selection { return }
+    if !isInsideSelection && selection.overlapsOrTouches(range) {
+      appendToken(.enableFormatting(range.lowerBound))
       isInsideSelection = true
     }
   }
 
   func generateDisable(_ position: AbsolutePosition) {
-    guard let selection else { return }
+    if case .infinite = selection { return }
     if isInsideSelection && !selection.contains(position) {
       appendToken(.disableFormatting(position))
       isInsideSelection = false
@@ -3270,7 +3270,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
       switch piece {
       case .lineComment(let text):
         if index > 0 || isStartOfFile {
-          generateEnable(Selection.Range(start: position, end: position + piece.sourceLength))
+          generateEnable(position ..< position + piece.sourceLength)
           appendToken(.comment(Comment(kind: .line, text: text), wasEndOfLine: false))
           generateDisable(position + piece.sourceLength)
           appendNewlines(.soft)
@@ -3280,7 +3280,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
 
       case .blockComment(let text):
         if index > 0 || isStartOfFile {
-          generateEnable(Selection.Range(start: position, end: position + piece.sourceLength))
+          generateEnable(position ..< position + piece.sourceLength)
           appendToken(.comment(Comment(kind: .block, text: text), wasEndOfLine: false))
           generateDisable(position + piece.sourceLength)
           // There is always a break after the comment to allow a discretionary newline after it.
@@ -3297,15 +3297,15 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
         requiresNextNewline = false
 
       case .docLineComment(let text):
-        generateEnable(Selection.Range(start: position, end: position + piece.sourceLength))
+        generateEnable(position ..< position + piece.sourceLength)
         appendToken(.comment(Comment(kind: .docLine, text: text), wasEndOfLine: false))
         generateDisable(position + piece.sourceLength)
         appendNewlines(.soft)
         isStartOfFile = false
         requiresNextNewline = true
 
       case .docBlockComment(let text):
-        generateEnable(Selection.Range(start: position, end: position + piece.sourceLength))
+        generateEnable(position ..< position + piece.sourceLength)
         appendToken(.comment(Comment(kind: .docBlock, text: text), wasEndOfLine: false))
         generateDisable(position + piece.sourceLength)
         appendNewlines(.soft)
@@ -4049,7 +4049,7 @@ extension Syntax {
   /// Creates a pretty-printable token stream for the provided Syntax node.
   func makeTokenStream(
     configuration: Configuration,
-    selection: Selection?,
+    selection: Selection,
     operatorTable: OperatorTable
   ) -> [Token] {
     let commentsMoved = CommentMovingRewriter(selection: selection).rewrite(self)
@@ -4067,11 +4067,11 @@ extension Syntax {
 /// For example, comments after binary operators are relocated to be before the operator, which
 /// results in fewer line breaks with the comment closer to the relevant tokens.
 class CommentMovingRewriter: SyntaxRewriter {
-  init(selection: Selection? = nil) {
+  init(selection: Selection = .infinite) {
     self.selection = selection
   }
 
-  var selection: Selection?
+  var selection: Selection
 
   override func visit(_ node: SourceFileSyntax) -> SourceFileSyntax {
     if shouldFormatterIgnore(file: node) {

Sources/_SwiftFormatTestSupport/DiagnosingTestCase.swift

@@ -15,7 +15,7 @@ open class DiagnosingTestCase: XCTestCase {
   public func makeContext(
     sourceFileSyntax: SourceFileSyntax,
     configuration: Configuration? = nil,
-    selection: Selection?,
+    selection: Selection,
     findingConsumer: @escaping (Finding) -> Void
   ) -> Context {
     let context = Context(

Sources/_SwiftFormatTestSupport/MarkedText.swift

@@ -22,23 +22,23 @@ public struct MarkedText {
   public let textWithoutMarkers: String
 
   /// If the marked text contains "⏩" and "⏪", they're used to create a selection
-  public var selection: Selection?
+  public var selection: Selection
 
   /// Creates a new `MarkedText` value by extracting emoji markers from the given text.
   public init(textWithMarkers markedText: String) {
     var text = ""
     var markers = [String: Int]()
     var lastIndex = markedText.startIndex
-    var ranges = [Selection.Range]()
-    var lastRangeStart = 0
+    var ranges = [Range<AbsolutePosition>]()
+    var lastRangeStart = AbsolutePosition(utf8Offset: 0)
     for marker in findMarkedRanges(in: markedText) {
       text += markedText[lastIndex..<marker.range.lowerBound]
       lastIndex = marker.range.upperBound
 
       if marker.name == "⏩" {
-        lastRangeStart = text.utf8.count
+        lastRangeStart = AbsolutePosition(utf8Offset: text.utf8.count)
       } else if marker.name == "⏪" {
-        ranges.append(Selection.Range(start: lastRangeStart, end: text.utf8.count))
+        ranges.append(lastRangeStart ..< AbsolutePosition(utf8Offset: text.utf8.count))
       } else {
         assert(markers[marker.name] == nil, "Marker names must be unique")
         markers[marker.name] = text.utf8.count
@@ -49,9 +49,7 @@ public struct MarkedText {
 
     self.markers = markers
     self.textWithoutMarkers = text
-    if !ranges.isEmpty {
-      selection = Selection(ranges: ranges)
-    }
+    self.selection = Selection(ranges: ranges)
   }
 }
 

Sources/swift-format/Frontend/Frontend.swift

@@ -36,7 +36,7 @@ class Frontend {
     let configuration: Configuration
 
     /// the selected ranges to process
-    let selection: Selection?
+    let selection: Selection
 
     /// Returns the string contents of the file.
     ///
@@ -48,7 +48,12 @@ class Frontend {
       return String(data: sourceData, encoding: .utf8)
     }()
 
-    init(fileHandle: FileHandle, url: URL, configuration: Configuration, selection: Selection? = nil) {
+    init(
+      fileHandle: FileHandle,
+      url: URL,
+      configuration: Configuration,
+      selection: Selection = .infinite
+    ) {
       self.fileHandle = fileHandle
       self.url = url
       self.configuration = configuration
@@ -120,7 +125,7 @@ class Frontend {
       return
     }
 
-    var selection: Selection?
+    var selection: Selection = .infinite
     if let selectionString = lintFormatOptions.selection {
       // TODO: do this when parsing args to catch errors?
       selection = Selection(json: selectionString)
@@ -173,7 +178,7 @@ class Frontend {
       return nil
     }
 
-    var selection: Selection?
+    var selection: Selection = .infinite
     if let selectionString = lintFormatOptions.selection {
       // TODO: do this when parsing args to catch errors?
       selection = Selection(json: selectionString)

Tests/SwiftFormatPerformanceTests/WhitespaceLinterPerformanceTests.swift

@@ -58,7 +58,7 @@ final class WhitespaceLinterPerformanceTests: DiagnosingTestCase {
   ///   - expected: The formatted text.
   private func performWhitespaceLint(input: String, expected: String) {
     let sourceFileSyntax = Parser.parse(source: input)
-    let context = makeContext(sourceFileSyntax: sourceFileSyntax, selection: nil,
+    let context = makeContext(sourceFileSyntax: sourceFileSyntax, selection: .infinite,
       findingConsumer: { _ in })
     let linter = WhitespaceLinter(user: input, formatted: expected, context: context)
     linter.lint()