Higher order type parameter promotion fixes

[ahejlsberg]

This PR makes a couple of tweaks to our higher order type parameter promotion algorithm. Specifically, we don't promote type parameters of a function type argument when the contextual signature doesn't reference type parameters in parameter positions, and we now treat inferences made from parameter positions in type parameter promotion as contravariant.

Fixes #30324.

[weswigham]

@typescript-bot run dt let's see if this fixes ramda completely~

[typescript-bot]

Heya @weswigham, I've started to run the Definitely Typed test suite on this PR at e7afa6c. You can monitor the build here. It should now contribute to this PR's status checks.

[weswigham]

@ahejlsberg this looks new:

Error in css-tree
Error: /home/vsts/work/1/s/dtslint-runner/DefinitelyTyped/types/css-tree/css-tree-tests.ts:211:1
ERROR: 211:1  expect  Expected type to be:
  List<Test>
got:
  List<Test | null>
ERROR: 219:1  expect  Expected type to be:
  List<Test>
got:
  List<Test | null>

might be unrelated, though, don't know. The other failures are caused by #30334 and fixed by #30375.

[weswigham]

Here's the testcase from css-tree:

// @strict: true
interface ListItem<TData> {
    prev: ListItem<TData> | null;
    next: ListItem<TData> | null;
    data: TData;
}
type IteratorFn<TData, TResult, TContext = List<TData>> = (this: TContext, item: TData, node: ListItem<TData>, list: List<TData>) => TResult;
type FilterFn<TData, TResult extends TData, TContext = List<TData>> = (this: TContext, item: TData, node: ListItem<TData>, list: List<TData>) => item is TResult;

declare class List<TData> {
    filter<TContext, TResult extends TData>(fn: FilterFn<TData, TResult, TContext>, context: TContext): List<TResult>;
    filter<TResult extends TData>(fn: FilterFn<TData, TResult>): List<TResult>;
    filter<TContext>(fn: IteratorFn<TData, boolean, TContext>, context: TContext): List<TData>;
    filter(fn: IteratorFn<TData, boolean>): List<TData>;
}
interface Test {
    a: string;
}
const list2 = new List<Test | null>();
const filter1 = list2.filter(function(item, node, list): item is Test {
    this.b; // $ExpectType string
    item; // $ExpectType Test | null
    node; // $ExpectType ListItem<Test | null>
    list; // $ExpectType List<Test | null>
    return !!item;
}, {b: 'c'});
filter1; // $ExpectType List<Test>

the change in expected type of filter1 does seem to be isolated to this branch. It seems like we're inferring Test | null for TResult when we should just be inferring Test - this is likely due to the contravariant parameter inference?

[weswigham]

Noooooope, I think it's due to the subtle change in inference for return type predicates.

[weswigham]

Specifically, we check for a matching parameter index.... except we don't account for this parameters not really counting.

[weswigham]

I opened a PR against this PR - #30386 - with a fix.

[ahejlsberg]

Latest commit changes IdentifierTypePredicate.parameterIndex to be an index into Signature.parameters. It previously was an index into the parameter list of the declaration from which the signature was obtained, which meant we had latent off-by-one errors in signatures with a this parameter (signatures don't include this parameters in the parameters array). Since types always operate on Signature instances and since it is possible for Signature instances to have no declaration associated with them, it really doesn't make sense to store declaration indices.

[weswigham]

Wanna add the test from #30386 so we have more coverage?

[ahejlsberg]

@weswigham Sure, I can do that.

[ahejlsberg]

@typescript-bot run dt

[typescript-bot]

Heya @ahejlsberg, I've started to run the Definitely Typed test suite on this PR at 3dd0814. You can monitor the build here. It should now contribute to this PR's status checks.

[weswigham]

The DT run looks great - just has the instantiation error that was in master back when this branch was first created, so this branch fixes ramda, as intended, and breaks nothing else now. 💓