Skip to content

Commit d83d32e

Browse files
authored
Fix false positive multiplicity validation (#1559)
1 parent eeb0707 commit d83d32e

File tree

6 files changed

+49
-22
lines changed

6 files changed

+49
-22
lines changed

package-lock.json

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/generator-langium/templates/core/.package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
"langium:watch": "langium generate --watch"
1616
},
1717
"dependencies": {
18-
"langium": "~3.1.0"
18+
"langium": "~3.1.1"
1919
},
2020
"devDependencies": {
2121
"@types/node": "^18.0.0",

packages/langium-vscode/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "langium-vscode",
33
"publisher": "langium",
4-
"version": "3.1.0",
4+
"version": "3.1.1",
55
"displayName": "Langium",
66
"description": "Support for the Langium Grammar Language",
77
"homepage": "https://langium.org",
@@ -95,7 +95,7 @@
9595
"lint": "eslint src --ext ts"
9696
},
9797
"dependencies": {
98-
"langium": "3.1.0",
98+
"langium": "3.1.1",
9999
"langium-railroad": "3.1.0",
100100
"vscode-languageserver": "~9.0.1",
101101
"ignore": "~5.2.4"

packages/langium/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "langium",
3-
"version": "3.1.0",
3+
"version": "3.1.1",
44
"description": "A language engineering tool for the Language Server Protocol",
55
"homepage": "https://langium.org",
66
"engines": {

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

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import * as ast from '../../languages/generated/ast.js';
2323
import { getTypeNameWithoutError, hasDataTypeReturn, isPrimitiveGrammarType, isStringGrammarType, resolveImport, resolveTransitiveImports } from '../internal-grammar-util.js';
2424
import { typeDefinitionToPropertyType } from '../type-system/type-collector/declared-types.js';
2525
import { flattenPlainType, isPlainReferenceType } from '../type-system/type-collector/plain-types.js';
26-
import { AstUtils } from '../../index.js';
2726

2827
export function registerValidationChecks(services: LangiumGrammarServices): void {
2928
const registry = services.validation.ValidationRegistry;
@@ -731,20 +730,26 @@ export class LangiumGrammarValidator {
731730
}
732731
return result;
733732
};
734-
// Locate called rule, ensure it is not a fragment.
735-
const refersToFragment = call.rule.ref !== undefined && call.rule.ref.fragment;
736-
// Data type rules do not cause problems, too.
737-
const callInDataTypeRule = (AstUtils.getContainerOfType(call, ast.isParserRule))?.dataType !== undefined;
738-
if (refersToFragment || callInDataTypeRule) {
733+
const ref = call.rule.ref;
734+
// Parsing an unassigned terminal rule is fine.
735+
if (!ref || ast.isTerminalRule(ref)) {
736+
return;
737+
}
738+
// Fragment or data type rules are fine too.
739+
if (ref.fragment || isDataTypeRule(ref)) {
740+
return;
741+
}
742+
// Multiple unassigned calls if inside a data type rule is fine too.
743+
const parentRule = getContainerOfType(call, ast.isParserRule);
744+
if (!parentRule || isDataTypeRule(parentRule)) {
739745
return;
740746
}
741747

742748
const appearsMultipleTimes = findContainerWithCardinality(call) !== undefined;
743-
const hasAssignment = AstUtils.getContainerOfType(call, ast.isAssignment) !== undefined;
749+
const hasAssignment = getContainerOfType(call, ast.isAssignment) !== undefined;
744750
if (appearsMultipleTimes && !hasAssignment) {
745-
accept('error', `Rule call ${call.rule.$refText} requires assignment when used with multiplicity.`, {
746-
node: call,
747-
property: 'cardinality'
751+
accept('error', `Rule call '${ref.name}' requires assignment when parsed multiple times.`, {
752+
node: call
748753
});
749754
}
750755
}

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

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,6 @@ describe('Langium grammar validation', () => {
156156
Plus+;
157157
List3:
158158
Exp+;
159-
// addresses https://github.com/eclipse-langium/langium/pull/1437#pullrequestreview-1994232830
160159
List4:
161160
elems += (Minus | Div);
162161
@@ -171,13 +170,36 @@ describe('Langium grammar validation', () => {
171170

172171
const validationResult = await validate(grammar);
173172
expect(validationResult.diagnostics).to.have.length(2);
174-
expectError(validationResult, 'Rule call Mult requires assignment when used with multiplicity.', {
175-
property: 'cardinality'
173+
expectError(validationResult, "Rule call 'Mult' requires assignment when parsed multiple times.", {
174+
property: undefined
176175
});
177-
expectError(validationResult, 'Rule call Plus requires assignment when used with multiplicity.', {
178-
property: 'cardinality'
176+
expectError(validationResult, "Rule call 'Plus' requires assignment when parsed multiple times.", {
177+
property: undefined
179178
});
180179
});
180+
181+
test('Rule calls with multiplicity - negative', async () => {
182+
const grammar = `
183+
grammar Test
184+
185+
entry Main:
186+
// Fragment rule
187+
Body*
188+
// Data type rule
189+
FQN*
190+
// Terminal rule
191+
ID*;
192+
fragment Body:
193+
value+='test';
194+
FQN returns string:
195+
ID ('.' ID)*;
196+
197+
terminal ID: /[_a-zA-Z][\\w_]*/;
198+
`.trim();
199+
200+
const validationResult = await validate(grammar);
201+
expectNoIssues(validationResult);
202+
});
181203
});
182204

183205
describe('Data type rule return type', () => {

0 commit comments

Comments
 (0)