Repository: DmitrySoshnikov/regexp-tree
Pull request #103 (2017-11-14 08:12:19)

Optimizer: don't throw errors on empty groups
This PR prevents the optimizer from throwing errors when handling empty groups or empty parts:

Optimizing `/a(?:)b/` used to throw in the `ungroup` transform. Now it leaves the regexp as it is.
Optimizing `/a||/` used to throw in the `disjunction-remove-duplicates` transform. Now it handles it properly and optimizes as `/a|/`.
Optimizing `/a|b|/` used to throw in the `group-single-chars-to-char-class` transform. Now it leaves the regexp as it is.

In the last case, I wonder if `[ab]?` would be an appropriate optimization. What is your opinion on this @DmitrySoshnikov ?

Also, I intend to add a specific transform to handle empty (non-capturing) groups at some point.



Patch:
diff --git a/src/optimizer/transforms/disjunction-remove-duplicates-transform.js b/src/optimizer/transforms/disjunction-remove-duplicates-transform.js
index 759dc35..d1fbb68 100644
--- a/src/optimizer/transforms/disjunction-remove-duplicates-transform.js
+++ b/src/optimizer/transforms/disjunction-remove-duplicates-transform.js
@@ -25,7 +25,7 @@ module.exports = {
     const uniqueNodesMap = {};
 
     const parts = disjunctionToList(node).filter(part => {
-      const encoded = NodePath.getForNode(part).jsonEncode();
+      const encoded = part ? NodePath.getForNode(part).jsonEncode() : 'null';
 
       // Already recorded this part, filter out.
       if (uniqueNodesMap.hasOwnProperty(encoded)) {
diff --git a/src/optimizer/transforms/group-single-chars-to-char-class.js b/src/optimizer/transforms/group-single-chars-to-char-class.js
index 8169b8f..d539a35 100644
--- a/src/optimizer/transforms/group-single-chars-to-char-class.js
+++ b/src/optimizer/transforms/group-single-chars-to-char-class.js
@@ -60,6 +60,11 @@ const handlers = {
 };
 
 function shouldProcess(expression, charset) {
+  if (!expression) {
+    // Abort on empty disjunction part
+    return false;
+  }
+
   const {type} = expression;
 
   if (type === 'Disjunction') {
diff --git a/src/optimizer/transforms/ungroup-transform.js b/src/optimizer/transforms/ungroup-transform.js
index 490c423..11f8255 100644
--- a/src/optimizer/transforms/ungroup-transform.js
+++ b/src/optimizer/transforms/ungroup-transform.js
@@ -16,12 +16,11 @@ module.exports = {
     const {node, parent} = path;
     const childPath = path.getChild();
 
-    if (node.capturing) {
+    if (node.capturing || !childPath) {
       return;
     }
 
     if (
-      childPath &&
       childPath.node.type === 'Disjunction' &&
       parent.type !== 'RegExp'
     ) {


Test patch:
diff --git a/src/optimizer/transforms/__tests__/disjunction-remove-duplicates-transform-test.js b/src/optimizer/transforms/__tests__/disjunction-remove-duplicates-transform-test.js
index 538bd33..b4f9b4d 100644
--- a/src/optimizer/transforms/__tests__/disjunction-remove-duplicates-transform-test.js
+++ b/src/optimizer/transforms/__tests__/disjunction-remove-duplicates-transform-test.js
@@ -50,4 +50,11 @@ describe('(ab|bc|ab) -> (ab|bc)', () => {
     expect(re2.toString()).toBe('/(?:ab|bc)/');
   });
 
+  it('handles empty parts', () => {
+    const re = transform(/a|b|||/, [
+      disjunctionRemoveDuplicates
+    ]);
+    expect(re.toString()).toBe('/a|b|/');
+  });
+
 });
\ No newline at end of file
diff --git a/src/optimizer/transforms/__tests__/group-single-chars-to-char-class-test.js b/src/optimizer/transforms/__tests__/group-single-chars-to-char-class-test.js
index 2752a69..a4cf67c 100644
--- a/src/optimizer/transforms/__tests__/group-single-chars-to-char-class-test.js
+++ b/src/optimizer/transforms/__tests__/group-single-chars-to-char-class-test.js
@@ -59,4 +59,11 @@ describe('(a|b|c) -> ([abc])', () => {
     expect(re.toString()).toBe('/(a|b|no)/');
   });
 
+  it('has no effet on empty values', () => {
+    const re = transform(/(a|b|)/, [
+      singleCharsGroupToCharClass,
+    ]);
+    expect(re.toString()).toBe('/(a|b|)/');
+  });
+
 });
\ No newline at end of file
diff --git a/src/optimizer/transforms/__tests__/ungroup-transform-test.js b/src/optimizer/transforms/__tests__/ungroup-transform-test.js
index fe61afe..3d02d77 100644
--- a/src/optimizer/transforms/__tests__/ungroup-transform-test.js
+++ b/src/optimizer/transforms/__tests__/ungroup-transform-test.js
@@ -64,4 +64,11 @@ describe('ungroup', () => {
     expect(re.toString()).toBe('/(a)/');
   });
 
+  it('does not ungroup empty groups', () => {
+    const re = transform(/(?:)+/, [
+      ungroup,
+    ]);
+    expect(re.toString()).toBe('/(?:)+/');
+  });
+
 });
\ No newline at end of file