improvements according to the review

examples/lox/src/language/type-system/lox-type-checking.ts

@@ -58,7 +58,7 @@ export class LoxTypeCreator extends AbstractLangiumTypeCreator {
                 (node: unknown) => isReturnStatement(node) && node.value === undefined
             ] });
         const typeNil = this.primitiveKind.createPrimitiveType({ primitiveName: 'nil',
-            inferenceRules: isNilLiteral }); // for what is this used in LOX?
+            inferenceRules: isNilLiteral }); // From "Crafting Interpreters" no value, like null in other languages. Uninitialised variables default to nil. When the execution reaches the end of the block of a function body without hitting a return, nil is implicitly returned.
         const typeAny = this.anyKind.createTopType({});
 
         // extract inference rules, which is possible here thanks to the unified structure of the Langium grammar (but this is not possible in general!)
@@ -135,12 +135,13 @@ export class LoxTypeCreator extends AbstractLangiumTypeCreator {
             // ... variable declarations
             if (isVariableDeclaration(domainElement)) {
                 if (domainElement.type) {
+                    // the user declared this variable with a type
                     return domainElement.type;
                 } else if (domainElement.value) {
-                    // the type might be null; no type declared => do type inference of the assigned value instead!
+                    // the didn't declared a type for this variable => do type inference of the assigned value instead!
                     return domainElement.value;
                 } else {
-                    return InferenceRuleNotApplicable; // this case is impossible, there is a validation in the "usual LOX validator" for this case
+                    return InferenceRuleNotApplicable; // this case is impossible, there is a validation in the Langium LOX validator for this case
                 }
             }
             return InferenceRuleNotApplicable;

packages/typir-langium/src/features/langium-caching.ts

@@ -50,7 +50,7 @@ export class LangiumDomainElementInferenceCaching implements DomainElementInfere
 }
 
 
-// TODO this is copied from Langium, since the introducing PR #1659 will be included in the upcoming Langium version 3.3 (+ PR #1712), after realising v3.3 this class can be removed completely!
+// TODO this is copied from Langium, since the introducing PR #1659 will be included in the upcoming Langium version 3.3 (+ PR #1712), after releasing v3.3 this class can be removed completely!
 /**
  * Every key/value pair in this cache is scoped to a document.
  * If this document is changed or deleted, all associated key/value pairs are deleted.

packages/typir-langium/src/features/langium-type-creator.ts

@@ -9,7 +9,7 @@ import { LangiumSharedServices } from 'langium/lsp';
 import { Type, TypeEdge, TypeGraph, TypeGraphListener, TypirServices } from 'typir';
 import { getDocumentKeyForDocument, getDocumentKeyForURI } from '../utils/typir-langium-utils.js';
 
-export interface LangiumTypeCreator {
+export interface LangiumTypeCreator { // TODO Registry instead?
     triggerInitialization(): void;
 
     /**
@@ -44,7 +44,7 @@ export abstract class AbstractLangiumTypeCreator implements LangiumTypeCreator,
         langiumServices.workspace.DocumentBuilder.onUpdate((_changed, deleted) => {
             deleted
                 .map(del => getDocumentKeyForURI(del))
-                .forEach(del => this.handleDeletedDocument(del));
+                .forEach(del => this.invalidateTypesOfDocument(del));
         });
 
         // get informed about added/removed types
@@ -70,20 +70,27 @@ export abstract class AbstractLangiumTypeCreator implements LangiumTypeCreator,
         this.triggerInitialization();
         this.currentDocumentKey = getDocumentKeyForDocument(document); // remember the key in order to map newly created types to the current document
 
-        // remove all types which were associated with the current document
-        this.handleDeletedDocument(this.currentDocumentKey);
+        // For a NEW document, this is called, but nothing happens.
+        // For an UPDATED document, Langium deletes the whole previous AST and creates a complete new AST.
+        // Therefore all types which were created for such (now invalid) AstNodes and therefore associated with the current document need to be removed.
+        this.invalidateTypesOfDocument(this.currentDocumentKey);
 
         // create all types for this document
         AstUtils.streamAst(document.parseResult.value)
             .forEach((node: AstNode) => this.onNewAstNode(node));
 
-        this.currentDocumentKey = '';
+        this.currentDocumentKey = ''; // reset the key, newly created types will be associated with no document now
     }
 
-    protected handleDeletedDocument(documentKey: string): void {
-        (this.documentTypesMap.get(documentKey) ?? [])
+    protected invalidateTypesOfDocument(documentKey: string): void {
+        // grab all types which were created for the document
+        (this.documentTypesMap.get(documentKey)
+            // there are no types, if the document is new or if no types were created for the previous document version
+            ?? [])
             // this is the central way to remove types from the type systems, there is no need to inform the kinds
             .forEach(typeToRemove => this.typeGraph.removeNode(typeToRemove));
+        // remove the deleted types from the map
+        this.documentTypesMap.delete(documentKey);
     }
 
     addedType(newType: Type): void {
@@ -102,24 +109,24 @@ export abstract class AbstractLangiumTypeCreator implements LangiumTypeCreator,
     }
 
     removedType(_type: Type): void {
-        // do nothing
+        // since this type creator actively removes types from the type graph itself, there is no need to react on removed types
     }
     addedEdge(_edge: TypeEdge): void {
-        // do nothing
+        // this type creator does not care about edges => do nothing
     }
     removedEdge(_edge: TypeEdge): void {
-        // do nothing
+        // this type creator does not care about edges => do nothing
     }
 }
 
-export class IncompleteLangiumTypeCreator extends AbstractLangiumTypeCreator {
+export class PlaceholderLangiumTypeCreator extends AbstractLangiumTypeCreator {
     constructor(typirServices: TypirServices, langiumServices: LangiumSharedServices) {
         super(typirServices, langiumServices);
     }
     override onInitialize(): void {
-        throw new Error('This method needs to be implemented!');
+        throw new Error('This method needs to be implemented! Extend the AbstractLangiumTypeCreator and register it in the Typir module: TypeCreator: (typirServices) => new MyLangiumTypeCreator(typirServices, langiumServices)');
     }
     override onNewAstNode(_domainElement: AstNode): void {
-        throw new Error('This method needs to be implemented!');
+        throw new Error('This method needs to be implemented! Extend the AbstractLangiumTypeCreator and register it in the Typir module: TypeCreator: (typirServices) => new MyLangiumTypeCreator(typirServices, langiumServices)');
     }
 }

packages/typir-langium/src/typir-langium.ts

@@ -8,7 +8,7 @@ import { LangiumServices, LangiumSharedServices } from 'langium/lsp';
 import { DeepPartial, DefaultTypeRelationshipCaching, DefaultTypirServiceModule, Module, TypirServices } from 'typir';
 import { LangiumDomainElementInferenceCaching } from './features/langium-caching.js';
 import { LangiumProblemPrinter } from './features/langium-printing.js';
-import { IncompleteLangiumTypeCreator, LangiumTypeCreator } from './features/langium-type-creator.js';
+import { PlaceholderLangiumTypeCreator, LangiumTypeCreator } from './features/langium-type-creator.js';
 import { LangiumTypirValidator, registerTypirValidationChecks } from './features/langium-validation.js';
 
 /**
@@ -40,7 +40,7 @@ export function createLangiumModuleForTypirBinding(langiumServices: LangiumShare
         },
         // provide implementations for the additional services for the Typir-Langium-binding:
         TypeValidation: (typirServices) => new LangiumTypirValidator(typirServices),
-        TypeCreator: (typirServices) => new IncompleteLangiumTypeCreator(typirServices, langiumServices),
+        TypeCreator: (typirServices) => new PlaceholderLangiumTypeCreator(typirServices, langiumServices),
     };
 }
 
@@ -50,9 +50,10 @@ export function initializeLangiumTypirServices(services: LangiumServices & Langi
 
     // initialize the type creation (this is not done automatically by dependency injection!)
     services.TypeCreator.triggerInitialization();
-    // TODO why does the following not work?
+    // TODO This does not work, if there is no Language Server used, e.g. in test cases!
     // services.shared.lsp.LanguageServer.onInitialized(_params => {
     //     services.TypeCreator.triggerInitialization();
     // });
-
+    // maybe using services.shared.workspace.WorkspaceManager.initializeWorkspace/loadAdditionalDocuments
+    // another idea is to use eagerLoad(inject(...)) when creating the services
 }

packages/typir/src/features/inference.ts

@@ -100,7 +100,7 @@ export interface TypeInferenceCollector {
      * When inferring the type for an element, all registered inference rules are checked until the first match.
      * @param rule a new inference rule
      * @param boundToType an optional type, if the new inference rule is dedicated for exactly this type.
-     * If the given type is removed from the type system, this rule will be removed as well.
+     * If the given type is removed from the type system, this rule will be automatically removed as well.
      */
     addInferenceRule(rule: TypeInferenceRule, boundToType?: Type): void;
 }

packages/typir/src/kinds/function-kind.ts

@@ -404,7 +404,7 @@ export class FunctionKind implements Kind, TypeGraphListener {
         this.services.graph.addNode(functionType);
 
         // output parameter for function calls
-        const outputTypeForFunctionCalls = this.getOtputTypeForFunctionCalls(functionType);
+        const outputTypeForFunctionCalls = this.getOutputTypeForFunctionCalls(functionType);
 
         // remember the new function for later in order to enable overloaded functions!
         let overloaded = this.mapNameTypes.get(functionName);
@@ -444,7 +444,7 @@ export class FunctionKind implements Kind, TypeGraphListener {
         const functionName = typeDetails.functionName;
         const mapNameTypes = this.mapNameTypes;
         const overloaded = mapNameTypes.get(functionName)!;
-        const outputTypeForFunctionCalls = this.getOtputTypeForFunctionCalls(functionType);
+        const outputTypeForFunctionCalls = this.getOutputTypeForFunctionCalls(functionType);
         if (typeDetails.inferenceRuleForCalls) {
             /** Preconditions:
              * - there is a rule which specifies how to infer the current function type
@@ -534,7 +534,7 @@ export class FunctionKind implements Kind, TypeGraphListener {
         }
     }
 
-    protected getOtputTypeForFunctionCalls(functionType: FunctionType): Type | undefined {
+    protected getOutputTypeForFunctionCalls(functionType: FunctionType): Type | undefined {
         return functionType.getOutput()?.type ?? // by default, use the return type of the function ...
             // ... if this type is missing, use the specified type for this case in the options:
             // 'THROW_ERROR': an error will be thrown later, when this case actually occurs!

packages/typir/src/typir.ts

@@ -76,6 +76,12 @@ export const DefaultTypirServiceModule: Module<TypirServices> = {
     },
 };
 
+/**
+ * Creates the TypirServices with the default module containing the default implements for Typir, which might be exchanged by the given optional customized modules.
+ * @param customization1 optional Typir module with customizations
+ * @param customization2 optional Typir module with customizations
+ * @returns a Typir instance, i.e. the TypirServices with implementations
+ */
 export function createTypirServices(
     customization1: Module<TypirServices, PartialTypirServices> = {},
     customization2: Module<TypirServices, PartialTypirServices> = {}