From e828ca6b12c434ae1ba9832781e3769d4ee86632 Mon Sep 17 00:00:00 2001 From: Johannes Meier Date: Tue, 12 Dec 2023 10:41:11 +0100 Subject: [PATCH] more accurate validation hint for parser rules which are not used for parsing, but for cross-references #1309 --- .../src/grammar/validation/validator.ts | 21 +++++++++---- packages/langium/src/utils/grammar-utils.ts | 22 +++++++++++++ .../test/grammar/grammar-validator.test.ts | 31 +++++++++++++++++++ 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/packages/langium/src/grammar/validation/validator.ts b/packages/langium/src/grammar/validation/validator.ts index cc5736de8..d52ec3c24 100644 --- 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 index 32d730527..3683d690e 100644 --- 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, 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 { + const result = new Set(); + 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 index a2cf26d12..f1a20b567 100644 --- 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', () => {