Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation for multiple assignments takes rewrite actions into account now #1766

Merged
merged 7 commits into from
Dec 11, 2024
Merged

Validation for multiple assignments takes rewrite actions into account now #1766

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
95c425d
improved error logging in test utils
JohannesMeierSE Dec 3, 2024
e411ba0
derived test cases from the discussion in #1756
JohannesMeierSE Dec 3, 2024
847aaa9
simplified and reworked the validation to support actions as top-leve…
JohannesMeierSE Dec 3, 2024
458400e
refactoring: inlined method
JohannesMeierSE Dec 3, 2024
6096fd1
improved error logging for some more test cases
JohannesMeierSE Dec 3, 2024
7ea4ea9
recursively take created objects in groups/alternatives into account,…
JohannesMeierSE Dec 3, 2024
f28181f
fixes according to the review
JohannesMeierSE Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 70 additions & 67 deletions packages/langium/src/grammar/validation/validator.ts
View file
Open in desktop
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], new Map(), 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[], map: Map<string, AssignmentUse>, accept: ValidationAcceptor): void {
JohannesMeierSE marked this conversation as resolved.
Show resolved Hide resolved
// 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), mapForNewObject, accept);
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)!
JohannesMeierSE marked this conversation as resolved.
Show resolved Hide resolved
}

// 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, 1 * currentMultiplicity, currentNode);
JohannesMeierSE marked this conversation as resolved.
Show resolved Hide resolved
}

// 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
Open in desktop
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
Loading