apply review suggestions

Co-authored-by: Ashley Engelund (weedySeaDragon @ github) <[email protected]>
Signed-off-by: Reda Al Sulais <[email protected]>

packages/parser/src/language/common/tokenBuilder.ts

@@ -18,16 +18,28 @@ export abstract class AbstractMermaidTokenBuilder extends DefaultTokenBuilder {
     this.commentProvider = services.documentation.CommentProvider;
   }
 
+  // TODO: This responsibility might better belong in CommentProvider (e.g. AbstractMermaidCommentProvider that is a subclass of CommentProvider).
+  private ruleHasGreedyComment(rule: GrammarAST.AbstractRule): boolean {
+    const comment = this.commentProvider.getComment(rule);
+    return !!comment && /@greedy/.test(comment);
+  }
+
   protected override buildTerminalTokens(rules: Stream<GrammarAST.AbstractRule>): TokenType[] {
-    // put the greedy annotated terminal rules at the end of the array
-    const rulesArray = rules.toArray();
-    rules.forEach((rule, index) => {
-      const comment = this.commentProvider.getComment(rule);
-      if (comment && /@greedy/.test(comment)) {
-        rulesArray.push(rulesArray.splice(index, 1)[0]);
-      }
-    });
-    return super.buildTerminalTokens(stream(rulesArray));
+    if (rules.some((rule: GrammarAST.AbstractRule) => this.ruleHasGreedyComment(rule))) {
+      const notGreedyRules: GrammarAST.AbstractRule[] = [];
+      const lastRules: GrammarAST.AbstractRule[] = [];
+      // put terminal rules with @greedy in their comment at the end of the array
+      rules.forEach((rule) => {
+        if (this.ruleHasGreedyComment(rule)) {
+          lastRules.push(rule);
+        } else {
+          notGreedyRules.push(rule);
+        }
+      });
+      return super.buildTerminalTokens(stream([...notGreedyRules, ...lastRules]));
+    } else {
+      return super.buildTerminalTokens(rules);
+    }
   }
 
   protected override buildKeywordTokens(

packages/parser/src/language/sankey/sankey.langium

@@ -16,11 +16,12 @@ entry Sankey returns Sankey:
 ;
 
 SankeyLink:
-  source=SANKEY_LINK_NODE "," target=SANKEY_LINK_NODE "," value=SANKEY_LINK_VALUE EOL
+  source=SANKEY_LINK_NODE "," target=SANKEY_LINK_NODE ","
+  value=SANKEY_LINK_VALUE EOL
 ;
 
 terminal SANKEY_LINK_VALUE returns number: /"(0|[1-9][0-9]*)(\.[0-9]+)?"|[\t ]*(0|[1-9][0-9]*)(\.[0-9]+)?/;
 /**
- * @greedy
+ * @greedy This ensures that this rule is put at the bottom of the list of tokens.
  */
 terminal SANKEY_LINK_NODE: /sankey-link-node/;

packages/parser/tests/sankey-tokenBuilder.test.ts

@@ -0,0 +1,50 @@
+import { beforeAll, describe, expect, it } from 'vitest';
+import type { TokenType, TokenVocabulary } from 'chevrotain';
+
+import { sankeyServices } from './test-util.js';
+
+describe('SankeyTokenBuilder', () => {
+  describe('token order', () => {
+    let tokenVocab: TokenVocabulary;
+    let tokenVocabNames: string[];
+
+    beforeAll(() => {
+      // Get the ordered tokens (the vocabulary) from the grammar
+      tokenVocab = sankeyServices.parser.TokenBuilder.buildTokens(sankeyServices.Grammar, {
+        caseInsensitive: sankeyServices.LanguageMetaData.caseInsensitive,
+      });
+      // coerce the tokenVocab to a type that can use .map
+      tokenVocabNames = (tokenVocab as TokenType[]).map((tokenVocabEntry: TokenType) => {
+        return tokenVocabEntry.name;
+      });
+    });
+
+    it('whitespace is always first', () => {
+      expect(tokenVocabNames[0]).toEqual('WHITESPACE');
+    });
+    it('sankey-beta comes after whitespace', () => {
+      expect(tokenVocabNames[1]).toEqual('sankey-beta');
+    });
+
+    describe('terminal rules with @greedy in comments are put at the end of the ordered list of tokens', () => {
+      const NUM_EXPECTED_GREEDY_RULES = 2;
+
+      let greedyGroupStartIndex = 0;
+      beforeAll(() => {
+        greedyGroupStartIndex = tokenVocabNames.length - NUM_EXPECTED_GREEDY_RULES - 1;
+      });
+
+      it('SANKEY_LINK_NODE rule has @greedy so it is in the last group of all @greedy terminal rules', () => {
+        expect(tokenVocabNames.indexOf('SANKEY_LINK_NODE')).toBeGreaterThanOrEqual(
+          greedyGroupStartIndex
+        );
+      });
+
+      it('SANKEY_LINK_VALUE rule has @greedy so it is in the last group of all @greedy terminal rules', () => {
+        expect(tokenVocabNames.indexOf('SANKEY_LINK_VALUE')).toBeGreaterThanOrEqual(
+          greedyGroupStartIndex
+        );
+      });
+    });
+  });
+});

packages/parser/tests/test-util.ts

@@ -52,7 +52,7 @@ export function createPieTestServices() {
 }
 export const pieParse = createPieTestServices().parse;
 
-const sankeyServices: SankeyServices = createSankeyServices().Sankey;
+export const sankeyServices: SankeyServices = createSankeyServices().Sankey;
 const sankeyParser: LangiumParser = sankeyServices.parser.LangiumParser;
 export function createSankeyTestServices() {
   const parse = (input: string) => {