Show warnings on optional parser rules (#1459)

Co-authored-by: Mark Sujew <mark.sujew@typefox.io>

packages/langium/src/grammar/validation/validator.ts

@@ -42,6 +42,7 @@ export function registerValidationChecks(services: LangiumGrammarServices): void
         ParserRule: [
             validator.checkParserRuleDataType,
             validator.checkRuleParametersUsed,
+            validator.checkEmptyParserRule,
             validator.checkParserRuleReservedName,
         ],
         TerminalRule: [
@@ -112,6 +113,7 @@ export namespace IssueCodes {
     export const MissingReturns = 'missing-returns';
     export const SuperfluousInfer = 'superfluous-infer';
     export const OptionalUnorderedGroup = 'optional-unordered-group';
+    export const ParsingRuleEmpty = 'parsing-rule-empty';
 }
 
 export class LangiumGrammarValidator {
@@ -411,6 +413,47 @@ export class LangiumGrammarValidator {
         }
     }
 
+    checkEmptyParserRule(parserRule: ast.ParserRule, accept: ValidationAcceptor): void {
+        // Rule body needs to be set;
+        // Entry rules and fragments may consume no input.
+        if (!parserRule.definition || parserRule.entry || parserRule.fragment) {
+            return;
+        }
+
+        const consumesAnything = (element: ast.AbstractElement): boolean => {
+            // First, check cardinality of the element.
+            if (element.cardinality === '?' || element.cardinality === '*') {
+                return false;
+            }
+            // Actions themselves count as optional.
+            if (ast.isAction(element)) {
+                return false;
+            }
+            // Unordered groups act as alternatives surrounded by `*`
+            if (ast.isUnorderedGroup(element)) {
+                return false;
+            }
+            // Only one element of the group needs to consume something
+            if (ast.isGroup(element)) {
+                return element.elements.some(consumesAnything);
+            }
+            // Every altneratives needs to consume something
+            if (ast.isAlternatives(element)) {
+                return element.elements.every(consumesAnything);
+            }
+            // If the element is a direct rule call
+            // We need to check whether the element consumes anything
+            if (ast.isRuleCall(element) && element.rule.ref?.definition) {
+                return consumesAnything(element.rule.ref.definition);
+            }
+            // Else, assert that we consume something.
+            return true;
+        };
+        if (!consumesAnything(parserRule.definition)) {
+            accept('warning', 'This parser rule potentially consumes no input.', { node: parserRule, property: 'name', code: IssueCodes.ParsingRuleEmpty });
+        }
+    }
+
     checkInvalidRegexFlags(token: ast.RegexToken, accept: ValidationAcceptor): void {
         const regex = token.regex;
         if (regex) {

packages/langium/test/grammar/grammar-validator.test.ts

@@ -912,3 +912,108 @@ describe('Cross-reference to type union is only valid if all alternatives are AS
         expectNoIssues(validationResult);
     });
 });
+
+describe('Prohibit empty parser rules', async () => {
+
+    function detectEmptyRules(validationResult: ValidationResult<GrammarTypes.Grammar>, ...ruleNames: string[]) {
+        for (const rule of validationResult.document.parseResult.value.rules) {
+            if (GrammarAST.isParserRule(rule)) {
+                if (ruleNames.includes(rule.name)) {
+                    expectWarning(validationResult, 'This parser rule potentially consumes no input.', {
+                        node: rule, property: 'name'
+                    });
+                } else {
+                    expectNoIssues(validationResult, {
+                        node: rule, property: 'name', message: 'This parser rule potentially consumes no input.'
+                    });
+                }
+            }
+        }
+    }
+
+    test('Keywords, Rule calls, cross references', async () => {
+        const grammarWithoutOptionals = `
+        Name: name=ID;
+        Ref: 'Hello' 'to' who=[Name:ID];
+        Call: Name;
+        Word returns string: 'word';
+
+        terminal ID: /[a-zA-Z]/+;
+        hidden terminal WS: /\\s+/;`;
+        const validationResult = await validate(grammarWithoutOptionals);
+        detectEmptyRules(validationResult);
+    });
+
+    test('Optional cardinality', async () => {
+        const grammarWithOptionals = `
+        B: (x=ID);
+        C: (x=ID)?;
+        D: (x=ID)*;
+
+        terminal ID: /[a-zA-Z]+/;
+        `;
+        const validationResult = await validate(grammarWithOptionals);
+        detectEmptyRules(validationResult, 'C', 'D');
+    });
+
+    test('Alternatives', async () => {
+        const grammarWithGroups = `
+        A: (name=ID  | 'test');
+        B: (name=ID? | 'test');
+        C: (name=ID  | 'test')?;
+
+        terminal ID: /[a-zA-Z]+/;
+        `;
+        const validationResult = await validate(grammarWithGroups);
+        expect(validationResult.diagnostics).toHaveLength(2);
+        detectEmptyRules(validationResult, 'B', 'C');
+    });
+
+    test('Groups', async () => {
+        const grammarWithGroups = `
+        A: name=ID 'test';
+        B: name=ID? 'test';
+        C: name=ID? 'test'?;
+
+        terminal ID: /[a-zA-Z]+/;
+        `;
+        const validationResult = await validate(grammarWithGroups);
+        detectEmptyRules(validationResult, 'C');
+    });
+
+    test('Unordered Groups', async () => {
+        const grammarWithGroups = `
+        A: (name=ID & 'test');
+
+        terminal ID: /[a-zA-Z]+/;
+        `;
+        const validationResult = await validate(grammarWithGroups);
+        detectEmptyRules(validationResult, 'A');
+    });
+
+    test('Actions', async () => {
+        const specialGrammar = `
+        A: {infer Empty};
+        B: {infer Value} name=ID?;
+        C: {infer Value} name=ID;
+
+        terminal ID: /[a-zA-Z]+/;
+        `;
+        const validationResult = await validate(specialGrammar);
+        detectEmptyRules(validationResult, 'A', 'B');
+    });
+
+    test('Fragments and entry rules', async () => {
+        const specialGrammar = `
+        grammar Test
+        entry Rule: items+=Item*;
+        Item: name=ID Content;
+        EmptyItem: Content;
+        fragment Content: value=ID?;
+
+        terminal ID: /[a-zA-Z]+/;
+        `;
+        const validationResult = await validate(specialGrammar);
+        detectEmptyRules(validationResult, 'EmptyItem');
+    });
+});

packages/langium/test/grammar/type-system/type-validator.test.ts

@@ -724,7 +724,7 @@ describe('Property types validation takes in account types hierarchy', () => {
             value: A;
         }
 
-        B returns B: {B};
+        B returns B: 'B' {B};
 
         Test returns Test:
             value=B