Skip to content

Feature Request/Question: Lift the explicit type requirement in assertions for participation in CFA #45385

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

Open
5 tasks done
devanshj opened this issue Aug 9, 2021 Β· 5 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@devanshj
Copy link

devanshj commented Aug 9, 2021 β€’

Suggestion

πŸ” Search Terms

explicit type, assertion, CFA

βœ… Viability Checklist

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

As stated in #32695, functions using asserts require explicitly typed/annotated like so:

declare let x: unknown;
const aFoo = a.literal("foo");
aFoo(x); // error
// Assertions require every name in the call target to be declared with an explicit type annotation.(2775)
//  input.ts(2, 7): 'aFoo' needs an explicit type annotation.

const _aBar = a.literal("bar")
const aBar: typeof _aBar = _aBar;
aBar(x); // works
let test1: "bar" = x

declare let y: unknown;
a.string(y) // works
let test2: string = y;

namespace a {
  export function literal<T>(t: InferLiteral<T>){
    return function(v: unknown): asserts v is T {
      if (v !== t) throw new Error()
    }
  }

  export function string(v: unknown): asserts v is string {
    if (typeof v !== "string") throw new Error();
  }
}
type InferLiteral<T> =
  | (T extends string ? T : string)
  | (T extends number ? T : number)
  | (T extends boolean ? T : boolean)

The reason stated in the PR is "This particular rule exists so that control flow analysis of potential assertion calls doesn't circularly trigger further analysis."

My question is, umm, what does this mean in more layman terms? My impression was it's to not allow recursive usage but looks like that's not the case because this compiles... So not sure what the above statement means

  namespace a {
    export function literal<T>(t: InferLiteral<T>){
      return function(v: unknown): asserts v is T {
+        let _aLol = a.literal("lol")
+        let aLol: typeof _aLol = _aLol;
+        let x = {} as unknown;
+        aLol(x)
+        let test: "lol" = x;
        if (v !== t) throw new Error()
      }
    }

Also I understand it's a "design limitation" but, excuse my ignorance, is it really that hard to lift it? Because it's quite annoying to make make two variables for the same function then annotated the other, makes me think "Eh why can't the compiler do this for itself" haha. Maybe lift the restriction in some scenarios?

πŸ“ƒ Motivating Example

πŸ’» Use Cases

I was writing a fail-fast parser that basically composes assertion functions something like this... But I have to use the mentioned workaround. Even if this is too much of a feature request, an explanation why the requirement exists would make me feel less annoyed when I redeclare and annotate assertions :P

Playground (Hit on run to see the ParseError with message "At Person.age: Expected a number")

namespace a {
  export const string: Asserter<string> =
    (v, p) => invariant(typeof v === "string", "Expected a string", v, p)

  export const number: Asserter<number> =
    (v, p) => invariant(typeof v === "number", "Expected a number", v, p)
  
  export const object =
    <O extends { [_ in string]: Asserter }>(tO: O):
      Asserter<{ [K in keyof O]: O[K] extends Asserter<infer T> ? T : never }> =>
        (v, p) => {
          invariant((v): v is object => typeof v === "object", "Expected an object", v, p);
          for (let k in tO) {
            (tO[k] as any)((v as any)[k], `${p}.${k}`)
          }
        }
  
  export class ParseError extends Error {
    constructor(message: string, public actual: unknown, public path: string) {
      super(`At ${path}: ${message}`)
    }
  }

  function invariant(test: boolean, message: string, actual: unknown, path: string): void
  function invariant<T, U extends T>(test: (v: T) => v is U, message: string, actual: T, path: string): asserts actual is U
  function invariant<T>(test: (v: T) => boolean, message: string, actual: T, path: string): void
  function invariant(test: boolean | ((v: unknown) => boolean), message: string, actual: unknown, path: string) {
    if (typeof test === "function" ? !test(actual) : !test)
      throw new ParseError(message, actual, path);
  }
}
type Asserter<T = unknown> = (v: unknown, path: string) => asserts v is T


const _aPerson = a.object({ name: a.string, age: a.number })
const aPerson: typeof _aPerson = _aPerson;

let person = { name: "Devansh", age: true } as unknown;
aPerson(person, "Person");
let test: string = person.name
@RyanCavanaugh
Copy link
Member

This rule exists so that the feature could be added without drastically impacting performance.

Let's say you had some sequence of calls

let x = `e`;
fn1(x);
fn2(x);
fn3(x);
fn4(x);
let m = x; // <-

where all of fn1 .. fn4 had non-explicitly-declared types. If you wanted to query the type of m (independently, like you would in an editor), today there is only one step required -- compute the type of e. This can be safely done because the rule says that the calls to fn1 et al. cannot legally resolve to an assertion signature.

Without the rule, we would have to compute types for all fn1 et al., because anyone one of them could resolve to an assertion signature.

The worst-case scenario here could look something like

declare function mut1(x: unknown): asserts x is (n: unknown) => asserts n is (m: unknown) => m is string;
function nightmare(a: unknown, b: unknown, c: unknown, n: unknown) {
  mut1(a);
  a(b);
  b(c);
  c(n);
  const a = n;
}

where the computation of n, because it's an argument to c, requires a full evaluation of every inbound reference to an identifier used in its initialization. The only rule that "gets it right" without effectively computing the side effects of the entire function body for any arbitrary identifier reference is the one that says the target must be explicitly-annotated, because at that annotation boundary we can stop the control flow analysis.

@devanshj
Copy link
Author

devanshj commented Aug 11, 2021 β€’

TLDR: Got it, explicitness is the only way to avoid a negative impact on performance -- but explicitness can be achieved by a new less-annoying syntax too! -- for example a.literal("foo") as assertion


Thanks a lot for the explanation! I'm convinced that it indeed is a hard problem to solve without impacting the performance, I did thought about this for a while and it's true that requiring an explicit annotation is the only solution that "gets it right". Although I think I have an improvement on that.

So if I understand it correctly I think when you said "today there is only one step required -- compute the type of e" you were simplifying it bit and this is a less simplified version of what happens (assuming a little different scenario) -

  1. User does a quickinfo query for type of x
  2. TS starts looking for function calls that pass x as an (direct) argument in the lines above it
  3. Finds fn4, checks if it's annotated (which is probably easy to check as I'd be in the ast or some quickly derived tree of that); nope it isn't, continue
  4. Finds fn3 does the same thing; nope it's not annotated, continue
  5. Finds fn2 does the same thing; okay it's annotated, resolve it's type to see if it's making an assertion; okay it is, grab the asserted type
  6. Stop no need to go upward, show the asserted type (TS stops even if fn1 was annotated and makes an assertion but we don't care as whatever assertion fn2 would be making would be the final -- you see it's an "assertion" :P -- it might be asserting an impossibility because of what fn1 is asserting and we'd show red squiggles on fn2 (afaik) but that's an entirely different operation which we aren't concerned about when making a quickinfo query)

Okay we saved ourselves from resolving types of fn4 and fn3 which is great. So essentially, the explicit annotation requirement acts as some sort of a "first pass" as checking if a function is annotated or not is cheap.

But the problem here is it's too much of a requirement, if we want a marker for "this function might be making an assertion" one could introduce a syntax for it -- say as assertion

So basically this...

let aFoo = a.literal("foo") as assertion

would be equivalent to writing this...

let _aFoo = a.literal("foo")
let aFoo: typeof _aFoo = _aFoo

And now the error would say "Assertions require call target to be annotated with 'as assertion' or every name in the call target to be declared with an explicit type annotation"

Afaict this would not add much cost, in fact checking if a function asserts via "as assertion" would be faster than checking if all call targets are explicitly annotated (not sure tho maybe both would be just flags on the node)

And if I were to make a critic: using explicit type annotation as a first pass for assertions is a bit cheating as both have no correlation as such, it took me a while to get that the ultimate goal is to have a first-pass that is cheap to check

I think this is a pretty new feature and people aren't leveraging it like I'm doing -- I already found myself redeclaring 6 assertions and the project isn't even complete yet :P -- ultimately it just makes the complier look a little stupid, the explicitness requirement for a first-pass is very much valid, I totally get it, but it would be nice if there was a syntax to aid it.

What are your thoughts?

(Imagine if I got this all wrong hahaha xD)

@devanshj
Copy link
Author

@RyanCavanaugh senpai wdyt? Also you haven't labelled it anything yet.

@andrewbranch andrewbranch added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Aug 23, 2021
@jcalz
Copy link
Contributor

jcalz commented Sep 21, 2021

crosslinking to #33622 for the error message implementation and to #32695 for the assertion function implementation

@jcalz
Copy link
Contributor

jcalz commented Sep 21, 2021

I definitely want this, but since it's apparently too hard to do it, could this at least be a request for a "quick fix" that gives you some idea what you should be doing? #34596 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants