Fix trivial parser removal

barsdeveloper · Nov 19, 2023 · 493cf46 · 493cf46
1 parent b4826f7
commit 493cf46
Show file tree

Hide file tree

Showing 9 changed files with 70 additions and 47 deletions.
diff --git a/dist/regexer.js b/dist/regexer.js
@@ -35,8 +35,8 @@ class Parser {
     }
 
     /**
-     * @template {Parser<any>[]} T
-     * @param {T} parsers
+     * @template {Parser<any>[]} P
+     * @param {P} parsers
      * @returns {Parser<any>}
      */
     wrap(...parsers) {
@@ -85,25 +85,35 @@ class Parser {
 
     /**
      * @param {Context} context
-     * @param {Parser<any>} other
+     * @param {Parser<any>} rhs
      * @param {Boolean} strict
      */
-    equals(context, other, strict) {
-        let self = /** @type {Parser<any>} */(this);
-        if (self === other) {
+    equals(context, rhs, strict) {
+        let lhs = /** @type {Parser<any>} */(this);
+        if (lhs === rhs) {
             return true
         }
         if (!strict) {
-            self = this.actualParser();
-            other = other.actualParser();
+            lhs = this.actualParser();
+            rhs = rhs.actualParser();
         }
-        let memoized = context.visited.get(self, other);
+        if (
+            rhs instanceof lhs.constructor && !(lhs instanceof rhs.constructor)
+            // @ts-expect-error
+            || rhs.resolve && !lhs.resolve
+        ) {
+            // Take advantage of polymorphism or compare a lazy against a non lazy (not the other way around)
+            const temp = lhs;
+            lhs = rhs;
+            rhs = temp;
+        }
+        let memoized = context.visited.get(lhs, rhs);
         if (memoized !== undefined) {
             return memoized
         } else if (memoized === undefined) {
-            context.visited.set(self, other, true);
-            memoized = self.doEquals(context, other, strict);
-            context.visited.set(self, other, memoized);
+            context.visited.set(lhs, rhs, true);
+            memoized = lhs.doEquals(context, rhs, strict);
+            context.visited.set(lhs, rhs, memoized);
         }
         return memoized
     }
@@ -877,7 +887,7 @@ class TimesParser extends Parser {
      * @param {P} parsers
      */
     wrap(...parsers) {
-        return new TimesParser(parsers[0], this.#min, this.#max)
+        return /** @type {TimesParser<typeof parsers[0]>} */(new TimesParser(parsers[0], this.#min, this.#max))
     }
 
     /**
@@ -1012,10 +1022,6 @@ class Regexer {
     static equals(lhs, rhs, strict = false) {
         const a = lhs.getParser();
         const b = rhs.getParser();
-        if (b instanceof a.constructor && !(a instanceof b.constructor)) {
-            // typeof b extends typeof a, invert to take advantage of polymorphism
-            return b.equals(Reply.makeContext(rhs), a, strict)
-        }
         return a.equals(Reply.makeContext(lhs), b, strict)
     }
 

diff --git a/dist/regexer.min.js b/dist/regexer.min.js
diff --git a/src/parser/Parser.js b/src/parser/Parser.js
@@ -35,8 +35,8 @@ export default class Parser {
     }
 
     /**
-     * @template {Parser<any>[]} T
-     * @param {T} parsers
+     * @template {Parser<any>[]} P
+     * @param {P} parsers
      * @returns {Parser<any>}
      */
     wrap(...parsers) {

diff --git a/src/parser/TimesParser.js b/src/parser/TimesParser.js
@@ -42,7 +42,7 @@ export default class TimesParser extends Parser {
      * @param {P} parsers
      */
     wrap(...parsers) {
-        return new TimesParser(parsers[0], this.#min, this.#max)
+        return /** @type {TimesParser<typeof parsers[0]>} */(new TimesParser(parsers[0], this.#min, this.#max))
     }
 
     /**

diff --git a/src/transformers/InlineParsersTransformer.js b/src/transformers/InlineParsersTransformer.js
@@ -17,10 +17,12 @@ export default class InlineParsersTransformer extends ParentChildTransformer {
      * @returns {Parser<any>[]}
      */
     doTransformChild(parent, child, index) {
-        return parent instanceof AlternativeParser && child instanceof AlternativeParser
+        if (parent instanceof AlternativeParser && child instanceof AlternativeParser
             || parent instanceof SequenceParser && child instanceof SequenceParser
-            ? child.unwrap()
-            : null
+        ) {
+            return child.unwrap()
+        }
+        return [child]
     }
 
 }
diff --git a/src/transformers/ParentChildTransformer.js b/src/transformers/ParentChildTransformer.js
@@ -38,15 +38,15 @@ export default class ParentChildTransformer extends Transformer {
                         /** @type {UnionFromArray<ChildTypes>} */(current),
                         i
                     )
-                    if (newParent) {
+                    if (newParent != parser) {
                         return this.doTransform(newParent)
                     }
                     const newChildren = this.doTransformChild(
                         /** @type {UnionFromArray<ParentTypes>} */(parser),
                         /** @type {UnionFromArray<ChildTypes>} */(current),
                         i
                     )
-                    if (newChildren) {
+                    if (newChildren.length !== 1 || newChildren[0] !== current) {
                         children.splice(
                             i,
                             1,
@@ -71,30 +71,32 @@ export default class ParentChildTransformer extends Transformer {
             })
         }
         if (changed) {
-            return parser.wrap(...children)
+            return this.doTransform(parser.wrap(...children))
         }
         return parser
     }
 
     /**
+     * Replace the parent parser with another parser
      * @protected
      * @param {UnionFromArray<ParentTypes>} parent
      * @param {UnionFromArray<ChildTypes>} child
      * @param {Number} index
      * @returns {Parser<any>?}
      */
     doTransformParent(parent, child, index) {
-        return null
+        return parent
     }
 
     /**
+     * Replace the given child with other children
      * @protected
      * @param {UnionFromArray<ParentTypes>} parent
      * @param {UnionFromArray<ChildTypes>} child
      * @param {Number} index
      * @returns {Parser<any>[]?}
      */
     doTransformChild(parent, child, index) {
-        return null
+        return [child]
     }
 }
diff --git a/src/transformers/RemoveDiscardedMapTransformer.js b/src/transformers/RemoveDiscardedMapTransformer.js
@@ -20,6 +20,7 @@ export default class RemoveDiscardedMapTransformer extends ParentChildTransforme
         if (parent instanceof MapParser && child instanceof LookaroundParser) {
             return parent.parser.withActualParser(child, this.traverse, this.opaque)
         }
+        return parent
     }
 
     /**
@@ -32,5 +33,6 @@ export default class RemoveDiscardedMapTransformer extends ParentChildTransforme
         if (parent instanceof LookaroundParser && child instanceof MapParser) {
             return [child.parser]
         }
+        return [child]
     }
 }
diff --git a/src/transformers/RemoveTrivialParsersTransformer.js b/src/transformers/RemoveTrivialParsersTransformer.js
@@ -1,19 +1,22 @@
 import AlternativeParser from "../parser/AlternativeParser.js"
+import CapturingGroupParser from "../parser/CapturingGroupParser.js"
 import FailureParser from "../parser/FailureParser.js"
+import NonCapturingGroupParser from "../parser/NonCapturingGroupParser.js"
 import ParentChildTransformer from "./ParentChildTransformer.js"
 import SequenceParser from "../parser/SequenceParser.js"
 import SuccessParser from "../parser/SuccessParser.js"
+import TimesParser from "../parser/TimesParser.js"
 
-/** @extends {ParentChildTransformer<[AlternativeParser, SequenceParser], [SuccessParser, FailureParser]>} */
+/** @extends {ParentChildTransformer<[AlternativeParser, SequenceParser, TimesParser, CapturingGroupParser, NonCapturingGroupParser], [SuccessParser, FailureParser]>} */
 export default class RemoveTrivialParsersTransformer extends ParentChildTransformer {
 
     constructor() {
-        super([AlternativeParser, SequenceParser], [SuccessParser, FailureParser])
+        super([AlternativeParser, SequenceParser, TimesParser, CapturingGroupParser, NonCapturingGroupParser], [SuccessParser, FailureParser])
     }
 
     /**
      * @protected
-     * @param {AlternativeParser<Parser<any>[]> | SequenceParser<Parser<any>[]>} parent
+     * @param {AlternativeParser<Parser<any>[]> | SequenceParser<Parser<any>[]> | TimesParser<Parser<any>> | CapturingGroupParser | NonCapturingGroupParser} parent
      * @param {SuccessParser | FailureParser} child
      * @param {Number} index
      * @returns {Parser<any>?}
@@ -22,32 +25,28 @@ export default class RemoveTrivialParsersTransformer extends ParentChildTransfor
         if (parent instanceof AlternativeParser && child instanceof SuccessParser) {
             return parent.wrap(...parent.parsers.slice(0, index))
         }
-        if (
-            parent instanceof AlternativeParser && child instanceof FailureParser
-            || parent instanceof SequenceParser && child instanceof SuccessParser
-        ) {
-            const children = parent.unwrap()
-            children.splice(index, 1)
-            return parent.wrap(...children)
-        }
         if (parent instanceof SequenceParser && child instanceof FailureParser) {
-            return new FailureParser()
+            return child
+        }
+        if (parent instanceof TimesParser || parent instanceof CapturingGroupParser || parent instanceof NonCapturingGroupParser) {
+            return child
         }
+        return parent
     }
 
     /**
      * @protected
-     * @param {AlternativeParser<Parser<any>[]> | SequenceParser<Parser<any>[]>} parent
+     * @param {AlternativeParser<Parser<any>[]> | SequenceParser<Parser<any>[]> | TimesParser<Parser<any>> | CapturingGroupParser | NonCapturingGroupParser} parent
      * @param {SuccessParser | FailureParser} child
      * @param {Number} index
      * @returns {Parser<any>[]}
      */
     doTransformChild(parent, child, index) {
-        if (parent instanceof AlternativeParser && child instanceof FailureParser) {
-            return []
-        }
-        if (parent instanceof SequenceParser && child instanceof SuccessParser) {
-            return []
+        if (
+            parent instanceof AlternativeParser && child instanceof FailureParser
+            || parent instanceof SequenceParser && child instanceof SuccessParser
+        ) {
+            return [] // Remove the child from the list
         }
     }
 

diff --git a/tests/transformers.spec.js b/tests/transformers.spec.js
@@ -431,6 +431,18 @@ test("Remove trivial parsers 4", ({ page }) => {
     ).toBeTruthy()
 })
 
+test("Remove trivial parsers 5", ({ page }) => {
+    const removeTrivialParsers = new RemoveTrivialParsersTransformer()
+    expect(
+        R.equals(
+            removeTrivialParsers.transform(
+                R.seq(R.number, R.grp(R.nonGrp(R.success()).map(f1).atLeast(20)).map(f3), R.str("a"))
+            ),
+            R.seq(R.number, R.str("a"))
+        )
+    ).toBeTruthy()
+})
+
 // test("Test 1", ({ page }) => {
 //     expect(R.equals(
 //         transformer.transform(