more accurate validation hint for parser rules which are not used for…

… parsing, but for cross-references eclipse-langium#1309
JohannesMeierSE · Dec 12, 2023 · 1fcd7de · 1fcd7de
1 parent 750ff44
commit 1fcd7de
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 6 deletions.
diff --git a/packages/langium/src/grammar/validation/validator.ts b/packages/langium/src/grammar/validation/validator.ts
@@ -16,7 +16,7 @@ import { DiagnosticTag } from 'vscode-languageserver-types';
 import { getContainerOfType, streamAllContents } from '../../utils/ast-util.js';
 import { MultiMap } from '../../utils/collections.js';
 import { toDocumentSegment } from '../../utils/cst-util.js';
-import { findNameAssignment, findNodeForKeyword, findNodeForProperty, getAllReachableRules } from '../../utils/grammar-util.js';
+import { findNameAssignment, findNodeForKeyword, findNodeForProperty, getAllReachableRules, getAllRulesUsedForCrossReferences } from '../../utils/grammar-util.js';
 import { stream } from '../../utils/stream.js';
 import { diagnosticData } from '../../validation/validation-registry.js';
 import * as ast from '../generated/ast.js';
@@ -545,17 +545,25 @@ export class LangiumGrammarValidator {
 
     checkGrammarForUnusedRules(grammar: ast.Grammar, accept: ValidationAcceptor): void {
         const reachableRules = getAllReachableRules(grammar, true);
+        const parserRulesUsedByCrossReferences = getAllRulesUsedForCrossReferences(grammar);
 
         for (const rule of grammar.rules) {
             if (ast.isTerminalRule(rule) && rule.hidden || isEmptyRule(rule)) {
                 continue;
             }
             if (!reachableRules.has(rule)) {
-                accept('hint', 'This rule is declared but never referenced.', {
-                    node: rule,
-                    property: 'name',
-                    tags: [DiagnosticTag.Unnecessary]
-                });
+                if (isParserRule(rule) && parserRulesUsedByCrossReferences.has(rule)) {
+                    accept('hint', 'This parser rule is not used for parsing, but referenced by cross-references. Consider to replace this rule by a type declaration.', {
+                        node: rule,
+                        property: 'name'
+                    });
+                } else {
+                    accept('hint', 'This rule is declared but never referenced.', {
+                        node: rule,
+                        property: 'name',
+                        tags: [DiagnosticTag.Unnecessary]
+                    });
+                }
             }
         }
     }

diff --git a/packages/langium/src/utils/grammar-util.ts b/packages/langium/src/utils/grammar-util.ts
@@ -79,6 +79,21 @@ function ruleDfs(rule: ast.AbstractRule, visitedSet: Set<string>, allTerminals:
     });
 }
 
+/**
+ * Returns all parser rules which are used in the grammar as type in cross-references.
+ * @param grammar the grammar to investigate
+ * @returns the set of parser rules used as type in cross-references
+ */
+export function getAllRulesUsedForCrossReferences(grammar: ast.Grammar): Set<ast.ParserRule> {
+    const result = new Set<ast.ParserRule>();
+    streamAllContents(grammar).forEach(node => {
+        if (ast.isCrossReference(node) && ast.isParserRule(node.type.ref)) {
+            result.add(node.type.ref);
+        }
+    });
+    return result;
+}
+
 /**
  * Determines the grammar expression used to parse a cross-reference (usually a reference to a terminal rule).
  * A cross-reference can declare this expression explicitly in the form `[Type : Terminal]`, but if `Terminal`

diff --git a/packages/langium/test/grammar/grammar-validator.test.ts b/packages/langium/test/grammar/grammar-validator.test.ts
@@ -393,6 +393,37 @@ describe('Unused rules validation', () => {
         });
     });
 
+    test('Parser rules used only as type in cross-references are correctly identified as used', async () => {
+        // this test case targets https://github.com/eclipse-langium/langium/issues/1309
+        const text = `
+        grammar HelloWorld
+
+        entry Model:
+            (persons+=Neighbor | friends+=Friend | greetings+=Greeting)*;
+
+        Neighbor:
+            'neighbor' name=ID;
+        Friend:
+            'friend' name=ID;
+
+        Person: Neighbor | Friend; // 'Person' is used only for cross-references, not as parser rule
+
+        Greeting:
+            'Hello' person=[Person:ID] '!';
+
+        hidden terminal WS: /\\s+/;
+        terminal ID: /[_a-zA-Z][\\w_]*/;
+        `;
+        const validation = await validate(text);
+        expect(validation.diagnostics).toHaveLength(1);
+        const ruleWithHint = validation.document.parseResult.value.rules.find(e => e.name === 'Person')!;
+        expectIssue(validation, {
+            node: ruleWithHint,
+            property: 'name',
+            severity: DiagnosticSeverity.Hint
+        });
+    });
+
 });
 
 describe('Reserved names', () => {