no-regex-spaces: Add RegExp object form

[IanVS]

The docs for this rule specifically call out that the following will not be considered a warning:

/*eslint no-regex-spaces: 2*/

var re = new RegExp("foo   bar");

This seems to be a limitation, as it is not asserted in the tests, and intuitively it seems that the RegExp object format should be treated the same way as a literal, multiple spaces are equally hard to read in both.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[nzakas]

This rule is only designed to test regex literals. It's really hard to do anything for the constructor since you can pass non-literals or expressions.

[IanVS]

Wouldn't it at least be possible to check for spaces in literals within the constructor? Failing that, I think the docs could be clarified that it is a known limitation of the rule, rather than by-design.

[nzakas]

I'm not against adding it, just pointing out that this rule only checked literals by design. It's not a bug. It was intended to map to what JSLint and JSHint do, which only check literals. We can also check strings.

Failing that, I think the docs could be clarified that it is a known limitation of the rule, rather than by-design.

I have no idea what this means. :)

[IanVS]

From the rule docs:

The following patterns are considered warnings:

var re = /foo   bar/;

The following patterns are not warnings:

var re = new RegExp("foo   bar");

In most of the docs, the "not warnings" section is used to show how to "fix" the code so that the rule will no longer flag. So, what I'm proposing is:

The following patterns are considered warnings:

var re = /foo   bar/;

The following patterns are not warnings:

var re = /foo {3}bar/;

Known Limitations

This rule does not apply to the constructor form of RegExp, and therefore the following will not warn:

var re = new RegExp("foo   bar");

[nzakas]

"Not warnings" are definitely not "fixes", they are designed to show patterns that won't be flagged either because they are correct or they are not covered by the rule. They are provided to avoid confusion of people saying, "should this be flagged?"

[alberto]

@IanVS are you still interested in implementing this?

[IanVS]

Yeah, this dropped off my radar. I'll submit a docs PR within the next day or two.

[jacksonrayhamilton]

@IanVS, I'd also like to have this functionality. If you'd like, I could implement it as an enhancement to the rule.

I'm not entirely convinced the best option is always to convert a RegExp with a single string literal to a regular expression literal. For instance, one might have code where RegExp is used often to construct dynamic regular expressions, and it could look inconsistent to switch between the literal and constructor syntaxes. In this case, we wouldn't want to lose out on potential linting.

no-invalid-regexp indicates a willingness for core rules to handle this case, too. This could also be a matter of consistency.

[IanVS]

@jacksonrayhamilton Sorry, I guess I'm not exactly sure what enhancement you're proposing. Are you saying you want to teach no-regex-spaces to flag spaces in literals within RexExp constructors?

[jacksonrayhamilton]

Yep!

[IanVS]

Cool, I'd be all for that, and @nzakas has said

I'm not against adding it

So I'd say go for it. :)