Skip to content
This repository has been archived by the owner on Aug 18, 2021. It is now read-only.

Fix: Convert RegExpLieteral value to RegExp object (fixes #477) #478

Merged
merged 1 commit into from
May 30, 2017
Merged

Fix: Convert RegExpLieteral value to RegExp object (fixes #477) #478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion babylon-to-espree/toAST.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@ var astTransformVisitor = {
if (path.isRegExpLiteral()) {
node.type = "Literal";
node.raw = node.extra.raw;
node.value = {};
try {
node.value = new RegExp(node.pattern, node.flags);
} catch (err) {
node.value = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you give a scenario in tests that handles when the value is null? Also, perhaps we should have it default to {} as it was before? Otherwise this could be a breaking change right?

Copy link
Contributor Author

@soda0289 soda0289 May 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A scenario where value is null is when the flags u and y are not supported. I have tests cases for these and it will produce null when run in node v4 and v5.

I'm not sure this is a breaking change since the output is was producing is a bug and did not match what espree, or esprima produce. The reason the test pass is it only compared the typeof, which were both objects, but not from the same class. We could set the default to {} but that would not match the ESTree spec for RegExp Literal nodes.
https://github.com/estree/estree/blob/master/es5.md#regexpliteral

This does fix an issue with eslint eslint/eslint#8655 and setting the default to {} might cause other rule failures.

The estree plugin for babylon does produce the correct output so if this were to be considered a breaking change we could wait till that is implemented in this parser.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've convinced me. Thanks 😄

}
node.regex = {
pattern: node.pattern,
flags: node.flags
Expand Down
14 changes: 14 additions & 0 deletions test/babel-eslint.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ function assertImplementsAST(target, source, path) {
var typeB = source === null ? "null" : typeof source;
if (typeA !== typeB) {
error(`have different types (${typeA} !== ${typeB}) (${target} !== ${source})`);
} else if (typeA === "object" && ["RegExp"].indexOf(target.constructor.name) !== -1 && target.constructor.name !== source.constructor.name) {
error(`object have different constructors (${target.constructor.name} !== ${source.constructor.name}`);
} else if (typeA === "object") {
var keysTarget = Object.keys(target);
for (var i in keysTarget) {
Expand Down Expand Up @@ -305,6 +307,18 @@ describe("babylon-to-esprima", () => {
parseAndAssertSame("/affix-top|affix-bottom|affix|[a-z]/");
});

it("regexp", () => {
parseAndAssertSame("const foo = /foo/;");
});

it("regexp y flag", () => {
parseAndAssertSame("const foo = /foo/y;");
});

it("regexp u flag", () => {
parseAndAssertSame("const foo = /foo/u;");
});

it("regexp in a template string", () => {
parseAndAssertSame("`${/\\d/.exec(\"1\")[0]}`");
});
Expand Down