Remove exotic Modifier APIs, fix normalization

redwood-runtime/src/commonMain/kotlin/app/cash/redwood/Modifier.kt

@@ -25,68 +25,33 @@ import kotlin.native.ObjCName
 @Stable
 @ObjCName("Modifier", exact = true)
 public interface Modifier {
-
-  /**
-   * Accumulates a value starting with [initial] and applying [operation] to the current value
-   * and each element from outside in.
-   *
-   * Elements wrap one another in a chain from left to right; an [Element] that appears to the
-   * left of another in a `+` expression or in [operation]'s parameter order affects all
-   * of the elements that appear after it. [foldIn] may be used to accumulate a value starting
-   * from the parent or head of the modifier chain to the final wrapped child.
-   */
-  public fun <R> foldIn(initial: R, operation: (R, Element) -> R): R
-
-  /**
-   * Accumulates a value starting with [initial] and applying [operation] to the current value
-   * and each element from inside out.
-   *
-   * Elements wrap one another in a chain from left to right; an [Element] that appears to the
-   * left of another in a `+` expression or in [operation]'s parameter order affects all
-   * of the elements that appear after it. [foldOut] may be used to accumulate a value starting
-   * from the child or tail of the modifier chain up to the parent or head of the chain.
-   */
-  public fun <R> foldOut(initial: R, operation: (Element, R) -> R): R
-
   /**
    * Iterates over all [Element]s in this [Modifier].
    */
   public fun forEach(block: (Element) -> Unit)
 
-  /**
-   * Returns `true` if [predicate] returns true for any [Element] in this [Modifier].
-   */
-  public fun any(predicate: (Element) -> Boolean): Boolean
-
-  /**
-   * Returns `true` if [predicate] returns true for all [Element]s in this [Modifier] or if
-   * this [Modifier] contains no [Element]s.
-   */
-  public fun all(predicate: (Element) -> Boolean): Boolean
-
   /**
    * Concatenates this modifier with another.
    *
    * Returns a [Modifier] representing this modifier followed by [other] in sequence.
    */
   public infix fun then(other: Modifier): Modifier =
-    if (other === Modifier) this else CombinedModifier(this, other)
+    if (other === Modifier) {
+      this
+    } else if (other is CombinedModifier) {
+      // Normalize the element chain to be left-associative when multiple are added on the right.
+      var result: Modifier = this
+      other.forEach { element -> result = CombinedModifier(result, element) }
+      result
+    } else {
+      CombinedModifier(this, other)
+    }
 
   /**
    * A single element contained within a [Modifier] chain.
    */
   public interface Element : Modifier {
-    override fun <R> foldIn(initial: R, operation: (R, Element) -> R): R =
-      operation(initial, this)
-
-    override fun <R> foldOut(initial: R, operation: (Element, R) -> R): R =
-      operation(this, initial)
-
     override fun forEach(block: (Element) -> Unit): Unit = block(this)
-
-    override fun any(predicate: (Element) -> Boolean): Boolean = predicate(this)
-
-    override fun all(predicate: (Element) -> Boolean): Boolean = predicate(this)
   }
 
   /**
@@ -98,48 +63,38 @@ public interface Modifier {
   // modifier extension factory expression.
   @ObjCName("EmptyModifier", exact = true)
   public companion object : Modifier {
-    override fun <R> foldIn(initial: R, operation: (R, Element) -> R): R = initial
-    override fun <R> foldOut(initial: R, operation: (Element, R) -> R): R = initial
     override fun forEach(block: (Element) -> Unit) {}
-    override fun any(predicate: (Element) -> Boolean): Boolean = false
-    override fun all(predicate: (Element) -> Boolean): Boolean = true
     override infix fun then(other: Modifier): Modifier = other
     override fun toString(): String = "Modifier"
   }
 }
 
-/**
- * A node in a [Modifier] chain. A CombinedModifier always contains at least two elements;
- * a Modifier [outer] that wraps around the Modifier [inner].
- */
-@ObjCName("CombinedModifier", exact = true)
-public class CombinedModifier(
+private class CombinedModifier(
   private val outer: Modifier,
   private val inner: Modifier,
 ) : Modifier {
-  override fun <R> foldIn(initial: R, operation: (R, Modifier.Element) -> R): R =
-    inner.foldIn(outer.foldIn(initial, operation), operation)
-
-  override fun <R> foldOut(initial: R, operation: (Modifier.Element, R) -> R): R =
-    outer.foldOut(inner.foldOut(initial, operation), operation)
-
   override fun forEach(block: (Modifier.Element) -> Unit) {
     outer.forEach(block)
     inner.forEach(block)
   }
 
-  override fun any(predicate: (Modifier.Element) -> Boolean): Boolean =
-    outer.any(predicate) || inner.any(predicate)
-
-  override fun all(predicate: (Modifier.Element) -> Boolean): Boolean =
-    outer.all(predicate) && inner.all(predicate)
-
   override fun equals(other: Any?): Boolean =
     other is CombinedModifier && outer == other.outer && inner == other.inner
 
   override fun hashCode(): Int = outer.hashCode() + 31 * inner.hashCode()
 
-  override fun toString(): String = "[" + foldIn("") { acc, element ->
-    if (acc.isEmpty()) element.toString() else "$acc, $element"
-  } + "]"
+  override fun toString(): String = buildString {
+    append('[')
+
+    var first = true
+    val appendElement: (Modifier.Element) -> Unit = { element ->
+      if (!first) append(", ")
+      first = false
+      append(element)
+    }
+    outer.forEach(appendElement)
+    inner.forEach(appendElement)
+
+    append(']')
+  }
 }

redwood-runtime/src/commonTest/kotlin/app/cash/redwood/ui/ModifierTest.kt

@@ -0,0 +1,114 @@
+/*
+ * Copyright (C) 2023 Square, Inc.
+ *
+ * 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
+ *
+ *      http://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 app.cash.redwood.ui
+
+import app.cash.redwood.Modifier
+import assertk.assertThat
+import assertk.assertions.hashCodeFun
+import assertk.assertions.isEqualTo
+import assertk.assertions.isSameAs
+import assertk.assertions.toStringFun
+import kotlin.test.Test
+import kotlin.test.fail
+
+class ModifierTest {
+  @Test fun forEachUnitModifierNeverInvoked() {
+    Modifier.forEach {
+      fail()
+    }
+  }
+
+  @Test fun forEachOneElement() {
+    val a = NamedModifier("A")
+    var called = 0
+    a.forEach { element ->
+      assertThat(element).isSameAs(a)
+      called++
+    }
+    assertThat(called).isEqualTo(1)
+  }
+
+  @Test fun forEachManyElements() {
+    val a = NamedModifier("A")
+    val b = NamedModifier("B")
+    val c = NamedModifier("C")
+    val expected = listOf(a, b, c)
+    var called = 0
+    (a then b then c).forEach { element ->
+      assertThat(element).isSameAs(expected[called])
+      called++
+    }
+    assertThat(called).isEqualTo(3)
+  }
+
+  @Test fun thenIgnoresUnitModifier() {
+    assertThat(Modifier then Modifier).isSameAs(Modifier)
+
+    val a = NamedModifier("A")
+    assertThat(a then Modifier).isSameAs(a)
+    assertThat(Modifier then a).isSameAs(a)
+  }
+
+  @Test fun thenElementsToElements() {
+    val a = NamedModifier("A")
+    val b = NamedModifier("B")
+    val c = NamedModifier("C")
+    val d = NamedModifier("D")
+    val expected = listOf(a, b, c, d)
+    var called = 0
+    ((a then b) then (c then d)).forEach { element ->
+      assertThat(element).isSameAs(expected[called])
+      called++
+    }
+    assertThat(called).isEqualTo(4)
+  }
+
+  @Test fun toStringEmpty() {
+    assertThat(Modifier).toStringFun().isEqualTo("Modifier")
+  }
+
+  @Test fun toStringOne() {
+    val a = NamedModifier("A")
+    assertThat(a).toStringFun().isEqualTo("A")
+  }
+
+  @Test fun toStringMany() {
+    val a = NamedModifier("A")
+    val b = NamedModifier("B")
+    val c = NamedModifier("C")
+    assertThat((a then b) then c).toStringFun().isEqualTo("[A, B, C]")
+    assertThat(a then (b then c)).toStringFun().isEqualTo("[A, B, C]")
+  }
+
+  @Test fun hashCodeMany() {
+    val a = NamedModifier("A")
+    val b = NamedModifier("B")
+    val c = NamedModifier("C")
+    assertThat((a then b) then c).hashCodeFun().isEqualTo((a then (b then c)).hashCode())
+  }
+
+  @Test fun equalsMany() {
+    val a = NamedModifier("A")
+    val b = NamedModifier("B")
+    val c = NamedModifier("C")
+    assertThat((a then b) then c).isEqualTo(a then (b then c))
+    assertThat(a then (b then c)).isEqualTo((a then b) then c)
+  }
+}
+
+private class NamedModifier(private val name: String) : Modifier.Element {
+  override fun toString() = name
+}