Skip to content

Commit

Permalink
Correct redundant escaping
Browse files Browse the repository at this point in the history
Whoever wrote these `String.replaceAll` calls was a bit confused about
how backslashes behave in the replacement text.  They actually behave
less like backslashes in generic `String` literals, and more like
backslashes in regular expressions.  For example, to replace a newline
with a backslash followed by an `n`, the replacement text should be
`\\\\n`, not `\\n`.

I've added a test for
`com.ibm.wala.types.generics.MethodTypeSignature.getArguments()` to
affirm that my changes there leave us with an implementation that has
the intended effects.  I haven't added a
`com.ibm.wala.cast.js.html.DomLessSourceExtractor.HtmlCallback.quotify(String)`
test, though, because this method is buried too deep inside `protected`
APIs that are difficult for a unit test to get access to.  Instead, to
argue that the new code matches the original `quotify` authors' likely
intent, contrast these two JShell approximations:

* old code:

  ```plain
  jshell> System.out.println("foo\n\"bar".replaceAll("\"", "\\\"").replaceAll("\n", "\\n"))
  foon"bar
  ```

* new code:

  ```plain
  System.out.println("foo\n\"bar".replaceAll("\"", "\\\\\"").replaceAll("\n", "\\\\n"))
  foo\n\"bar
  ```
  • Loading branch information
liblit committed Sep 4, 2023
1 parent 6a24203 commit df52cbb
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,11 @@ protected static Pair<String, Character> quotify(String value) {
quote = '"';
} else {
quote = '"';
value = value.replaceAll("\"", "\\\"");
value = value.replaceAll("\"", "\\\\\"");
}

if (value.indexOf('\n') >= 0) {
value = value.replaceAll("\n", "\\n");
value = value.replaceAll("\n", "\\\\n");
}

return Pair.make(value, quote);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public static MethodTypeSignature make(String genericsSignature) throws IllegalA
* @return null if no arguments
*/
public TypeSignature[] getArguments() {
String typeSig = rawString().replaceAll(".*\\(", "\\(").replaceAll("\\).*", "\\)");
String typeSig = rawString().replaceAll(".*\\(", "(").replaceAll("\\).*", ")");
String[] args = TypeSignature.parseForTypeSignatures(typeSig);
if (args == null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.ibm.wala.types.generics;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.is;

import org.junit.jupiter.api.Test;

/** Unit tests for {@link MethodTypeSignature}. */
class MethodTypeSignatureTest {

/** Unit test for {@link MethodTypeSignature#getArguments()}. */
@Test
void getArguments() {
assertThat(
MethodTypeSignature.make("(I)V").getArguments(),
arrayContaining(is(TypeSignature.make("I"))));
}
}

0 comments on commit df52cbb

Please sign in to comment.