Skip to content

Commit

Permalink
Validation for multiple assignments takes rewrite actions into accoun…
Browse files Browse the repository at this point in the history 
…t now (#1766)

* recursively take created objects in groups/alternatives into account, added some more specific test cases
* simplified and reworked the validation to support actions as top-level elements in parser rules as well (was another bug)
* derived test cases from the discussion in #1756
* refactoring: inlined method
* improved error logging
  • Loading branch information
@JohannesMeierSE
JohannesMeierSE authored Dec 11, 2024
1 parent 88f176c commit e004619
Show file tree
Hide file tree
Showing 3 changed files with 190 additions and 85 deletions.
137 changes: 70 additions & 67 deletions packages/langium/src/grammar/validation/validator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -877,18 +877,13 @@ export class LangiumGrammarValidator {
checkOperatorMultiplicitiesForMultiAssignments(rule: ast.ParserRule, accept: ValidationAcceptor): void {
// for usual parser rules AND for fragments, but not for data type rules!
if (!rule.dataType) {
this.checkOperatorMultiplicitiesForMultiAssignmentsLogic([rule.definition], accept);
this.checkOperatorMultiplicitiesForMultiAssignmentsIndependent([rule.definition], accept);
}
}

private checkOperatorMultiplicitiesForMultiAssignmentsLogic(startNodes: AstNode[], accept: ValidationAcceptor): void {
// new map to store usage information of the assignments
const map: Map<string, AssignmentUse> = new Map();

// top-down traversal for all starting nodes
for (const node of startNodes) {
this.checkAssignmentNumbersForNode(node, 1, map, accept);
}
private checkOperatorMultiplicitiesForMultiAssignmentsIndependent(startNodes: AstNode[], accept: ValidationAcceptor, map: Map<string, AssignmentUse> = new Map()): void {
// check all starting nodes
this.checkOperatorMultiplicitiesForMultiAssignmentsNested(startNodes, 1, map, accept);

// create the warnings
for (const entry of map.values()) {
Expand All @@ -906,72 +901,79 @@ export class LangiumGrammarValidator {
}
}

private checkAssignmentNumbersForNode(currentNode: AstNode, parentMultiplicity: number, map: Map<string, AssignmentUse>, accept: ValidationAcceptor) {
// the current element can occur multiple times => its assignments can occur multiple times as well
let currentMultiplicity = parentMultiplicity;
if (ast.isAbstractElement(currentNode) && isArrayCardinality(currentNode.cardinality)) {
currentMultiplicity *= 2; // note, that the result is not exact (but it is sufficient for the current case)!
}
private checkOperatorMultiplicitiesForMultiAssignmentsNested(nodes: AstNode[], parentMultiplicity: number, map: Map<string, AssignmentUse>, accept: ValidationAcceptor): boolean {
let resultCreatedNewObject = false;
// check all given elements
for (let i = 0; i < nodes.length; i++) {
const currentNode = nodes[i];

// Tree-Rewrite-Actions are a special case: a new object is created => following assignments are put into the new object
if (ast.isAction(currentNode) && currentNode.feature) {
// (This does NOT count for unassigned actions, i.e. actions without feature name, since they change only the type of the current object.)
const mapForNewObject = new Map();
storeAssignmentUse(mapForNewObject, currentNode.feature, 1, currentNode); // remember the special rewriting feature
// all following nodes are put into the new object => check their assignments independently
this.checkOperatorMultiplicitiesForMultiAssignmentsIndependent(nodes.slice(i + 1), accept, mapForNewObject);
resultCreatedNewObject = true;
break; // breaks the current loop
}

// assignment
if (ast.isAssignment(currentNode)) {
storeAssignmentUse(map, currentNode.feature, 1 * currentMultiplicity, currentNode);
}
// all other elements don't create new objects themselves:

// Search for assignments in used fragments as well, since their property values are stored in the current object.
// But do not search in calls of regular parser rules, since parser rules create new objects.
if (ast.isRuleCall(currentNode) && ast.isParserRule(currentNode.rule.ref) && currentNode.rule.ref.fragment) {
this.checkAssignmentNumbersForNode(currentNode.rule.ref.definition, currentMultiplicity, map, accept);
}
// the current element can occur multiple times => its assignments can occur multiple times as well
let currentMultiplicity = parentMultiplicity;
if (ast.isAbstractElement(currentNode) && isArrayCardinality(currentNode.cardinality)) {
currentMultiplicity *= 2; // note that the result is not exact (but it is sufficient for the current use case)!
}

// rewriting actions are a special case for assignments
if (ast.isAction(currentNode) && currentNode.feature) {
storeAssignmentUse(map, currentNode.feature, 1 * currentMultiplicity, currentNode);
}
// assignment
if (ast.isAssignment(currentNode)) {
storeAssignmentUse(map, currentNode.feature, currentMultiplicity, currentNode);
}

// look for assignments to the same feature nested within groups
if (ast.isGroup(currentNode) || ast.isUnorderedGroup(currentNode) || ast.isAlternatives(currentNode)) {
const mapAllAlternatives: Map<string, AssignmentUse> = new Map(); // store assignments for Alternatives separately
let nodesForNewObject: AstNode[] = [];
// check all elements inside the current group
for (const child of currentNode.elements) {
if (ast.isAction(child)) {
// Actions are a special case: a new object is created => following assignments are put into the new object
// (This counts for rewriting actions as well as for unassigned actions, i.e. actions without feature name)
if (nodesForNewObject.length > 0) {
// all collected nodes are put into the new object => check their assignments independently
this.checkOperatorMultiplicitiesForMultiAssignmentsLogic(nodesForNewObject, accept);
// is it possible to have two or more Actions within the same parser rule? the grammar allows that ...
nodesForNewObject = [];
}
// push the current node into a new object
nodesForNewObject.push(child);
} else {
// for non-Actions
if (nodesForNewObject.length > 0) {
// nodes go into a new object
nodesForNewObject.push(child);
} else {
// count the relevant child assignments
if (ast.isAlternatives(currentNode)) {
// for alternatives, only a single alternative is used => assume the worst case and take the maximum number of assignments
const mapCurrentAlternative: Map<string, AssignmentUse> = new Map();
this.checkAssignmentNumbersForNode(child, currentMultiplicity, mapCurrentAlternative, accept);
mergeAssignmentUse(mapCurrentAlternative, mapAllAlternatives, (s, t) => Math.max(s, t));
} else {
// all members of the group are relavant => collect them all
this.checkAssignmentNumbersForNode(child, currentMultiplicity, map, accept);
}
// Search for assignments in used fragments as well, since their property values are stored in the current object.
// But do not search in calls of regular parser rules, since parser rules create new objects.
if (ast.isRuleCall(currentNode) && ast.isParserRule(currentNode.rule.ref) && currentNode.rule.ref.fragment) {
const createdNewObject = this.checkOperatorMultiplicitiesForMultiAssignmentsNested([currentNode.rule.ref.definition], currentMultiplicity, map, accept);
resultCreatedNewObject = createdNewObject || resultCreatedNewObject;
}

// look for assignments to the same feature nested within groups
if (ast.isGroup(currentNode) || ast.isUnorderedGroup(currentNode)) {
// all members of the group are relavant => collect them all
const mapGroup: Map<string, AssignmentUse> = new Map(); // store assignments for Alternatives separately
const createdNewObject = this.checkOperatorMultiplicitiesForMultiAssignmentsNested(currentNode.elements, 1, mapGroup, accept);
mergeAssignmentUse(mapGroup, map, createdNewObject
? (s, t) => (s + t) // if a new object is created in the group: ignore the current multiplicity, since a new object is created for each loop cycle!
: (s, t) => (s * currentMultiplicity + t) // otherwise as usual: take the current multiplicity into account
);
resultCreatedNewObject = createdNewObject || resultCreatedNewObject;
}

// for alternatives, only a single alternative is used => assume the worst case and take the maximum number of assignments
if (ast.isAlternatives(currentNode)) {
const mapAllAlternatives: Map<string, AssignmentUse> = new Map(); // store assignments for Alternatives separately
let countCreatedObjects = 0;
for (const child of currentNode.elements) {
const mapCurrentAlternative: Map<string, AssignmentUse> = new Map();
const createdNewObject = this.checkOperatorMultiplicitiesForMultiAssignmentsNested([child], 1, mapCurrentAlternative, accept);
mergeAssignmentUse(mapCurrentAlternative, mapAllAlternatives, createdNewObject
? (s, t) => Math.max(s, t) // if a new object is created in an alternative: ignore the current multiplicity, since a new object is created for each loop cycle!
: (s, t) => Math.max(s * currentMultiplicity, t) // otherwise as usual: take the current multiplicity into account
);
if (createdNewObject) {
countCreatedObjects++;
}
}
}
// merge alternatives
mergeAssignmentUse(mapAllAlternatives, map);
if (nodesForNewObject.length >= 1) {
// these nodes are put into a new object => check their assignments independently
this.checkOperatorMultiplicitiesForMultiAssignmentsLogic(nodesForNewObject, accept);
// merge alternatives
mergeAssignmentUse(mapAllAlternatives, map);
// since we assume the worst case, we define, that the entire Alternatives node created a new object, if ALL its alternatives created a new object
if (countCreatedObjects === currentNode.elements.length) {
resultCreatedNewObject = true;
}
}
}
return resultCreatedNewObject; // indicates, whether a new object was created
}

checkInterfacePropertyTypes(interfaceDecl: ast.Interface, accept: ValidationAcceptor): void {
Expand Down Expand Up @@ -1243,6 +1245,7 @@ function mergeAssignmentUse(mapSoure: Map<string, AssignmentUse>, mapTarget: Map
target.counter = counterOperation(source.counter, target.counter);
} else {
mapTarget.set(key, source);
source.counter = counterOperation(source.counter, 0);
}
}
mapSoure.clear();
Expand Down
28 changes: 16 additions & 12 deletions packages/langium/src/test/langium-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,24 @@
* terms of the MIT License, which is available in the project root.
******************************************************************************/

import * as assert from 'node:assert';
import type { CompletionItem, CompletionList, Diagnostic, DocumentSymbol, FoldingRange, FormattingOptions, Range, ReferenceParams, SemanticTokensParams, SemanticTokenTypes, TextDocumentIdentifier, TextDocumentPositionParams, WorkspaceSymbol } from 'vscode-languageserver-protocol';
import { DiagnosticSeverity, MarkupContent } from 'vscode-languageserver-types';
import { normalizeEOL } from '../generate/template-string.js';
import type { LangiumServices, LangiumSharedLSPServices } from '../lsp/lsp-services.js';
import { SemanticTokensDecoder } from '../lsp/semantic-token-provider.js';
import type { ParserOptions } from '../parser/langium-parser.js';
import type { LangiumCoreServices, LangiumSharedCoreServices } from '../services.js';
import type { AstNode, CstNode, Properties } from '../syntax-tree.js';
import { type LangiumDocument, TextDocument } from '../workspace/documents.js';
import type { BuildOptions } from '../workspace/document-builder.js';
import type { AsyncDisposable } from '../utils/disposable.js';
import type { LangiumServices, LangiumSharedLSPServices } from '../lsp/lsp-services.js';
import type { ParserOptions } from '../parser/langium-parser.js';
import { DiagnosticSeverity, MarkupContent } from 'vscode-languageserver-types';
import { escapeRegExp } from '../utils/regexp-utils.js';
import { URI } from '../utils/uri-utils.js';
import { Disposable } from '../utils/disposable.js';
import { findNodeForProperty } from '../utils/grammar-utils.js';
import { SemanticTokensDecoder } from '../lsp/semantic-token-provider.js';
import * as assert from 'node:assert';
import { escapeRegExp } from '../utils/regexp-utils.js';
import { stream } from '../utils/stream.js';
import { Disposable } from '../utils/disposable.js';
import { normalizeEOL } from '../generate/template-string.js';
import { URI } from '../utils/uri-utils.js';
import { DocumentValidator } from '../validation/document-validator.js';
import type { BuildOptions } from '../workspace/document-builder.js';
import { TextDocument, type LangiumDocument } from '../workspace/documents.js';

export interface ParseHelperOptions extends BuildOptions {
/**
Expand Down Expand Up @@ -682,7 +682,7 @@ export function filterByOptions<T extends AstNode = AstNode, N extends AstNode =

export function expectNoIssues<T extends AstNode = AstNode, N extends AstNode = AstNode>(validationResult: ValidationResult<T>, filterOptions?: ExpectDiagnosticOptions<N>): void {
const filtered = filterOptions ? filterByOptions<T, N>(validationResult, filterOptions) : validationResult.diagnostics;
expectedFunction(filtered.length, 0, `Expected no issues, but found ${filtered.length}`);
expectedFunction(filtered.length, 0, `Expected no issues, but found ${filtered.length}:\n${printDiagnostics(filtered)}`);
}

export function expectIssue<T extends AstNode = AstNode, N extends AstNode = AstNode>(validationResult: ValidationResult<T>, filterOptions?: ExpectDiagnosticOptions<N>): void {
Expand Down Expand Up @@ -711,6 +711,10 @@ export function expectWarning<T extends AstNode = AstNode, N extends AstNode = A
});
}

export function printDiagnostics(diagnostics: Diagnostic[] | undefined): string {
return diagnostics?.map(d => `line ${d.range.start.line}, column ${d.range.start.character}: ${d.message}`).join('\n') ?? '';
}

/**
* Add the given document to the `TextDocuments` service, simulating it being opened in an editor.
*
Expand Down
Loading

0 comments on commit e004619

Please sign in to comment.