-
Notifications
You must be signed in to change notification settings - Fork 13k
Fix contextual typing on yield and return expressions in generator function #49736
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
Changes from 3 commits
5f4e6fb
156d7a8
e955108
236e10e
b93c25b
f6132ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26638,12 +26638,17 @@ namespace ts { | |
if (contextualReturnType) { | ||
const functionFlags = getFunctionFlags(func); | ||
if (functionFlags & FunctionFlags.Generator) { // Generator or AsyncGenerator function | ||
const use = functionFlags & FunctionFlags.Async ? IterationUse.AsyncGeneratorReturnType : IterationUse.GeneratorReturnType; | ||
const iterationTypes = getIterationTypesOfIterable(contextualReturnType, use, /*errorNode*/ undefined); | ||
if (!iterationTypes) { | ||
if (contextualReturnType.flags & TypeFlags.Union) { | ||
contextualReturnType = filterType(contextualReturnType, type => { | ||
const generator = createGeneratorReturnType(anyType, anyType, anyType, (functionFlags & FunctionFlags.Async) !== 0); | ||
return checkTypeAssignableTo(generator, type, /*errorNode*/ undefined); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
const isAsyncGenerator = (functionFlags & functionFlags.Async) !== 0;
const iterationReturnType = mapType(contextualReturnType,
type => getIterationTypeOfGeneratorFunctionReturnType(IterationTypeKind.Return, type, isAsyncGenerator));
if (iterationReturnType) {
return undefined;
}
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It returns Most of the relevant functions used by if (isTypeAny(type)) {
return anyIterationTypes;
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it returns any. What happens is: we call
I don't understand all the subtleties involved, so that's why I went with filtering the union by checking generator type assignability, but maybe I could be filtering using some of the other I think filtering by calling Let me know what you think. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems function getIterationTypesOfIteratorSlow(type: Type, resolver: IterationTypesResolver, errorNode: Node | undefined) {
const iterationTypes = combineIterationTypes([
getIterationTypesOfMethod(type, resolver, "next", errorNode),
getIterationTypesOfMethod(type, resolver, "return", errorNode),
getIterationTypesOfMethod(type, resolver, "throw", errorNode),
]);
return setCachedIterationTypes(type, resolver.iteratorCacheKey, iterationTypes);
}
The results of those three methods are passed to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I need to look into this a bit further, as changing this results in some other additional diagnostics that we may not want. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The return value for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ended up not using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should have produced the same type for |
||
}); | ||
} | ||
const iterationReturnType = getIterationTypeOfGeneratorFunctionReturnType(IterationTypeKind.Return, contextualReturnType, (functionFlags & FunctionFlags.Async) !== 0); | ||
if (!iterationReturnType) { | ||
return undefined; | ||
} | ||
contextualReturnType = iterationTypes.returnType; | ||
contextualReturnType = iterationReturnType; | ||
// falls through to unwrap Promise for AsyncGenerators | ||
} | ||
|
||
|
@@ -26672,11 +26677,18 @@ namespace ts { | |
const func = getContainingFunction(node); | ||
if (func) { | ||
const functionFlags = getFunctionFlags(func); | ||
const contextualReturnType = getContextualReturnType(func); | ||
let contextualReturnType = getContextualReturnType(func); | ||
if (contextualReturnType) { | ||
const isAsyncGenerator = (functionFlags & FunctionFlags.Async) !== 0; | ||
if (!node.asteriskToken && contextualReturnType.flags & TypeFlags.Union) { | ||
contextualReturnType = filterType(contextualReturnType, type => { | ||
const generator = createGeneratorReturnType(anyType, anyType, anyType, isAsyncGenerator); | ||
return checkTypeAssignableTo(generator, type, /*errorNode*/ undefined); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same idea applies here as above. |
||
}); | ||
} | ||
return node.asteriskToken | ||
? contextualReturnType | ||
: getIterationTypeOfGeneratorFunctionReturnType(IterationTypeKind.Yield, contextualReturnType, (functionFlags & FunctionFlags.Async) !== 0); | ||
: getIterationTypeOfGeneratorFunctionReturnType(IterationTypeKind.Yield, contextualReturnType, isAsyncGenerator); | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
//// [contextualTypeOnYield1.ts] | ||
type FuncOrGeneratorFunc = () => (number | Generator<(arg: number) => void, any, void>) | ||
|
||
const f: FuncOrGeneratorFunc = function*() { | ||
yield (num) => console.log(num); // `num` should be inferred to have type `number`. | ||
} | ||
|
||
//// [contextualTypeOnYield1.js] | ||
"use strict"; | ||
const f = function* () { | ||
yield (num) => console.log(num); // `num` should be inferred to have type `number`. | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
=== tests/cases/compiler/contextualTypeOnYield1.ts === | ||
type FuncOrGeneratorFunc = () => (number | Generator<(arg: number) => void, any, void>) | ||
>FuncOrGeneratorFunc : Symbol(FuncOrGeneratorFunc, Decl(contextualTypeOnYield1.ts, 0, 0)) | ||
>Generator : Symbol(Generator, Decl(lib.es2015.generator.d.ts, --, --)) | ||
>arg : Symbol(arg, Decl(contextualTypeOnYield1.ts, 0, 54)) | ||
|
||
const f: FuncOrGeneratorFunc = function*() { | ||
>f : Symbol(f, Decl(contextualTypeOnYield1.ts, 2, 5)) | ||
>FuncOrGeneratorFunc : Symbol(FuncOrGeneratorFunc, Decl(contextualTypeOnYield1.ts, 0, 0)) | ||
|
||
yield (num) => console.log(num); // `num` should be inferred to have type `number`. | ||
>num : Symbol(num, Decl(contextualTypeOnYield1.ts, 3, 9)) | ||
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) | ||
>console : Symbol(console, Decl(lib.dom.d.ts, --, --)) | ||
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) | ||
>num : Symbol(num, Decl(contextualTypeOnYield1.ts, 3, 9)) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
=== tests/cases/compiler/contextualTypeOnYield1.ts === | ||
type FuncOrGeneratorFunc = () => (number | Generator<(arg: number) => void, any, void>) | ||
>FuncOrGeneratorFunc : () => number | Generator<(arg: number) => void, any, void> | ||
>arg : number | ||
|
||
const f: FuncOrGeneratorFunc = function*() { | ||
>f : FuncOrGeneratorFunc | ||
>function*() { yield (num) => console.log(num); // `num` should be inferred to have type `number`.} : () => Generator<(num: number) => void, void, any> | ||
|
||
yield (num) => console.log(num); // `num` should be inferred to have type `number`. | ||
>yield (num) => console.log(num) : any | ||
>(num) => console.log(num) : (num: number) => void | ||
>num : number | ||
>console.log(num) : void | ||
>console.log : (...data: any[]) => void | ||
>console : Console | ||
>log : (...data: any[]) => void | ||
>num : number | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
//// [contextualTypeOnYield2.ts] | ||
type OrGen = () => (number | Generator<string, (arg: number) => void, undefined>); | ||
const g: OrGen = function* () { | ||
return (num) => console.log(num); | ||
} | ||
|
||
//// [contextualTypeOnYield2.js] | ||
"use strict"; | ||
const g = function* () { | ||
return (num) => console.log(num); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
=== tests/cases/compiler/contextualTypeOnYield2.ts === | ||
type OrGen = () => (number | Generator<string, (arg: number) => void, undefined>); | ||
>OrGen : Symbol(OrGen, Decl(contextualTypeOnYield2.ts, 0, 0)) | ||
>Generator : Symbol(Generator, Decl(lib.es2015.generator.d.ts, --, --)) | ||
>arg : Symbol(arg, Decl(contextualTypeOnYield2.ts, 0, 48)) | ||
|
||
const g: OrGen = function* () { | ||
>g : Symbol(g, Decl(contextualTypeOnYield2.ts, 1, 5)) | ||
>OrGen : Symbol(OrGen, Decl(contextualTypeOnYield2.ts, 0, 0)) | ||
|
||
return (num) => console.log(num); | ||
>num : Symbol(num, Decl(contextualTypeOnYield2.ts, 2, 12)) | ||
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) | ||
>console : Symbol(console, Decl(lib.dom.d.ts, --, --)) | ||
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) | ||
>num : Symbol(num, Decl(contextualTypeOnYield2.ts, 2, 12)) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
=== tests/cases/compiler/contextualTypeOnYield2.ts === | ||
type OrGen = () => (number | Generator<string, (arg: number) => void, undefined>); | ||
>OrGen : () => number | Generator<string, (arg: number) => void, undefined> | ||
>arg : number | ||
|
||
const g: OrGen = function* () { | ||
>g : OrGen | ||
>function* () { return (num) => console.log(num);} : () => Generator<never, (num: number) => void, any> | ||
|
||
return (num) => console.log(num); | ||
>(num) => console.log(num) : (num: number) => void | ||
>num : number | ||
>console.log(num) : void | ||
>console.log : (...data: any[]) => void | ||
>console : Console | ||
>log : (...data: any[]) => void | ||
>num : number | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,29 +1,53 @@ | ||
tests/cases/conformance/generators/generatorReturnContextualType.ts(17,3): error TS2322: Type '{ x: string; }' is not assignable to type '{ x: "x"; }'. | ||
tests/cases/conformance/generators/generatorReturnContextualType.ts(29,3): error TS2322: Type '{ x: string; }' is not assignable to type '{ x: "x"; }'. | ||
Types of property 'x' are incompatible. | ||
Type 'string' is not assignable to type '"x"'. | ||
tests/cases/conformance/generators/generatorReturnContextualType.ts(34,3): error TS2322: Type '{ x: string; }' is not assignable to type '{ x: "x"; }'. | ||
Types of property 'x' are incompatible. | ||
Type 'string' is not assignable to type '"x"'. | ||
|
||
|
||
==== tests/cases/conformance/generators/generatorReturnContextualType.ts (1 errors) ==== | ||
==== tests/cases/conformance/generators/generatorReturnContextualType.ts (2 errors) ==== | ||
// #35995 | ||
|
||
function* f1(): Generator<any, { x: 'x' }, any> { | ||
return { x: 'x' }; | ||
} | ||
|
||
function* g1(): Iterator<any, { x: 'x' }, any> { | ||
return { x: 'x' }; | ||
} | ||
|
||
async function* f2(): AsyncGenerator<any, { x: 'x' }, any> { | ||
return { x: 'x' }; | ||
} | ||
|
||
async function* g2(): AsyncIterator<any, { x: 'x' }, any> { | ||
return { x: 'x' }; | ||
} | ||
|
||
async function* f3(): AsyncGenerator<any, { x: 'x' }, any> { | ||
return Promise.resolve({ x: 'x' }); | ||
} | ||
|
||
async function* g3(): AsyncIterator<any, { x: 'x' }, any> { | ||
return Promise.resolve({ x: 'x' }); | ||
} | ||
|
||
async function* f4(): AsyncGenerator<any, { x: 'x' }, any> { | ||
const ret = { x: 'x' }; | ||
return Promise.resolve(ret); // Error | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
!!! error TS2322: Type '{ x: string; }' is not assignable to type '{ x: "x"; }'. | ||
!!! error TS2322: Types of property 'x' are incompatible. | ||
!!! error TS2322: Type 'string' is not assignable to type '"x"'. | ||
} | ||
|
||
async function* g4(): AsyncIterator<any, { x: 'x' }, any> { | ||
const ret = { x: 'x' }; | ||
return Promise.resolve(ret); // Error | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
!!! error TS2322: Type '{ x: string; }' is not assignable to type '{ x: "x"; }'. | ||
!!! error TS2322: Types of property 'x' are incompatible. | ||
!!! error TS2322: Type 'string' is not assignable to type '"x"'. | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be hoisted out to a
createTypeChecker
singletonanyGenerator
? (Maybe lazily initialized though)