Optional Iterator parameter "incorrectly" accepts spread arguments

[DanielRosenwasser]

declare const strs1: string[];
declare const strs2: IterableIterator<string>;

declare function badFoo(x?: Iterable<string>): void

// This function is BAD.
// The last two calls are INCORRECTLY accepted.

badFoo(strs1); // accepted - fine
badFoo(strs2); // accepted - fine

badFoo(...strs1); // accepted, BAD ❌
badFoo(...strs2); // accepted, BAD ❌

Playground Link

[jakebailey]

@bschnurr thanks 😄

[DanielRosenwasser]

So from other discussions, this "works" because spreading a string produces another set of strings for each optional position. This is two slightly undesirable behaviors combined into one harder-to-explain one.

declare function takeOptionals(x?: number, y?: number): void;
declare const nums: Iterable<number>;
takeOptionals(...nums); // fine, but suspicious

declare function eatStrings(strs: Iterable<string>): void;
eatStrings("hello"); // almost never desirable to begin with?

[Jamesernator]

takeOptionals(...nums); // fine, but suspicious

In most cases this is gonna be a forward-compatibility hazard, like if takeOptionals gets another parameter in future ...Iterable<number> might not be compatible.

eatStrings("hello"); // almost never desirable to begin with?

This on the other hand is something I have used in cases where any sequence of characters is valid. In general TypeScript is pretty weak here as it lacks any way to specify Iterable<codePoint> or Iterable<char>.

Mind you it is certainly somewhat of a hazard that someone passes non-char strings, which is why whenever I use it I just go straight to flattening it to counter this exact problem:

for (const str in input) {
    for (const char in string) {
        // feed char into the parser or whatever
    }
}

[fatcerberus]

I was under the impression that spread calls weren’t even allowed unless 1. You’re spreading into a rest parameter or 2. The spread source is a tuple type matching the function’s parameter list. When did that change?

[RyanCavanaugh]

I don't quite get what the defect is supposed to be? We also let you write the equivalent

badFoo(strs1[0]);
badFoo([...strs2][0]);

The behavior we actually work hard to exhibit here is that a spread is valid if all its possible arities result in valid calls, including the generally-allowed pattern of dynamic excess arity.

Strings being iterables of themselves means it's always going to be possible to be "off by one" in terms of the number of times you spreaded something. I don't know how we can detect that unless we somehow said that string iterations produced some special supertype of string that itself was not iterable by convention, but then we'd need some way to undo that despecialization once they combined with other chars.

[jakebailey]

This came from a bug in pyright/pylance that came down to a the spread hiding the fact that ... shouldn't have been there for creating a set; I hadn't realized it had to do with the optional parameter; I agree that it's unfortunate but working as intended. One can write similar code with tuples/lists and *args in Python, for comparison, and get it wrong by adding a * in the wrong place.

[DanielRosenwasser]

Ultimately I think it's a case where strings being at all compatible with Iterable<string> is extremely suspicious unless it's made explicit with a type assertion. Seems a bit heavy-handed, but I can't think of the last time I needed to spread a string except for emoji examples.

[dkolman-vendavo]

Doesn't have to be Iterable, function with an optional parameter (of any type) incorrectly accepts spread arguments:

declare function required(a: number): void;
declare function optional(a?: number): void;

const nums = [1, 2, 3];

required(...nums); // error as expected 
optional(...nums); // accepted, BAD ❌

Playground link