fix: false positives

Actions counts as non-empty. Fragments may be empty, Alternatives now consume nothing if all options do, too.
eclipse-langium · Apr 19, 2024 · 1f6b0da · 1f6b0da
1 parent 1e04feb
commit 1f6b0da
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 18 deletions.
diff --git a/packages/langium/src/grammar/validation/validator.ts b/packages/langium/src/grammar/validation/validator.ts
@@ -415,8 +415,8 @@ export class LangiumGrammarValidator {
 
     checkEmptyParserRule(parserRule: ast.ParserRule, accept: ValidationAcceptor): void {
         // Rule body needs to be set.
-        // Entry rules may consume no input.
-        if (!parserRule.definition || parserRule.entry) {
+        // Entry rules and fragments may consume no input.
+        if (!parserRule.definition || parserRule.entry || parserRule.fragment) {
             return;
         }
 
@@ -426,9 +426,9 @@ export class LangiumGrammarValidator {
             if (element.cardinality === '?' || element.cardinality === '*') {
                 return true;
             }
-            // Actions themselves consume nothing.
+            // Actions themselves count as non-optional.
             if (ast.isAction(element)) {
-                return true;
+                return false;
             }
             // Assignments with ?= consume nothing, groups without + too.
             if (ast.isAssignment(element)) {
@@ -438,12 +438,9 @@ export class LangiumGrammarValidator {
             if (ast.isUnorderedGroup(element)) {
                 return false;
             }
-            // Alternatives consume nothing if one alternative does,
-            // Ordered groups if all elements consume nothing.
-            if (ast.isAlternatives(element)) {
-                return element.elements.some(mayConsumeEmpty);
-            }
-            if (ast.isGroup(element)) {
+            // Ordered groups consume if all elements do so;
+            // and alternative groups, too.
+            if (ast.isGroup(element) || ast.isAlternatives(element)) {
                 return element.elements.every(mayConsumeEmpty);
             }
             // Else, assert that we consume _something_.

diff --git a/packages/langium/test/grammar/grammar-validator.test.ts b/packages/langium/test/grammar/grammar-validator.test.ts
@@ -1003,9 +1003,8 @@ describe('Prohibit empty parser rules', async () => {
         hidden terminal WS: /\\s+/;
         `;
         const validationResult = await validate(grammarWithGroups);
-        expect(validationResult.diagnostics).toHaveLength(3);
+        expect(validationResult.diagnostics).toHaveLength(2);
         detectEmptyRule(validationResult, 6, 12, 6, 57);
-        detectEmptyRule(validationResult, 13, 12, 13, 50);
         detectEmptyRule(validationResult, 14, 12, 14, 55);
     });
 
@@ -1024,18 +1023,17 @@ describe('Prohibit empty parser rules', async () => {
         I returns string: '/' ID ('.' ID)*;
         
         J: Student | Teacher;
-        Teacher: 'Teacher' Name;
+        Teacher: 'Teacher' Name Tel;
         Student: 'Student' Name;
         fragment Name: name=ID?;
+        fragment Tel: 'telephone' ':' number=NUM;
 
         terminal ID: /[a-zA-Z]+/;
+        terminal NUM: /[0-9]+/;
         hidden terminal WS: /\\s+/;
         `;
         const validationResult = await validate(specialGrammar);
-        expect(validationResult.diagnostics).toHaveLength(4);
-        detectEmptyRule(validationResult, 6, 11, 6, 18);
-        detectEmptyRule(validationResult, 7, 17, 7, 59);
+        expect(validationResult.diagnostics).toHaveLength(1);
         detectEmptyRule(validationResult, 10, 26, 10, 35);
-        detectEmptyRule(validationResult, 16, 23, 16, 31);
     });
 });
diff --git a/packages/langium/test/grammar/type-system/type-validator.test.ts b/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: 'BRuleForBInterface' {B};
+        B returns B: {B};
 
         Test returns Test:
             value=B