From db3346105904df18b38cccee7a7fe3b2c9cb8ddf Mon Sep 17 00:00:00 2001
From: Ray Ryan <ray@squareup.com>
Date: Wed, 4 Oct 2023 09:04:03 -0700
Subject: [PATCH] Improves legibility of `Compatible.keyFor`, `NamedScreen`,
 `Named`.

This:

```
object Hey : Screen
NamedScreen(NamedScreen(Hey, "one"), "ho")
```

is now logged as this:

```
NamedScreen:ho(NamedScreen:one(com.squareup.workflow1.ui.NamedScreenTest$Hey))
```

instead of this:

```
com.squareup.workflow1.ui.NamedScreenTest\$Hey+NamedScreen(one)+NamedScreen(ho)
```
---
 .../main/java/com/squareup/workflow1/ui/Compatible.kt | 11 ++++++++---
 .../src/main/java/com/squareup/workflow1/ui/Named.kt  |  2 +-
 .../java/com/squareup/workflow1/ui/NamedScreen.kt     |  2 +-
 .../java/com/squareup/workflow1/ui/NamedScreenTest.kt |  2 +-
 .../test/java/com/squareup/workflow1/ui/NamedTest.kt  |  4 ++--
 5 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/Compatible.kt b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/Compatible.kt
index bd653a688..0636662ff 100644
--- a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/Compatible.kt
+++ b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/Compatible.kt
@@ -36,14 +36,19 @@ public interface Compatible {
 
   public companion object {
     /**
-     * Calculates a suitable [Compatible.compatibilityKey] for a given [value] and [name].
+     * Calculates a suitable [Compatible.compatibilityKey] for a given [value], incorporating
+     * [name] if that is not blank. Includes the [compatibilityKey] for [value] if it
+     * implements [Compatible], to support recursion from wrapping.
+     *
+     * Style note: [name] is given more prominence than the key generate
      */
     public fun keyFor(
       value: Any,
       name: String = ""
     ): String {
-      return ((value as? Compatible)?.compatibilityKey ?: value::class.java.name) +
-        if (name.isEmpty()) "" else "+$name"
+      val key = (value as? Compatible)?.compatibilityKey ?: value::class.java.name
+
+      return name.takeIf { it.isNotEmpty() }?.let { "$name($key)" } ?: key
     }
   }
 }
diff --git a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/Named.kt b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/Named.kt
index 85db722c1..289200655 100644
--- a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/Named.kt
+++ b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/Named.kt
@@ -15,7 +15,7 @@ public data class Named<W : Any>(
     require(name.isNotBlank()) { "name must not be blank." }
   }
 
-  override val compatibilityKey: String = Compatible.keyFor(wrapped, "Named($name)")
+  override val compatibilityKey: String = Compatible.keyFor(wrapped, "Named:$name")
 
   override fun toString(): String {
     return "${super.toString()}: $compatibilityKey"
diff --git a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/NamedScreen.kt b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/NamedScreen.kt
index 519c87a8c..79fc1a808 100644
--- a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/NamedScreen.kt
+++ b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/NamedScreen.kt
@@ -16,7 +16,7 @@ public data class NamedScreen<C : Screen>(
     require(name.isNotBlank()) { "name must not be blank." }
   }
 
-  override val compatibilityKey: String = Compatible.keyFor(content, "NamedScreen($name)")
+  override val compatibilityKey: String = Compatible.keyFor(content, "NamedScreen:$name")
 
   @Deprecated("Use content", ReplaceWith("content"))
   public val wrapped: C = content
diff --git a/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/NamedScreenTest.kt b/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/NamedScreenTest.kt
index 602f56edf..c36bae731 100644
--- a/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/NamedScreenTest.kt
+++ b/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/NamedScreenTest.kt
@@ -57,7 +57,7 @@ internal class NamedScreenTest {
 
   @Test fun `recursive keys are legible`() {
     assertThat(NamedScreen(NamedScreen(Hey, "one"), "ho").compatibilityKey)
-      .isEqualTo("com.squareup.workflow1.ui.NamedScreenTest\$Hey+NamedScreen(one)+NamedScreen(ho)")
+      .isEqualTo("NamedScreen:ho(NamedScreen:one(com.squareup.workflow1.ui.NamedScreenTest\$Hey))")
   }
 
   private class Foo(override val compatibilityKey: String) : Compatible, Screen
diff --git a/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/NamedTest.kt b/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/NamedTest.kt
index 5be393a60..01caafc88 100644
--- a/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/NamedTest.kt
+++ b/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/NamedTest.kt
@@ -5,7 +5,7 @@ import org.junit.Test
 
 // If you try to replace isTrue() with isTrue compilation fails.
 @OptIn(WorkflowUiExperimentalApi::class)
-@Suppress("UsePropertyAccessSyntax", "DEPRECATION")
+@Suppress("DEPRECATION")
 internal class NamedTest {
   object Whut
   object Hey
@@ -58,7 +58,7 @@ internal class NamedTest {
 
   @Test fun `recursive keys are legible`() {
     assertThat(Named(Named(Hey, "one"), "ho").compatibilityKey)
-      .isEqualTo("com.squareup.workflow1.ui.NamedTest\$Hey+Named(one)+Named(ho)")
+      .isEqualTo("Named:ho(Named:one(com.squareup.workflow1.ui.NamedTest\$Hey))")
   }
 
   private class Foo(override val compatibilityKey: String) : Compatible