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

ReturnType support for assertions #34636

Closed
sirian opened this issue Oct 22, 2019 · 17 comments
Closed

ReturnType support for assertions #34636

sirian opened this issue Oct 22, 2019 · 17 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@sirian
Copy link
Contributor

sirian commented Oct 22, 2019

Suggestion

Not found a way to provide return type for assert function. Look at use case

Use Cases

function assert1(e: any, m?: string): asserts e {
    if (!e) throw new Error();
    return e;
}
function assert2<T>(e: T, m?: string): asserts e {
    if (!e) throw new Error();
    return e; // error:  T is not assignable to void
}

declare const p: Promise<string | undefined>;
p.then(assert1).then((x) => x.length); // error: x is string | undefined
p.then(assert2).then((x) => x.length
); // error: x is string | undefined

This doesn't work also

const assertAndReturn = <T>(x: T) => { 
    assert(x);
    return x;
}
p.then((x) => { assert(x); return x; }).then((x) => x.length) // ok
p.then(assertAndReturn).then((x) => x.length); // error. x is string | undefined

Playground

Related issues

#34596

@dragomirtitian
Copy link
Contributor

This is by design, according to the PR introducing this features:

A function call is analyzed as an assertion call or never-returning call when

  1. the call occurs as a top-level expression statement, and
  2. the call specifies a single identifier or a dotted sequence of identifiers for the function name, and
  3. each identifier in the function name references an entity with an explicit type, and
  4. the function name resolves to a function type with an asserts return type or an explicit never return type annotation.

The first item states that the call must occur as a top-level statement, this means that the call can't be nested in an expression (such as a parameter to another function or in an assignment). This means that effectively, even if you could return from an assertion function, using it in a position where the return value could be use, the function would not be analyzed as an assertion but rather as a regular function call. So it is best to implicitly just say that an assertion function returns void. This is also outlined in the PR:

An asserts return type predicate implies that the returned value is of type void, and there is no provision for returning values of other types.

Your first example (assert1) allows a return because of a quirk in assignability rules, since e is any it is also assignable to void. Change to unknown and you will get an error.

Suggestion: @ahejlsberg Maybe disallow any return statement that has an expression in assertion functions?

@nmain
Copy link

nmain commented Oct 22, 2019

The real world use case outlined here is handled fine by typescript if, instead of then chains, you make an async function:

async function f() {
    const val = await p;
    assert1(val);
    val.length; // number
}

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Oct 22, 2019
@sirian
Copy link
Contributor Author

sirian commented Oct 22, 2019

@nmain There are many other use cases. Especially in arrow functions

const bar = () => assert(someTask());
// instead of
const bar = () => {
     const result = someTask();
     assert(result);
     return result;
}

@nmain
Copy link

nmain commented Oct 22, 2019

You can do that easily without getting involved with the assert feature at all:

function assert<T>(v: T): NonNullable<T> {
    if (v == null) {
        throw null;
    }
    return v as any;
}

declare function someTask(): string | undefined;

const bar = () => assert(someTask()); // () => string, instead of () => string | undefined

I certainly see the draw of being able to use the same assert function in both ways, though, with some sort of type annotation that encompasses both the asserting and the returning a subset of the values idea.

@sirian
Copy link
Contributor Author

sirian commented Oct 23, 2019

@nmain I know. But this increase entropy - i should have two assert functions with same code;

function assert(value: any): asserts value {
    if (!value) {
        throw new Error("Assertion failed.")
    }

function assertAndReturn<T>(value: T): Exclude<T, null | undefined | void | 0 | false | ''> {
    if (!value) {
        throw new Error("Assertion failed.")
    }
    return value as any;
}

It's silly IMO. And as i said before - why not to add narrowing at least?

const assertAndReturn = <T>(x: T) => { 
    assert(x);
    return x;
}
p.then((x) => { assert(x); return x; }).then((x) => x.length) // Works fine 
p.then(assertAndReturn).then((x) => x.length); // Doesn't narrow

@RyanCavanaugh @dragomirtitian Is it hard to add narrowing for code above?

@mprobst
Copy link
Contributor

mprobst commented Jan 22, 2020

As a data point, both returning a value and narrowing the type of the input is also how Closure's goog.asserts.assert() & Co work.

@eyedean
Copy link

eyedean commented Feb 1, 2020

I also tried to get around this by having two assert functions, one to assert and one to return, and have the latter call the former. e.g.

function _assert<T = any>(x: T): asserts x { // say, private one
    if (!x) { throw new Error(); }
}

function myAssert<T = any>(x: T): T { // say, public one that returns
    _assert(x);
    return x;
}

type Employee = { name: string; }

function getEmployee(id: number): Employee | null {
    if (id !== 2) {
        return null;
    }
    return { name: "James" };
}

const emp = getEmployee(3);

console.log(myAssert(emp).name); // Object is possibly null :(

// Pure check:
// _assert(emp); // without this line, I get "Object possibly null" in the console.log
// console.log(emp.name);

But unfortunately, while myAssert is calling _assert(x) on its own, typescript doesn't infer that an assert x is performed. :(

@LinusU
Copy link
Contributor

LinusU commented Feb 7, 2020

I ran into to trying to do this today, my use case is this:

interface Provider {
  initiatePayment (items: Items): Promise<string>
}

function getProvider (id: string): asserts id is ('foo' | 'bar') & Provider {
  switch (id) {
    case 'foo': return fooProvider
    case 'bar': return barProvider
    default: throw new Error(`Unknown payment provider: ${id}`)
  }
}

function makePurchase (provider: string, items: Items) {
  const providerImpl = getProvider(provider)

  const paymentId = await providerImpl.initiatePayment(items)

  savePurchase({
    provider,      // <---- Provider needs to be typed as 'foo' | 'bar' here!
    paymentId,
    items
  })
}

@RReverser
Copy link
Contributor

Another datapoint: combining asserts with an actual return type would allow to improve typings for in-place Object.assign. See https://twitter.com/RReverser/status/1350524568075710464.

@ghost ghost mentioned this issue Feb 7, 2021
5 tasks
@ghost
Copy link

ghost commented Feb 7, 2021

I would really love to see this happen, and for the restriction

  1. the call occurs as a top-level expression statement

to be made obsolete.
I have a function in plain, simple JavaScript that does both, asserts something, and returns another type assertion as a boolean.
The natural use is to use it in an if statement or ternary condition, but TS can't give it a correct signature.

@RReverser
Copy link
Contributor

@RyanCavanaugh Just checking - was this closed because it's implemented now or because it's not considered or?

@ttencate
Copy link

@RReverser Presumably because #40562 is a duplicate of this one.

@RReverser
Copy link
Contributor

Hmm 1) it wasn't linked during closing and 2) it's a newer issue, so I'd expect the newer one would be closed as a duplicate...

@benwainwright
Copy link

benwainwright commented Apr 3, 2023

@nightlyherb no it doesn't; that function you've written isn't returning an assertion, it's just returning a value. It's not a type guard.

Here is my use case for this

type AssertionType = { toBeUndefined: () => void }

const expect: <T>(bar: T | undefined) => asserts bar is T & AssertionType = (bar) => {
  return {
    toBeUndefined: () => {},
  };
};

// Currently there is an error here
expect(foo).toBeUndefined()

If this feature were implemented in TS, I'd probably submit a contribution to the jest codebase to make certain matchers type guards.

@joakimbeng
Copy link

@nightlyherb no it doesn't; that function you've written isn't returning an assertion, it's just returning a value. It's not a type guard.

Here is my use case for this

type AssertionType = { toBeUndefined: () => void }

const expect: <T>(bar: T | undefined) => asserts bar is T & AssertionType = (bar) => {
  return {
    toBeUndefined: () => {},
  };
};

// Currently there is an error here
expect(foo).toBeUndefined()

If this feature were implemented in TS, I'd probably submit a contribution to the jest codebase to make certain matchers type guards.

Having type guard matchers with the Jest matchers interface is a highly wanted feature IMO!
Creating a custom wrapper like expectToBeDefined<T>(value: T | undefined): asserts value is T works but gives bad error reports when the assertion fails

@ExE-Boss
Copy link
Contributor

@benwainwright

Here is my use case for this

type AssertionType = { toBeUndefined: () => void }

const expect: <T>(bar: T | undefined) => asserts bar is T & AssertionType = (bar) => {
  return {
    toBeUndefined: () => {},
  };
};

// Currently there is an error here
expect(foo).toBeUndefined()

If this feature were implemented in TS, I'd probably submit a contribution to the jest codebase to make certain matchers type guards.

That’s because your code is equivalent to:

type AssertionType = { toBeUndefined: () => void }

const expect: <T>(bar: T | undefined) => asserts bar is (T & AssertionType) = (bar) => {
	return {
		toBeUndefined: () => {},
	};
};

// Currently there is an error here
expect(foo).toBeUndefined();

What you want, and is currently impossible:

type AssertionType = { toBeUndefined: () => void }

// The error is now here:
const expect: <T>(bar: T | undefined) => (asserts bar is T) & AssertionType = (bar) => {
	return {
		toBeUndefined: () => {},
	};
};

expect(foo).toBeUndefined();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests