Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate TypeScript types from matcher patterns #6160

Open
turadg opened this issue Sep 8, 2022 · 34 comments
Open

Generate TypeScript types from matcher patterns #6160

turadg opened this issue Sep 8, 2022 · 34 comments
Assignees
Labels
code-style defensive correctness patterns; readability thru consistency devex developer experience enhancement New feature or request vaults_triage DO NOT USE

Comments

@turadg
Copy link
Member

turadg commented Sep 8, 2022

What is the Problem Being Solved?

With the typeGuards we're repeating type descriptions that are in TS/JSdoc. This is extra work and ambiguous source of truth.

Description of the Design

When a type is described by matchers, that becomes the source of truth. TS types can be auto-generated from it.

Related work

Security Considerations

Test Plan

@turadg turadg added enhancement New feature or request code-style defensive correctness patterns; readability thru consistency devex developer experience labels Sep 8, 2022
@turadg turadg self-assigned this Sep 8, 2022
@kriskowal
Copy link
Member

kriskowal commented Oct 11, 2022

I believe we agree the ideal outcome would be to solve this problem entirely with TypeScript annotations and not resort to code generation. With the ideal outcome, a program like the following would pass a type check.

const pattern = pl.or(pl.number(), pl.string());
/** @type {{ value: number | string } | null} */
const matched = pattern.match(10);
if (matched !== null) {
  /** @type {number | string} */
  const value = matched.value;
}

Other acceptable outcomes might involve a callback.

However, if this is not possible, we could generalize pattern definitions such that a definition can be used either to generate a passable pattern object or the equivalent TypeScript definition text.

/** @type {<T>(p: Pattern<T>) => T} */
const patterner = $ => $.or($.number(), $.string());
const pattern = patterner(toPattern);

And alternately elsewhere:

console.log(patterner(toTypeScriptNotation, 'MyNumber'));
// output: number | string

cc @erights

@mhofman
Copy link
Member

mhofman commented Oct 11, 2022

Here is a TS playground showing it should be possible to make the parameter inference work given an statically typed runtime validator which enforces a shape.

The remaining step is to adapt this to an object with methods, and have the validator's static type assertion being automatically derived / inferred from a pattern.

@erights
Copy link
Member

erights commented Oct 11, 2022

Erasing the comments and just looking at the JS program, I don't understand. | in JS is the bitwise-or operator, and cannot be overloaded, so I don't see how this can work.

const pattern = pl.number() | pl.string();
const matched = pattern.match(10);
if (matched !== null) {
  const value = matched.value;
}

Also, we need to avoid confusing validators (which ensures that an argument conforms) with coercers (or guards, which ensures only that the returned result conforms). Validators provide a stronger guarantee. Coercers are more flexible. Our pattern framework is more focussed on validators.

In any case, I don't yet understand why we need to invent a new API. Expressing the above JS code in our existing API:

const pattern = M.or(M.number(), M.string());
if (matches(value, pattern) {
  // Here we know that `value` is a number or a string
}

IIUC, the open question is whether we can do TS magic without a separate build step so that TS derives the type number | string and statically knows what we know, that within those curly brackets value is a number | string. TS already does something similar with if (typeof ...) { .

Is the change in API motivated by what TS needs to infer this from the JS code?

Btw, if we can achieve the above with the current API, then we should also be able to do, with the current API:

const pattern = M.or(M.number(), M.string());
fit(value, pattern); // throws good diagnostic if doesn't match
// Here we know that `value` is a number or a string

@michaelfig already did something similar with our assert.typeof.

@erights
Copy link
Member

erights commented Oct 11, 2022

Here is a TS playground showing it should be possible to make the parameter inference work given an statically typed runtime validator which enforces a shape.

The remaining step is to adapt this to an object with methods, and have the validator's static type assertion being automatically derived / inferred from a pattern.

Hi @mhofman very cool. Can this be built on the current patterns API, or does it require an API change?

@mhofman
Copy link
Member

mhofman commented Oct 11, 2022

I admittedly do not know enough about the current patterns API to say if it would require changes or not.

Edit: In the example above, fit would be able to assert that value (previously of type any or unknown) would now be string | number.

My example was mostly about how patterns could interact with function / object definitions, and the inference of function/method parameters, which is usually the hardest place to make type inference work seamlessly.

@kriskowal
Copy link
Member

Regarding pl.number() | pl.string(), that was a typo. I’ve edited the example to pl.or(pl.number(), pl.string()). I’m also not proposing a new API. Substitute ML for pl and my intention is the same.

I also am focusing on validators, not coercers. With the exception of asserts T is U clauses, I don’t think TypeScript has a way to narrow the type of an existing variable: it needs to be rebound. Mathieu argues for a callback and my example is intended to imply that matcher.value is just passed thru, but the type is narrowed from unknown to a type corresponding to the pattern.

@kriskowal
Copy link
Member

Oh, if the alternative to matching is throwing, the assert T is U trick we used in assert.typeof will work fine here. I don’t know of a way to narrow the type only if the function returns true.

@mhofman
Copy link
Member

mhofman commented Oct 11, 2022

Right, narrowing through assert type functions is easy, but I don't believe it helps the original issue, which is about deduplicating static type definitions and runtime type guards. The best DX is for the static types to be automatically inferred from the runtime guards, however the inference flow is fairly restrictive for function / methods, and basically requires the function/method definition to be coupled with a parameter pattern, so that the type inference of the pattern can be reflected onto the params.

@erights
Copy link
Member

erights commented Oct 11, 2022

Substitute ML for pl and my intention is the same.

Just checking. You meant M, right?

@erights
Copy link
Member

erights commented Oct 11, 2022

I don’t know of a way to narrow the type only if the function returns true.

See static conditional example which looks like

image

Notice what is and is not red-squiggled.

@erights
Copy link
Member

erights commented Oct 11, 2022

Our pattern API has both

matches(specimen: Passable, pattern: Pattern) => boolean

and

fit(specimen: Passable, pattern: Pattern, label?: string) => void

I'd expect we'd change the return type of matches to specimen is T, for some appropriate T somehow derived from pattern.

I'd expect we'd similarly change the return type of fit to asserts specimen is T.

@erights
Copy link
Member

erights commented Oct 11, 2022

Speculating here since I know so little TS. The definitions could be

/**
 * @template {Passable} T
 * @param {Passable} specimen
 * @param {Pattern<T>} pattern
 * @returns {specimen is T}
 */
const matches = (specimen, pattern) {

where all the magic is in the definition of Pattern which could be in a *.d.ts file?

@mhofman
Copy link
Member

mhofman commented Oct 11, 2022

@erights how is the pattern API integrated with kind behavior and far objects, if any?

@mhofman
Copy link
Member

mhofman commented Oct 11, 2022

Speculating here since I know so little TS. The definitions could be

/**
 * @template {Passable} T
 * @param {Passable} specimen
 * @param {Pattern<T>} pattern
 * @returns {specimen is T}
 */
const matches = (specimen, pattern) {

where all the magic is in the definition of Pattern which could be in a *.d.ts file?

Most likely it'd be something closer to

/**
 * @template {Pattern<Passable>} T
 * @param {Passable} specimen
 * @param {T} pattern
 * @returns {specimen is TypeFromPattern<T>}
 */
const matches = (specimen, pattern) {

@erights
Copy link
Member

erights commented Oct 11, 2022

@erights how is the pattern API integrated with kind behavior and far objects, if any?

Pervasively. First, patterns themselves are a subset of kinds. The pattern M.key(x) tests that x is a Key, which is a kind/store level concept, not a passStyle/marshal level concept.

matches(left, M.gte(right)) iff keyCompare(left, right) >= 0.

All the pattern inequalities use keyCompare rather than rankOrder or fullOrder. keyCompare is designed to have a useful semantics across keys. For example, on CopyMaps keyCompare is about subsets, which are necessarily a partial order.

@mhofman
Copy link
Member

mhofman commented Oct 11, 2022

I think my question was about virtual / durable object kind where patterns are likely to be enforced for method parameters.

For collection and other stores, I'm less worried since I assume we can make (if not already) the pattern an argument of the store/collection constructor?

@erights
Copy link
Member

erights commented Oct 11, 2022

... and far objects ...?

Patterns are passable. They match only Passables. Whether they match or not is pass-invariant, which is an important invariant for our distributed object semantics.

For any x and p

matches(x, p) // here
// iff
matches(passedX, passedP) // elsewhere

Thus, the pattern system must not be able to distinguish a far object from its remote presences. Together, they jointly designate the same remotable. Thus passable CopySets, CopyBags, and CopyMaps can use remotables as keys which mean the same think where that collection is itself copied elsewhere.

@erights
Copy link
Member

erights commented Oct 11, 2022

Most likely it'd be something closer to

Cool! I understood that and like it much better. Thanks!

@erights
Copy link
Member

erights commented Oct 11, 2022

Catching up

I think my question was about virtual / durable object kind where patterns are likely to be enforced for method parameters.

Typing wrapper used when making a FarClass or FarInstance is not itself meaningfully a Pattern, but instead described as a InterfaceGuard. An interface guard of course pervasively contains pattens for method parameters and return results. InterfaceGuards are themselves pass-by-copy passables, as are the patterns they contain.

Currently, for an InterfaceGuard, and its component MethodGuards and AwaitGuards, I just use CopyRecords of a recognized shape, but this is wrong. Instead, they should be kinds of tagged records, recognized by kindOf like patterns are, but distinct from patterns.

This is something I'm planning to do anyway. If it would help this typing effort, I can prioritize this sooner.

@erights
Copy link
Member

erights commented Oct 11, 2022

For collection and other stores, I'm less worried since I assume we can make (if not already) the pattern an argument of the store/collection constructor?

Currently, for stores, only via keyShape and valueShape options, which are a bit awkward, but IIUC would not make the needed TS mechanics any harder.

The interfaceGuard is an explicit parameter to vivifyFarClass, etc. Because the raw methods of a farClass are not defensive by themselves, I think they should only be written inline, like normal class methods are.

Does this make it straightforward that a type derived from the interface guard could be applied to the raw methods and to the instance that the returned making function makes? That would be great!

@erights
Copy link
Member

erights commented Oct 11, 2022

Even more awesome is if we could generate distinct types for the raw methods and for the made instance. For example, for getAmountOf in the Issuer interfaceGuard is

    getAmountOf: M.callWhen(M.await(PaymentShape)).returns(amountShape),

whereas the raw method is

    getAmountOf(payment) {
      // IssuerI delays calling this method until `payment` is a Remotable
      assertLivePayment(payment);
      return paymentLedger.get(payment);
    },

The type of the method in the instance is the external defensive API, and should be

(payment: ERef<Payment>) => ERef<Amount>

whereas the raw method should be typed

(payment: Payment) => Amount

Thus, M.await(PaymentShape) is an async coercer/guard rather than a pattern.

@erights
Copy link
Member

erights commented Oct 11, 2022

To be explicit: The interfaceGuard is an explicit parameter to vivifyFarClass, etc.

There's one more coming for FarClasses with #6432

In the same way that collections can be parameterized with optional keyShape, valueShape options, vivifyFarClass etc can be parameterize by a stateShape serving as a schema for that class's state records. As of #6432 , both valueShare for collections and stateShape for FarClasses, if provided, are used for compression. Having looked at this in the debugger, this is a lot of compression.

Perhaps this same typing effort can be used to derive a static TS type from stateShape for a FarClass's stateRecord? That would be so cool!

@erights
Copy link
Member

erights commented Oct 11, 2022

As far as the passStyleOf, kindOf, and pattern matching are concerned, a virtual or durable object is a just a far object and therefore a Remotable.

@mhofman
Copy link
Member

mhofman commented Oct 12, 2022

It should be possible to have the following types inferred correctly solely based on the interface guards and state shape, if those are made generic in order to carry the type information of their constituents:

  • The type of parameters of the implementation
  • The type of parameters of the made instances
  • The type of the state properties

For the made instance that would mean we could forego the manual typedef we currently have. However we may want/need to keep explicit types around for that case since unfortunately I'm not aware of any way to generate or pass-through the JS doc descriptions of methods and parameters. In that case we should make sure to avoid as much duplication with the pattern based interface guard as possible, or at least cause a static type error if they don't match.

@Chris-Hibbert
Copy link
Contributor

The thing I don't see explicitly addressed above is that the proposal seems to be to use Pattern declarations as the source of truth, but MarkM has said

Patterns are passable. They match only Passables.

and

the pattern system must not be able to distinguish a far object from its remote presences.

One thing I've learned is that in declaring signatures of methods, you don't get to declare the types of Remotables that will be returned. I think there are other places that the Pattern system obscures information that we want in our type system.

How will this be addressed in a scheme to generate TS from Patterns?

@turadg
Copy link
Member Author

turadg commented Dec 7, 2022

How will this be addressed in a scheme to generate TS from Patterns?

I've heard concern that types without runtime validation would give false confidence to the programmer so it's not obvious we should let TS express anything the Patterns didn't. I consider it outside the scope of this PR, but it is worth considering in the design of this work how it would be compatible with that future requirement.

One way we could add non-validated type information is for pattern objects to be optionally generic, with a type parameter for a hint as to what they return in a particular case, even if that's not validated by fit.

@erights
Copy link
Member

erights commented Dec 7, 2022

I've heard concern that types without runtime validation would give false confidence to the programmer so it's not obvious we should let TS express anything the Patterns didn't.

Yes, that's exactly my concern. We already have a notation, typescript, for expressing unsound types. I'd like to see the patterns produce only types that soundly represent what the patterns enforce.

One way we could add non-validated type information is for pattern objects to be optionally generic, with a type parameter for a hint as to what they return in a particular case, even if that's not validated by fit.

I don't understand this well enough to have an opinion, but I would like to.

@kriskowal
Copy link
Member

I found an example that may be useful for “patterning” our solution. JSON Schema is a JSOM pattern meta language and these are TypeScript types for it https://github.com/ajv-validator/ajv/blob/master/lib/types/json-schema.ts

@Chris-Hibbert
Copy link
Contributor

it's not obvious we should let TS express anything the Patterns didn't

This seems inconsistent with a desire to unify TS, Patterns, and JSDoc. One of the purposes of the JSDoc (and I thought of the TS types) is giving programmers hints, in context, about the required types of parameters as they are programming. Even if Patterns, and hence TS don't want to express anything about the required types of remotables, promises, etc. as a programmer, I want to know what the recipient is expecting me to send them.

Does this leave us with two sets of type descriptions to write? The JSDoc and the Patterns, and then the TS version would be derived from the Patterns. Isn't the support in our IDEs based on the TS info?

@erights
Copy link
Member

erights commented Dec 7, 2022

I think I'm understanding better @turadg 's suggestion:

One way we could add non-validated type information is for pattern objects to be optionally generic, with a type parameter for a hint as to what they return in a particular case, even if that's not validated by fit.

and why it may be the way to bridge these considerations. When calling a function with a parameterized type, how does one call it explicitly providing a type argument for its type parameter? In Java, we'd use <ConcreteType> at the call site. Possibly in TS too? But what about JS with jsdoc TS types? Some way to use the jsdoc cast syntax?

Assuming there is some such notation, then, IIUC, M.remotable would be a type parameterized by T and a call to it would be of static type Remotable<T>. At the call site, the distinction between code and TS type parameters would keep the notational separation between enforced vs unsound types, while still guiding the IDE with the unsound combined type info?

@turadg , am I on the right track?

@dckc
Copy link
Member

dckc commented May 12, 2023

to blow off some steam, I played around with this some and made some non-tangible progress... enough to make a typebox schema for swingset configs, and hence generate a JSON schema to facilitate editing swingset config files:

#7309 (comment)
schema-gen.js
test-schema-gen.js
50576f7

@turadg
Copy link
Member Author

turadg commented Aug 13, 2023

Experiment with dynamically narrowing types using @endo/patterns assertions. We should be able to extract types from shapes and patterns as needed, without codegen.

endojs/endo#1721

mergify bot added a commit that referenced this issue Jul 12, 2024
refs: #6160

## Description

I ran again into the need for type narrowing with a shape object: https://github.com/Agoric/agoric-sdk/pull/8385/files#diff-b17d46d065ac769cdf1d70471b16d141f28672965c18522d58d39722d4852ad4R329-R331

endojs/endo#1721 approached the general problem of inferring the type from the shape. But until we have that, we can at least move the type description onto the shape object so users of that shape can automatically get the type a match implies.

With this, the code referenced above would be,
```js
const questionDesc = cast(info, QuestionSpecShape);
```

### Security Considerations

n/a
### Scaling Considerations

n/a

### Documentation Considerations

Once this settles in agoric-sdk, we may want to move it into Endo. Alternately, Endo waits for more general support.

### Testing Considerations

Has a test. Maybe should use tsd instead of Ava

### Upgrade Considerations

n/a
@dckc
Copy link
Member

dckc commented Aug 8, 2024

recent related work:

@turadg
Copy link
Member Author

turadg commented Aug 8, 2024

@gibson042 filed endojs/endo#2392 to add this ability to Endo. That's where it should live eventually, regardless of how it is adopted in agoric-sdk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-style defensive correctness patterns; readability thru consistency devex developer experience enhancement New feature or request vaults_triage DO NOT USE
Projects
None yet
Development

No branches or pull requests

8 participants