first working WIP version for a quick-fix for eclipse-langium#1309

packages/langium/src/grammar/lsp/grammar-code-actions.ts

@@ -10,7 +10,7 @@ import type { CodeAction, Command, Position, TextEdit } from 'vscode-languageser
 import type { URI } from '../../utils/uri-utils.js';
 import type { CodeActionProvider } from '../../lsp/code-action.js';
 import type { LangiumServices } from '../../lsp/lsp-services.js';
-import type { AstReflection, Reference, ReferenceInfo } from '../../syntax-tree.js';
+import type { AstNode, AstReflection, Reference, ReferenceInfo } from '../../syntax-tree.js';
 import type { MaybePromise } from '../../utils/promise-utils.js';
 import type { LinkingErrorData } from '../../validation/document-validator.js';
 import type { DiagnosticData } from '../../validation/validation-registry.js';
@@ -63,6 +63,9 @@ export class LangiumGrammarCodeActionProvider implements CodeActionProvider {
             case IssueCodes.CrossRefTokenSyntax:
                 accept(this.fixCrossRefSyntax(diagnostic, document));
                 break;
+            case IssueCodes.ParserRuleToTypeDecl:
+                accept(this.replaceParserRuleByTypeDeclaration(diagnostic, document));
+                break;
             case IssueCodes.UnnecessaryFileExtension:
                 accept(this.fixUnnecessaryFileExtension(diagnostic, document));
                 break;
@@ -180,6 +183,61 @@ export class LangiumGrammarCodeActionProvider implements CodeActionProvider {
         return undefined;
     }
 
+    private isReplaceableRule(rule: ast.ParserRule): boolean {
+        // OK are: definition use only Alternatives! infers! (TODO return ginge theoretisch auch)
+        return !rule.fragment && !rule.entry && rule.parameters.length === 0 && !rule.definesHiddenTokens && !rule.wildcard && !rule.returnType && !rule.dataType;
+    }
+    private replaceRule(rule: ast.ParserRule): string {
+        return rule.name; // TODO alternative Namen/Types berücksichtigen!
+    }
+    private isReplaceable(node: AstNode): node is ast.AbstractElement {
+        return (ast.isRuleCall(node) && node.arguments.length === 0 && ast.isParserRule(node.rule.ref) && this.isReplaceableRule(node.rule.ref))
+            || (ast.isAlternatives(node) && node.elements.every(child => this.isReplaceable(child)))
+            || (ast.isUnorderedGroup(node) && node.elements.every(child => this.isReplaceable(child)));
+    }
+    private replace(node: ast.AbstractElement): string {
+        if (ast.isRuleCall(node)) {
+            return this.replaceRule(node.rule.ref as ast.ParserRule);
+        }
+        if (ast.isAlternatives(node)) {
+            return node.elements.map(child => this.replace(child)).join(' | ');
+        }
+        if (ast.isUnorderedGroup(node)) {
+            return node.elements.map(child => this.replace(child)).join(' & '); // TODO
+        }
+        throw new Error('missing code for ' + node);
+    }
+
+    private replaceParserRuleByTypeDeclaration(diagnostic: Diagnostic, document: LangiumDocument): CodeAction | undefined {
+        const rootCst = document.parseResult.value.$cstNode;
+        if (rootCst) {
+            const offset = document.textDocument.offsetAt(diagnostic.range.start);
+            const cstNode = findLeafNodeAtOffset(rootCst, offset);
+            const rule = getContainerOfType(cstNode?.astNode, ast.isParserRule);
+            if (rule && rule.$cstNode) {
+                const isFixable = this.isReplaceableRule(rule) && this.isReplaceable(rule.definition);
+                if (isFixable) {
+                    const newText = `type ${this.replaceRule(rule)} = ${this.replace(rule.definition)};`;
+                    return {
+                        title: 'Replace parser rule by type declaration',
+                        kind: CodeActionKind.QuickFix,
+                        diagnostics: [diagnostic],
+                        isPreferred: true,
+                        edit: {
+                            changes: {
+                                [document.textDocument.uri]: [{
+                                    range: diagnostic.range,
+                                    newText
+                                }]
+                            }
+                        }
+                    };
+                }
+            }
+        }
+        return undefined;
+    }
+
     private fixUnnecessaryFileExtension(diagnostic: Diagnostic, document: LangiumDocument): CodeAction {
         const end = {...diagnostic.range.end};
         end.character -= 1;

packages/langium/src/grammar/validation/validator.ts

@@ -106,6 +106,7 @@ export namespace IssueCodes {
     export const UseRegexTokens = 'use-regex-tokens';
     export const EntryRuleTokenSyntax = 'entry-rule-token-syntax';
     export const CrossRefTokenSyntax = 'cross-ref-token-syntax';
+    export const ParserRuleToTypeDecl = 'parser-rule-to-type-decl';
     export const UnnecessaryFileExtension = 'unnecessary-file-extension';
     export const InvalidReturns = 'invalid-returns';
     export const InvalidInfers = 'invalid-infers';
@@ -612,7 +613,8 @@ export class LangiumGrammarValidator {
                 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'
+                        // property: 'name',
+                        data: diagnosticData(IssueCodes.ParserRuleToTypeDecl)
                     });
                 } else {
                     accept('hint', 'This rule is declared but never referenced.', {

packages/langium/test/grammar/grammar-validator.test.ts

@@ -8,10 +8,11 @@ import type { AstNode, Properties } from 'langium';
 import type { GrammarAST as GrammarTypes } from 'langium';
 import type { ValidationResult } from 'langium/test';
 import { afterEach, beforeAll, describe, expect, test } from 'vitest';
-import { DiagnosticSeverity } from 'vscode-languageserver';
+import { CodeAction, DiagnosticSeverity } from 'vscode-languageserver';
 import { AstUtils, EmptyFileSystem, GrammarAST } from 'langium';
 import { IssueCodes, createLangiumGrammarServices } from 'langium/grammar';
-import { clearDocuments, expectError, expectIssue, expectNoIssues, expectWarning, parseHelper, validationHelper } from 'langium/test';
+import { clearDocuments, expectError, expectIssue, expectNoIssues, expectWarning, parseHelper, textDocumentParams, validationHelper } from 'langium/test';
+import { TextDocument } from 'vscode-languageserver-textdocument';
 
 const services = createLangiumGrammarServices(EmptyFileSystem);
 const parse = parseHelper(services.grammar);
@@ -515,7 +516,7 @@ 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
+        grammar ParserRulesOnlyForCrossReferences
 
         entry Model:
             (persons+=Neighbor | friends+=Friend | greetings+=Greeting)*;
@@ -533,14 +534,59 @@ describe('Unused rules validation', () => {
         hidden terminal WS: /\\s+/;
         terminal ID: /[_a-zA-Z][\\w_]*/;
         `;
+        // check, that the expected validation hint is available
         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',
+            // property: 'name',
             severity: DiagnosticSeverity.Hint
         });
+        // check, that the quick-fix is generated
+        const actionProvider = services.grammar.lsp.CodeActionProvider;
+        expect(actionProvider).toBeTruthy();
+        const currentAcctions = await actionProvider!.getCodeActions(validation.document, {
+            ...textDocumentParams(validation.document),
+            range: validation.diagnostics[0].range,
+            context: {
+                diagnostics: validation.diagnostics,
+                triggerKind: 1 // explicitly triggered by users (or extensions)
+            }
+        });
+        // there is one quick-fix
+        expect(currentAcctions).toBeTruthy();
+        expect(Array.isArray(currentAcctions)).toBeTruthy();
+        expect(currentAcctions!.length).toBe(1);
+        expect(CodeAction.is(currentAcctions![0])).toBeTruthy();
+        const action: CodeAction = currentAcctions![0] as CodeAction;
+        // execute the found quick-fix
+        expect(action.title).toBe('Replace parser rule by type declaration');
+        const edits = action.edit?.changes![validation.document.textDocument.uri];
+        expect(edits).toBeTruthy();
+        const updatedText = TextDocument.applyEdits(validation.document.textDocument, edits!);
+
+        // check the result
+        const textExpected = `
+        grammar ParserRulesOnlyForCrossReferences
+
+        entry Model:
+            (persons+=Neighbor | friends+=Friend | greetings+=Greeting)*;
+
+        Neighbor:
+            'neighbor' name=ID;
+        Friend:
+            'friend' name=ID;
+
+        type 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_]*/;
+        `;
+        expect(updatedText).toBe(textExpected);
     });
 
 });