Refactor tests.

This PR significantly improves the testing infrastructure by adopting
the emoji markers used by SwiftSyntax's parser tests. This makes it
easier to read and write tests, because the markers indicate in the
string where the finding should be emitted and we don't have to compute
line/column numbers manually.

Additional testing improvements include:

- Tests are no longer stateful w.r.t. collecting findings, so a single
  test can safely call its `assert*` helpers multiple times.
- Tests verify the actual text of the findings, not just their symbolic
  values/constructors. Previously, we weren't actually verifying that
  the *messages* were what we wanted them to be, and this has revealed
  a couple minor issues.
- I've dropped the `XCT*` prefix on our own custom assertions, because
  we shouldn't be using their namespace.

I've added a handful of FIXME comments for things I caught during the
migration that need to be addressed.

Author: allevato

Sources/SwiftFormat/Core/Context.swift

@@ -53,7 +53,7 @@ public final class Context {
   public var importsXCTest: XCTestImportState
 
   /// An object that converts `AbsolutePosition` values to `SourceLocation` values.
-  let sourceLocationConverter: SourceLocationConverter
+  public let sourceLocationConverter: SourceLocationConverter
 
   /// Contains the rules have been disabled by comments for certain line numbers.
   let ruleMask: RuleMask

Sources/_SwiftFormatTestSupport/DiagnosingTestCase.swift

@@ -7,100 +7,183 @@ import XCTest
 /// DiagnosingTestCase is an XCTestCase subclass meant to inject diagnostic-specific testing
 /// routines into specific formatting test cases.
 open class DiagnosingTestCase: XCTestCase {
-  /// Set during lint tests to indicate that we should check for unasserted diagnostics when the
-  /// test is torn down and fail if there were any.
-  public var shouldCheckForUnassertedDiagnostics = false
-
-  /// A helper that will keep track of the findings that were emitted.
-  private var consumer = TestingFindingConsumer()
-
-  override open func setUp() {
-    shouldCheckForUnassertedDiagnostics = false
-  }
-
-  override open func tearDown() {
-    guard shouldCheckForUnassertedDiagnostics else { return }
-
-    // This will emit a test failure if a diagnostic is thrown but we don't explicitly call
-    // XCTAssertDiagnosed for it.
-    for finding in consumer.emittedFindings {
-      XCTFail("unexpected finding '\(finding)' emitted")
-    }
-  }
-
   /// Creates and returns a new `Context` from the given syntax tree and configuration.
   ///
-  /// The returned context is configured with a diagnostic consumer that records diagnostics emitted
-  /// during the tests, which can then be asserted using the `XCTAssertDiagnosed` and
-  /// `XCTAssertNotDiagnosed` methods.
+  /// The returned context is configured with the given finding consumer to record findings emitted
+  /// during the tests, so that they can be asserted later using the `assertFindings` method.
   @_spi(Testing)
-  public func makeContext(sourceFileSyntax: SourceFileSyntax, configuration: Configuration? = nil)
-    -> Context
-  {
-    consumer = TestingFindingConsumer()
+  public func makeContext(
+    sourceFileSyntax: SourceFileSyntax,
+    configuration: Configuration? = nil,
+    findingConsumer: @escaping (Finding) -> Void
+  ) -> Context {
     let context = Context(
       configuration: configuration ?? Configuration(),
       operatorTable: .standardOperators,
-      findingConsumer: consumer.consume,
+      findingConsumer: findingConsumer,
       fileURL: URL(fileURLWithPath: "/tmp/test.swift"),
       sourceFileSyntax: sourceFileSyntax,
       ruleNameCache: ruleNameCache)
     return context
   }
 
-  /// Stops tracking diagnostics emitted during formatting/linting.
-  ///
-  /// This used by the pretty-printer tests to suppress any diagnostics that might be emitted during
-  /// the second format pass (which checks for idempotence).
-  public func stopTrackingDiagnostics() {
-    consumer.stopTrackingFindings()
+  /// Asserts that the given list of findings matches a set of specs.
+  @_spi(Testing)
+  public final func assertFindings(
+    expected specs: [FindingSpec],
+    markerLocations: [String: Int],
+    emittedFindings: [Finding],
+    context: Context,
+    file: StaticString = #file,
+    line: UInt = #line
+  ) {
+    var emittedFindings = emittedFindings
+
+    // Check for a finding that matches each spec, removing it from the array if found.
+    for spec in specs {
+      assertAndRemoveFinding(
+        findingSpec: spec,
+        markerLocations: markerLocations,
+        emittedFindings: &emittedFindings,
+        context: context,
+        file: file,
+        line: line)
+    }
+
+    // Emit test failures for any findings that did not have matches.
+    for finding in emittedFindings {
+      let locationString: String
+      if let location = finding.location {
+        locationString = "line:col \(location.line):\(location.column)"
+      } else {
+        locationString = "no location provided"
+      }
+      XCTFail(
+        "Unexpected finding '\(finding.message)' was emitted (\(locationString))",
+        file: file,
+        line: line)
+    }
   }
 
-  /// Asserts that a specific diagnostic message was emitted.
-  ///
-  /// - Parameters:
-  ///   - message: The diagnostic message expected to be emitted.
-  ///   - file: The file in which failure occurred. Defaults to the file name of the test case in
-  ///     which this function was called.
-  ///   - line: The line number on which failure occurred. Defaults to the line number on which this
-  ///     function was called.
-  public final func XCTAssertDiagnosed(
-    _ message: Finding.Message,
-    line diagnosticLine: Int? = nil,
-    column diagnosticColumn: Int? = nil,
+  private func assertAndRemoveFinding(
+    findingSpec: FindingSpec,
+    markerLocations: [String: Int],
+    emittedFindings: inout [Finding],
+    context: Context,
     file: StaticString = #file,
     line: UInt = #line
   ) {
-    let wasEmitted: Bool
-    if let diagnosticLine = diagnosticLine, let diagnosticColumn = diagnosticColumn {
-      wasEmitted = consumer.popFinding(
-        containing: message.text, atLine: diagnosticLine, column: diagnosticColumn)
-    } else {
-      wasEmitted = consumer.popFinding(containing: message.text)
+    guard let utf8Offset = markerLocations[findingSpec.marker] else {
+      XCTFail("Marker '\(findingSpec.marker)' was not found in the input", file: file, line: line)
+      return
+    }
+
+    let markerLocation =
+      context.sourceLocationConverter.location(for: AbsolutePosition(utf8Offset: utf8Offset))
+
+    // Find a finding that has the expected line/column location, ignoring the text.
+    // FIXME: We do this to provide a better error message if the finding is in the right place but
+    // doesn't have the right message, but this also introduces an order-sensitivity among the
+    // specs. Fix this if it becomes an issue.
+    let maybeIndex = emittedFindings.firstIndex {
+      markerLocation.line == $0.location?.line && markerLocation.column == $0.location?.column
+    }
+    guard let index = maybeIndex else {
+      XCTFail(
+        """
+        Finding '\(findingSpec.message)' was not emitted at marker '\(findingSpec.marker)' \
+        (line:col \(markerLocation.line):\(markerLocation.column), offset \(utf8Offset))
+        """,
+        file: file,
+        line: line)
+      return
     }
-    if !wasEmitted {
-      XCTFail("diagnostic '\(message.text)' not emitted", file: file, line: line)
+
+    // Verify that the finding text also matches what we expect.
+    let matchedFinding = emittedFindings.remove(at: index)
+    XCTAssertEqual(
+      matchedFinding.message.text,
+      findingSpec.message,
+      """
+      Finding emitted at marker '\(findingSpec.marker)' \
+      (line:col \(markerLocation.line):\(markerLocation.column), offset \(utf8Offset)) \
+      had the wrong message
+      """,
+      file: file,
+      line: line)
+
+    // Assert that a note exists for each of the expected nodes in the finding.
+    var emittedNotes = matchedFinding.notes
+    for noteSpec in findingSpec.notes {
+      assertAndRemoveNote(
+        noteSpec: noteSpec,
+        markerLocations: markerLocations,
+        emittedNotes: &emittedNotes,
+        context: context,
+        file: file,
+        line: line)
+    }
+
+    // Emit test failures for any notes that weren't specified.
+    for note in emittedNotes {
+      let locationString: String
+      if let location = note.location {
+        locationString = "line:col \(location.line):\(location.column)"
+      } else {
+        locationString = "no location provided"
+      }
+      XCTFail(
+        "Unexpected note '\(note.message)' was emitted (\(locationString))",
+        file: file,
+        line: line)
     }
   }
 
-  /// Asserts that a specific diagnostic message was not emitted.
-  ///
-  /// - Parameters:
-  ///   - message: The diagnostic message expected to not be emitted.
-  ///   - file: The file in which failure occurred. Defaults to the file name of the test case in
-  ///     which this function was called.
-  ///   - line: The line number on which failure occurred. Defaults to the line number on which this
-  ///     function was called.
-  public final func XCTAssertNotDiagnosed(
-    _ message: Finding.Message,
+  private func assertAndRemoveNote(
+    noteSpec: NoteSpec,
+    markerLocations: [String: Int],
+    emittedNotes: inout [Finding.Note],
+    context: Context,
     file: StaticString = #file,
     line: UInt = #line
   ) {
-    let wasEmitted = consumer.popFinding(containing: message.text)
-    XCTAssertFalse(
-      wasEmitted,
-      "diagnostic '\(message.text)' should not have been emitted",
-      file: file, line: line)
+    guard let utf8Offset = markerLocations[noteSpec.marker] else {
+      XCTFail("Marker '\(noteSpec.marker)' was not found in the input", file: file, line: line)
+      return
+    }
+
+    let markerLocation =
+      context.sourceLocationConverter.location(for: AbsolutePosition(utf8Offset: utf8Offset))
+
+    // FIXME: We do this to provide a better error message if the note is in the right place but
+    // doesn't have the right message, but this also introduces an order-sensitivity among the
+    // specs. Fix this if it becomes an issue.
+    let maybeIndex = emittedNotes.firstIndex {
+      markerLocation.line == $0.location?.line && markerLocation.column == $0.location?.column
+    }
+    guard let index = maybeIndex else {
+      XCTFail(
+        """
+        Note '\(noteSpec.message)' was not emitted at marker '\(noteSpec.marker)' \
+        (line:col \(markerLocation.line):\(markerLocation.column), offset \(utf8Offset))
+        """,
+        file: file,
+        line: line)
+      return
+    }
+
+    // Verify that the note text also matches what we expect.
+    let matchedNote = emittedNotes.remove(at: index)
+    XCTAssertEqual(
+      matchedNote.message.text,
+      noteSpec.message,
+      """
+      Note emitted at marker '\(noteSpec.marker)' \
+      (line:col \(markerLocation.line):\(markerLocation.column), offset \(utf8Offset)) \
+      had the wrong message
+      """,
+      file: file,
+      line: line)
   }
 
   /// Asserts that the two strings are equal, providing Unix `diff`-style output if they are not.
@@ -113,7 +196,7 @@ open class DiagnosingTestCase: XCTestCase {
   ///     which this function was called.
   ///   - line: The line number on which failure occurred. Defaults to the line number on which this
   ///     function was called.
-  public final func XCTAssertStringsEqualWithDiff(
+  public final func assertStringsEqualWithDiff(
     _ actual: String,
     _ expected: String,
     _ message: String = "",

Sources/_SwiftFormatTestSupport/FindingSpec.swift

@@ -0,0 +1,45 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+/// A description of a `Finding` that can be asserted during tests.
+public struct FindingSpec {
+  /// The marker that identifies the finding.
+  public var marker: String
+
+  /// The message text associated with the finding.
+  public var message: String
+
+  /// A description of a `Note` that should be associated with this finding.
+  public var notes: [NoteSpec]
+
+  /// Creates a new `FindingSpec` with the given values.
+  public init(_ marker: String = "1️⃣", message: String, notes: [NoteSpec] = []) {
+    self.marker = marker
+    self.message = message
+    self.notes = notes
+  }
+}
+
+/// A description of a `Note` that can be asserted during tests.
+public struct NoteSpec {
+  /// The marker that identifies the note.
+  public var marker: String
+
+  /// The message text associated with the note.
+  public var message: String
+
+  /// Creates a new `NoteSpec` with the given values.
+  public init(_ marker: String, message: String) {
+    self.marker = marker
+    self.message = message
+  }
+}

Sources/_SwiftFormatTestSupport/MarkedText.swift

@@ -0,0 +1,80 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+/// Encapsulates the locations of emoji markers extracted from source text.
+public struct MarkedText {
+  /// A mapping from marker names to the UTF-8 offset where the marker was found in the string.
+  public let markers: [String: Int]
+
+  /// The text with all markers removed.
+  public let textWithoutMarkers: String
+
+  /// 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
+    for marker in findMarkedRanges(in: markedText) {
+      text += markedText[lastIndex..<marker.range.lowerBound]
+      lastIndex = marker.range.upperBound
+
+      assert(markers[marker.name] == nil, "Marker names must be unique")
+      markers[marker.name] = text.utf8.count
+    }
+
+    text += markedText[lastIndex..<markedText.endIndex]
+
+    self.markers = markers
+    self.textWithoutMarkers = text
+  }
+}
+
+private struct Marker {
+  /// The name (i.e., emoji identifier) of the marker.
+  var name: String
+
+  /// The range of the marker.
+  ///
+  /// If the marker contains all the non-whitespace characters on the line, then this is the range
+  /// of the entire line. Otherwise, it's the range of the marker itself.
+  var range: Range<String.Index>
+}
+
+private func findMarkedRanges(in text: String) -> [Marker] {
+  var markers = [Marker]()
+  while let marker = nextMarkedRange(in: text, from: markers.last?.range.upperBound ?? text.startIndex) {
+    markers.append(marker)
+  }
+  return markers
+}
+
+private func nextMarkedRange(in text: String, from index: String.Index) -> Marker? {
+  guard let start = text[index...].firstIndex(where: { $0.isMarkerEmoji }) else {
+    return nil
+  }
+
+  let end = text.index(after: start)
+  let markerRange = start..<end
+  let name = String(text[start..<end])
+  return Marker(name: name, range: markerRange)
+}
+
+extension Character {
+  /// A value indicating whether or not the character is an emoji that we recognize as a source
+  /// location marker.
+  fileprivate var isMarkerEmoji: Bool {
+    switch self {
+    case "0️⃣", "1️⃣", "2️⃣", "3️⃣", "4️⃣", "5️⃣", "6️⃣", "7️⃣", "8️⃣", "9️⃣", "🔟", "ℹ️": return true
+    default: return false
+    }
+  }
+}