Avoid Recursive grammar rule calls

comunica · Jan 17, 2025 · b7fd73e · b7fd73e
1 parent 39be54e
commit b7fd73e
Show file tree

Hide file tree

Showing 24 changed files with 331 additions and 383 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -6,29 +6,29 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-#  test:
-#    strategy:
-#      fail-fast: false
-#      matrix:
-#        node-version:
-#          - 18.x
-#          - 20.x
-#          - 22.x
-#        os:
-#          - macos-latest
-#          - ubuntu-latest
-#          - windows-latest
-#    runs-on: ${{ matrix.os }}
-#    steps:
-#      - uses: actions/checkout@v4
-#      - name: Use Node.js ${{ matrix.node-version }}
-#        uses: actions/setup-node@v4
-#        with:
-#          node-version: ${{ matrix.node-version }}
-#      - run: yarn install --immutable
-#      - run: yarn build
-#      - run: yarn depcheck
-#      - run: yarn test
+  test:
+    strategy:
+      fail-fast: false
+      matrix:
+        node-version:
+          - 18.x
+          - 20.x
+          - 22.x
+        os:
+          - macos-latest
+          - ubuntu-latest
+          - windows-latest
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v4
+      - name: Use Node.js ${{ matrix.node-version }}
+        uses: actions/setup-node@v4
+        with:
+          node-version: ${{ matrix.node-version }}
+      - run: yarn install --immutable
+      - run: yarn build
+      - run: yarn depcheck
+      - run: yarn test
 
   spec:
     strategy:
@@ -42,9 +42,9 @@ jobs:
           - 22.x
         os:
           - macos-latest
-#          - ubuntu-latest
-#          - windows-latest
-#          - macos-13
+          - ubuntu-latest
+          - windows-latest
+          - macos-13
     runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v4
@@ -60,4 +60,4 @@ jobs:
           path: |
             .rdf-test-suite-cache
           key: rdftestsuite-${{ hashFiles('yarn.lock') }}
-      - run: node --max-old-space-size=500 ./node_modules/rdf-test-suite/bin/Runner.js engines/engine-sparql-1-1/spec/parser.cjs http://w3c.github.io/rdf-tests/sparql/sparql11/manifest-all.ttl -c ./.rdf-test-suite-cache/
+      - run: yarn ${{ matrix.spec }}
diff --git a/engines/engine-sparql-1-1/lib/Parser.ts b/engines/engine-sparql-1-1/lib/Parser.ts
@@ -26,7 +26,7 @@ import type * as RDF from '@rdfjs/types';
 // ```
 const queryOrUpdate: RuleDef<'queryOrUpdate', Query | Update | Pick<Update, 'base' | 'prefixes'>> = {
   name: 'queryOrUpdate',
-  impl: ({ ACTION, SUBRULE, OR1, OR2, CONSUME, OPTION1, OPTION2, context }) => () => {
+  impl: ({ ACTION, SUBRULE, SUBRULE2, OR1, OR2, CONSUME, OPTION1, OPTION2, MANY, context }) => () => {
     const prologueValues = SUBRULE(gram.prologue);
     return OR1<Query | Update | Pick<Update, 'base' | 'prefixes'>>([
       { ALT: () => {
@@ -45,60 +45,65 @@ const queryOrUpdate: RuleDef<'queryOrUpdate', Query | Update | Pick<Update, 'bas
         }));
       } },
       { ALT: () => {
+        // Prologue ( Update1 ( ';' Update )? )?
+        // Is equivalent to:
 
-        let result: Update | Pick<Update, 'base' | 'prefixes'> = prologueValues;
-        OPTION1(() => {
-          const updateOperation = SUBRULE(gram.update1);
+        let parsedPrologue = true;
+        const updateResult: Update = {
+          ...prologueValues,
+          type: 'update',
+          updates: [],
+        }
+        MANY({
+          GATE: () => parsedPrologue,
+          DEF: () => {
+            parsedPrologue = false;
+            const updateOperation = SUBRULE(gram.update1);
 
-          const recursiveRes = OPTION2(() => {
-            CONSUME(l.symbols.semi);
-            return SUBRULE(gram.update);
-          });
+            updateResult.updates.push(updateOperation);
 
-          return ACTION(() => {
-            const updateResult: Update = {
-              ...result,
-              type: 'update',
-              updates: [ updateOperation ],
-            };
-            if (recursiveRes) {
-              updateResult.updates.push(...recursiveRes.updates);
-              updateResult.base = recursiveRes.base ?? result.base;
-              updateResult.prefixes = recursiveRes.prefixes ?
-                  { ...result.prefixes, ...recursiveRes.prefixes } :
-                updateResult.prefixes;
-            }
-            result = updateResult;
-          });
+            OPTION1(() => {
+              CONSUME(l.symbols.semi);
+              const prologueValues = SUBRULE2(gram.prologue);
+
+              ACTION(() => {
+                updateResult.base = prologueValues.base ?? updateResult.base;
+                updateResult.prefixes = prologueValues.prefixes ?
+                  { ...updateResult.prefixes, ...prologueValues.prefixes } :
+                  updateResult.prefixes;
+              });
+
+              parsedPrologue = true;
+            })
+          }
         });
 
         ACTION(() => {
           const blankLabelsUsedInInsertData = new Set<string>();
-          if ('updates' in result) {
-            for (const updateOperation of result.updates) {
-              const iterBlankNodes = (callback: (blankNodeLabel: string) => void) => {
-                if ('updateType' in updateOperation && updateOperation.updateType === 'insert') {
-                  for (const quad of updateOperation.insert) {
-                    for (const triple of quad.triples) {
-                      for (const position of <const> ['subject', 'object']) {
-                        if (triple[position].termType === 'BlankNode') {
-                          callback(triple[position].value);
-                        }
+          for (const updateOperation of updateResult.updates) {
+            const iterBlankNodes = (callback: (blankNodeLabel: string) => void) => {
+              if ('updateType' in updateOperation && updateOperation.updateType === 'insert') {
+                for (const quad of updateOperation.insert) {
+                  for (const triple of quad.triples) {
+                    for (const position of <const> ['subject', 'object']) {
+                      if (triple[position].termType === 'BlankNode') {
+                        callback(triple[position].value);
                       }
                     }
                   }
                 }
               }
-              iterBlankNodes(label => {
-                if (blankLabelsUsedInInsertData.has(label)) {
-                  throw new Error('Detected reuse blank node across different INSERT DATA clauses');
-                }
-              });
-              iterBlankNodes(label => blankLabelsUsedInInsertData.add(label));
             }
+            iterBlankNodes(label => {
+              if (blankLabelsUsedInInsertData.has(label)) {
+                throw new Error('Detected reuse blank node across different INSERT DATA clauses');
+              }
+            });
+            iterBlankNodes(label => blankLabelsUsedInInsertData.add(label));
           }
         });
-        return result;
+
+        return updateResult.updates.length > 0 ? updateResult : prologueValues;
       } },
     ]);
   },

diff --git a/engines/engine-sparql-1-2/test/statics.test.ts b/engines/engine-sparql-1-2/test/statics.test.ts
@@ -40,6 +40,18 @@ describe('a SPARQL 1.2 parser', () => {
     }
   });
 
+  it(`should NOT parse $only thing}`, async ({expect}) => {
+    const query = `
+    PREFIX : <http://example.com/ns#>
+
+SELECT * WHERE {
+   <<( ?s ?p ?o )>> .
+}
+    `
+    parser._resetBlanks();
+    expect(() => parser.parse(query)).toThrow();
+  });
+
   describe('negative sparql 1.2', () => {
     for (const {name, statics} of [...negativeTest('sparql-1-2-invalid')]) {
       const parser = new Parser({prefixes: {ex: 'http://example.org/'}});

diff --git a/packages/core/lib/grammar-builder/parserBuilder.ts b/packages/core/lib/grammar-builder/parserBuilder.ts
@@ -56,7 +56,7 @@ export class Builder<Names extends string, RuleDefs extends RuleDefMap<Names>> {
   /**
    * Add a rule to the grammar. If the rule already exists, but the implementation differs, an error will be thrown.
    */
-  public addRuleRedundant<U extends string, RET, ARGS extends undefined[]>(rule: RuleDef<U, RET, ARGS>):
+  public addRuleRedundant<U extends string, RET, ARGS extends unknown[]>(rule: RuleDef<U, RET, ARGS>):
   Builder<Names | U, {[K in Names | U]: K extends U ? RuleDef<K, RET, ARGS> : ( K extends Names ? (RuleDefs[K] extends RuleDef<K> ? RuleDefs[K] : never ) : never) }> {
     const self = <Builder<Names | U, {[K in Names | U]: K extends U ? RuleDef<K, RET, ARGS> : ( K extends Names ? (RuleDefs[K] extends RuleDef<K> ? RuleDefs[K] : never ) : never) }>>
       <unknown> this;
@@ -156,7 +156,7 @@ export class Builder<Names extends string, RuleDefs extends RuleDefMap<Names>> {
     const selfSufficientParser: Partial<ParserFromRules<Names, RuleDefs>> = {};
     // eslint-disable-next-line ts/no-unnecessary-type-assertion
     for (const rule of <RuleDef<Names>[]> Object.values(this.rules)) {
-      selfSufficientParser[rule.name] = <any> ((input: string, ...args: unknown[]) => {
+      selfSufficientParser[rule.name] = <any> ((input: string, arg: unknown) => {
         // Transform input in accordance to 19.2
         input = input.replaceAll(
           /\\u([0-9a-fA-F]{4})|\\U([0-9a-fA-F]{8})/gu,
@@ -183,7 +183,7 @@ export class Builder<Names extends string, RuleDefs extends RuleDefMap<Names>> {
 
         parser.reset();
         parser.input = lexResult.tokens;
-        const result = parser[rule.name](...args);
+        const result = parser[rule.name](arg);
         if (parser.errors.length > 0) {
           // Console.log(lexResult.tokens);
           throw new Error(`Parse error on line ${parser.errors.map(x => x.token.startLine).join(', ')}
@@ -251,6 +251,11 @@ ${parser.errors.map(x => `${x.token.startLine}: ${x.message}`).join('\n')}`);
       }
 
       private getSelfRef(): CstDef {
+        const subRuleImpl = (subrule: typeof this.SUBRULE): CstDef['SUBRULE'] => {
+          return ((cstDef, ...args) => {
+            return subrule(<any> this[<keyof (typeof this)> cstDef.name], <any> { ARGS: args });
+          }) satisfies CstDef['SUBRULE'];
+        }
         return {
           CONSUME: (tokenType, option) => this.CONSUME(tokenType, option),
           CONSUME1: (tokenType, option) => this.CONSUME1(tokenType, option),
@@ -330,76 +335,16 @@ ${parser.errors.map(x => `${x.token.startLine}: ${x.message}`).join('\n')}`);
               throw error;
             }
           },
-          SUBRULE: (cstDef, ...args) => {
-            try {
-              return this.SUBRULE(<any> this[<keyof (typeof this)> cstDef.name], <any> { ARGS: args });
-            } catch (error: unknown) {
-              throw error;
-            }
-          },
-          SUBRULE1: (cstDef, ...args) => {
-            try {
-              return this.SUBRULE1(<any> this[<keyof (typeof this)> cstDef.name], <any> { ARGS: args });
-            } catch (error: unknown) {
-              throw error;
-            }
-          },
-          SUBRULE2: (cstDef, ...args) => {
-            try {
-              return this.SUBRULE2(<any> this[<keyof (typeof this)> cstDef.name], <any> { ARGS: args });
-            } catch (error: unknown) {
-              throw error;
-            }
-          },
-          SUBRULE3: (cstDef, ...args) => {
-            try {
-              return this.SUBRULE3(<any> this[<keyof (typeof this)> cstDef.name], <any> { ARGS: args });
-            } catch (error: unknown) {
-              throw error;
-            }
-          },
-          SUBRULE4: (cstDef, ...args) => {
-            try {
-              return this.SUBRULE4(<any> this[<keyof (typeof this)> cstDef.name], <any> { ARGS: args });
-            } catch (error: unknown) {
-              throw error;
-            }
-          },
-          SUBRULE5: (cstDef, ...args) => {
-            try {
-              return this.SUBRULE5(<any> this[<keyof (typeof this)> cstDef.name], <any> { ARGS: args });
-            } catch (error: unknown) {
-              throw error;
-            }
-          },
-          SUBRULE6: (cstDef, ...args) => {
-            try {
-              return this.SUBRULE6(<any> this[<keyof (typeof this)> cstDef.name], <any> { ARGS: args });
-            } catch (error: unknown) {
-              throw error;
-            }
-          },
-          SUBRULE7: (cstDef, ...args) => {
-            try {
-              return this.SUBRULE7(<any> this[<keyof (typeof this)> cstDef.name], <any> { ARGS: args });
-            } catch (error: unknown) {
-              throw error;
-            }
-          },
-          SUBRULE8: (cstDef, ...args) => {
-            try {
-              return this.SUBRULE8(<any> this[<keyof (typeof this)> cstDef.name], <any> { ARGS: args });
-            } catch (error: unknown) {
-              throw error;
-            }
-          },
-          SUBRULE9: (cstDef, ...args) => {
-            try {
-              return this.SUBRULE9(<any> this[<keyof (typeof this)> cstDef.name], <any> { ARGS: args });
-            } catch (error: unknown) {
-              throw error;
-            }
-          },
+          SUBRULE: subRuleImpl((rule, args) => this.SUBRULE(rule, args)),
+          SUBRULE1: subRuleImpl((rule, args) => this.SUBRULE1(rule, args)),
+          SUBRULE2: subRuleImpl((rule, args) => this.SUBRULE2(rule, args)),
+          SUBRULE3: subRuleImpl((rule, args) => this.SUBRULE3(rule, args)),
+          SUBRULE4: subRuleImpl((rule, args) => this.SUBRULE4(rule, args)),
+          SUBRULE5: subRuleImpl((rule, args) => this.SUBRULE5(rule, args)),
+          SUBRULE6: subRuleImpl((rule, args) => this.SUBRULE6(rule, args)),
+          SUBRULE7: subRuleImpl((rule, args) => this.SUBRULE7(rule, args)),
+          SUBRULE8: subRuleImpl((rule, args) => this.SUBRULE8(rule, args)),
+          SUBRULE9: subRuleImpl((rule, args) => this.SUBRULE9(rule, args)),
         };
       }
     }

diff --git a/packages/core/lib/grammar-builder/ruleDefTypes.ts b/packages/core/lib/grammar-builder/ruleDefTypes.ts
@@ -17,13 +17,13 @@ export type RuleDef<
   ParamType extends unknown[] = unknown[],
 > = {
   name: NameType;
-  impl: (def: ImplArgs) => (...args: ArrayElementsUndefinable<ParamType>) => ReturnType;
+  impl: (def: ImplArgs) => (...args: ArrayMagicWork<ParamType>) => ReturnType;
 };
 
 export type RuleDefReturn<T> = T extends RuleDef<any, infer Ret, any> ? Ret : never;
 
-type ArrayElementsUndefinable<ArrayType extends any[]> =
-  ArrayType extends [infer First, ...infer Rest] ? [First | undefined, ...ArrayElementsUndefinable<Rest>] : [];
+type ArrayMagicWork<ArrayType extends unknown[]> =
+  ArrayType extends [infer First, ...infer Rest] ? [First, ...ArrayMagicWork<Rest>] : [];
 
 export interface ImplArgs extends CstDef {
   cache: WeakMap<RuleDef, unknown>;

diff --git a/packages/rules-sparql-1-1/lib/Sparql11types.ts b/packages/rules-sparql-1-1/lib/Sparql11types.ts
@@ -18,9 +18,6 @@ export type Triple = {
   object: Term;
 };
 
-export type TripleCreatorS = (part: Pick<Triple, 'subject'>) => Triple;
-export type TripleCreatorSP = (part: Pick<Triple, 'subject' | 'predicate'>) => Triple;
-
 export interface IGraphNode {
   node: ITriplesNode['node'] | Term;
   triples: Triple[];

diff --git a/packages/rules-sparql-1-1/lib/grammar/general.ts b/packages/rules-sparql-1-1/lib/grammar/general.ts
@@ -74,23 +74,32 @@ export const prefixDecl: RuleDef<'prefixDecl', [string, string]> = <const> {
   },
 };
 
+
 /**
  * [[52]](https://www.w3.org/TR/sparql11-query/#rTriplesTemplate)
  */
 export const triplesTemplate: RuleDef<'triplesTemplate', Triple[]> = <const> {
   name: 'triplesTemplate',
-  impl: ({ SUBRULE, CONSUME, OPTION1, OPTION2 }) => () => {
-    const triples: Triple[][] = [];
+  impl: ({ ACTION, AT_LEAST_ONE, AT_LEAST_ONE_SEP, SUBRULE, CONSUME, OPTION }) => () => {
+    const triples: Triple[] = [];
 
-    triples.push(SUBRULE(triplesSameSubject));
-    OPTION1(() => {
-      CONSUME(l.symbols.dot);
-      OPTION2(() => {
-        triples.push(SUBRULE(triplesTemplate));
-      });
+    let parsedDot = true;
+    AT_LEAST_ONE({
+      GATE: () => parsedDot,
+      DEF: () => {
+        parsedDot = false;
+        const template = SUBRULE(triplesSameSubject)
+        ACTION(() => {
+          triples.push(...template);
+        });
+        OPTION(() => {
+          CONSUME(l.symbols.dot);
+          parsedDot = true;
+        });
+      }
     });
 
-    return triples.flat(1);
+    return triples;
   },
 };