pointfreeco / swift-snapshot-testing

📸 Delightful Swift snapshot testing.
https://www.pointfree.co/episodes/ep41-a-tour-of-snapshot-testing
MIT License
3.68k stars 556 forks source link

Exclude newlines from indent string in SnapshotRewriter. #811

Closed scogeo closed 4 months ago

scogeo commented 7 months ago

This PR fixes an issue that was first raised in pointfreeco/swift-macro-testing#11. I mysteriously hit this same issue of extra newlines in the generated expansion while editing a macro test source file that was previously working. It was a real head scratcher, but I was able to track it down.

The underlying cause is the algorithm to determine the default indentation level in SnapshotRewriter. The code looks for the first use of leading whitespace in the source file, and then uses that to determine the indention size. However, it didn't account for the case where the first use of leading whitespace, may be an empty "indented line" followed by a newline, with no non-whitespace characters.

A trivial example would be:

import Foundation
import InlineSnapshotTesting
import SnapshotTesting
import XCTest

final class WhitespaceTests: XCTestCase {

  func testInlineSnapshot() {
    assertInlineSnapshot(of: ["Hello", "World"], as: .dump)
  }
}

In the above example there are two spaces after the class declaration and before the function declaration which causes the indention string to be calculated as " \n" instead of the expected " ", resulting in extra newlines being introduced in the generated output. When run the following is produced:

import Foundation
import InlineSnapshotTesting
import SnapshotTesting
import XCTest

final class WhitespaceTests: XCTestCase {

  func testInlineSnapshot() {
    assertInlineSnapshot(of: ["Hello", "World"], as: .dump) {

"""

▿ 2 elements

  - "Hello"

  - "World"

"""
    }
  }
}

With this PR, the correct output is now produced:

import Foundation
import InlineSnapshotTesting
import SnapshotTesting
import XCTest

final class WhitespaceTests: XCTestCase {

  func testInlineSnapshot() {
    assertInlineSnapshot(of: ["Hello", "World"], as: .dump) {
      """
      ▿ 2 elements
        - "Hello"
        - "World"

      """
    }
  }
}
scogeo commented 7 months ago

.prefix(while: { $0.isWhitespace && !$0.isNewline})

@adozenlines Thanks, I added this change.

I also changed the detection of the first indented line to be more inline with what I believe was the original intention and now ignore all lines that contain only whitespace when computing indentation.

scogeo commented 6 months ago

@stephencelis I think this may have gone overlooked over the holidays. I would have thought it would have been merged by now. Perhaps my original PR comment wasn't clear enough so I've updated it to include a trivial example you can copy and paste into a file to verify the issue. Make sure to preserve all of the whitespace!

scogeo commented 4 months ago

I noticed this was recently fixed in #830, so I'm closing this PR.