microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.92k stars 12.47k forks source link

array filter/find has inconsistent results and incorrect constraint specifications #56572

Open craigphicks opened 11 months ago

craigphicks commented 11 months ago

🔎 Search Terms

type predicate error 2677 union of arrays type predicate array filter result infer return type from constraint

One error noted in this bug report (2345 on the rhs of a1 in the code) is a known problem with design-limitation status, I believe. That is incidental and not the main focus of this bug report.

🕗 Version & Regression Information

⏯ Playground Link

playground

💻 Code

interface Fizz {
    id: number;
    fizz: string;
}

interface Buzz {
    id: number;
    buzz: string;
}

declare function isFizz(x: unknown): x is Fizz;
const x1 = ([] as Fizz[]|Buzz[]).find(isFizz);
//    ?^  x1: Fizz | undefined
const x2 = ([] as Buzz[]).find(isFizz);
//    ?^  x2: Buzz | undefined

const y1 = ([] as Fizz[]|Buzz[]).filter(isFizz);
//   ?^  y1: Fizz[]
const y2 = ([] as Buzz[]).filter(isFizz);
//   ?^  y2: Buzz[]

// Different results obtained for this non library version
declare function filter2<T,S extends T>(predicate:(t:T)=>t is S, arr:T[]): S[] ;
declare function filter2<T>(predicate:(t:T)=>unknown, arr:T[]): T[] ;

const z1 = filter2(isFizz, ([] as Fizz[]|Buzz[]));
//   ?^  z1: Fizz[]
const z2 = filter2(isFizz, ([] as Buzz[]));
//   ?^  z2: Fizz[]

// Related: If the constraint "S extends T" is ommitted, an error occurs.
declare function filter3<T,S>(predicate:(t:T)=>t is S, arr:T[]): (T&S)[];
//                                                  ~ Type 'S' is not assignable to type 'T'. 2677

// Assuming the object type T is open (i.e., could have other keys, which is the TS default assumption)
// then the filter result should be (T&S)[], so try defining that result explicitly (without an overload, to avoid confusion).
// This should work:
declare function typePredicateFilter4<D, T extends D,S extends D>(predicate:(t:D)=>t is S, arr:T[]): (T&S)[];

const a1 = typePredicateFilter4(isFizz, ([] as Fizz[]|Buzz[])); // error
//   ?^  z1: Fizz[] (invalidated by error)
const a2 = typePredicateFilter4(isFizz, ([] as Buzz[]));
//   ?^  z2: (Buzz & Fizz)[]

🙁 Actual behavior

  1. The types of y2 (Buzz[]) and z2 (Fizz) differ.

  2. As shown in function3, omitting the constraint S extends T results in error 2677.

  3. The result a2 is (Fizz & Buzz)[] as desired (good), but the result of a1 invalidated because of the error occurring on Fizz[]|Buzz[]. So typePredicateFilter4 is not currently usable in the general case.

🙂 Expected behavior

  1. The types of y2 and z2 match.

  2. Want to be able to omit the that constraint S extends T which triggers the error.
    2.1. One reason we want to omit it is because S extends T triggers the inference logic we don't need if (T&S) is explicitly provided as the return type.
    2.2. The other reason is that S extends T unnecessarily constrains the domain of the predicate type function. See typePredicateFilter4 for the correct constraints - D is the domain of the predicate function, and both T and S independently extends D.

  3. typePredicateFilter4 should not produce an error when Fizz[]|Buzz[] is the array argument type. This is an already known issue. 1 and 2 can be solved without solving this problem #3.

Additional information about the issue

Some special type inference logic to calculate the return type is triggered by the presence of the type predicate function arg is S as an argument, and that logic uses the constraint S extends T to infer the return type. If the requirement of having a constraint is dropped, and return type was specified explicitly (as in function filter3 and typePredicateFilter4, then that logic would not be required.

Would such a change be good? If (T & S) is what is actually expected to pass the type predicate function, then I think yes.

Moreover, the entire result signature output of typePredicateFilter4is "linear" with respect to it type inputs. This allows faster resolution.

RyanCavanaugh commented 11 months ago

There's a lot of intermingled concerns here.

Regarding improvements to find/filter - we're not opposed to well-justified improvements here

Regarding improvements to inference in general - same, though probably more difficult

Want to be able to omit the that constraint S extends T which triggers the error.

Not really on the table IMO - the subtype invariant on user-defined type predicates is well-justified. If things you have to do to satisfy that cause problems elsewhere, we should address those consequences on their own terms.

craigphicks commented 11 months ago

@RyanCavanaugh

Want to be able to omit the that constraint S extends T which triggers the error.

Not really on the table IMO - the subtype invariant on user-defined type predicates is well-justified. If things you have to do to satisfy that cause problems elsewhere, we should address those consequences on their own terms.

Let me revise my poor choice of words. Those words should be - I want to replace the constraint S extends T.

Specifically, by replacing

filter<S extends T>(predicate: (value: T, index: number, array: T[]) => value is S, thisArg?: any): S[];

with

filter<D, T extends D, S extends D>(predicate: (value: D, index: number, array: T[]) => value is S, thisArg?: any): (T&S)[];

the constraint S extends T is replaced with two constraints:

Why do I say S extends T is wrong? After all, in the context of the filter function S extends T implies both T extends D and S extends D, which is good!

However, S extends T also implies something else - that every element of S must be an element of T.

Consider this -

declare function predicate(d:1|2|3): d is 1;
declare const arr: (2|3)[];
const x = arr.filter(predicate);

Here S does not extend T. Therefore, the function

filter<S extends T>(predicate: (value: T, index: number, array: T[]) => value is S, thisArg?: any): S[];

considered as a stand alone function (not part of an overload), should fail, right? Apparently not:

declare function filterStandAlone<T, S extends T>(predicate:(t:T)=>t is S, arr:T[]): S[];
const z = filterStandAlone(predicate,arr); // no error
//    ^? const z: 1[]

I believe what is happening is that the infer logic is using S extends T not as a hard constraint, but rather an instruction to infer the output type - but it doesn't finally calculate the minimal correct output never[].

Rather than analyzing why the infer logic doesn't calculate the minimal correct output never[], I want to assert that in this scenario with filter we should not be putting the onus on the infer logic to to calculate the return type. It is overthinking the problem, resulting in over complexity. (As well as altering the meaning of S extends T from a hard constraint to something else, which again requires more thinking to understand).

If instead we use the typePredicateFilter4 defined in the OP -

const y = typePredicateFilter4(predicate,arr);
//      ?^  const y = never[]

the minimal correct return type is produced without any contribution from the infer logic at all. So easy - simply because we didn't make it harder than it needed to be.