Skip to content
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

Regression: Intersection of array and tuple not assignable to tuple #38348

Closed
NWilson opened this issue May 5, 2020 · 6 comments · Fixed by #38395
Closed

Regression: Intersection of array and tuple not assignable to tuple #38348

NWilson opened this issue May 5, 2020 · 6 comments · Fixed by #38395
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@NWilson
Copy link

NWilson commented May 5, 2020

TypeScript Version: Nightly

Search Terms: tuple intersection array

Expected behavior:
Expected code to compile. It does compile with tsc 3.8, but does not compile with tsc 3.9 (Nightly).

Actual behavior:
In the code below, the following error is reported:

const x: [number, ...number[]]
Type 'number[] & [number, ...number[]]' is not assignable to type '[number, ...number[]]'.(2322)

Code

const y: number[] & [number, ...number[]] = [1];
const x: [number, ...number[]] = y;

Discussion

Presumably, this is related to the breaking change in 3.9 around handling of tuple types more strictly. I believe the code is correct however and should compile, because the array type number[] is a strict supertype of the tuple type [number, ...number[]] (surely?) - perhaps the special case unifying tuple types with matching element types to an array type hasn't been implemented?

Discovered by @JasonGore and @NWilson (MSFT).

Output
"use strict";
const y = [1];
const x = y;
Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "useDefineForClassFields": false,
    "alwaysStrict": true,
    "allowUnreachableCode": false,
    "allowUnusedLabels": false,
    "downlevelIteration": false,
    "noEmitHelpers": false,
    "noLib": false,
    "noStrictGenericChecks": false,
    "noUnusedLocals": false,
    "noUnusedParameters": false,
    "esModuleInterop": true,
    "preserveConstEnums": false,
    "removeComments": false,
    "skipLibCheck": false,
    "checkJs": false,
    "allowJs": false,
    "declaration": true,
    "experimentalDecorators": false,
    "emitDecoratorMetadata": false,
    "target": "ES2017",
    "module": "ESNext"
  }
}

Playground Link: Provided

@JasonGore
Copy link
Member

Also applies to 3.9.1-rc.

@DanielRosenwasser
Copy link
Member

I believe that this is because we no longer just consider intersections to be assignable if any constituent is assignable - the type has to be compatible as a whole. @ahejlsberg made that change at #37195

Is there a specific reason you had a number[] & [number, ...number[]]?

@ahejlsberg
Copy link
Member

The issue here is that we lack the ability to relate a tuple type with a rest element on the target side to anything but another tuple type on the source side. In the extra check introduced by #37195, the source type is an intersection, and so the check fails. I think an easy and reasonable fix is to omit the extra check when the target is a tuple type with a rest element.

@NWilson
Copy link
Author

NWilson commented May 6, 2020

I can see it's correct that number[] won't assign to [number, ...number[]]. But the intersection checking logic should realise that the two types being interescted are not unrelated, but that the tuple is a refinement of the array.

For example, this works in Nightly:

const y: { x: 1 } & { x: number } = { x: 1 };
const x: { x: 1 } = y;

Even though { x: number } can't be assigned to { x : 1 } the checker doesn't do the thing where it requires both parts of the intersection type to be assignable to the target. So the logic is there correctly for objects; the case I ran into is the corresponding handling for an array+tuple intersection.

@NWilson
Copy link
Author

NWilson commented May 6, 2020

Is there a specific reason you had a number[] & [number, ...number[]]?

Apologies, I didn't motivate this very well! The example was perhaps too minimal.

The full code looked more like this (reworked to a vaguely minimal example). I hope you can see what was being attempted. No apologies made for the code - it is what it is - and thinking some more I can see that the types could be improved, but it's less clear what's going on in the original code which is a bit more involved.

type Checker<T> = (x: unknown) => x is T;

type NonEmptyArray<T> = [T, ...T[]];

function bothChecker<T1, T2>(c1: Checker<T1>, c2: (x: T1) => Checker<T2>): Checker<T1 & T2> {
    return (x: unknown): x is T1 & T2 => c1(x) && c2(x)(x);
}

const numChecker: Checker<number> = (x: unknown): x is number => typeof x === 'number';
function arrayChecker<T>(elt: Checker<T>): Checker<T[]> {
    return (x: unknown): x is T[] => Array.isArray(x) && x.every(i => elt(i));
}
function nonEmptyChecker<T>(): Checker<NonEmptyArray<T>> {
        return (x: unknown): x is NonEmptyArray<T> => (x as T[]).length > 0;
}

const theProblem: Checker<NonEmptyArray<number>> = bothChecker(
    arrayChecker(numChecker),
    _asArray => nonEmptyChecker()
);

@ahejlsberg ahejlsberg self-assigned this May 7, 2020
@ahejlsberg ahejlsberg added the Bug A bug in TypeScript label May 7, 2020
@ahejlsberg ahejlsberg added this to the TypeScript 3.9.2 milestone May 7, 2020
@ahejlsberg ahejlsberg added the Fix Available A PR has been opened for this issue label May 7, 2020
@NWilson
Copy link
Author

NWilson commented May 8, 2020

Wow, thank you so much for a quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants