fix: apply review suggestions

eclipse-langium · Apr 29, 2024 · 90d6661 · 90d6661
1 parent 1f6b0da
commit 90d6661
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 25 deletions.
diff --git a/packages/langium/src/grammar/validation/validator.ts b/packages/langium/src/grammar/validation/validator.ts
@@ -414,7 +414,7 @@ export class LangiumGrammarValidator {
     }
 
     checkEmptyParserRule(parserRule: ast.ParserRule, accept: ValidationAcceptor): void {
-        // Rule body needs to be set.
+        // Rule body needs to be set;
         // Entry rules and fragments may consume no input.
         if (!parserRule.definition || parserRule.entry || parserRule.fragment) {
             return;
@@ -426,20 +426,15 @@ export class LangiumGrammarValidator {
             if (element.cardinality === '?' || element.cardinality === '*') {
                 return true;
             }
-            // Actions themselves count as non-optional.
+            // Actions themselves count as optional.
             if (ast.isAction(element)) {
-                return false;
-            }
-            // Assignments with ?= consume nothing, groups without + too.
-            if (ast.isAssignment(element)) {
-                return element.operator === '?=';
+                return true;
             }
             // Unordered groups consume something.
             if (ast.isUnorderedGroup(element)) {
                 return false;
             }
-            // Ordered groups consume if all elements do so;
-            // and alternative groups, too.
+            // Ordered groups consume if all elements do so, and alternative groups, too.
             if (ast.isGroup(element) || ast.isAlternatives(element)) {
                 return element.elements.every(mayConsumeEmpty);
             }
@@ -448,7 +443,7 @@ export class LangiumGrammarValidator {
 
         };
         if (mayConsumeEmpty(parserRule.definition)) {
-            accept('error', 'This parser rule would succeed without consuming input.', { node: parserRule, property: 'definition', code: IssueCodes.ParsingRuleEmpty });
+            accept('error', 'This parser rule would succeed without consuming input.', { node: parserRule, property: 'name', code: IssueCodes.ParsingRuleEmpty });
         }
     }
 

diff --git a/packages/langium/test/grammar/grammar-validator.test.ts b/packages/langium/test/grammar/grammar-validator.test.ts
@@ -958,8 +958,8 @@ describe('Prohibit empty parser rules', async () => {
 
         const validationResult = await validate(grammarWithMultiplicity);
         expect(validationResult.diagnostics).toHaveLength(2);
-        detectEmptyRule(validationResult, 6, 20, 6, 32);
-        detectEmptyRule(validationResult, 8, 23, 8, 39);
+        detectEmptyRule(validationResult, 6, 8, 6, 18);
+        detectEmptyRule(validationResult, 8, 8, 8, 21);
     });
 
     test('Optional assignments and cardinality', async () => {
@@ -975,13 +975,12 @@ describe('Prohibit empty parser rules', async () => {
         hidden terminal WS: /\\s+/;
         `;
         const validationResult = await validate(grammarWithOptionals);
-        expect(validationResult.diagnostics).toHaveLength(3);
-        detectEmptyRule(validationResult, 4, 11, 4, 18);
-        detectEmptyRule(validationResult, 5, 11, 5, 19);
-        detectEmptyRule(validationResult, 6, 11, 6, 18);
+        expect(validationResult.diagnostics).toHaveLength(2);
+        detectEmptyRule(validationResult, 5, 8, 5, 9);
+        detectEmptyRule(validationResult, 6, 8, 6, 9);
     });
 
-    test('Unordered groups', async () => {
+    test('Unordered groups and alternatives', async () => {
         const grammarWithGroups = `
         grammar GroupGrammars
 
@@ -1004,8 +1003,8 @@ describe('Prohibit empty parser rules', async () => {
         `;
         const validationResult = await validate(grammarWithGroups);
         expect(validationResult.diagnostics).toHaveLength(2);
-        detectEmptyRule(validationResult, 6, 12, 6, 57);
-        detectEmptyRule(validationResult, 14, 12, 14, 55);
+        detectEmptyRule(validationResult, 6, 8, 6, 10);
+        detectEmptyRule(validationResult, 14, 8, 14, 10);
     });
 
     test('Actions, guard conditions, fragments, data types', async () => {
@@ -1023,17 +1022,16 @@ describe('Prohibit empty parser rules', async () => {
         I returns string: '/' ID ('.' ID)*;
         
         J: Student | Teacher;
-        Teacher: 'Teacher' Name Tel;
+        Teacher: 'Teacher' Name;
         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(1);
-        detectEmptyRule(validationResult, 10, 26, 10, 35);
+        expect(validationResult.diagnostics).toHaveLength(2);
+        detectEmptyRule(validationResult, 6, 8, 6, 9);
+        detectEmptyRule(validationResult, 10, 8, 10, 9);
     });
 });
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: {B};
+        B returns B: 'B' {B};
 
         Test returns Test:
             value=B