Soundness issue with Array.prototype.concat and arrays of arrays

[andyfriesen]

TypeScript Version: 2.7.0-dev.201xxxxx

Code

const foo: [number, string][] = [[1, 'one']];
console.log(foo.concat([2, 'two']));

Expected behavior:
Either [[1, 'one'], [2, 'two']] or a type error.

Actual behavior:
[[1, 'one'], 2, 'two']

[Ghabriel]

a.concat(b) pushes the elements of b to a, so the actual behavior is correct (also, what the functions do at runtime is JavaScript-world, not TypeScript). Your expected behavior happens if you use push instead or add extra brackets around [2, 'two']:

const foo: [number, string][] = [[1, 'one']];
foo.push([2, 'two']);
console.log(foo);

or

const foo: [number, string][] = [[1, 'one']];
console.log(foo.concat([[2, 'two']]));

[andyfriesen]

This is absolutely true.

My concern primarily lies with the fact that it was so easy to violate type safety. It would be nice if this showed up in the editor rather than something I had to debug at runtime.

[mhegazy]

One option is to change the definition of concat to be:

concat<U>(...items: (T | ConcatArray<T>)[]): T extends ConcatArray<infer E> ? (E | T)[] : T[];

[mhegazy]

PRs welcomed

[Ghabriel]

I'm working on it, but I've encountered the following error after doing the change:

src/compiler/core.ts:840:42 - error TS2345: Argument of type 'SortedArray<T>' is not assignable to parameter of type 'ReadonlyArray<T>'.
  Types of property 'concat' are incompatible.
    Type '(...items: (T | ConcatArray<T>)[]) => T extends ConcatArray<infer E> ? (E | T)[] : T[]' is not assignable to type '(...items: (T | ConcatArray<T>)[]) => T extends ConcatArray<infer E> ? (E | T)[] : T[]'. Two different types with this name exist, but they are unrelated.
      Type 'T extends ConcatArray<infer E> ? (E | T)[] : T[]' is not assignable to type 'T extends ConcatArray<infer E> ? (E | T)[] : T[]'. Two different types with this name exist, but they are unrelated.

840         const insertIndex = binarySearch(array, insert, identity, compare);
                                             ~~~~~

...and a bunch of errors of the form Argument of type 'T[]' is not assignable to parameter of type 'ReadonlyArray<T>'. This seems to be a bug, or am I overlooking something @mhegazy?

[mhegazy]

did you change the declaration of both Array and ReadonlyArray?

[andersk]

Another example:

const a: number[][] = [[1]].concat([[2]], [3])
// → [[1], [2], 3]

and a related issue with flatMap:

const b: number[][] = [true, false].flatMap<number[]>(x => x ? [1] : [[2]]);
// → [1, [2]]

[RyanCavanaugh]

Rolling up into #36554