Skip to content

Commit

Permalink
Pass test for consecutive unnecessarily escaped characters in strings
Browse files Browse the repository at this point in the history
See #1575 (comment)

This looping is hacky. We might be able to emulate lookbehind instead.
  • Loading branch information
josephfrazier committed May 10, 2017
1 parent 2e25f4b commit 2323c8c
Showing 1 changed file with 18 additions and 8 deletions.
26 changes: 18 additions & 8 deletions src/printer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3859,10 +3859,6 @@ function makeString(rawContent, enclosingQuote) {
// Matches _any_ escape and unescaped quotes (both single and double).
const regex = /\\([\s\S])|(['"])/g;

// Matches any unnecessarily escaped character.
// Adapted from https://github.com/eslint/eslint/blob/de0b4ad7bd820ade41b1f606008bea68683dc11a/lib/rules/no-useless-escape.js#L27
const regexUnnecessaryStringEscapes = /((?:^|[^\\])(?:\\\\)*)\\([^\\nrvtbfux\r\n\u2028\u2029"'0-7])/g;

// Escape and unescape single and double quotes as needed to be able to
// enclose `rawContent` with `enclosingQuote`.
const newContent = rawContent.replace(regex, (match, escaped, quote) => {
Expand All @@ -3882,11 +3878,25 @@ function makeString(rawContent, enclosingQuote) {

// Otherwise return the escape or unescaped quote as-is.
return match;
})
// Unescape unnecessarily escaped characters.
.replace(regexUnnecessaryStringEscapes, (match, prev, escaped) => prev + escaped);
});

// Matches any unnecessarily escaped character.
// Adapted from https://github.com/eslint/eslint/blob/de0b4ad7bd820ade41b1f606008bea68683dc11a/lib/rules/no-useless-escape.js#L27
const regexUnnecessaryStringEscapes = /((?:^|[^\\])(?:\\\\)*)\\([^\\nrvtbfux\r\n\u2028\u2029"'0-7])/g;

let minimallyEscapedContent = newContent;
let minimallyEscapedContentPrev;

while (minimallyEscapedContent !== minimallyEscapedContentPrev) {
minimallyEscapedContentPrev = minimallyEscapedContent;
// Unescape unnecessarily escaped characters.
minimallyEscapedContent = minimallyEscapedContentPrev.replace(
regexUnnecessaryStringEscapes,
(match, prev, escaped) => prev + escaped

This comment has been minimized.

Copy link
@bakkot

bakkot May 10, 2017

Collaborator

I'm not entirely sure it's worth doing - I don't have a good sense for how expensive string comparison is - but fwiw you can abuse the "replace" function to avoid having to do the string comparison at all; something like

let touched = true;
while (touched) {
  touched = false;
  minimallyEscapedContent = minimallyEscapedContent.replace(regexUnnecessaryStringEscapes, (match, prev, escaped) => { 
    touched = true;
    return prev + escaped;
  });
}

This comment has been minimized.

Copy link
@josephfrazier

josephfrazier May 10, 2017

Author Collaborator

Ooh, good point. I'm actually currently looking into emulating lookbehind à la http://blog.stevenlevithan.com/archives/mimic-lookbehind-javascript, so hopefully that will work out. If not, I'll probably make the suggested change.

This comment has been minimized.

Copy link
@josephfrazier

josephfrazier May 10, 2017

Author Collaborator

Hmm, I didn't get lookbehind working, so I made the change in 5725d48

);
}

return enclosingQuote + newContent + enclosingQuote;
return enclosingQuote + minimallyEscapedContent + enclosingQuote;
}

function printRegex(node) {
Expand Down

0 comments on commit 2323c8c

Please sign in to comment.