Calling terminal fragment in another fragment does not work as expected

[luan-xiaokun]

NOTE: This title is kind of misleading. The problem here is not about calling fragment in another fragment, but just a subtle bug that some regexes are not surrounded by parenthesis in a mixed one.

Hey guys, I ran into a problem with the terminal fragment.

According to my tests, these two following ways to define DECFRAC do not have the same effects.

terminal fragment DECSEQ: /[1-9]\d*|0/;
terminal fragment DECFRAC: ((DECSEQ)? '.' /\d+/ | (DECSEQ) '.');
// vs
terminal fragment DECFRAC: /(([1-9]\d*|0)?\.\d+)|([1-9]\d*|0)\./;

The second one is just manually copying&pasting the regex and it has the desired behaviour, but the first one does not work as expected.

Am I missing something about the terminal fragment (like fragment is not supposed to be called in another fragment?), or is there something wrong with my code? As far as I understand, it works like copy and paste, except that it relieves the burden of adding parentheses manually.

By the way, when I use terminal fragment, the generator complains about never using this rule, it reports something like

src/language-server/mediator.langium:329:19 - This rule is declared but never referenced.

Do I need to do something to suppress such warnings, or this feature is not fully supported yet?

[msujew]

Hey @luan-xiaokun

Do I need to do something to suppress such warnings, or this feature is not fully supported yet?

It's supported, but the missing validation was a bug, see #633.

The second one is just manually copying&pasting the regex and it has the desired behaviour, but the first one does not work as expected.

So Chevrotain (our underlying parser library) is heavily reliant on the order of terminals. It will always perform a first match approach (i.e. find the first tokentype that matches the next characters) instead of a longest fit (as one might expect). So in your case, the DECSEQ terminal is always parsed first, even though DECFRAG might fit as well. To fix this, you'll just have to reverse their order:

terminal fragment DECFRAC: ((DECSEQ)? '.' /\d+/ | (DECSEQ) '.');
terminal fragment DECSEQ: /[1-9]\d*|0/;

[luan-xiaokun]

Reversing the order does not help. According to the document, the fragment terminals are not consumed by the lexer, so the order of fragments does not matter here, right?

Here is a more complete snippet of my terminal rules.

terminal FLOAT returns number: (DECFRAC) EXPONENT? | DECSEQ EXPONENT;
terminal INT returns number: BINSEQ | OCTSEQ | HEXSEQ | DECSEQ;
terminal fragment BINSEQ: /0[bB][01]+/;
terminal fragment OCTSEQ: /0[oO][0-7]+/;
terminal fragment HEXSEQ: /0[xX][\da-fA-F]+/;
terminal fragment DECFRAC: ((DECSEQ)? '.' /\d+/ | (DECSEQ) '.');
// terminal fragment DECFRAC: /(([1-9]\d*|0)?\.\d+)|([1-9]\d*|0)\./;
terminal fragment DECSEQ: /[1-9]\d*|0/;
terminal fragment EXPONENT: /[eE][+-]?\d+/;

and I have the following parse rule

PrimaryExpression infers Expression:
    Atom
    (
        ('[' {infer SliceExpression.primary=current} slice=Expression ']') |
        ('(' {infer CallExpression.primary=current} (arguments+=Expression ((',' arguments+=Expression)+ ','?)?)? ')') |
        ('.' {infer QualifiedExpression.primary=current} field=ID)
    )*;

Note that the terminal FLOAT comes before INT, so I guess that the parser fails to parse 3.e-02 as a float, and then fall backs to parse it as an int, and finally comes the syntax error.

[luan-xiaokun]

I can provide a snippet of the corresponding grammar.ts if necessary. Also, please let me know if there is any error in the code.

[msujew]

Ah, yeah now I see it. It's less about using a fragment within another fragment rule, but rather due to a bug which doesn't add parenthesis sometimes. Can you modify the DECSEQ regex to be sorrounded by parenthesis? terminal fragment DECSEQ: /([1-9]\d*|0)/;

[luan-xiaokun]

That solves the problem, thanks! Glad to catch this subtle bug with you!

[luan-xiaokun]

Just a reminder, when I use lookbehind to write the regex of DECFRAC, it pops up an error, and after a little bit searching, it seems that regexp-to-ast does not support lookbehind, bd82/regexp-to-ast#84 (comment), and the author does not plan to support it.