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

no-useless-escape rule doesn't work when a template string contains regex with backslash #1265

Closed
Zephilim opened this issue Mar 28, 2019 · 5 comments

Comments

@Zephilim
Copy link

commented Mar 28, 2019

What version of standard?
standard version 12.0.1 (implicitly via semistandard version 13.0.1)

What operating system, Node.js, and npm version?
Node: 10.13.0,
npm: 6.9.0
What did you expect to happen?
I should be able to use XML/regular expressions verbatim inside a template string without have to escape anything

What actually happened?
no-useless-escape rule fires when it encounters the \ inside a CDATA section. See following description:

In a test case, I have a hardcoded xml fragment:

    const data = `<?xml version="1.0"?>
      <Application name="pez">
        <Expressions>
          <Expression name="person's-name-expression" eg="Mick Mars">
            <Pattern><![CDATA[[a-zA-Z\s']+]]></Pattern>
          </Expression>
        </Expressions>
      </Application>`;

The no-useless-escape rule fires on encountering the \s inside the CDATA section of the Pattern element. The whole point of use template strings is that you should be able to use any string without escaping anything. The way to get round this is to escape the slash, giving \s, which I should not have to do.

Has regex content been overlooked here when it comes to the no-useless-escape rule? The slash is not an escape as far as the template string is concerned, so the rule shouldn't be firing here. This is the first rule I have encountered that really can't be worked around without fudging something (like escaping inside a construct designed to not have to be escaped).
The backslash inside the template string left in works as expected, so this really is just an issue with this rule and the way it has been implemented.

@LinusU

This comment has been minimized.

Copy link
Member

commented Apr 2, 2019

The whole point of use template strings is that you should be able to use any string without escaping anything.

I don't think that this is true, e.g. `\n` produces a string with a single new-line character, not a backslash and an n.

AFAIK, the correct thing to do here is to escape your backslash since you want that in the resulting string. That is: [a-zA-Z\\s']+

@stale

This comment has been minimized.

Copy link

commented Jul 1, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Jul 1, 2019

@Zephilim

This comment has been minimized.

Copy link
Author

commented Jul 4, 2019

The whole point of use template strings is that you should be able to use any string without escaping anything.

I don't think that this is true, e.g. `\n` produces a string with a single new-line character, not a backslash and an n.

AFAIK, the correct thing to do here is to escape your backslash since you want that in the resulting string. That is: [a-zA-Z\\s']+

Hi LinusS, I take your point about your '\n' example, but that's not the same as my xml example. The string I'm using in my test case includes the standard xml? discriminator so the string should be treated as xml content and thus \s should not be flagged. I have a lot of large complicated regex sequences that have been externally tested, then pasted into xml config verbatim. If I have to start re-escaping in test cases, this is going to be onerous, tedious and error prone. I would rather disable the rule but this would be such a shame to have to do this, because one of the things that attracted me to using standard was not having to disable rules. I have totally submitted to accepting the 'standard' as it is and so far this has worked well. But this rule firing like this feels like a fly in the ointment! To me the fix would be modify the rule for strings with xml content (marked by the presence of xml? discriminator and not fire for valid xml "".

@stale stale bot removed the stale label Jul 4, 2019

@Zephilim

This comment has been minimized.

Copy link
Author

commented Jul 4, 2019

I am closing this, because I have found another solution that doesn't require disabling the rule

@Zephilim Zephilim closed this Jul 4, 2019

@LinusU

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

Neither Node.js nor Standard treats strings with an "xml? discriminator" any differently to normal strings. Even if Standard wouldn't complain, you still wouldn't get the correct string in your variable:

Screenshot 2019-07-09 at 09 55 39

Notice how the \s is just s in the resulting string...

Anyhow, glad you managed to solve it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.