From 9f238d3c1453ef6fb681c2b64bc340227d8ec42e Mon Sep 17 00:00:00 2001 From: Johannes Meier Date: Tue, 12 Dec 2023 16:39:30 +0100 Subject: [PATCH] first working WIP version for a quick-fix for #1309 --- .../src/grammar/lsp/grammar-code-actions.ts | 60 ++++++++++++++++++- .../src/grammar/validation/validator.ts | 4 +- .../test/grammar/grammar-validator.test.ts | 54 +++++++++++++++-- 3 files changed, 112 insertions(+), 6 deletions(-) diff --git a/packages/langium/src/grammar/lsp/grammar-code-actions.ts b/packages/langium/src/grammar/lsp/grammar-code-actions.ts index c4baaddf9..e6785504e 100644 --- a/packages/langium/src/grammar/lsp/grammar-code-actions.ts +++ b/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; diff --git a/packages/langium/src/grammar/validation/validator.ts b/packages/langium/src/grammar/validation/validator.ts index d52ec3c24..12015d51b 100644 --- a/packages/langium/src/grammar/validation/validator.ts +++ b/packages/langium/src/grammar/validation/validator.ts @@ -104,6 +104,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'; @@ -565,7 +566,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.', { diff --git a/packages/langium/test/grammar/grammar-validator.test.ts b/packages/langium/test/grammar/grammar-validator.test.ts index f1a20b567..38dfb229f 100644 --- a/packages/langium/test/grammar/grammar-validator.test.ts +++ b/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); @@ -432,7 +433,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)*; @@ -450,14 +451,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); }); });