fixed bugs: circular calls of fragments, validate rewrite actions as starting points, more test cases

Author: JohannesMeierSE

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

@@ -13,7 +13,7 @@ import type { AstNode, Properties, Reference } from '../../syntax-tree.js';
 import { getContainerOfType, getDocument, streamAllContents } from '../../utils/ast-utils.js';
 import { MultiMap } from '../../utils/collections.js';
 import { toDocumentSegment } from '../../utils/cst-utils.js';
-import { assertUnreachable } from '../../utils/errors.js';
+import { assertCondition, assertUnreachable } from '../../utils/errors.js';
 import { findNameAssignment, findNodeForKeyword, findNodeForProperty, getAllReachableRules, getAllRulesUsedForCrossReferences, isArrayCardinality, isDataTypeRule, isOptionalCardinality, terminalRegex } from '../../utils/grammar-utils.js';
 import type { Stream } from '../../utils/stream.js';
 import { stream } from '../../utils/stream.js';
@@ -44,6 +44,7 @@ export function registerValidationChecks(services: LangiumGrammarServices): void
  const checks: ValidationChecks<ast.LangiumGrammarAstType> = {
  Action: [
  validator.checkAssignmentReservedName,
+ validator.checkActionAssignmentsToTheSameFeature,
  ],
  AbstractRule: validator.checkRuleName,
  Assignment: [
@@ -58,7 +59,7 @@ export function registerValidationChecks(services: LangiumGrammarServices): void
  validator.checkRuleParameters,
  validator.checkEmptyParserRule,
  validator.checkParserRuleReservedName,
- validator.checkAssignmentsToTheSameFeature,
+ validator.checkRuleAssignmentsToTheSameFeature,
  ],
  InfixRule: [
  validator.checkInfixRuleDataType,
@@ -1020,20 +1021,47 @@ export class LangiumGrammarValidator {
  }
 
  /**
- * This validation recursively collects all assignments (and rewriting actions) to the same feature and validates them:
+ * This validation recursively collects all assignments (and rewriting actions: see the check/entry point in the next function) to the same feature and validates them:
  * Assignment operators '?=', '=' and '+=' should not be mixed.
  * Assignments with '=' as assignment operator should be replaced be '+=', if assignments to the feature might occure more than once.
  */
- checkAssignmentsToTheSameFeature(rule: ast.ParserRule, accept: ValidationAcceptor): void {
- // for usual parser rules AND for fragments, but not for data type rules!
- if (!rule.dataType) {
- this.checkAssignmentsToTheSameFeatureIndependent(rule, [rule.definition], accept);
+ checkRuleAssignmentsToTheSameFeature(rule: ast.ParserRule, accept: ValidationAcceptor): void {
+ // validate only usual parser rules, but no fragments (they are validated when validating their using parser rules) and no data type rules!
+ if (!rule.dataType && !rule.fragment) {
+ this.checkAssignmentsToTheSameFeature(rule, [rule.definition], new Map(), accept);
  }
  }
 
- private checkAssignmentsToTheSameFeatureIndependent(startRule: ast.ParserRule|ast.Action, startNodes: AstNode[], accept: ValidationAcceptor, map: Map<string, AssignmentUse> = new Map()): void {
+ checkActionAssignmentsToTheSameFeature(action: ast.Action, accept: ValidationAcceptor): void {
+ if (action.feature) {
+ // tree rewriting action
+ const mapForNewObject = new Map();
+ storeAssignmentUse(mapForNewObject, action.feature, 1, action); // remember the special rewriting feature
+ // all following nodes are put into the new object => check their assignments independently
+ const sibblings = ast.isGroup(action.$container) || ast.isUnorderedGroup(action.$container) ? action.$container.elements : [action];
+ this.checkAssignmentsToTheSameFeature(action, sibblings.slice(sibblings.indexOf(action) + 1), mapForNewObject, accept);
+ } else {
+ // this action does not create a new AstNode => no assignments to check
+ }
+ }
+
+ private checkAssignmentsToTheSameFeature(
+ startRule: ast.ParserRule|ast.Action, startNodes: AstNode[], map: Map<string, AssignmentUse>, accept: ValidationAcceptor
+ ): void {
  // check all starting nodes
- this.checkAssignmentsToTheSameFeatureNested(startRule, startNodes, 1, map, accept);
+ const circularFragments = new Set<ast.ParserRule>();
+ this.checkAssignmentsToTheSameFeatureNested(startRule, startNodes, 1, map, [], circularFragments, accept);
+
+ // handle properties of fragments which are called in circular way => count its properties multiple times
+ for (const values of map.values()) {
+ for (const assignment of values.assignments) {
+ // for each found assignment, check whether it is defined inside a fragment which is called in circular way
+ const fragmentContainer = getContainerOfType(assignment, ast.isParserRule);
+ if (fragmentContainer && circularFragments.has(fragmentContainer)) {
+ values.counter += 1; // This calculation is not accurate, it is only important to know, that this assignment is called multiple times (>= 2).
+ }
+ }
+ }
 
  // create the warnings
  for (const entry of map.values()) {
@@ -1071,11 +1099,10 @@ export class LangiumGrammarValidator {
  }
  }
 
- const startDocument = getDocument(startRule);
  // Utility to handle the case, that the critical assignment is located in another document (for which validation markers cannot be reported now).
  function createInfo(assignment: ast.Assignment | ast.Action, code: string): [DiagnosticInfo<ast.Assignment | ast.Action | ast.ParserRule>, string] {
  const assignmentDocument = getDocument(assignment);
- if (assignmentDocument === startDocument) {
+ if (assignmentDocument === getDocument(startRule)) {
  // the assignment is located in the document of the start rule which is currently validated
  return [<DiagnosticInfo<ast.Assignment | ast.Action>>{
  node: assignment,
@@ -1093,23 +1120,18 @@ export class LangiumGrammarValidator {
  }
  }
 
- private checkAssignmentsToTheSameFeatureNested(startRule: ast.ParserRule|ast.Action, nodes: AstNode[], parentMultiplicity: number, map: Map<string, AssignmentUse>, accept: ValidationAcceptor): boolean {
+ private checkAssignmentsToTheSameFeatureNested(
+ startRule: ast.ParserRule|ast.Action, nodes: AstNode[], parentMultiplicity: number, map: Map<string, AssignmentUse>,
+ visiting: ast.ParserRule[], circularFragments: Set<ast.ParserRule>, accept: ValidationAcceptor
+ ): boolean {
  let resultCreatedNewObject = false;
  // check all given elements
- for (let i = 0; i < nodes.length; i++) {
- const currentNode = nodes[i];
+ for (const currentNode of nodes) {
 
  // 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
- if (getDocument(currentNode) === getDocument(startRule)) {
- this.checkAssignmentsToTheSameFeatureIndependent(currentNode, nodes.slice(i + 1), accept, mapForNewObject);
- } else {
- // if the rewrite action is inside another document, validate it, when its document is validated (otherwise, it is done twice and validation markers are added to the wrong document)
- }
+ // The assignments to AstNodes created by this action are validated by another validation check => nothing to do here
  resultCreatedNewObject = true;
  break; // breaks the current loop
  }
@@ -1130,15 +1152,27 @@ export class LangiumGrammarValidator {
  // 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.checkAssignmentsToTheSameFeatureNested(startRule, [currentNode.rule.ref.definition], currentMultiplicity, map, accept);
- resultCreatedNewObject = createdNewObject || resultCreatedNewObject;
+ const foundIndex = visiting.indexOf(currentNode.rule.ref);
+ if (foundIndex >= 0) {
+ // remember fragments which are called in circular way (their assignments will be counted more often later)
+ // all currently visited fragments are part of the circle/path
+ for (let i = foundIndex; i < visiting.length; i++) {
+ const circleParticipant = visiting[i];
+ circularFragments.add(circleParticipant);
+ }
+ } else {
+ visiting.push(currentNode.rule.ref);
+ const createdNewObject = this.checkAssignmentsToTheSameFeatureNested(startRule, [currentNode.rule.ref.definition], currentMultiplicity, map, visiting, circularFragments, accept);
+ resultCreatedNewObject = createdNewObject || resultCreatedNewObject;
+ assertCondition(visiting.pop() === currentNode.rule.ref); // prevent circles, but don't "cache" the results, since the fragment could be called multiple times in non-circular way
+ }
  }
 
  // 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.checkAssignmentsToTheSameFeatureNested(startRule, currentNode.elements, 1, mapGroup, accept);
+ const createdNewObject = this.checkAssignmentsToTheSameFeatureNested(startRule, currentNode.elements, 1, mapGroup, visiting, circularFragments, 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
@@ -1152,7 +1186,7 @@ export class LangiumGrammarValidator {
  let countCreatedObjects = 0;
  for (const alternative of currentNode.elements) {
  const mapCurrentAlternative: Map<string, AssignmentUse> = new Map();
- const createdNewObject = this.checkAssignmentsToTheSameFeatureNested(startRule, [alternative], 1, mapCurrentAlternative, accept);
+ const createdNewObject = this.checkAssignmentsToTheSameFeatureNested(startRule, [alternative], 1, mapCurrentAlternative, visiting, circularFragments, 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