Skip to content

Commit fec54d6

Browse files
som-snyttlrytz
authored andcommitted
SI-9666: Use inline group names in Regex (scala#4990)
Delegate `Match group name` to the underlying `matcher`. If that fails, try explicit group names as a fall back. No attempt is made to correlate inline and explicit names. In the following case, either name is accepted: ``` new Regex("a(?<Bar>b*)c", "Bee") ``` But if names are reversed, the error is undetected: ``` new Regex("a(?<Bee>b*)(?<Bar>c)", "Bar", "Bee") ``` Throw IllegalArg on bad group name to be consistent with Java.
1 parent a7bc60b commit fec54d6

File tree

3 files changed

+86
-8
lines changed

3 files changed

+86
-8
lines changed

src/library/scala/util/matching/Regex.scala

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,9 @@ class Regex private[matching](val pattern: Pattern, groupNames: String*) extends
182182
* val namedYears = for (m <- namedDate findAllMatchIn dates) yield m group "year"
183183
* }}}
184184
*
185+
* Group names supplied to the constructor are preferred to inline group names
186+
* when retrieving matched groups by name. Not all platforms support inline names.
187+
*
185188
* This constructor does not support options as flags, which must be
186189
* supplied as inline flags in the pattern string: `(?idmsux-idmsux)`.
187190
*
@@ -578,6 +581,9 @@ object Regex {
578581
*/
579582
trait MatchData {
580583

584+
/** Basically, wraps a platform Matcher. */
585+
protected def matcher: Matcher
586+
581587
/** The source from which the match originated */
582588
val source: CharSequence
583589

@@ -650,24 +656,33 @@ object Regex {
650656

651657
private lazy val nameToIndex: Map[String, Int] = Map[String, Int]() ++ ("" :: groupNames.toList).zipWithIndex
652658

653-
/** Returns the group with given name.
659+
/** Returns the group with the given name.
660+
*
661+
* Uses explicit group names when supplied; otherwise,
662+
* queries the underlying implementation for inline named groups.
663+
* Not all platforms support inline group names.
654664
*
655665
* @param id The group name
656666
* @return The requested group
657-
* @throws NoSuchElementException if the requested group name is not defined
667+
* @throws IllegalArgumentException if the requested group name is not defined
658668
*/
659-
def group(id: String): String = nameToIndex.get(id) match {
660-
case None => throw new NoSuchElementException("group name "+id+" not defined")
661-
case Some(index) => group(index)
662-
}
669+
def group(id: String): String = (
670+
if (groupNames.isEmpty)
671+
matcher group id
672+
else
673+
nameToIndex.get(id) match {
674+
case Some(index) => group(index)
675+
case None => matcher group id
676+
}
677+
)
663678

664679
/** The matched string; equivalent to `matched.toString`. */
665680
override def toString = matched
666681
}
667682

668683
/** Provides information about a successful match. */
669684
class Match(val source: CharSequence,
670-
private[matching] val matcher: Matcher,
685+
protected[matching] val matcher: Matcher,
671686
val groupNames: Seq[String]) extends MatchData {
672687

673688
/** The index of the first matched character. */

test/files/run/reify_printf.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import scala.tools.reflect.ToolBox
66
import scala.reflect.api._
77
import scala.reflect.api.Trees
88
import scala.reflect.internal.Types
9-
import scala.util.matching.Regex
109

1110
object Test extends App {
1211
//val output = new ByteArrayOutputStream()

test/junit/scala/util/matching/RegexTest.scala

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import org.junit.Test
66
import org.junit.runner.RunWith
77
import org.junit.runners.JUnit4
88

9+
import scala.tools.testing.AssertUtil._
10+
911
@RunWith(classOf[JUnit4])
1012
class RegexTest {
1113
@Test def t8022CharSequence(): Unit = {
@@ -44,4 +46,66 @@ class RegexTest {
4446
}
4547
assertEquals(List((1,2),(3,4),(5,6)), z)
4648
}
49+
50+
@Test def `SI-9666: use inline group names`(): Unit = {
51+
val r = new Regex("a(?<Bee>b*)c")
52+
val ms = r findAllIn "stuff abbbc more abc and so on"
53+
assertTrue(ms.hasNext)
54+
assertEquals("abbbc", ms.next())
55+
assertEquals("bbb", ms group "Bee")
56+
assertTrue(ms.hasNext)
57+
assertEquals("abc", ms.next())
58+
assertEquals("b", ms group "Bee")
59+
assertFalse(ms.hasNext)
60+
}
61+
62+
@Test def `SI-9666: use explicit group names`(): Unit = {
63+
val r = new Regex("a(b*)c", "Bee")
64+
val ms = r findAllIn "stuff abbbc more abc and so on"
65+
assertTrue(ms.hasNext)
66+
assertEquals("abbbc", ms.next())
67+
assertEquals("bbb", ms group "Bee")
68+
assertTrue(ms.hasNext)
69+
assertEquals("abc", ms.next())
70+
assertEquals("b", ms group "Bee")
71+
assertFalse(ms.hasNext)
72+
}
73+
74+
@Test def `SI-9666: fall back to explicit group names`(): Unit = {
75+
val r = new Regex("a(?<Bar>b*)c", "Bee")
76+
val ms = r findAllIn "stuff abbbc more abc and so on"
77+
assertTrue(ms.hasNext)
78+
assertEquals("abbbc", ms.next())
79+
assertEquals("bbb", ms group "Bee")
80+
assertEquals("bbb", ms group "Bar")
81+
assertTrue(ms.hasNext)
82+
assertEquals("abc", ms.next())
83+
assertEquals("b", ms group "Bee")
84+
assertEquals("b", ms group "Bar")
85+
assertFalse(ms.hasNext)
86+
}
87+
88+
//type NoGroup = NoSuchElementException
89+
type NoGroup = IllegalArgumentException
90+
91+
@Test def `SI-9666: throw on bad name`(): Unit = {
92+
assertThrows[NoGroup] {
93+
val r = new Regex("a(?<Bar>b*)c")
94+
val ms = r findAllIn "stuff abbbc more abc and so on"
95+
assertTrue(ms.hasNext)
96+
ms group "Bee"
97+
}
98+
assertThrows[NoGroup] {
99+
val r = new Regex("a(?<Bar>b*)c", "Bar")
100+
val ms = r findAllIn "stuff abbbc more abc and so on"
101+
assertTrue(ms.hasNext)
102+
ms group "Bee"
103+
}
104+
assertThrows[NoGroup] {
105+
val r = new Regex("a(b*)c", "Bar")
106+
val ms = r findAllIn "stuff abbbc more abc and so on"
107+
assertTrue(ms.hasNext)
108+
ms group "Bee"
109+
}
110+
}
47111
}

0 commit comments

Comments
 (0)