Merge pull request #16 from diffplug/feat/carriage-return

Carriage return handling

Author: jknack

selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SnapshotFile.kt

@@ -38,7 +38,7 @@ sealed interface SnapshotValue {
 
   companion object {
     fun of(binary: ByteArray): SnapshotValue = SnapshotValueBinary(binary)
-    fun of(string: String): SnapshotValue = SnapshotValueString(string)
+    fun of(string: String): SnapshotValue = SnapshotValueString(unixNewlines(string))
   }
 }
 
@@ -66,7 +66,8 @@ data class Snapshot(
     get() = lensData
   fun lens(key: String, value: ByteArray) = lens(key, SnapshotValue.of(value))
   fun lens(key: String, value: String) = lens(key, SnapshotValue.of(value))
-  fun lens(key: String, value: SnapshotValue) = Snapshot(this.value, lensData.plus(key, value))
+  fun lens(key: String, value: SnapshotValue) =
+      Snapshot(this.value, lensData.plus(unixNewlines(key), value))
   override fun toString(): String = "[${value} ${lenses}]"
 
   companion object {
@@ -82,12 +83,17 @@ private fun String.efficientReplace(find: String, replaceWith: String): String {
   val idx = this.indexOf(find)
   return if (idx == -1) this else this.replace(find, replaceWith)
 }
+private fun unixNewlines(str: String) = str.efficientReplace("\r\n", "\n")
 
 class SnapshotFile {
+  internal var unixNewlines = true
   // this will probably become `<String, JsonObject>` we'll cross that bridge when we get to it
   var metadata: Map.Entry<String, String>? = null
   var snapshots = ArrayMap.empty<String, Snapshot>()
-  fun serialize(valueWriter: StringWriter) {
+  fun serialize(valueWriterRaw: StringWriter) {
+    val valueWriter =
+        if (unixNewlines) valueWriterRaw
+        else StringWriter { valueWriterRaw.write(it.efficientReplace("\n", "\r\n")) }
     metadata?.let {
       writeKey(valueWriter, "📷 ${it.key}", null)
       writeValue(valueWriter, SnapshotValue.of(it.value))
@@ -147,6 +153,7 @@ class SnapshotFile {
     fun parse(valueReader: SnapshotValueReader): SnapshotFile {
       try {
         val result = SnapshotFile()
+        result.unixNewlines = valueReader.unixNewlines
         val reader = SnapshotReader(valueReader)
         // only if the first value starts with 📷
         if (reader.peekKey()?.startsWith(HEADER_PREFIX) == true) {
@@ -162,6 +169,11 @@ class SnapshotFile {
         throw if (e is ParseException) e else ParseException(valueReader.lineReader, e)
       }
     }
+    fun createEmptyWithUnixNewlines(unixNewlines: Boolean): SnapshotFile {
+      val result = SnapshotFile()
+      result.unixNewlines = unixNewlines
+      return result
+    }
   }
 }
 
@@ -207,6 +219,7 @@ class SnapshotReader(val valueReader: SnapshotValueReader) {
 /** Provides the ability to parse a snapshot file incrementally. */
 class SnapshotValueReader(val lineReader: LineReader) {
   var line: String? = null
+  val unixNewlines = lineReader.unixNewlines()
 
   /** The key of the next value, does not increment anything about the reader's state. */
   fun peekKey(): String? {
@@ -311,6 +324,7 @@ class SnapshotValueReader(val lineReader: LineReader) {
 expect class LineReader {
   fun getLineNumber(): Int
   fun readLine(): String?
+  fun unixNewlines(): Boolean
 
   companion object {
     fun forString(content: String): LineReader

selfie-lib/src/jsMain/kotlin/com/diffplug/selfie/SnapshotFile.js.kt

@@ -22,4 +22,5 @@ actual class LineReader {
   }
   actual fun getLineNumber(): Int = TODO()
   actual fun readLine(): String? = TODO()
+  actual fun unixNewlines(): Boolean = TODO()
 }

selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/SnapshotFile.jvm.kt

@@ -15,16 +15,74 @@
  */
 package com.diffplug.selfie
 
+import java.io.BufferedReader
 import java.io.InputStreamReader
 import java.io.LineNumberReader
 import java.io.Reader
 import java.io.StringReader
+import java.nio.CharBuffer
 import java.nio.charset.StandardCharsets
 
-actual class LineReader(reader: Reader) : LineNumberReader(reader) {
+actual class LineReader(reader: Reader) {
+  private val reader = LineTerminatorAware(LineTerminatorReader(reader))
+
   actual companion object {
     actual fun forString(content: String) = LineReader(StringReader(content))
     actual fun forBinary(content: ByteArray) =
         LineReader(InputStreamReader(content.inputStream(), StandardCharsets.UTF_8))
   }
+  actual fun getLineNumber(): Int = reader.lineNumber
+  actual fun readLine(): String? = reader.readLine()
+  actual fun unixNewlines(): Boolean = reader.lineTerminator.unixNewlines()
+}
+
+/**
+ * Keep track of carriage return char to figure it out if we need unix new line or not. The first
+ * line is kept in memory until we require the next line.
+ */
+private open class LineTerminatorAware(val lineTerminator: LineTerminatorReader) :
+    LineNumberReader(lineTerminator) {
+  /** First line is initialized as soon as possible. */
+  private var firstLine: String? = super.readLine()
+  override fun readLine(): String? {
+    if (this.firstLine != null) {
+      val result = this.firstLine
+      this.firstLine = null
+      return result
+    }
+    return super.readLine()
+  }
+}
+
+/**
+ * Override all read operations to find the carriage return. We want to keep lazy/incremental reads.
+ */
+private class LineTerminatorReader(reader: Reader) : BufferedReader(reader) {
+  private val CR: Int = '\r'.code
+  private var unixNewlines = true
+  override fun read(cbuf: CharArray): Int {
+    val result = super.read(cbuf)
+    unixNewlines = cbuf.indexOf(CR.toChar()) == -1
+    return result
+  }
+  override fun read(target: CharBuffer): Int {
+    val result = super.read(target)
+    unixNewlines = target.indexOf(CR.toChar()) == -1
+    return result
+  }
+  override fun read(cbuf: CharArray, off: Int, len: Int): Int {
+    val result = super.read(cbuf, off, len)
+    unixNewlines = cbuf.indexOf(CR.toChar()) == -1
+    return result
+  }
+  override fun read(): Int {
+    val ch = super.read()
+    if (ch == CR) {
+      unixNewlines = false
+    }
+    return ch
+  }
+  fun unixNewlines(): Boolean {
+    return unixNewlines
+  }
 }

selfie-lib/src/jvmTest/kotlin/com/diffplug/selfie/LineReaderTest.kt

@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2023 DiffPlug
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.diffplug.selfie
+
+import io.kotest.matchers.shouldBe
+import kotlin.test.Test
+
+class LineReaderTest {
+
+  @Test
+  fun shouldFindUnixSeparatorFromBinary() {
+    val reader = LineReader.forBinary("This is a new line\n".encodeToByteArray())
+    reader.unixNewlines() shouldBe true
+    reader.readLine() shouldBe "This is a new line"
+  }
+
+  @Test
+  fun shouldFindWindowsSeparatorFromBinary() {
+    val reader = LineReader.forBinary("This is a new line\r\n".encodeToByteArray())
+    reader.unixNewlines() shouldBe false
+    reader.readLine() shouldBe "This is a new line"
+  }
+
+  @Test
+  fun shouldFindUnixSeparatorFromString() {
+    val reader = LineReader.forString("This is a new line\n")
+    reader.unixNewlines() shouldBe true
+    reader.readLine() shouldBe "This is a new line"
+  }
+
+  @Test
+  fun shouldFindWindowsSeparatorFromString() {
+    val reader = LineReader.forString("This is a new line\r\n")
+    reader.unixNewlines() shouldBe false
+    reader.readLine() shouldBe "This is a new line"
+  }
+
+  @Test
+  fun shouldGetUnixLineSeparatorWhenThereIsNone() {
+    val reader = LineReader.forBinary("This is a new line".encodeToByteArray())
+    reader.unixNewlines() shouldBe true
+    reader.readLine() shouldBe "This is a new line"
+  }
+
+  @Test
+  fun shouldReadNextLineWithoutProblem() {
+    val reader = LineReader.forBinary("First\r\nSecond\r\n".encodeToByteArray())
+    reader.unixNewlines() shouldBe false
+    reader.readLine() shouldBe "First"
+    reader.unixNewlines() shouldBe false
+    reader.readLine() shouldBe "Second"
+    reader.unixNewlines() shouldBe false
+  }
+
+  @Test
+  fun shouldUseFirstLineSeparatorAndIgnoreNext() {
+    val reader = LineReader.forBinary("First\r\nAnother separator\n".encodeToByteArray())
+    reader.unixNewlines() shouldBe false
+    reader.readLine() shouldBe "First"
+    reader.unixNewlines() shouldBe false
+    reader.readLine() shouldBe "Another separator"
+    reader.unixNewlines() shouldBe false
+  }
+}

selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieConfig.kt

@@ -19,7 +19,11 @@ import java.nio.file.Files
 import java.nio.file.Path
 import java.nio.file.Paths
 
-internal class SnapshotFileLayout(val rootFolder: Path, val snapshotFolderName: String?) {
+internal class SnapshotFileLayout(
+    val rootFolder: Path,
+    val snapshotFolderName: String?,
+    val unixNewlines: Boolean
+) {
   val extension: String = ".ss"
   fun snapshotPathForClass(className: String): Path {
     val lastDot = className.lastIndexOf('.')
@@ -64,12 +68,21 @@ internal class SnapshotFileLayout(val rootFolder: Path, val snapshotFolderName:
             "src/test/scala",
             "src/test/resources")
     fun initialize(className: String): SnapshotFileLayout {
-      val selfieDotProp = SnapshotFileLayout.javaClass.getResource("/selfie.properties")
+      val selfieDotProp = SnapshotFileLayout::class.java.getResource("/selfie.properties")
       val properties = java.util.Properties()
       selfieDotProp?.openStream()?.use { properties.load(selfieDotProp.openStream()) }
       val snapshotFolderName = snapshotFolderName(properties.getProperty("snapshot-dir"))
       val snapshotRootFolder = rootFolder(properties.getProperty("output-dir"))
-      return SnapshotFileLayout(snapshotRootFolder, snapshotFolderName)
+      // it's pretty easy to preserve the line endings of existing snapshot files, but it's
+      // a bit harder to create a fresh snapshot file with the correct line endings.
+      val cr =
+          snapshotRootFolder
+              .resolve(snapshotFolderName!!)
+              .toFile()
+              .walkTopDown()
+              .filter { it.isFile }
+              .any { it.readText().contains('\r') }
+      return SnapshotFileLayout(snapshotRootFolder, snapshotFolderName, !cr)
     }
     private fun snapshotFolderName(snapshotDir: String?): String? {
       if (snapshotDir == null) {

selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt

@@ -171,7 +171,7 @@ internal class ClassProgress(val className: String) {
             val content = Files.readAllBytes(snapshotPath)
             SnapshotFile.parse(SnapshotValueReader.of(content))
           } else {
-            SnapshotFile()
+            SnapshotFile.createEmptyWithUnixNewlines(Router.layout!!.unixNewlines)
           }
     }
     return file!!

selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/CarriageReturnTest.kt

@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2023 DiffPlug
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.diffplug.selfie.junit5
+
+import kotlin.test.Test
+import org.junit.jupiter.api.MethodOrderer
+import org.junit.jupiter.api.Order
+import org.junit.jupiter.api.TestMethodOrder
+import org.junitpioneer.jupiter.DisableIfTestFails
+
+/** Verify selfie's carriage-return handling. */
+@TestMethodOrder(MethodOrderer.OrderAnnotation::class)
+@DisableIfTestFails
+class CarriageReturnTest : Harness("undertest-junit5") {
+  @Test @Order(1)
+  fun noSelfie() {
+    ut_snapshot().deleteIfExists()
+    ut_snapshot().assertDoesNotExist()
+  }
+  val expectedContent =
+      """
+            ╔═ git_makes_carriage_returns_unrepresentable ═╗
+            hard
+            to
+            preserve
+            this
+            
+            ╔═ [end of file] ═╗
+            
+        """
+          .trimIndent()
+
+  @Test @Order(2)
+  fun write_and_assert_ss_has_unix_newlines() {
+    gradleWriteSS()
+    ut_snapshot().assertContent(expectedContent)
+  }
+
+  @Test @Order(3)
+  fun if_ss_has_cr_then_it_will_stay_cr() {
+    val contentWithCr = expectedContent.replace("\n", "\r\n")
+    ut_snapshot().setContent(contentWithCr)
+    gradleWriteSS()
+    ut_snapshot().assertContent(contentWithCr)
+  }
+
+  @Test @Order(4)
+  fun go_back_to_unix_and_it_stays_unix() {
+    ut_snapshot().setContent(expectedContent)
+    gradleWriteSS()
+    ut_snapshot().assertContent(expectedContent)
+  }
+
+  @Test @Order(5)
+  fun deleteSelfie() {
+    ut_snapshot().deleteIfExists()
+  }
+}

undertest-junit5/src/test/kotlin/undertest/junit5/UT_CarriageReturnTest.kt

@@ -0,0 +1,10 @@
+package undertest.junit5
+
+import com.diffplug.selfie.expectSelfie
+import kotlin.test.Test
+
+class UT_CarriageReturnTest {
+  @Test fun git_makes_carriage_returns_unrepresentable() {
+    expectSelfie("hard\r\nto\npreserve\r\nthis\r\n").toMatchDisk()
+  }
+}