From c17cd103c64d8e74bad15f311cd688b23236e088 Mon Sep 17 00:00:00 2001
From: Daniel Sobral <dcsobral@gmail.com>
Date: Fri, 20 Mar 2020 18:53:53 -0600
Subject: [PATCH] Make RuleTranformer fully recursive [#257]

RuleTransformer for the past eleven years or more has first recursed,
then applied the rewrite rules as it backed out of the recursion.

That prevented rewrite rules from acting on changes of previous rules
unless they recursed themselves.

The workaround has always been chain rule transformers instead of
calling one rule transformer with all the rules. This change basically
re-implements RuleTransformer as that workaround, and introduces
a NestingTransformer which does the nesting.

Clearly, if you have N rules you'll now recurse N times instead of
once, though each rule is still only applied once for each element.

On the other hand, a RewriteRule that recursed would incur in
exponential times, and now they don't need to.

The original behavior has no reason to be. It didn't prevent
rules from seeing each other changes, nor was it particularly
concerned with performance.  With API changes coming on 2.0, I
believe this is the right time to introduce this change.
---
 .../xml/transform/NestingTransformer.scala    | 19 +++++++++++++++++++
 .../scala/xml/transform/RuleTransformer.scala |  7 +++++--
 .../scala/xml/TransformersTest.scala          | 17 ++++++++++++++++-
 3 files changed, 40 insertions(+), 3 deletions(-)
 create mode 100644 shared/src/main/scala/scala/xml/transform/NestingTransformer.scala

diff --git a/shared/src/main/scala/scala/xml/transform/NestingTransformer.scala b/shared/src/main/scala/scala/xml/transform/NestingTransformer.scala
new file mode 100644
index 000000000..321e195b1
--- /dev/null
+++ b/shared/src/main/scala/scala/xml/transform/NestingTransformer.scala
@@ -0,0 +1,19 @@
+/*                     __                                               *\
+**     ________ ___   / /  ___     Scala API                            **
+**    / __/ __// _ | / /  / _ |    (c) 2002-2020, LAMP/EPFL             **
+**  __\ \/ /__/ __ |/ /__/ __ |    (c) 2011-2020, Lightbend, Inc.       **
+** /____/\___/_/ |_/____/_/ | |    http://scala-lang.org/               **
+**                          |/                                          **
+\*                                                                      */
+
+package scala
+package xml
+package transform
+
+import scala.collection.Seq
+
+class NestingTransformer(rule: RewriteRule) extends BasicTransformer {
+  override def transform(n: Node): Seq[Node] = {
+    rule.transform(super.transform(n))
+  }
+}
diff --git a/shared/src/main/scala/scala/xml/transform/RuleTransformer.scala b/shared/src/main/scala/scala/xml/transform/RuleTransformer.scala
index d8c1a56d0..b7e62644a 100644
--- a/shared/src/main/scala/scala/xml/transform/RuleTransformer.scala
+++ b/shared/src/main/scala/scala/xml/transform/RuleTransformer.scala
@@ -13,6 +13,9 @@ package transform
 import scala.collection.Seq
 
 class RuleTransformer(rules: RewriteRule*) extends BasicTransformer {
-  override def transform(n: Node): Seq[Node] =
-    rules.foldLeft(super.transform(n)) { (res, rule) => rule transform res }
+  private val transformers = rules.map(new NestingTransformer(_))
+  override def transform(n: Node): Seq[Node] = {
+    if (transformers.isEmpty) n
+    else transformers.tail.foldLeft(transformers.head.transform(n)) { (res, transformer) => transformer.transform(res) }
+  }
 }
diff --git a/shared/src/test/scala-2.x/scala/xml/TransformersTest.scala b/shared/src/test/scala-2.x/scala/xml/TransformersTest.scala
index 59ca25240..f8c5b661a 100644
--- a/shared/src/test/scala-2.x/scala/xml/TransformersTest.scala
+++ b/shared/src/test/scala-2.x/scala/xml/TransformersTest.scala
@@ -60,7 +60,7 @@ class TransformersTest {
   @Test
   def preserveReferentialComplexityInLinearComplexity = { // SI-4528
     var i = 0
- 
+
     val xmlNode = <a><b><c><h1>Hello Example</h1></c></b></a>
 
     new RuleTransformer(new RewriteRule {
@@ -77,4 +77,19 @@ class TransformersTest {
 
     assertEquals(1, i)
   }
+
+  @Test
+  def appliesRulesRecursivelyOnPreviousChanges = { // #257
+    def add(outer: Elem, inner: Node) = new RewriteRule {
+      override def transform(n: Node): Seq[Node] = n match {
+        case e: Elem if e.label == outer.label => e.copy(child = e.child ++ inner)
+        case other => other
+      }
+    }
+
+    def transformer = new RuleTransformer(add(<element/>, <new/>), add(<new/>, <thing/>))
+
+    assertEquals(<element><new><thing/></new></element>, transformer(<element/>))
+  }
 }
+