Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 207 forks source link

Generate TypeScript types from matcher patterns #6160

Open turadg opened 2 years ago

turadg commented 2 years ago

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

kriskowal commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Substitute ML for pl and my intention is the same.

Just checking. You meant M, right?

erights commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

mhofman commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

... 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 commented 2 years ago

Most likely it'd be something closer to

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

erights commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

There's one more coming for FarClasses with https://github.com/Agoric/agoric-sdk/pull/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 https://github.com/Agoric/agoric-sdk/pull/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 commented 2 years ago

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 commented 2 years ago

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:

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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:

https://github.com/Agoric/agoric-sdk/issues/7309#issuecomment-1543998300 schema-gen.js test-schema-gen.js 50576f7

turadg commented 1 year ago

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

https://github.com/endojs/endo/pull/1721

dckc commented 3 months ago

recent related work:

turadg commented 3 months ago

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