Fix references in fragment rules

packages/langium/src/parser/cst-node-builder.ts

@@ -18,7 +18,7 @@ export class CstNodeBuilder {
     private nodeStack: CompositeCstNodeImpl[] = [];
 
     private get current(): CompositeCstNodeImpl {
-        return this.nodeStack[this.nodeStack.length - 1];
+        return this.nodeStack[this.nodeStack.length - 1] ?? this.rootNode;
     }
 
     buildRootNode(input: string): RootCstNode {

packages/langium/src/parser/langium-parser.ts

@@ -94,7 +94,7 @@ export interface BaseParser {
      * Requires a unique index within the rule for a specific sub rule.
      * Arguments can be supplied to the rule invocation for semantic predicates
      */
-    subrule(idx: number, rule: RuleResult, feature: AbstractElement, args: Args): void;
+    subrule(idx: number, rule: RuleResult, fragment: boolean, feature: AbstractElement, args: Args): void;
     /**
      * Executes a grammar action that modifies the currently active AST node
      */
@@ -161,7 +161,7 @@ export abstract class AbstractLangiumParser implements BaseParser {
 
     abstract rule(rule: ParserRule, impl: RuleImpl): RuleResult;
     abstract consume(idx: number, tokenType: TokenType, feature: AbstractElement): void;
-    abstract subrule(idx: number, rule: RuleResult, feature: AbstractElement, args: Args): void;
+    abstract subrule(idx: number, rule: RuleResult, fragment: boolean, feature: AbstractElement, args: Args): void;
     abstract action($type: string, action: Action): void;
     abstract construct(): unknown;
 
@@ -251,7 +251,9 @@ export class LangiumParser extends AbstractLangiumParser {
 
     private startImplementation($type: string | symbol | undefined, implementation: RuleImpl): RuleImpl {
         return (args) => {
-            if (!this.isRecording()) {
+            // Only create a new AST node in case the calling rule is not a fragment rule
+            const createNode = $type !== undefined;
+            if (!this.isRecording() && createNode) {
                 const node: any = { $type };
                 this.stack.push(node);
                 if ($type === DatatypeSymbol) {
@@ -264,7 +266,7 @@ export class LangiumParser extends AbstractLangiumParser {
             } catch (err) {
                 result = undefined;
             }
-            if (!this.isRecording() && result === undefined) {
+            if (!this.isRecording() && result === undefined && createNode) {
                 result = this.construct();
             }
             return result;
@@ -300,9 +302,11 @@ export class LangiumParser extends AbstractLangiumParser {
         return !token.isInsertedInRecovery && !isNaN(token.startOffset) && typeof token.endOffset === 'number' && !isNaN(token.endOffset);
     }
 
-    subrule(idx: number, rule: RuleResult, feature: AbstractElement, args: Args): void {
+    subrule(idx: number, rule: RuleResult, fragment: boolean, feature: AbstractElement, args: Args): void {
         let cstNode: CompositeCstNode | undefined;
-        if (!this.isRecording()) {
+        if (!this.isRecording() && !fragment) {
+            // If the called rule is a fragment rule, we just add all parsed CST nodes to the already existing CST node
+            // Note that this also skips the subrule assignment later on. This is intended, as fragment rules only enrich the current AST node
             cstNode = this.nodeBuilder.buildCompositeNode(feature);
         }
         const subruleResult = this.wrapper.wrapSubrule(idx, rule, args) as any;
@@ -325,11 +329,7 @@ export class LangiumParser extends AbstractLangiumParser {
             if (isDataTypeNode(current)) {
                 current.value += result.toString();
             } else if (typeof result === 'object' && result) {
-                const resultKind = result.$type;
                 const object = this.assignWithoutOverride(result, current);
-                if (resultKind) {
-                    object.$type = resultKind;
-                }
                 const newItem = object;
                 this.stack.pop();
                 this.stack.push(newItem);
@@ -592,7 +592,7 @@ export class LangiumCompletionParser extends AbstractLangiumParser {
         }
     }
 
-    subrule(idx: number, rule: RuleResult, feature: AbstractElement, args: Args): void {
+    subrule(idx: number, rule: RuleResult, fragment: boolean, feature: AbstractElement, args: Args): void {
         this.before(feature);
         this.wrapper.wrapSubrule(idx, rule, args);
         this.after(feature);

packages/langium/src/parser/parser-builder-base.ts

@@ -99,8 +99,9 @@ function buildRuleCall(ctx: RuleContext, ruleCall: RuleCall): Method {
     const rule = ruleCall.rule.ref;
     if (isParserRule(rule)) {
         const idx = ctx.subrule++;
+        const fragment = rule.fragment;
         const predicate = ruleCall.arguments.length > 0 ? buildRuleCallPredicate(rule, ruleCall.arguments) : () => ({});
-        return (args) => ctx.parser.subrule(idx, getRule(ctx, rule), ruleCall, predicate(args));
+        return (args) => ctx.parser.subrule(idx, getRule(ctx, rule), fragment, ruleCall, predicate(args));
     } else if (isTerminalRule(rule)) {
         const idx = ctx.consume++;
         const method = getToken(ctx, rule.name);
@@ -273,8 +274,9 @@ function buildCrossReference(ctx: RuleContext, crossRef: CrossReference, termina
         }
         return buildCrossReference(ctx, crossRef, assignTerminal);
     } else if (isRuleCall(terminal) && isParserRule(terminal.rule.ref)) {
+        const rule = terminal.rule.ref;
         const idx = ctx.subrule++;
-        return (args) => ctx.parser.subrule(idx, getRule(ctx, terminal.rule.ref as ParserRule), crossRef, args);
+        return (args) => ctx.parser.subrule(idx, getRule(ctx, rule), false, crossRef, args);
     } else if (isRuleCall(terminal) && isTerminalRule(terminal.rule.ref)) {
         const idx = ctx.consume++;
         const terminalRule = getToken(ctx, terminal.rule.ref.name);

packages/langium/test/parser/langium-parser-builder.test.ts

@@ -4,9 +4,11 @@
  * terms of the MIT License, which is available in the project root.
  ******************************************************************************/
 
+/* eslint-disable @typescript-eslint/no-explicit-any */
+
 import type { TokenType, TokenVocabulary } from 'chevrotain';
-import type { AstNode, CstNode, GenericAstNode, Grammar, GrammarAST, LangiumParser, ParseResult, TokenBuilderOptions } from 'langium';
-import { EmptyFileSystem, DefaultTokenBuilder, GrammarUtils, CstUtils } from 'langium';
+import type { AstNode, CstNode, GenericAstNode, Grammar, GrammarAST, LangiumParser, ParseResult, ReferenceInfo, Scope, TokenBuilderOptions } from 'langium';
+import { EmptyFileSystem, DefaultTokenBuilder, GrammarUtils, CstUtils, DefaultScopeProvider, URI } from 'langium';
 import { describe, expect, test, onTestFailed, beforeAll } from 'vitest';
 import { createLangiumGrammarServices, createServicesForGrammar } from 'langium/grammar';
 import { expandToString } from 'langium/generate';
@@ -706,6 +708,56 @@ describe('Fragment rules', () => {
         expect(result.value).toHaveProperty('values', ['ab', 'cd', 'ef']);
     });
 
+    test("Fragment rules don't create AST elements during parsing", async () => {
+        // This test is mostly based on a bug report in a GitHub discussion:
+        // https://github.com/eclipse-langium/langium/discussions/1638
+        // In particular, the issue was that fragment rules used to create AST elements with type "undefined"
+        // When passing those into references, the reference would fail to resolve because the created AST element was just a placeholder
+        // Eventually, the parser would assign the properties of the fragment rule to the actual AST element
+        // But the reference would still use the old reference
+        // The fixed implementation no longer creates these fake AST elements, thereby skipping any potential issues completely.
+        let resolved = false;
+        const services = await createServicesForGrammar({
+            grammar: `
+                grammar FragmentRuleOverride
+                entry Entry: items+=(MemberCall|Def)*;
+                Def: 'def' name=ID;
+                MemberCall:
+                    Start ({infer MemberCall.previous=current} "." element=[Def]
+                    ArrayCallSignature? )*;
+
+                Start:
+                    element=[Def]
+                    ArrayCallSignature?;
+
+                fragment ArrayCallSignature:
+                    (isArray?='[' ']');
+
+                terminal ID: /[_a-zA-Z][\\w_]*/;
+                hidden terminal WS: /\\s+/;
+            `, module: {
+                references: {
+                    ScopeProvider: (services: any) => new class extends DefaultScopeProvider {
+                        override getScope(context: ReferenceInfo): Scope {
+                            const item = context.container as any;
+                            const previous = item.previous;
+                            if (previous) {
+                                const previousElement = previous.element.ref;
+                                // Ensure that the reference can be resolved
+                                resolved = Boolean(previousElement);
+                            }
+                            return super.getScope(context);
+                        }
+                    }(services)
+                }
+            }
+        });
+        const document = services.shared.workspace.LangiumDocumentFactory.fromString('def a def b a[].b', URI.parse('file:///test'));
+        await services.shared.workspace.DocumentBuilder.build([document]);
+        expect(document.parseResult.lexerErrors).toHaveLength(0);
+        expect(document.parseResult.parserErrors).toHaveLength(0);
+        expect(resolved).toBe(true);
+    });
 });
 
 describe('Unicode terminal rules', () => {