SI-8818 FreshName extractor forgives suffix

The test is corrected (inverted) and the extractor is made
more succinct.  Succinctness isn't enforced by the test,
but I checked it manually.

3 files changed, 30 insertions, 27 deletions

diff --git a/src/reflect/scala/reflect/internal/FreshNames.scala b/src/reflect/scala/reflect/internal/FreshNames.scala
index 7e9a568266..17883d12ad 100644
--- a/src/reflect/scala/reflect/internal/FreshNames.scala
+++ b/src/reflect/scala/reflect/internal/FreshNames.scala
@@ -7,6 +7,7 @@ package reflect
 package internal
 
 import scala.reflect.internal.util.FreshNameCreator
+import scala.util.matching.Regex
 
 trait FreshNames { self: Names with StdNames =>
   // SI-6879 Keeps track of counters that are supposed to be globally unique
@@ -23,17 +24,20 @@ trait FreshNames { self: Names with StdNames =>
   // Extractor that matches names which were generated by some
   // FreshNameCreator with known prefix. Extracts user-specified
   // prefix that was used as a parameter to newName by stripping
-  // global creator prefix and unique number in the end of the name.
+  // global creator prefix and unique numerical suffix.
+  // The creator prefix and numerical suffix may both be empty.
   class FreshNameExtractor(creatorPrefix: String = "") {
-    // quote prefix so that it can be used with replaceFirst
-    // which expects regExp rather than simple string
-    val quotedCreatorPrefix = java.util.regex.Pattern.quote(creatorPrefix)
-
-    def unapply(name: Name): Option[String] = {
-      val sname = name.toString
-      // name should start with creatorPrefix and end with number
-      if (!sname.startsWith(creatorPrefix) || !sname.matches("^.*\\d*$")) None
-      else Some(NameTransformer.decode(sname.replaceFirst(quotedCreatorPrefix, "").replaceAll("\\d*$", "")))
+
+    // name should start with creatorPrefix and end with number
+    val freshlyNamed = {
+      val pre = if (!creatorPrefix.isEmpty) Regex quote creatorPrefix else ""
+      s"""$pre(.*?)\\d*""".r
     }
+
+    def unapply(name: Name): Option[String] =
+      name.toString match {
+        case freshlyNamed(prefix) => Some(prefix)
+        case _                    => None
+      }
   }
 }
diff --git a/test/junit/scala/tools/nsc/symtab/FreshNameExtractorTest.scala b/test/junit/scala/tools/nsc/symtab/FreshNameExtractorTest.scala
index effbfb2f7c..7796345351 100644
--- a/test/junit/scala/tools/nsc/symtab/FreshNameExtractorTest.scala
+++ b/test/junit/scala/tools/nsc/symtab/FreshNameExtractorTest.scala
@@ -32,16 +32,16 @@ class FreshNameExtractorTest {
     val Creator = new FreshNameCreator(prefixes.head)
     val Extractor = new FreshNameExtractor(prefixes.tail.head)
     assertThrows[MatchError] {
-      val Extractor(_) = TermName(Creator.newName("foo"))
+      TermName(Creator.newName("foo")) match { case Extractor(_) => }
     }
   }
 
-  @Test @org.junit.Ignore // SI-8818
-  def extractionsFailsIfNameDoesntEndWithNumber = {
-    val Creator = new FreshNameCreator(prefixes.head)
+  @Test
+  def `no numeric suffix? no problem!` = {
+    val Creator   = new FreshNameCreator(prefixes.head)
     val Extractor = new FreshNameExtractor(prefixes.head)
-    assertThrows[MatchError] {
-      val Extractor(_) = TermName(Creator.newName("foo") + "bar")
+    TermName(Creator.newName("foo") + "bar") match {
+      case Extractor(_) =>
     }
   }
 }
diff --git a/test/junit/scala/tools/testing/AssertUtil.scala b/test/junit/scala/tools/testing/AssertUtil.scala
index d29f9a473f..d798f2e53e 100644
--- a/test/junit/scala/tools/testing/AssertUtil.scala
+++ b/test/junit/scala/tools/testing/AssertUtil.scala
@@ -36,21 +36,20 @@ object AssertUtil {
     }
   }
 
-  /** Check if throwable T (or a subclass) was thrown during evaluation of f, and that its message
-   *  satisfies the `checkMessage` predicate. If any other exception will be re-thrown.
+  /** Check that throwable T (or a subclass) was thrown during evaluation of `body`,
+   *  and that its message satisfies the `checkMessage` predicate.
+   *  Any other exception is propagated.
    */
-  def assertThrows[T <: Throwable](f: => Any,
+  def assertThrows[T <: Throwable](body: => Any,
                                    checkMessage: String => Boolean = s => true)
                                   (implicit manifest: Manifest[T]): Unit = {
-    try f
-    catch {
-      case e: Throwable if checkMessage(e.getMessage) =>
-        val clazz = manifest.runtimeClass
-        if (!clazz.isAssignableFrom(e.getClass))
-          throw e
-        else return
+    try {
+      body
+      fail("Expression did not throw!")
+    } catch {
+      case e: Throwable if (manifest.runtimeClass isAssignableFrom e.getClass) &&
+                            checkMessage(e.getMessage) =>
     }
-    fail("Expression did not throw!")
   }
 
   /** JUnit-style assertion for `IterableLike.sameElements`.