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

Analysis of multiple-use generic type parameters on DefinitelyTyped #19596

Closed
RyanCavanaugh opened this issue Oct 30, 2017 · 6 comments
Closed
Labels
Discussion Issues which may not have code impact

Comments

@RyanCavanaugh
Copy link
Member

Background: We're considering synthesizing union types during generic type inference, same as we do for functions:

function makeArr(x: T, y: T): T[] { return [x, y] }
const ar = makeArr("1", 2); // ar: Array<string | number>

This would likely need to be paired with some sort of syntax for making this not happen in cases like

function addToArray<T>(arr: T[], el: T) { arr.push(el); }
addToArray([1, 2, 3], "foo"); // bad

What follows is a sorting of all the cases I could find on DefinitelyTyped where a type parameter was declared and used in more than one position

T and Lower-priority T

In these cases, two or more uses appear, but there is only one T at any given "level" of inference. These functions would be mostly unchanged under a hypothetical union type inference world.

// adone/glosses/is.d.ts
function inArray<T>(value: T, array: any[], offset?: number, comparator?: (a: T, b: T) => boolean): boolean;

// adone/glosses/shani.d.ts
function spy<T>(object: T, method: keyof T): I.Spy;
function stub<T>(obj: T, method: keyof T): I.Stub;

// adone/glosses/utils.d.ts
function unique<T>(array: T[], projection?: (a: T) => any): T[];
interface BinarySearchFunction {
    <T>(aHaystack: T[], aNeedle: number, aLow?: number, aHigh?: number, aCompare?: (a: T, b: T) => number, aBias?: number): T;
}

function filter<T>(range: I.Range<T>, compare?: I.Comparator<T>): (key: T) => boolean;

// localforage/typings/localforage.d.ts
setItem<T>(key: string, value: T, callback: (err: any, value: T) => void): void;

// benchmark/index.d.ts
static filter<T>(arr: T[], callback: (value: T) => any, thisArg?: any): T[];
static forEach<T>(arr: T[], callback: (value: T) => any, thisArg?: any): void;

// core-js
sort<T>(array: ArrayLike<T>, compareFn?: (a: T, b: T) => number): T[];
reduce<T>(array: ArrayLike<T>, callbackfn: (previousValue: T, currentValue: T, currentIndex: number, array: T[]) => T, initialValue?: T): T;
reduceRight<T>(array: ArrayLike<T>, callbackfn: (previousValue: T, currentValue: T, currentIndex: number, array: T[]) => T, initialValue?: T): T;
findIndex<T>(array: ArrayLike<T>, predicate: (value: T) => boolean, thisArg?: any): number;
turn<U>(callbackfn: (memo: U, value: T, index: number, array: T[]) => void, memo?: U): U;
sort<T>(array: ArrayLike<T>, compareFn?: (a: T, b: T) => number): T[];

// d3-array
function scan<T>(array: ArrayLike<T>, comparator: (a: T, b: T) => number): number | undefined;

// d3-quadtree
function quadtree<T>(data: T[], x?: (d: T) => number, y?: (d: T) => number): Quadtree<T>;

// heap
static pop<T>(array: T[], cmp?: (a: T, b: T) => number): T;
static heapify<T>(array: T[], cmp?: (a: T, b: T) => number): Heap<T>;
static nlargest<T>(array: T[], n: number, cmp?: (a: T, b: T) => number): T[];
static nsmallest<T>(array: T[], n: number, cmp?: (a: T, b: T) => number): T[];

// icepick
function sort<T>(array: T[], compareFunction?: (a: T, b: T) => number): T[];

// jasmine
function spyOn<T>(object: T, method: keyof T): jasmine.Spy

// kefir
scan<W>(fn: (prev: T | W, next: T) => W): Stream<W, S>;
scan<W>(fn: (prev: W, next: T) => W, seed: W): Stream<W, S>;

// lokijs
function binarySearch<U>(array: U[], item: U, fun: (a: U, b: U) => number): { found: boolean; index: number; };

// multiplexjs
aggregate<TAccumulate>(seed: TAccumulate, func: (accumulate: TAccumulate, item: T) => TAccumulate): TAccumulate

// rx-lite-aggregates
aggregate<TAcc>(seed: TAcc, accumulator: (acc: TAcc, value: T) => TAcc): Observable<TAcc>;
reduce<TAcc>(accumulator: (acc: TAcc, value: T) => TAcc, seed: TAcc): Observable<TAcc>;
contains<TOther>(value: TOther, comparer: (value1: T, value2: TOther) => boolean): Observable<boolean>;
sequenceEqual<TOther>(second: Observable<TOther>, comparer: (value1: T, value2: TOther) => number): Observable<boolean>;

Promises...

Just putting this in its own bucket because 😠

// bluebird
export interface Thenable<R> {
	then<U>(onFulfilled: (value: R) => Thenable<U>, onRejected: (error: any) => Thenable<U>): Thenable<U>;
	then<U>(onFulfilled: (value: R) => Thenable<U>, onRejected?: (error: any) => U): Thenable<U>;
	then<U>(onFulfilled: (value: R) => U, onRejected: (error: any) => Thenable<U>): Thenable<U>;
	then<U>(onFulfilled?: (value: R) => U, onRejected?: (error: any) => U): Thenable<U>;
}

Equal-weight T

These use the same type parameter in at least two positions of equal weight in the inference process

// adone/glosses/utils.d.ts
function contains<T>(range: I.Range<T>, key: T, compare?: I.Comparator<T>): boolean;

// cache-manager
wrap<T>(key: string, wrapper: (callback: (error: any, result: T) => void) => void, options: CachingConfig, callback: (error: any, result: T) => void): void;

// cbor
export class Encoder extends stream.Transform {
    constructor();
    addSemanticType<T>(type: new (...args: any[]) => T, encodeFunction: (encoder: Encoder, t: T) => void): void;
}

// cli
getArrayValue<T>(arr: T[], defaultVal: T): T;

// core-js
set<T>(object: Dict<T>, key: PropertyKey, value: T): Dict<T>;
includes<T>(object: Dict<T>, value: T): boolean;
reduce<T>(object: Dict<T>, callbackfn: (previousValue: T, value: T, key: PropertyKey, dict: Dict<T>) => T, initialValue?: T): T;

// crossfilter
reduce<TValue>(add: (p: TValue, v: T) => TValue, remove: (p: TValue, v: T) => TValue, initial: () => TValue): GroupAll<T, TValue>;

// heap
static push<T>(array: T[], item: T, cmp?: (a: T, b: T) => number): void;
static replace<T>(array: T[], item: T, cmp?: (a: T, b: T) => number): T;
static pushpop<T>(array: T[], item: T, cmp?: (a: T, b: T) => number): T;
static updateItem<T>(array: T[], item: T, cmp?: (a: T, b: T) => number): void;

// icepick
function push<T>(array: T[], element: T): T[];
function unshift<T>(array: T[], element: T): T[];

// jasmine
compare<T>(actual: T, expected: T): CustomMatcherResult;

// knockout.postbox
defaultComparer<T>(newValue: T, oldValue: T): boolean;

// microsoft-ajax
add<T>(array: T[], element: T): void;
contains<T>(array: T[], element: T): boolean;
enqueue<T>(array: T[], element: T): void;
insert<T>(array: T[], index: number, item: T): void;
remove<T>(array: T[], item: T): boolean;

// multiplexjs
create<T>(comparison: (x: T, y: T) => number): Comparer<T>;
create<T>(hashCodeProvider: (obj: T) => number, equality: (x: T, y: T) => boolean): EqualityComparer<T>;
compare<T>(objA: T, objB: T): number;

// natural-sort
function naturalSort<T>(a: T, b: T): number;

// nedb
updateIndexes<T>(oldDoc: T, newDoc: T): void;
updateIndexes<T>(updates: Array<{ oldDoc: T; newDoc: T }>): void;

// plottable
function max<C>(array: C[], defaultValue: C): C;
function max<T, C>(array: T[], accessor: (t?: T, i?: number) => C, defaultValue: C): C;
function min<C>(array: C[], defaultValue: C): C;
function min<T, C>(array: T[], accessor: (t?: T, i?: number) => C, defaultValue: C): C;
minDomainExtent<D>(quantitativeScale: QuantitativeScale<D>, minDomainExtent: D): Interactions.PanZoom;
maxDomainExtent<D>(quantitativeScale: QuantitativeScale<D>, maxDomainExtent: D): Interactions.PanZoom;

// prelude-ls
export function max<Comparable>(x: Comparable): (y: Comparable) => Comparable;
export function max<Comparable>(x: Comparable, y: Comparable): Comparable;
export function min<Comparable>(x: Comparable): (y: Comparable) => Comparable;
export function min<Comparable>(x: Comparable, y: Comparable): Comparable;

// ramda
// Charachteristic signatures captured here; file has MANY instances of functions like this
ascend<T>(fn: (obj: T) => any, a: T, b: T): number;
ascend<T>(fn: (obj: T) => any): (a: T, b: T) => number;
clamp<T>(min: T, max: T, value: T): T;
clamp<T>(min: T, max: T): (value: T) => T;
clamp<T>(min: T): (max: T, value: T) => T;
clamp<T>(min: T): (max: T) => (value: T) => T;
comparator<T>(pred: (a: T, b: T) => boolean): (x: T, y: T) => number;
groupWith<T>(fn: (x: T, y: T) => boolean, list: string): string[];

// rx-lite-aggregates
minBy<TKey>(keySelector: (item: T) => TKey, comparer: (value1: TKey, value2: TKey) => number): Observable<T>;
maxBy<TKey>(keySelector: (item: T) => TKey, comparer: (value1: TKey, value2: TKey) => number): Observable<T>;
min(comparer?: (value1: T, value2: T) => number): Observable<number>;
max(comparer?: (value1: T, value2: T) => number): Observable<number>;
average(keySelector?: (value: T, index: number, source: Observable<T>) => number, thisArg?: any): Observable<number>;

// semver-compare
function semverCompare<T>(a: T, b: T): number;

// transducers.js
function push<T>(arr: T[], x: T): T[];

// vue
set<T>(array: T[], key: number, value: T): T;
use<T>(plugin: PluginObject<T> | PluginFunction<T>, options?: T): void;
indexOf<T>(arr: Array<T>, obj: T): number;
@RyanCavanaugh RyanCavanaugh added the Discussion Issues which may not have code impact label Oct 30, 2017
@RyanCavanaugh
Copy link
Member Author

/cc @ahejlsberg @mhegazy

@gcnew
Copy link
Contributor

gcnew commented Oct 31, 2017

I'm strictly against that. To the contrary, I've filed #16107 to fix function inference. Notice, that when the inferencer is strict, it's easy to make it more loose by providing a type annotation. However, when it's loose from the get-go, virtually every generic reference needs a fix. The latter also makes many safety and type-level programming patterns no longer possible.

Another ramification of this suggestion is that any two types should become equitable.

declare operator ===<T>(left: T, right: T): boolean;

"hello" === 7; // now an error, should be OK with the proposed logic
               // with T = string | number

@dead-claudia
Copy link

I'd be okay with it as long as it's something statically differentiable from normal generic parameters, it's something I can still specify explicitly if needed, and of course, if the type inference algorithm was improved substantially (so the type names are much less common). I'd rather not run into confusion over whether T is an interface type, explicit function generic or implicit function generic.

@markusjohnsson
Copy link
Contributor

This will have the effect of having to put type guards in a lot of places, places where I wouldn't have needed it in pure JS, b/c I "know" the types myself. Either that or resort to casts..

I use TypeScript to check that my code is sane and sound and give me errors if not.. not to accept whatever I'm writing and tell me what the result would be. And I expect TypeScript to report the error where it is most likely happening.

function assert<T>(expected: T, actual: T) { /* implementation */ }

assert(1, "foo")

I would expect TypeScript to report an error here and not even having to run the test.

@jack-williams
Copy link
Collaborator

I think a challenge with getting acceptance for this is that many users interpret generics in TypeScript in very different ways. I'd say there are roughly three different ways generics are interpreted in TypeScript:

  1. As a true parametric polymorphic generic function, such as functional map or fold.
  2. Using a generic parameter T that really means any, but wanting auto completion on the result type, such as the makeArr function in the example. Basically a local type alias.
  3. Using generics in a kind of ad-hoc polymorphism style, but without specifying the constraints they put on the type variables. They might say for all types T, but they really mean an implicitly constrained subset of T.

Those that use generics in style 1 and 2 should be fine with this, those that use generics in style 3 are probably against - it might break assumptions in their programs. For instance, in the assert example the function is not really defined for all types T, but those which are constant with respect to functions like typeof. This holds currently (and get's statically rejected), but is broken when the types get widened to unions.

Looking at the examples selected from DefinitelyTyped, I think a significant number fall into category 3. If you were to interpret those functions in a parametric way, most would be useless constant functions, so there must be some hidden constraints associated with the signatures. I'd be concerned that changing the inference mechanism by default might allow people to call those functions in ways that violate the hidden constraints.

As suggested it would probably please more people if it was a separate mechanism, disabled by default. The default behavior would guarantee that inference would always ensure that two values of generic type T have the same behavior under typeof (and possibly other functions like property lookup). Then, as an option the function can be written in a style closer to a true universal type, that works for all types, including widened unions, but does not guarantee uniform behavior for typeof.

@dead-claudia
Copy link

@RyanCavanaugh What was this closed in response to? Just curious because the issue isn't showing me any leads here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Issues which may not have code impact
Projects
None yet
Development

No branches or pull requests

5 participants