Some small refectorings and updates from review feedback. Add a few more test cases. For formatting a selection (<#297>).

Author: DaveEwing

Sources/SwiftFormat/API/Selection.swift

@@ -19,11 +19,11 @@ public enum Selection {
   case ranges([Range<AbsolutePosition>])
 
   /// Create a selection from an array of utf8 ranges. An empty array means an infinite selection.
-  public init(offsetPairs: [Range<Int>]) {
-    if offsetPairs.isEmpty {
+  public init(offsetRanges: [Range<Int>]) {
+    if offsetRanges.isEmpty {
       self = .infinite
     } else {
-      let ranges = offsetPairs.map {
+      let ranges = offsetRanges.map {
         AbsolutePosition(utf8Offset: $0.lowerBound) ..< AbsolutePosition(utf8Offset: $0.upperBound)
       }
       self = .ranges(ranges)
@@ -51,7 +51,7 @@ public enum Selection {
 
 
 public extension Syntax {
-  /// return true if the node is _completely_ inside any range in the selection
+  /// - Returns: `true` if the node is _completely_ inside any range in the selection
   func isInsideSelection(_ selection: Selection) -> Bool {
     switch selection {
     case .infinite:

Sources/SwiftFormat/API/SwiftFormatter.swift

@@ -21,9 +21,6 @@ public final class SwiftFormatter {
   /// The configuration settings that control the formatter's behavior.
   public let configuration: Configuration
 
-  /// the ranges of text to format
-  public var selection: Selection = .infinite
-
   /// An optional callback that will be notified with any findings encountered during formatting.
   public let findingConsumer: ((Finding) -> Void)?
 

Sources/SwiftFormat/CMakeLists.txt

@@ -14,6 +14,7 @@ add_library(SwiftFormat
   API/Finding.swift
   API/FindingCategorizing.swift
   API/Indent.swift
+  API/Selection.swift
   API/SwiftFormatError.swift
   API/SwiftFormatter.swift
   API/SwiftLinter.swift

Sources/SwiftFormat/Core/Context.swift

@@ -39,7 +39,7 @@ public final class Context {
   /// The configuration for this run of the pipeline, provided by a configuration JSON file.
   let configuration: Configuration
 
-  /// The optional ranges to process
+  /// The selection to process
   let selection: Selection
 
   /// Defines the operators and their precedence relationships that were used during parsing.
@@ -92,7 +92,7 @@ public final class Context {
 
   /// Given a rule's name and the node it is examining, determine if the rule is disabled at this
   /// location or not. Also makes sure the entire node is contained inside any selection.
-  func isRuleEnabled<R: Rule>(_ rule: R.Type, node: Syntax) -> Bool {
+  func shouldFormat<R: Rule>(_ rule: R.Type, node: Syntax) -> Bool {
     guard node.isInsideSelection(selection) else { return false }
 
     let loc = node.startLocation(converter: self.sourceLocationConverter)

Sources/SwiftFormat/Core/LintPipeline.swift

@@ -28,7 +28,7 @@ extension LintPipeline {
   func visitIfEnabled<Rule: SyntaxLintRule, Node: SyntaxProtocol>(
     _ visitor: (Rule) -> (Node) -> SyntaxVisitorContinueKind, for node: Node
   ) {
-    guard context.isRuleEnabled(Rule.self, node: Syntax(node)) else { return }
+    guard context.shouldFormat(Rule.self, node: Syntax(node)) else { return }
     let ruleId = ObjectIdentifier(Rule.self)
     guard self.shouldSkipChildren[ruleId] == nil else { return }
     let rule = self.rule(Rule.self)
@@ -54,7 +54,7 @@ extension LintPipeline {
     // more importantly because the `visit` methods return protocol refinements of `Syntax` that
     // cannot currently be expressed as constraints without duplicating this function for each of
     // them individually.
-    guard context.isRuleEnabled(Rule.self, node: Syntax(node)) else { return }
+    guard context.shouldFormat(Rule.self, node: Syntax(node)) else { return }
     guard self.shouldSkipChildren[ObjectIdentifier(Rule.self)] == nil else { return }
     let rule = self.rule(Rule.self)
     _ = visitor(rule)(node)

Sources/SwiftFormat/Core/SyntaxFormatRule.swift

@@ -32,7 +32,7 @@ public class SyntaxFormatRule: SyntaxRewriter, Rule {
   public override func visitAny(_ node: Syntax) -> Syntax? {
     // If the rule is not enabled, then return the node unmodified; otherwise, returning nil tells
     // SwiftSyntax to continue with the standard dispatch.
-    guard context.isRuleEnabled(type(of: self), node: node) else { return node }
+    guard context.shouldFormat(type(of: self), node: node) else { return node }
     return nil
   }
 }

Sources/SwiftFormat/PrettyPrint/PrettyPrint.swift

@@ -69,16 +69,14 @@ public class PrettyPrinter {
   private var tokens: [Token]
   private var source: String
 
-  /// keep track of where formatting was disabled in the original source
+  /// Keep track of where formatting was disabled in the original source
   ///
   /// To format a selection, we insert `enableFormatting`/`disableFormatting` tokens into the
   /// stream when entering/exiting a selection range. Those tokens include utf8 offsets into the
   /// original source. When enabling formatting, we copy the text between `disabledPosition` and the
   /// current position to `outputBuffer`. From then on, we continue to format until the next
   /// `disableFormatting` token.
   private var disabledPosition: AbsolutePosition? = nil
-  /// true if we're currently formatting
-  private var writingIsEnabled: Bool { disabledPosition == nil }
 
   private var outputBuffer: String = ""
 
@@ -204,7 +202,9 @@ public class PrettyPrinter {
   ///
   /// No further processing is performed on the string.
   private func writeRaw<S: StringProtocol>(_ str: S) {
-    outputBuffer.append(String(str))
+    if disabledPosition == nil {
+      outputBuffer.append(String(str))
+    }
   }
 
   /// Writes newlines into the output stream, taking into account any preexisting consecutive
@@ -233,9 +233,7 @@ public class PrettyPrinter {
     }
 
     guard numberToPrint > 0 else { return }
-    if writingIsEnabled {
-      writeRaw(String(repeating: "\n", count: numberToPrint))
-    }
+    writeRaw(String(repeating: "\n", count: numberToPrint))
     lineNumber += numberToPrint
     isAtStartOfLine = true
     consecutiveNewlineCount += numberToPrint
@@ -257,17 +255,13 @@ public class PrettyPrinter {
   /// leading spaces that are required before the text itself.
   private func write(_ text: String) {
     if isAtStartOfLine {
-      if writingIsEnabled {
-        writeRaw(currentIndentation.indentation())
-      }
+      writeRaw(currentIndentation.indentation())
       spaceRemaining = maxLineLength - currentIndentation.length(in: configuration)
       isAtStartOfLine = false
-    } else if pendingSpaces > 0 && writingIsEnabled {
+    } else if pendingSpaces > 0  {
       writeRaw(String(repeating: " ", count: pendingSpaces))
     }
-    if writingIsEnabled {
-      writeRaw(text)
-    }
+    writeRaw(text)
     consecutiveNewlineCount = 0
     pendingSpaces = 0
   }
@@ -546,9 +540,7 @@ public class PrettyPrinter {
       }
 
     case .verbatim(let verbatim):
-      if writingIsEnabled {
-        writeRaw(verbatim.print(indent: currentIndentation))
-      }
+      writeRaw(verbatim.print(indent: currentIndentation))
       consecutiveNewlineCount = 0
       pendingSpaces = 0
       lastBreak = false
@@ -596,38 +588,37 @@ public class PrettyPrinter {
       }
 
     case .enableFormatting(let enabledPosition):
-      // if we're not disabled, we ignore the token
-      if let disabledPosition {
-        let start = source.utf8.index(source.utf8.startIndex, offsetBy: disabledPosition.utf8Offset)
-        let end: String.Index
-        if let enabledPosition {
-          end = source.utf8.index(source.utf8.startIndex, offsetBy: enabledPosition.utf8Offset)
-        } else {
-          end = source.endIndex
-        }
-        var text = String(source[start..<end])
-        // strip trailing whitespace so that the next formatting can add the right amount
-        if let nonWhitespace = text.rangeOfCharacter(
-          from: CharacterSet.whitespaces.inverted, options: .backwards) {
-          text = String(text[..<nonWhitespace.upperBound])
-        }
+      guard let disabledPosition else {
+        // if we're not disabled, we ignore the token
+        break
+      }
+      let start = source.utf8.index(source.utf8.startIndex, offsetBy: disabledPosition.utf8Offset)
+      let end: String.Index
+      if let enabledPosition {
+        end = source.utf8.index(source.utf8.startIndex, offsetBy: enabledPosition.utf8Offset)
+      } else {
+        end = source.endIndex
+      }
+      var text = String(source[start..<end])
+      // strip trailing whitespace so that the next formatting can add the right amount
+      if let nonWhitespace = text.rangeOfCharacter(
+        from: CharacterSet.whitespaces.inverted, options: .backwards) {
+        text = String(text[..<nonWhitespace.upperBound])
+      }
 
-        writeRaw(text)
-        if text.hasSuffix("\n") {
-          isAtStartOfLine = true
-          consecutiveNewlineCount = 1
-        } else {
-          isAtStartOfLine = false
-          consecutiveNewlineCount = 0
-        }
-        self.disabledPosition = nil
+      self.disabledPosition = nil
+      writeRaw(text)
+      if text.hasSuffix("\n") {
+        isAtStartOfLine = true
+        consecutiveNewlineCount = 1
+      } else {
+        isAtStartOfLine = false
+        consecutiveNewlineCount = 0
       }
 
     case .disableFormatting(let newPosition):
-      // a second disable is ignored
-      if writingIsEnabled {
-        disabledPosition = newPosition
-      }
+      assert(disabledPosition == nil)
+      disabledPosition = newPosition
     }
   }
 

Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift

@@ -4071,7 +4071,7 @@ class CommentMovingRewriter: SyntaxRewriter {
     self.selection = selection
   }
 
-  var selection: Selection
+  private let selection: Selection
 
   override func visit(_ node: SourceFileSyntax) -> SourceFileSyntax {
     if shouldFormatterIgnore(file: node) {

Sources/SwiftFormat/Rules/OrderedImports.swift

@@ -310,7 +310,7 @@ fileprivate func generateLines(codeBlockItemList: CodeBlockItemListSyntax, conte
       if currentLine.syntaxNode != nil {
         appendNewLine()
       }
-      let sortable = context.isRuleEnabled(OrderedImports.self, node: Syntax(block))
+      let sortable = context.shouldFormat(OrderedImports.self, node: Syntax(block))
       var blockWithoutTrailingTrivia = block
       blockWithoutTrailingTrivia.trailingTrivia = []
       currentLine.syntaxNode = .importCodeBlock(blockWithoutTrailingTrivia, sortable: sortable)

Sources/_SwiftFormatTestSupport/MarkedText.swift

@@ -49,7 +49,7 @@ public struct MarkedText {
 
     self.markers = markers
     self.textWithoutMarkers = text
-    self.selection = Selection(offsetPairs: offsets)
+    self.selection = Selection(offsetRanges: offsets)
   }
 }
 

Sources/swift-format/Frontend/Frontend.swift

@@ -127,7 +127,7 @@ class Frontend {
 
     var selection: Selection = .infinite
     if let offsets = lintFormatOptions.offsets {
-      selection = Selection(offsetPairs: offsets)
+      selection = Selection(offsetRanges: offsets)
     }
     let fileToProcess = FileToProcess(
       fileHandle: FileHandle.standardInput,
@@ -178,7 +178,7 @@ class Frontend {
 
     var selection: Selection = .infinite
     if let offsets = lintFormatOptions.offsets {
-      selection = Selection(offsetPairs: offsets)
+      selection = Selection(offsetRanges: offsets)
     }
     return FileToProcess(
       fileHandle: sourceFile,

Sources/swift-format/Subcommands/LintFormatOptions.swift

@@ -125,7 +125,7 @@ struct LintFormatOptions: ParsableArguments {
   }
 }
 
-extension [Range<Int>] : @retroactive ExpressibleByArgument {
+extension [Range<Int>] {
   public init?(argument: String) {
     let pairs = argument.components(separatedBy: ",")
     let ranges: [Range<Int>] = pairs.compactMap {
@@ -139,3 +139,9 @@ extension [Range<Int>] : @retroactive ExpressibleByArgument {
     self = ranges
   }
 }
+
+#if compiler(>=6)
+extension [Range<Int>] : @retroactive ExpressibleByArgument {}
+#else
+extension [Range<Int>] : ExpressibleByArgument {}
+#endif

Tests/SwiftFormatTests/PrettyPrint/SelectionTests.swift

@@ -21,7 +21,6 @@ final class SelectionTests: PrettyPrintTestCase {
       }
       """
 
-    // The line length ends on the last paren of .Stuff()
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
   }
 
@@ -44,7 +43,6 @@ final class SelectionTests: PrettyPrintTestCase {
       }
       """
 
-    // The line length ends on the last paren of .Stuff()
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
   }
 
@@ -67,7 +65,6 @@ final class SelectionTests: PrettyPrintTestCase {
       }
       """
 
-    // The line length ends on the last paren of .Stuff()
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
   }
 
@@ -90,7 +87,6 @@ final class SelectionTests: PrettyPrintTestCase {
       }
       """
 
-    // The line length ends on the last paren of .Stuff()
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
   }
 
@@ -113,7 +109,6 @@ final class SelectionTests: PrettyPrintTestCase {
       }
       """
 
-    // The line length ends on the last paren of .Stuff()
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
   }
 
@@ -164,7 +159,6 @@ final class SelectionTests: PrettyPrintTestCase {
       }
       """
 
-    // The line length ends on the last paren of .Stuff()
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
   }
 
@@ -191,7 +185,50 @@ final class SelectionTests: PrettyPrintTestCase {
       }
       """
 
-    // The line length ends on the last paren of .Stuff()
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
+  }
+
+  func testSingleLineFunc() {
+    let input =
+      """
+      func foo()   ⏩{}⏪
+      """
+
+    let expected =
+      """
+      func foo() {}
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
+  }
+
+  func testSingleLineFunc2() {
+    let input =
+      """
+      func foo() /**/ ⏩{}⏪
+      """
+
+    let expected =
+      """
+      func foo() /**/ {}
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
+  }
+
+  func testSimpleFunc() {
+    let input =
+      """
+      func foo() /**/
+        ⏩{}⏪
+      """
+
+    let expected =
+      """
+      func foo() /**/
+      {}
+      """
+
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
   }
 
@@ -219,7 +256,6 @@ final class SelectionTests: PrettyPrintTestCase {
       }
       """
 
-    // The line length ends on the last paren of .Stuff()
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
   }