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

… parsing, but for cross-references eclipse-langium#1309
JohannesMeierSE · Feb 16, 2024 · dfa8aca · dfa8aca
1 parent e78aeba
commit dfa8aca
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 6 deletions.
diff --git a/packages/langium/src/grammar/validation/validator.ts b/packages/langium/src/grammar/validation/validator.ts
@@ -16,13 +16,14 @@ import { DiagnosticTag } from 'vscode-languageserver-types';
 import { getContainerOfType, streamAllContents } from '../../utils/ast-utils.js';
 import { MultiMap } from '../../utils/collections.js';
 import { toDocumentSegment } from '../../utils/cst-utils.js';
-import { findNameAssignment, findNodeForKeyword, findNodeForProperty, getAllReachableRules, isDataTypeRule, isOptionalCardinality, terminalRegex } from '../../utils/grammar-utils.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 '../../languages/generated/ast.js';
 import { getTypeNameWithoutError, hasDataTypeReturn, isPrimitiveGrammarType, isStringGrammarType, resolveImport, resolveTransitiveImports } from '../internal-grammar-util.js';
 import { typeDefinitionToPropertyType } from '../type-system/type-collector/declared-types.js';
 import { flattenPlainType, isPlainReferenceType } from '../type-system/type-collector/plain-types.js';
+import { isDataTypeRule, terminalRegex, isOptionalCardinality } from '../../utils/grammar-utils.js';
 
 export function registerValidationChecks(services: LangiumGrammarServices): void {
     const registry = services.validation.ValidationRegistry;
@@ -554,17 +555,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 (ast.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-utils.ts b/packages/langium/src/utils/grammar-utils.ts
@@ -68,6 +68,28 @@ 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)) {
+            // the cross-reference refers directly to a parser rule
+            if (ast.isParserRule(node.type.ref)) {
+                result.add(node.type.ref);
+            }
+            // the cross-reference refers to the explicitly inferred type of a parser rule
+            if (ast.isInferredType(node.type.ref) && ast.isParserRule(node.type.ref.$container)) {
+                result.add(node.type.ref.$container);
+            }
+        }
+    });
+    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
@@ -429,6 +429,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', () => {