From d3a7b4f730f5929dcd782dca50a0167c54c8a9b7 Mon Sep 17 00:00:00 2001 From: Johannes Meier Date: Fri, 24 Oct 2025 09:46:46 +0200 Subject: [PATCH 1/2] validate that no return type is specified for fragments --- .../src/grammar/validation/validator.ts | 15 ++++- .../test/grammar/grammar-validator.test.ts | 61 +++++++++++++------ 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/packages/langium/src/grammar/validation/validator.ts b/packages/langium/src/grammar/validation/validator.ts index 3abcee773..9276e4db0 100644 --- a/packages/langium/src/grammar/validation/validator.ts +++ b/packages/langium/src/grammar/validation/validator.ts @@ -55,6 +55,7 @@ export function registerValidationChecks(services: LangiumGrammarServices): void ], ParserRule: [ validator.checkParserRuleDataType, + validator.checkFragmentKeywords, validator.checkRuleParameters, validator.checkEmptyParserRule, validator.checkParserRuleReservedName, @@ -971,11 +972,23 @@ export class LangiumGrammarValidator { const dataTypeRule = isDataTypeRule(rule); if (!hasDatatypeReturnType && dataTypeRule) { accept('error', 'This parser rule does not create an object. Add a primitive return type or an action to the start of the rule to force object instantiation.', { node: rule, property: 'name' }); - } else if (hasDatatypeReturnType && !dataTypeRule) { + } else if (hasDatatypeReturnType && !dataTypeRule && !rule.fragment) { // fragments are validated in the next check below accept('error', 'Normal parser rules are not allowed to return a primitive value. Use a datatype rule for that.', { node: rule, property: rule.dataType ? 'dataType' : 'returnType' }); } } + checkFragmentKeywords(rule: ast.ParserRule, accept: ValidationAcceptor): void { + if (rule.fragment) { + if (rule.dataType || rule.returnType || rule.inferredType) { + accept('error', + "Fragments assign values to the object of the caller, but don't create a new object themselves. Therefore specifying the type of the returned object is not possible.", + { node: rule, property: rule.dataType ? 'dataType' : rule.returnType ? 'returnType' : 'inferredType' } + ); + } + // `rule.entry` don't need to be checked, since the grammar allows either `rule.fragment` or `rule.entry`. + } + } + checkInfixRuleDataType(rule: ast.InfixRule, accept: ValidationAcceptor): void { if (rule.dataType) { accept('error', 'Infix rules are not allowed to return a primitive value.', { node: rule, property: 'dataType' }); diff --git a/packages/langium/test/grammar/grammar-validator.test.ts b/packages/langium/test/grammar/grammar-validator.test.ts index bef09d9dc..6c0db1191 100644 --- a/packages/langium/test/grammar/grammar-validator.test.ts +++ b/packages/langium/test/grammar/grammar-validator.test.ts @@ -6,6 +6,7 @@ import type { AstNode, Grammar, LangiumDocument, Properties } from 'langium'; import { AstUtils, EmptyFileSystem, GrammarAST, URI } from 'langium'; +import { expandToString } from 'langium/generate'; import { IssueCodes, createLangiumGrammarServices } from 'langium/grammar'; import type { ValidationResult } from 'langium/test'; import { clearDocuments, expectError, expectIssue, expectNoIssues, expectWarning, parseHelper, validationHelper } from 'langium/test'; @@ -322,22 +323,52 @@ describe('checkReferenceToRuleButNotType', () => { }); describe('Check Rule Fragment Validation', () => { - const grammar = ` - grammar g - type Type = Fragment; - fragment Fragment: name=ID; - terminal ID: /[_a-zA-Z][\\w_]*/; - `.trim(); + test('Fragment used in type definition', async () => { + const grammar = expandToString` + grammar g + type Type = Fragment; + fragment Fragment: name=ID; + terminal ID: /[_a-zA-Z][\\w_]*/; + `; + const validationResult = await validate(grammar); + const range = { start: { character: 12, line: 1 }, end: { character: 20, line: 1 } }; + expectError(validationResult, 'Cannot use rule fragments in types.', { range }); + }); - let validationResult: ValidationResult; + test('Fragment with defined data type', async () => { + const grammar = expandToString` + grammar G + entry R: r1=ID F; + fragment F returns string: r2=ID; + terminal ID: /[_a-zA-Z][\\w_]*/; + `; + const validationResult = await validate(grammar); + const range = { start: { character: 19, line: 2 }, end: { character: 25, line: 2 } }; + expectError(validationResult, "Fragments assign values to the object of the caller, but don't create a new object themselves. Therefore specifying the type of the returned object is not possible.", { range }); + }); - beforeAll(async () => { - validationResult = await validate(grammar); + test('Fragment with defined returnType', async () => { + const grammar = expandToString` + grammar G + entry R: r1=ID F; + fragment F returns R: r2=ID; + terminal ID: /[_a-zA-Z][\\w_]*/; + `; + const validationResult = await validate(grammar); + const range = { start: { character: 19, line: 2 }, end: { character: 20, line: 2 } }; + expectError(validationResult, "Fragments assign values to the object of the caller, but don't create a new object themselves. Therefore specifying the type of the returned object is not possible.", { range }); }); - test('Rule Fragment Validation', () => { - const range = { start: { character: 16, line: 1 }, end: { character: 24, line: 1 } }; - expectError(validationResult, 'Cannot use rule fragments in types.', { range }); + test('Fragment with defined inferredType', async () => { + const grammar = expandToString` + grammar G + entry R: r1=ID F; + fragment F infers R: r2=ID; + terminal ID: /[_a-zA-Z][\\w_]*/; + `; + const validationResult = await validate(grammar); + const range = { start: { character: 11, line: 2 }, end: { character: 19, line: 2 } }; + expectError(validationResult, "Fragments assign values to the object of the caller, but don't create a new object themselves. Therefore specifying the type of the returned object is not possible.", { range }); }); }); @@ -354,9 +385,6 @@ describe('Check cross-references to inferred types', () => { terminal ID: /[a-zA-Z_][a-zA-Z0-9_]*/; `.trim()); expectNoIssues(validationResult); - // expect(validationResult.diagnostics).toHaveLength(1); - // expect(validationResult.diagnostics[0].message).toBe('Cannot infer terminal or data type rule for cross-reference.'); - // expectError(validationResult, 'Cannot use rule fragments in types.'); }); test('infer in the parser rules body', async () => { @@ -371,9 +399,6 @@ describe('Check cross-references to inferred types', () => { terminal ID: /[a-zA-Z_][a-zA-Z0-9_]*/; `.trim()); expectNoIssues(validationResult); - // expect(validationResult.diagnostics).toHaveLength(1); - // expect(validationResult.diagnostics[0].message).toBe('Cannot infer terminal or data type rule for cross-reference.'); - // expectError(validationResult, 'Cannot use rule fragments in types.'); }); }); From 23da376a368be4404ca53d40834a50350291e77e Mon Sep 17 00:00:00 2001 From: Johannes Meier Date: Wed, 29 Oct 2025 15:26:29 +0100 Subject: [PATCH 2/2] shorter error message --- packages/langium/src/grammar/validation/validator.ts | 2 +- packages/langium/test/grammar/grammar-validator.test.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/langium/src/grammar/validation/validator.ts b/packages/langium/src/grammar/validation/validator.ts index 9276e4db0..a1a6d0f8f 100644 --- a/packages/langium/src/grammar/validation/validator.ts +++ b/packages/langium/src/grammar/validation/validator.ts @@ -981,7 +981,7 @@ export class LangiumGrammarValidator { if (rule.fragment) { if (rule.dataType || rule.returnType || rule.inferredType) { accept('error', - "Fragments assign values to the object of the caller, but don't create a new object themselves. Therefore specifying the type of the returned object is not possible.", + 'Fragment rules cannot specify a return type.', { node: rule, property: rule.dataType ? 'dataType' : rule.returnType ? 'returnType' : 'inferredType' } ); } diff --git a/packages/langium/test/grammar/grammar-validator.test.ts b/packages/langium/test/grammar/grammar-validator.test.ts index 6c0db1191..715b3f632 100644 --- a/packages/langium/test/grammar/grammar-validator.test.ts +++ b/packages/langium/test/grammar/grammar-validator.test.ts @@ -344,7 +344,7 @@ describe('Check Rule Fragment Validation', () => { `; const validationResult = await validate(grammar); const range = { start: { character: 19, line: 2 }, end: { character: 25, line: 2 } }; - expectError(validationResult, "Fragments assign values to the object of the caller, but don't create a new object themselves. Therefore specifying the type of the returned object is not possible.", { range }); + expectError(validationResult, 'Fragment rules cannot specify a return type.', { range }); }); test('Fragment with defined returnType', async () => { @@ -356,7 +356,7 @@ describe('Check Rule Fragment Validation', () => { `; const validationResult = await validate(grammar); const range = { start: { character: 19, line: 2 }, end: { character: 20, line: 2 } }; - expectError(validationResult, "Fragments assign values to the object of the caller, but don't create a new object themselves. Therefore specifying the type of the returned object is not possible.", { range }); + expectError(validationResult, 'Fragment rules cannot specify a return type.', { range }); }); test('Fragment with defined inferredType', async () => { @@ -368,7 +368,7 @@ describe('Check Rule Fragment Validation', () => { `; const validationResult = await validate(grammar); const range = { start: { character: 11, line: 2 }, end: { character: 19, line: 2 } }; - expectError(validationResult, "Fragments assign values to the object of the caller, but don't create a new object themselves. Therefore specifying the type of the returned object is not possible.", { range }); + expectError(validationResult, 'Fragment rules cannot specify a return type.', { range }); }); });