False positives from no-octal-escape in regex literals #582

Closed
burnnat opened this Issue Feb 3, 2014 · 2 comments

Comments

Projects
None yet
3 participants
@burnnat
Contributor

burnnat commented Feb 3, 2014

Consider the following javascript:

var regex = /([abc]) \1/g;

Running ESLint on this sample code yields an error from the no-octal-escape rule:

sample.js: line 1, col 12, Error - Don't use octal: '\1'. Use '\u....' instead. (no-octal-escape)

1 problem

However, as the MDN explains, this isn't an octal escape in the regex literal, but rather a numeric backreference:

The '(foo)' and '(bar)' in the pattern /(foo) (bar) \1 \2/ match and remember the first two words in the string "foo bar foo bar". The \1 and \2 in the pattern match the string's last two words. Note that \1, \2, \n are used in the matching part of the regex. In the replacement part of a regex the syntax $1, $2, $n must be used, e.g.: 'bar foo'.replace( /(...) (...)/, '$2 $1' ).

As such, the no-octal-escape rule should not consider this an error.

@michaelficarra

This comment has been minimized.

Show comment Hide comment
@michaelficarra

michaelficarra Feb 3, 2014

Member

This rule was just meant to target strings. In the SpiderMonkey AST, they are both represented by the Literal type. We need to add an additional check that typeof node.value === "string".

Member

michaelficarra commented Feb 3, 2014

This rule was just meant to target strings. In the SpiderMonkey AST, they are both represented by the Literal type. We need to add an additional check that typeof node.value === "string".

SamuelEnglard added a commit to SamuelEnglard/eslint that referenced this issue Feb 4, 2014

@maxw3st

This comment has been minimized.

Show comment Hide comment
@maxw3st

maxw3st Feb 4, 2014

so, given the comment below this may be:
if( typeof node.value === "string" ) {
var regex = /([ abc ]) \1/g;
} else
return;

maybe?

maxw3st commented Feb 4, 2014

so, given the comment below this may be:
if( typeof node.value === "string" ) {
var regex = /([ abc ]) \1/g;
} else
return;

maybe?

SamuelEnglard added a commit to SamuelEnglard/eslint that referenced this issue Feb 4, 2014

@nzakas nzakas closed this in 5560029 Feb 4, 2014

nzakas added a commit that referenced this issue Feb 4, 2014

Merge pull request #583 from SamuelEnglard/master
Added check if node.value isn't a string just exit (fixes #582)

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.