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

Report "\8" and "\9" in no-octal-escape #13740

Closed
mdjermanovic opened this issue Oct 6, 2020 · 8 comments
Closed

Report "\8" and "\9" in no-octal-escape #13740

mdjermanovic opened this issue Oct 6, 2020 · 8 comments
Assignees
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules

Comments

@mdjermanovic
Copy link
Member

What rule do you want to change?

no-octal-escape

Does this change cause the rule to produce more or fewer warnings?

more

How will the change be implemented? (New option, new default behavior, etc.)?

new default behavior

Please provide some example code that this change will affect:

"\8"

"foo\9bar"

What does the rule currently do for this code?

no errors

What will the rule do after it's changed?

errors

Are you willing to submit a pull request to implement this change?

Yes.

\8 and \9 used to be invalid (unspecified) escape sequences in string literals. But, they were not not explicitly disallowed language extensions, and all engines used to allow "\8" and "\9", treating them as just "8" and "9" (i.e., as useless escapes).

Now, as of tc39/ecma262#2054, they are defined in Annex B as NonOctalDecimalEscapeSequence:

NonOctalDecimalEscapeSequence :: one of
89

and thus required in browsers, but only in non-strict mode. They are now explicitly disallowed in strict mode.

Although only recently added to the spec, the spec treats them as a legacy feature and in the same context as LegacyOctalEscapeSequence, so I think they should be reported by no-octal-escape.

Questions:

  • no-octal-escape or no-useless-escape? I think no-octal-escape, but there are arguments for no-useless-escape, too.
  • Can we make this change in a minor version?
@mdjermanovic mdjermanovic added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Oct 6, 2020
@mdjermanovic mdjermanovic self-assigned this Oct 6, 2020
@mdjermanovic mdjermanovic changed the title Report \8 and \9 in no-octal-escape Report "\8" and "\9" in no-octal-escape Oct 6, 2020
@nzakas
Copy link
Member

nzakas commented Oct 7, 2020

Given that the spec explicitly defines them as "non-octal", then I'd vote for making a change to no-useless-escape instead.

@mdjermanovic
Copy link
Member Author

To expand on why I'm slightly more in favor of no-octal-escape:

  • no-octal-escape disallows legacy syntax - escape sequences that are invalid except in non-strict mode in browsers, and that's the case with \8 and \9 now. no-useless-escape disallows valid escape sequences.
  • I think the term "non-octal" is used to emphasize the similarity (\ followed by a digit), rather than a difference. Otherwise, they could be named as just "DecimalEscapeSequence".
  • A similar rule no-octal disallows NonOctalDecimalIntegerLiteral literals, like 08 and 09 (demo).

On the other hand, "\8" and "\9" are indeed evaluated as useless escapes and there are no octal digits inside, so I don't have a strong opinion.

@nzakas
Copy link
Member

nzakas commented Oct 8, 2020

If this is added to no-octal-escape as a default behavior then I think it’s a breaking change.

I dug into this a bit. “disallowed” in this case simply means it’s not expanded, so \9 is interpreted as \9 rather than 9, so no-useless-escape doesn’t make sense. The escape isn’t useless because the slash isn’t ignored.

I’m more of the mind that this should be a new rule rather than modifying an existing one. The fact that \8 and \9 look like octal escapes doesn’t seem like a good enough reason to add to no-octal-escape.

@mdjermanovic
Copy link
Member Author

I believe the latest spec https://tc39.es/ecma262 says that \9 in a string literal must be either a syntax error or produce value "9" as it already works in engines:

"\9" === "9" // true

Either way, this looks like a different category than just "useless" escape, and while I still think that no-octal-escape makes sense, a new rule would certainly avoid confusion and breaking changes 👍

Any ideas for the name, maybe no-decimal-escape?

@nzakas
Copy link
Member

nzakas commented Oct 9, 2020

I’d go no-nonoctal-escapes to keep the terminology consistent.

@mdjermanovic
Copy link
Member Author

no-nonoctal-escapes might sound like it would disallow all escapes that aren't octal, including hexadecimal and unicode escape sequences.

Maybe no-nonoctal-decimal-escape? That would fully match the name from the specification.

@nzakas
Copy link
Member

nzakas commented Oct 16, 2020

Works for me.

@mdjermanovic
Copy link
Member Author

Closing this in favor of #13765

@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Apr 15, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

2 participants