Simplified API for custom types by JohannesMeierSE · Pull Request #73 · TypeFox/typir

JohannesMeierSE · 2025-04-16T11:50:54Z

This PR contributes a simplified API to enable users of Typir to define and use their project-specific custom types. Features of custom types as designed in this PR:

Custom types have custom properties ("data") including primitive values, Typir types and nesting/grouping (sets/arrays/maps and recursion)
TypeScript-safe descriptions for these properties with TypeScript-generics
ensure uniqueness for custom types
Cyclic dependencies of the given types for type properties are handled (see custom-cycles.test.ts and custom-selectors.test.ts)
specify rules for conversion, sub-type, names, identifiers, inference and validation (usually for all custom types OR for single ones)

For some more fixes/features of this PR beyond custom types, see the CHANGELOG.md.

Open questions to discuss during the review:

I needed to introduce LanguageService.isLanguageNode(node: unknown): node is LanguageType, which is used once and only for a detail (which has some relevance nevertheless): Is there a better solution? Should we make it optional (or is the default implementation OK)? For me, this feels not like a perfect solution.
There is one limitation for custom types: You cannot use simple string values for TypeSelectors (in order to specify properties of type Type), since they cannot be distinguished from string values for primitive properties. In my eyes, that is a reasonable limitation, since you will get a TypeScript error, if you try to do that.
Since custom types have builtin support for circular types, you have to call getTypeFinal(): Type|undefined on the initializer always, even in cases, where it is not required, since your custom type has no types as properties or you know, that all depending types are already available. Even in these cases you need the "work-around" with const myCustomType = ...finish().getTypeFinal()!;. An idea would be to provide SimpleCustomTypes (without support for circular types) and ComplexCustomTypes (with support for circular types) and the user of Typir has to select the appropriate solution for the current use case.
Another issue, which is not directly related to custom types: undefined is not supported as result to be returned by an inference rule. You need to return InferenceRuleNotApplicable, if the rule is for another case, or an InferenceProblem. Should we support undefined as well? That might simplify some things.

Hints for the review:

Try to get the general idea of this approach for custom types in order to evaluate, whether this approach is the one to go. Some more polishing could be done, when we decided on the way to go.
Look at the examples in custom-example-matrix.test.ts and custom-example-restricted.test.ts first to get an idea, how the API works.
The other four files with test cases for custom types are quite specific, but might give an impression, what is possible with custom types.
If you review the implementation commit-by-commit (which might be useful in general), notice, that the earlier commits develop custom types step-by-step including some reworked parts. Don't spent too much time to understand each detail in custom-type.ts and custom-definitions.ts 🙂

insafuhrmann

Thank you @JohannesMeierSE, great work (and a lot of it)! I think this will become a very valuable contribution. While I like the general approach a lot, I have a number of detailed remarks, see comments.

Regarding your open questions:

I think that users might choose other solutions than the isLanguageNode like specific properties or markers that are unique to LanguageType so it might make sense to make this optional. The default implementation is OK, but it is rather permissive.
Forbidding string values for TypeSelectors is fine for me
I think both approaches (SimpleCustomType/ComplexCustomType vs current solution) have their benefits and disadvantages. In any case the user needs to know about it. I think a good documentation is more important than the choice. I tend to support the current solution.
I'd rather not introduce more of the undefined. I rather have places in which I would like to get rid of it in favour of meaningful type checking (see detail comments). What are the simplifications you have in mind?

insafuhrmann · 2025-06-23T07:39:55Z

+        // now use this custom kind to create some custom types
+        const matrix2x2 = customKind // "lazy" to use matrix2x2 as 'baseType' => review ZOD, separate primitives and Typir-Types
+            .create({ typeName: 'My2x2MatrixType', properties: { baseType: integerType, width: 2, height: 2 } })
+            .finish().getTypeFinal()!; // we know, that the new custom type depends only on types which are already available


What needs to be done for custom types otherwise?

That should be well documented at some point I think. If we keep all in one and do not split into simple and complex custom types we should clarify what holds and is to be kept in mind for each case (no dependencies or dependencies to wait for).

Yes, we need to document that. If we merge #80 before this PR, I could already write some documentation for custom types.

What needs to be done for custom types otherwise?

If the type is not yet available, you can register a listener, which is called, when the type is available: ....finish().addListener(finishedType => /* now the type is available and can be used */ finishedType.getIdentifier());

Thanks, that should go into the docu, I think!

I put this into the documentation

insafuhrmann · 2025-07-03T09:48:44Z

+
+    test('Matrix type', () => {
+        const typir = createTypirServicesForTesting();
+        // TODO does not yet work: { factory: { Matrix: services => new CustomKind<MatrixType, TestLanguageNode>(services, { ... }) } }


Is this still planned?

Yes, it is already implemented in #78! #78 and this PR are highly related to each other, but I wanted to have more, but smaller PRs to ease reviews.

insafuhrmann · 2025-07-04T09:06:27Z

+
+export type CustomTypePropertyInitialization<T extends CustomTypePropertyTypes, LanguageType> =
+    // replace Type by a TypeSelector for it ...
+    T extends Type ? TypeSelectorForCustomTypes<T, LanguageType> : // note that TypeSelector includes "unknown" (if the LanguageType is not specified), which makes the TypeScript type-checking "useless" here!


Does this not negate the benefit of having strong typing for custom type initialisation? Is there any way to avoid unknown here? Maybe with an explicit error type?

The way to prevent this is not to use unknown as <LanguageType>. I improved the comment here

insafuhrmann · 2025-07-04T09:09:25Z

+        }
+    }
+
+    // TODO dieses Design für Class and Functions genau so umsetzen/angleichen


If this TODO should stay in, make it English?

This is a reminder for me to change all existing kinds accordingly, if the design for custom types is accepted in the review 🙂 I will do it next week

see the corresponding commit

insafuhrmann · 2025-07-04T09:41:44Z

+    /** Name for this custom kind. */
+    name: string;
+
+    /** This identifier needs to consider all properties which make the custom type unique. The identifiers are used to detect unique custom types. */


From the examples I saw that the identifiers do not consider structure for nested properties. Is uniqueness guaranteed nevertheless?

It is up to the user of Typir to consider the structure, if necessary. I improved the comment a bit.

JohannesMeierSE

Thank you @insafuhrmann very much for your helpful review!

Regarding the open questions:

I think that users might choose other solutions than the isLanguageNode like specific properties or markers that are unique to LanguageType so it might make sense to make this optional. The default implementation is OK, but it is rather permissive.

Yes. Let's discuss it in the next meeting.

Forbidding string values for TypeSelectors is fine for me

For me as well.

I think both approaches (SimpleCustomType/ComplexCustomType vs current solution) have their benefits and disadvantages. In any case the user needs to know about it. I think a good documentation is more important than the choice. I tend to support the current solution.

I tend to support only the current solution as well, since it reduces maintenance effort. I guess we need to extend custom types in the future and then it is easier to do it only once.

I'd rather not introduce more of the undefined. I rather have places in which I would like to get rid of it in favour of meaningful type checking (see detail comments). What are the simplifications you have in mind?

If you get const value: Type | undefined = ... from some calculation, you cannot use value as return value inside an inference rule. Instead you need to return value === undefined ? InferenceRuleNotApplicable : value instead.

JohannesMeierSE · 2025-07-04T14:18:35Z

+        // now use this custom kind to create some custom types
+        const matrix2x2 = customKind // "lazy" to use matrix2x2 as 'baseType' => review ZOD, separate primitives and Typir-Types
+            .create({ typeName: 'My2x2MatrixType', properties: { baseType: integerType, width: 2, height: 2 } })
+            .finish().getTypeFinal()!; // we know, that the new custom type depends only on types which are already available


Yes, we need to document that. If we merge #80 before this PR, I could already write some documentation for custom types.

What needs to be done for custom types otherwise?

If the type is not yet available, you can register a listener, which is called, when the type is available: ....finish().addListener(finishedType => /* now the type is available and can be used */ finishedType.getIdentifier());

JohannesMeierSE · 2025-07-04T15:03:18Z

+
+export type CustomTypePropertyInitialization<T extends CustomTypePropertyTypes, LanguageType> =
+    // replace Type by a TypeSelector for it ...
+    T extends Type ? TypeSelectorForCustomTypes<T, LanguageType> : // note that TypeSelector includes "unknown" (if the LanguageType is not specified), which makes the TypeScript type-checking "useless" here!


The way to prevent this is not to use unknown as <LanguageType>. I improved the comment here

JohannesMeierSE · 2025-07-04T15:04:18Z

+        }
+    }
+
+    // TODO dieses Design für Class and Functions genau so umsetzen/angleichen


This is a reminder for me to change all existing kinds accordingly, if the design for custom types is accepted in the review 🙂 I will do it next week

JohannesMeierSE · 2025-07-04T15:05:55Z

+    /** Name for this custom kind. */
+    name: string;
+
+    /** This identifier needs to consider all properties which make the custom type unique. The identifiers are used to detect unique custom types. */


It is up to the user of Typir to consider the structure, if necessary. I improved the comment a bit.

…Selectors, calculate identifiers only from custom properties

…sired

…ence rules

…, more comments, renamings

…ready exists

…or calculating identifiers of custom types, fixed minor bugs

JohannesMeierSE · 2025-07-09T13:14:41Z

Thanks @insafuhrmann for the helpful joint discussion yesterday! I improved the PR as discussed and wrote some documentation for custom types.

insafuhrmann

Thank you very much for the improvements, @JohannesMeierSE! Especially the very helpful documentation part and the additional comments! I left a few last detail remarks regarding typos/small mistakes, one of them (a choice of word problem) rather important (might confuse the meaning).

insafuhrmann

Thanks for the last fixes, @JohannesMeierSE. It will be great to have this large contribution merged!

insafuhrmann · 2025-07-11T13:22:54Z

JohannesMeierSE added the Predefined Types label Apr 16, 2025

JohannesMeierSE added this to the v0.3 milestone Apr 16, 2025

JohannesMeierSE requested a review from insafuhrmann April 16, 2025 11:50

JohannesMeierSE mentioned this pull request Jun 25, 2025

Support customized error messages #76

Closed

insafuhrmann reviewed Jul 4, 2025

View reviewed changes

JohannesMeierSE commented Jul 4, 2025

View reviewed changes

JohannesMeierSE added 24 commits July 9, 2025 14:11

first working version for defining simple custom types

d37784b

fixed some other smaller issues

edd4a32

fixed generics, small issues

fa9e3b5

no strings for TypeSelectors

9252fe4

calculate identifiers only from custom properties

44f5b37

tests for custom types depending on custom types, no strings for Type…

f98b7c7

…Selectors, calculate identifiers only from custom properties

test cases for type inference

bedb16c

fixed bug

f79d0df

register as listener and get informed about all existing types, if de…

53b3ad3

…sired

conversion and sub-types for custom types

8b60ada

test case for validation rules attached to custom type-specific infer…

bdbe76a

…ence rules

specify names and user representations once for all custom properties…

796630e

…, more comments, renamings

test multiple custom types in parallel

0c4659d

fixed eslint rule

1ced5ca

support nesting of Properties for custom types

74a6f90

new property to skip type-specific inference rules for types which al…

2571d82

…ready exists

removed testing stuff, fixed imports

7b94761

more test cases for TypeSelectors with custom types

4e9e31f

first improvements according to the review

d9ea8e1

more improvements

98ec8e9

improved comments according to the review

e50d304

properties of custom types are readonly now, default implementation f…

1f63167

…or calculating identifiers of custom types, fixed minor bugs

added missing implementation (which will be improved in future PRs)

7003a61

unified initialization design for classes, functions and custom types

7a98ce1

JohannesMeierSE added 2 commits July 9, 2025 15:11

check typeName for uniqueness

67de361

wrote some documentation for custom types

e551485

JohannesMeierSE force-pushed the jm/simple-custom-types branch from f5b593b to e551485 Compare July 9, 2025 13:12

insafuhrmann approved these changes Jul 11, 2025

View reviewed changes

fixes according to the review

6567a1e

insafuhrmann approved these changes Jul 11, 2025

View reviewed changes

Comment thread documentation/kinds/custom-types.md

insafuhrmann Jul 11, 2025

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

JohannesMeierSE merged commit 2aee774 into main Jul 11, 2025
2 checks passed

JohannesMeierSE deleted the jm/simple-custom-types branch July 11, 2025 13:50

JohannesMeierSE mentioned this pull request Jul 17, 2025

Simplified API for custom types #71

Closed

Uh oh!

Conversation

JohannesMeierSE commented Apr 16, 2025

Uh oh!

insafuhrmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JohannesMeierSE left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JohannesMeierSE commented Jul 9, 2025

Uh oh!

insafuhrmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

insafuhrmann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees