Reworked the infrastructure for validation rules and inference rules #64

JohannesMeierSE · 2025-03-07T13:59:25Z

Main motivation for this PR is to improve the performance of validation and inference rules:

Do not apply each validation/inference rule to each language node, since most rules are related only to few language nodes.
Therefore register these rules to language keys, calculate the language key of the current language node to validate/infer and apply only those rules, which are registered to this language key (inspired by the ValidationRegistry of Langium!)
undefined remains as a fall-back solution
Another benefit is, that validation and inference rules can be defined in the same way, as you would register validation checks in Langium! See OX and LOX for examples.

Beyond that, this PR contributes:

New concept of "language keys" of language nodes to register inference and validation rules, with the new service LanguageService
Nearly all TypeScript types in the source code have <LanguageType = unknown> in order to get rid of unknown in the source code. Benefit: In typir-langium projects, we work everywhere with AstNode (instead of unknown). In our internal test projects, we have now TestLanguageNode instead of unknown. Downside: The additional generics in the code make reading/writing the source code slower.
The factory API to create new types (e.g. typir.factory.Primitives.create({ ... })) is extended with a chaining API to register inference rules which are dedicated for the currently created type (e.g. typir.factory.Primitives.create({...}).inferenceRule({...}).finish()): This makes the definition of inference rules more (TypeScript-)type-safe and allows to define an arbitrary number of these rules.
Create the predefined validations using the factory API, e.g. typir.factory.Functions.createUniqueFunctionValidation()
Reworked the API of validation rules to create validation hints: Instead of returning ValidationProblems, they need to be given to the ValidationProblemAcceptor now, which is provided as additional argument inside validation rules (strongly inspired by Langium!)
Lots of fixed bugs, refactorings and new utilities

Suggestions for the review:

Start to read the CHANGELOG.md to get an overview.
Then look at the OX and LOX applications to see, how the API changed.
Look at the new test cases in Typir core to get an idea of the new features/possibilities. Please suggest ideas for additional test cases!
Now it is time to review the changes in more detail, I suggest to look at them commit by commit (note that the commits are less clean/separated from each other compared to my previous PRs). Check whether the CHANGELOG.md is complete.
Finally search for TODO review in the source code to see some more interesting points to discuss during the review.

…Stateless | ValidationRuleWithBeforeAfter

…in Typir-Langium as well

…oved

…ent language/DSL

…uage keys to improve performance

…itrary number of type-safe inference rules (for Functions, operators, classes, primitives), inference made TypeSelector type-safe

…tion for primitive types, fixed imports

…with `validateArgumentsOfFunctionCalls` inside inference rules, moved validation into its own file, fixed bug, more comments

…renamed files

…ference, prevent duplicates during registration

…ce rules

…tion rules, some test cases, fixed various bugs, improved TypeScript type-safety

…rence rule is removed

…to be given to the new ValidationProblemAcceptor now

…or better performance)

…oit language keys, combined validations for arguments matching signatures and for specific calls), renamings

…dation rules (these features are not yet used)

…o validate

…to a new file; fixed bug

…alls exchangable

…he properties `filter` and `match` were ignored

… wrote test cases

…e rules of calls of (overloaded) operators

insafuhrmann

Thank you very much @JohannesMeierSE this is not only a lot of work but also (again) very well described for review. I really appreciate the main contributions of the PR very much, especially the introduction of language keys and the chain notation. This should not only improve performance but also readability. I also like the possibility to create the predefined validations with the factory API.
I have hesitated with a general approval a lot though, as I was not 100% sure about the <LanguageType = unknown> additions. I really appreciate the benefit, but to see the changes overall with the additional generics is quite overwhelming. In the end I think I am OK with it.
I am a bit irritated by the discovery noted in your TODO review in line 19, of the class.test.ts and have no ready answer, I think this should be investigated further.
I added some detailed comments.
Regarding test coverage I had no idea for further tests at the moment but they can be added when something comes to mind.

insafuhrmann · 2025-03-21T08:52:06Z

CHANGELOG.md

+- Associate inference rules with language keys for an improved performance
+- Typir-Langium: new API to register inference rules to the `$type` of the `AstNode` to validate,
+  e.g. `addInferenceRulesForAstNodes({ MemberCall: <InferenceRule1>, VariableDeclaration: <InferenceRule2>, ...})`, see (L)OX for some examples
+- Thanks to the new chaining API for defining types (see corresponding breaking changes below), they can be annotated in TypeScript-type-safe way with multiple inference rules for the same purpose.


It is unclear to me at this point, what "for the same purpose" refers to (the point before)?

insafuhrmann · 2025-03-21T08:56:24Z

CHANGELOG.md

+- Typir-Langium: new API to register inference rules to the `$type` of the `AstNode` to validate,
+  e.g. `addInferenceRulesForAstNodes({ MemberCall: <InferenceRule1>, VariableDeclaration: <InferenceRule2>, ...})`, see (L)OX for some examples
+- Thanks to the new chaining API for defining types (see corresponding breaking changes below), they can be annotated in TypeScript-type-safe way with multiple inference rules for the same purpose.
+- Provide new `expectValidationHints()` utility for developers to ease the writing of test cases for Typir-based type systems.


I am wondering about the naming 'Hints' a bit. Should we maybe stick closer to the langium terminology?

insafuhrmann · 2025-03-21T09:01:49Z

CHANGELOG.md

+
+- Clear the cache for inferred types, when an inference rule is removed.
+- Remove removed functions from its internal storage in `FunctionKind`.
+- Update the returned function type during a performance optimization, when adding or removing some signatures of overloaded functions.


Not clear to me while just reading the Changelog.

insafuhrmann · 2025-03-21T09:06:01Z

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

-                (node: unknown) => isTypeReference(node) && node.primitive === 'boolean'
-            ]});
+        const typeBool = this.typir.factory.Primitives.create({ primitiveName: 'boolean' })
+            .inferenceRule({ languageKey: BooleanLiteral })


I like this new api in general, it also improves readability imho

insafuhrmann · 2025-03-21T09:07:32Z

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

+            // .inferenceRule({ filter: isBooleanLiteral }) // this is the alternative solution
+            .inferenceRule({ languageKey: TypeReference, matching: (node: TypeReference) => node.primitive === 'boolean' }) // this is the more performant notation
+            // .inferenceRule({ filter: isTypeReference, matching: node => node.primitive === 'boolean' }) // this is the "easier" notation
+            .finish();


Note to self: always remember to finish. This is a little downside, but fine for me.

insafuhrmann · 2025-03-21T12:18:51Z

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

+ * in particular, to support overloaded functions.
+ * In each type system, exactly one instance of this class is stored by the FunctionKind.
+ */
+// TODO review: better name


I would like a name that shows that it handles more that one function and describes better, what it does, like AvailableFunctionsManager. That would also hint to the overload tasks.

insafuhrmann · 2025-03-21T12:23:52Z

packages/typir/src/kinds/function/function-validation-calls.ts

+
+    beforeValidation(_languageRoot: LanguageType, _accept: ValidationProblemAcceptor<LanguageType>, _typir: TypirServices<LanguageType>): void {
+        // do nothing
+        // TODO review: Here ValidationRuleStateless is enough, but since it is a function type (and no interface type), it is not possible to implement it here in this class.


Should we have a new minimal interface here?

insafuhrmann · 2025-03-21T12:27:47Z

packages/typir/src/services/validation.ts

-export interface ValidationMessageDetails {
-    languageNode: unknown;
+export interface ValidationMessageDetails<LanguageType = unknown, T extends LanguageType = LanguageType> {
+    languageNode: T; // TODO review: in OX/LOX, 'unknown' instead of 'AstNode' is inferred by TypeScript, why?


I think this needs to be investigated more deeply independently of the review, I do not know out of the box.

insafuhrmann · 2025-03-21T12:33:59Z

packages/typir/src/utils/utils-definitions.ts

+export interface InferCurrentTypeRule<TypeType extends Type = Type, LanguageType = unknown, T extends LanguageType = LanguageType> {
+    languageKey?: string | string[];
+    filter?: (languageNode: LanguageType) => languageNode is T;
+    matching?: (languageNode: T) => boolean; // TODO review: Should we provide "typeToInfer: TypeType" as an additional property here?


Do you have reasons against this? Nothing comes to my mind spontaneously.

insafuhrmann · 2025-03-21T12:38:09Z

packages/typir/test/kinds/class/class.test.ts

+            .inferenceRulesForFieldAccess({
+                filter: node => node instanceof ClassFieldAccess,
+                matching: node => {
+                    const varType = typir.Inference.inferType(node.classVariable); // TODO review: doing type inference on your own here feels a bit strange


Could we maybe discuss this in the next meeting? I am not sure I really understand this.

JohannesMeierSE added 30 commits February 3, 2025 12:59

simplified validation API by defining ValidationRule = ValidationRule…

0a829c0

…Stateless | ValidationRuleWithBeforeAfter

refactoring: use the "validation namespace" of the core Typir module …

5056ae9

…in Typir-Langium as well

fixed bug: remove validations attached to Functions when they are rem…

9026a46

…oved

introduced removeFromArray utility

3d2faaf

new LanguageService to provide some static information about the curr…

8982ee2

…ent language/DSL

extended the validation API by associating validation rules with lang…

552fa5c

…uage keys to improve performance

renamed listener callbacks

305a66f

register inference rules with a language key, chaining API for an arb…

30aa25f

…itrary number of type-safe inference rules (for Functions, operators, classes, primitives), inference made TypeSelector type-safe

chaining API for bottom and top types, improved chaing API implementa…

d29824c

…tion for primitive types, fixed imports

explicitly enable validation of arguments of function/operator calls …

916d046

…with `validateArgumentsOfFunctionCalls` inside inference rules, moved validation into its own file, fixed bug, more comments

fixed bug in inference rules for function calls, test case for that, …

e2d128c

…renamed files

some small improvements (comments, TypeScript-types)

52f8979

enable multiple language keys to register rules for validation and in…

b9fd48a

…ference, prevent duplicates during registration

improved registration of rules: remove empty entries

664cfc9

new feature for the Language service: getAllSuperKeys

ceaf5ac

fixed composite inference rules

27b7fa5

support multiple boundToType, fixed the (de)registration of inferen…

a53cf08

…ce rules

developed dedicated registry for rules, used for inference and valida…

bbffb52

…tion rules, some test cases, fixed various bugs, improved TypeScript type-safety

use LanguageType = unknown> nearly everywhere in Typir now

ea45007

used LangiumType-generic even more

7127504

fixed bugs when resolving TypeSelectors

f5fda0a

fixed important bug: clear the cache for inferred types, when an infe…

3f86de2

…rence rule is removed

wrote test cases for the rule registry, fixed composite inference rule

4b266ef

Reworked the API to create validation hints: ValidationProblems need …

7cc5de7

…to be given to the new ValidationProblemAcceptor now

updated the TypeScript version

65003e8

simplified generics when defining validation rules

ea825b7

don't require $problem in the validation problem acceptor

c0fd60a

improved rule registry to lazily determine the unique set of rules (f…

0276eae

…or better performance)

reworked the validation of function calls regarding performance (expl…

3907061

…oit language keys, combined validations for arguments matching signatures and for specific calls), renamings

small performance improvements

8bc34d7

JohannesMeierSE added 13 commits March 6, 2025 15:52

added composite validation rules and listeners for added/removed vali…

5944874

…dation rules (these features are not yet used)

more TypeScript-safe generics

e152275

fixed some vulnerabilities

fb280dd

new API to register inference rules to the $type of the AstNode t…

e7bcb02

…o validate

refactoring: moved existing logic to manage (overloaded) functions in…

1536cbf

…to a new file; fixed bug

even more <LanguageType> generics, refactorings to make constructor c…

b8c0e41

…alls exchangable

annotate inference rules with validations (WIP)

89f6322

fixed bug: When inferring the types of accessing fields of classes, t…

33eaaed

…he properties `filter` and `match` were ignored

fixed bugs in generics

358ea69

finished the validations for the special inference rules for classes,…

a637950

… wrote test cases

provide new expectValidationHints() utility for testing

b26109d

additional test case for validations which are annotated for inferenc…

75e2206

…e rules of calls of (overloaded) operators

provide the predefined validations in the factory API

6c9fca0

JohannesMeierSE added the Infrastructure label Mar 7, 2025

JohannesMeierSE added this to the v0.2 milestone Mar 7, 2025

JohannesMeierSE requested review from Lotes and insafuhrmann March 7, 2025 13:59

JohannesMeierSE mentioned this pull request Mar 7, 2025

Add a simple example without Langium #59

Open

fixed some more generics

bf2bad2

insafuhrmann approved these changes Mar 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reworked the infrastructure for validation rules and inference rules #64

Reworked the infrastructure for validation rules and inference rules #64

JohannesMeierSE commented Mar 7, 2025

insafuhrmann left a comment

insafuhrmann Mar 21, 2025

insafuhrmann Mar 21, 2025

insafuhrmann Mar 21, 2025

insafuhrmann Mar 21, 2025

insafuhrmann Mar 21, 2025

insafuhrmann Mar 21, 2025

insafuhrmann Mar 21, 2025

insafuhrmann Mar 21, 2025

insafuhrmann Mar 21, 2025

insafuhrmann Mar 21, 2025

insafuhrmann Mar 21, 2025

Reworked the infrastructure for validation rules and inference rules #64

Are you sure you want to change the base?

Reworked the infrastructure for validation rules and inference rules #64

Conversation

JohannesMeierSE commented Mar 7, 2025

insafuhrmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment