Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regex.replaceAllIn does not escape '\' and '$' in group substitution #5437

Closed
scabug opened this issue Feb 6, 2012 · 5 comments
Closed

Regex.replaceAllIn does not escape '\' and '$' in group substitution #5437

scabug opened this issue Feb 6, 2012 · 5 comments
Assignees

Comments

@scabug
Copy link

scabug commented Feb 6, 2012

The bug was apparent in the trunk three days ago so I believe it affects all versions. The class under concern is scala.util.matching.Regex.

The following exception is somewhat surprising and undocumented in the scaladoc.

scala> "anything".r.replaceAllIn("anything", "\\")
java.lang.StringIndexOutOfBoundsException: String index out of range: 1
       at java.lang.String.charAt(String.java:694)
       at java.util.regex.Matcher.appendReplacement(Matcher.java:716)
       at java.util.regex.Matcher.replaceAll(Matcher.java:823)
       at scala.util.matching.Regex.replaceAllIn(Regex.scala:103)
       at .<init>(<console>:9)
       at .<clinit>(<console>)
       at .<init>(<console>:11)
       at .<clinit>(<console>)
       at $print(<console>)
       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
       at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
       at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
       at java.lang.reflect.Method.invoke(Method.java:616)
       at scala.tools.nsc.interpreter.IMain$ReadEvalPrint.call(IMain.scala:704)
       at scala.tools.nsc.interpreter.IMain$Request$$anonfun$14.apply(IMain.scala:920)
       at scala.tools.nsc.interpreter.Line$$anonfun$1.apply$mcV$sp(Line.scala:43)
       at scala.tools.nsc.io.package$$anon$2.run(package.scala:25)
       at java.lang.Thread.run(Thread.java:679)

As the stacktrace suggests, Java has the same behavior.

    (Pattern.compile("anything").matcher("anything").replaceAll("\\")) 

At the least, this should be documented. More unfortunately, there is an overload for replaceAllIn that takes an anonymous function Match=>String. Presently the user must remember to escape all backslashes and dollar signs or the code will break when certain match groups are used. This is undocumented and unreasonable; this problem does not arise in Java.

I like the new signature for replaceAllIn, but there are shortcomings which convince me it should not be used.

Java has the following documentation for replaceAll.

At the least, the behavior should be commented in the scaladoc. It is commented in the Javadoc.

"Note that backslashes () and dollar signs ($) in the replacement string may cause the results to be different than if it were being treated as a literal replacement string. Dollar signs may be treated as references to captured subsequences as described above, and backslashes are used to escape literal characters in the replacement string. "

@scabug
Copy link
Author

scabug commented Feb 6, 2012

Imported From: https://issues.scala-lang.org/browse/SI-5437?orig=1
Reporter: Michael Schmitz (schmmd)
Affected Versions: 2.9.1

@scabug
Copy link
Author

scabug commented Feb 6, 2012

Michael Schmitz (schmmd) said (edited on Feb 7, 2012 10:31:50 PM UTC):
As an example, it's not fun (nor particularly easy) to need to escape these characters oneself.

val rel = group.replaceAllIn(template, (gm: Regex.Match) => m.groups(gm.group(1)).text.replaceAll("""\\""", """\\\\""").replaceAll("""\$""", """\\\$"""))

I propose that for this overload, the escaping is done by scala.

@scabug
Copy link
Author

scabug commented Feb 7, 2012

@adriaanm said:
Daniel, since you've been improving the regex documentation, could you please look into this? thanks!

@scabug
Copy link
Author

scabug commented Feb 7, 2012

@dcsobral said:
This is a duplicate of SI-4750.

@scabug
Copy link
Author

scabug commented Feb 7, 2012

@dcsobral said:
See SI-4750.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants