microsoft / TypeScript

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

Implicit Symbol.iterator call in for..of loops / spread destructuring doesn't infer `this` generic type parameter #38388

Open Validark opened 4 years ago

Validark commented 4 years ago

TypeScript Version: 4.0.0-dev.20200507

Search Terms: Symbol.iterator type parameter implicit calling for..of loops object spread destructuring this generic

Code

What follows is 3 different ways of iterating over an object using a custom-made iterator. However, using Symbol.iterator implicitly via the for..of loop fails to produce the same behavior:

const obj = {
    x: 1,
    y: 2,
    z: 3,
    *[Symbol.iterator]<T extends object>(
        this: T,
    ): Generator<NonNullable<{ [K in keyof T]: [K, NonNullable<T[K]>] }[keyof T]>, void, unknown> {
        for (const entry of Object.entries(this)) yield entry as never;
    },
};

{
    const iter = obj[Symbol.iterator]();

    for (let result = iter.next(); !result.done; result = iter.next()) {
        const { value } = result;
        value; // ["x", number] | ["y", number] | ["z", number]
    }
}

for (const value of obj[Symbol.iterator]()) {
    value; // ["x", number] | ["y", number] | ["z", number]
}

for (const value of obj) {
    value; // NonNullable<{ [K in keyof T]: [K, NonNullable<T[K]>]; }[keyof T]>
    // bad! This should be ["x", number] | ["y", number] | ["z", number]
}

// also, spread destructuring suffers from the same problem!
const values = [...obj]; // Array<NonNullable<{ [K in keyof T]: [K, NonNullable<T[K]>]; }[keyof T]>>
// bad! This should be Array<["x", number] | ["y", number] | ["z", number]>

Expected behavior: for (const value of obj) should implicitly do the type equivalent of calling our iterator symbol, i.e. for (const value of obj[Symbol.iterator]()). That means our type parameter should be correctly inferred like so: obj[Symbol.iterator]<T>(this: T).

Actual behavior: T cannot be inferred as the local this type, so all derivative types cannot be evaluated and the type stays as its generic version.

Playground Link

Related Issues: n/a

Validark commented 4 years ago

I did my best to track down this problem on my own, and I believe the problem stems from this bit of code. However, I don't know the codebase well enough to know where to go from here:

// file: src/compiler/checker.ts
/**
 * Gets the *yield*, *return*, and *next* types of an `Iterable`-like or `AsyncIterable`-like
 * type from its members.
 *
 * If we successfully found the *yield*, *return*, and *next* types, an `IterationTypes`
 * record is returned. Otherwise, `noIterationTypes` is returned.
 *
 * NOTE: You probably don't want to call this directly and should be calling
 * `getIterationTypesOfIterable` instead.
 */
function getIterationTypesOfIterableSlow(type: Type, resolver: IterationTypesResolver, errorNode: Node | undefined) {
    const method = getPropertyOfType(type, getPropertyNameForKnownSymbolName(resolver.iteratorSymbolName));
    const methodType = method && !(method.flags & SymbolFlags.Optional) ? getTypeOfSymbol(method) : undefined;
    if (isTypeAny(methodType)) {
        return setCachedIterationTypes(type, resolver.iterableCacheKey, anyIterationTypes);
    }

    const signatures = methodType ? getSignaturesOfType(methodType, SignatureKind.Call) : undefined;
    if (!some(signatures)) {
        return setCachedIterationTypes(type, resolver.iterableCacheKey, noIterationTypes);
    }

    const iteratorType = getUnionType(map(signatures, getReturnTypeOfSignature), UnionReduction.Subtype);
    const iterationTypes = getIterationTypesOfIterator(iteratorType, resolver, errorNode) ?? noIterationTypes;
    return setCachedIterationTypes(type, resolver.iterableCacheKey, iterationTypes);
}

I think the problem is that map(signatures, getReturnTypeOfSignature) is used to grab the (raw) return type(s) and that neglects to account for the possibility of there being an inferred this parameter.

Consider this (simpler) code:

const obj = {
    x: 1,
    y: 2,
    z: 3,

    *[Symbol.iterator]<P extends object>(this: P): Generator<keyof P> {
        for (const k in this) yield k;
    },
};

for (const value of obj) {
    console.log(value); // value is `keyof P`, but should be `"x" | "y" | "z"`
}

Here are the relevant variables in getIterationTypesOfIterableSlow when inferring value from for (const value of obj)

type: { x: number; y: number; z: number; [Symbol.iterator]<P extends object>(this: P): Generator<keyof P, any, unknown>; }
methodType: <P extends object>(this: P) => Generator<keyof P, any, unknown>
signatures: [ <P extends object>(this: P): Generator<keyof P, any, unknown> ]
iteratorType: Generator<keyof P, any, unknown>

I am guessing that assignParameterType or assignContextualParameterTypes may have something to do with the solution, but again, I really have no idea. Please help!

Validark commented 4 years ago

I fixed it!! I'll submit a PR soon! image

Validark commented 3 years ago

Sorry for dropping off the face of the earth. I'm back in the game! My next priority and current goal is to get this PR out this week.

Validark commented 3 years ago

Update: I've been putting a bunch of hours into this, I hope I am almost finished with it now.

RyanCavanaugh commented 2 years ago

We'd take a PR on this so long as it's not invasive, but it doesn't seem like this is a high priority right now due to the low volume of feedback we've seen on it.

greggman commented 1 year ago

I think I ran into this issue recently. I had code like this

    [Symbol.iterator](): IterableIterator<CaptureState> {
        let i = 0;
        this.iterating = true;

        return {
            registry: this,
            next() {
                while (i < this.registry.objects.length) {
                    const obj = this.registry.objects[i++].deref();
                    if (obj === undefined) {
                        continue;
                    }
                    return { value: this.registry.get(obj), done: false };
                }
                this.registry.iterating = false;
                return { done: true };
            },
        };
    }

It complains about this.registry not existing but it does exist and the code actually runs fine. I worked around it by simplifying the code but was surprised TS lost track of this.

rotu commented 8 months ago

This also causes a leak of type parameters:

type ArrayIteratorMethod = <This extends ArrayLike<any>>(this: This) => IterableIterator<This[number]>
type MyStringArray = {
    [k:number]: string,
    length: number,
    [Symbol.iterator]: ArrayIteratorMethod
}
const y = ['a','b','c'] as MyStringArray

for (const b of y[Symbol.iterator]()){ // const b:string
    //     ^?
    b satisfies string
    // @ts-expect-error
    b satisfies symbol 
}
for (const b of y){ // const b: This[number]  (should be inferred as string, but instead `This` escapes from scope)
    //     ^?
    b satisfies string
    // so this unexpectedly passes, since the type of 'b' behaves like `any`
    // @ts-expect-error
    b satisfies symbol
}

Workbench Repro

typescript-bot commented 7 months ago

:wave: Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of this repro running against the nightly TypeScript.


Comment by @rotu

:x: Failed: -

Historical Information
Version Reproduction Outputs
4.9.3, 5.0.2, 5.1.3, 5.2.2, 5.3.2

:x: Failed: -

  • Unused '@ts-expect-error' directive.