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

Not assignable Type #34967

Closed
yuanfux opened this issue Nov 7, 2019 · 9 comments · Fixed by #39393
Closed

Not assignable Type #34967

yuanfux opened this issue Nov 7, 2019 · 9 comments · Fixed by #39393
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@yuanfux
Copy link

yuanfux commented Nov 7, 2019

TypeScript 3.7.2
Playground link

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,
    "experimentalDecorators": false,
    "emitDecoratorMetadata": false,
    "target": "ES2017",
    "module": "ESNext"
  }
}

Input:

const message: string = 'hello world';
console.log(message);

const b = message ? message : undefined;
const c = message ? message : undefined;

const a: [string, undefined] | [undefined, string] | [string, string] | [undefined, string] = [b, c];

Output:

"use strict";
const message = 'hello world';
console.log(message);
const b = message ? message : undefined;
const c = message ? message : undefined;
const a = [b, c];

Expected behavior:
To me, Type [string | undefined, string | undefined] is assignable to type [string, undefined] | [undefined, string] | [string, string] | [undefined, undefined].

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Nov 7, 2019

To me, Type [string | undefined, string | undefined] is assignable to type [string, undefined] | [undefined, string] | [string, string] | [undefined, undefined].

Except, it isn't.

declare const a : [
  string | undefined, 
  string | undefined
];
//Assume this is okay
const b : (
  | [string, undefined] 
  | [undefined, string] 
  | [string, string] 
  | [undefined, undefined]
) = a;

if (b[0] == undefined) {
  //b is now narrowed to 
  //[undefined, string] | [undefined, undefined] 
  a[0] = "hello, world"; //this is allowed
  console.log(b[0]); //"hello, world"
}

@jack-williams
Copy link
Collaborator

jack-williams commented Nov 7, 2019

This feels like a bug. As per this comment, recent changes to discriminant assignment should probably handle this. For example, this works:

const message: string = 'hello world';
console.log(message);

const b = message ? message : undefined;
const c = message ? message : undefined;

interface Pair<L, R> {
    0: L,
    1: R;
}

const a: Pair<string, undefined> | Pair<undefined, undefined> | Pair<string, string> | Pair<undefined, string> = [b, c];

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Nov 7, 2019

Sounds like a bad idea to allow the assignment, since it's unsound.

Somewhat related to #33205

I keep telling everyone assignments like these aren't sound and shouldn't be allowed but it's like I'm talking to the wind >.>
And everyone just goes on like I haven't said anything

@jack-williams
Copy link
Collaborator

I keep telling everyone assignments like these aren't sound and shouldn't be allowed but it's like I'm talking to the wind >.>
And everyone just goes on like I haven't said anything

TypeScript has always allowed covariant references, much like many other languages, which are known to be unsound in the presence of mutation and aliasing.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Nov 7, 2019

Except, in this case, it's a contravariant assigment, isn't it?

We're trying to assign string|undefined to string, or string|undefined to undefined

declare const a: (string | undefined)[];
//Fails because contravariant assignment
const b: string[] = a;

declare const c: (string | undefined)[];
//Fails because contravariant assignment
const d: undefined[] = c;

declare const e: (string | undefined)[];
//Fails because contravariant assignment
const f: (string[]) | (undefined[]) = e;

declare const g: [string | undefined];
//Fails because contravariant assignment
const h: [string] = g;

declare const i: [string | undefined];
//Fails because contravariant assignment
const j: [undefined] = i;

declare const k: [string | undefined];
//Fails because contravariant assignment
const l: [string]|[undefined] = k;

Playground


Also related,

declare const a : [
  string | undefined, 
  string | undefined
];

//This satisfies the type checker
const b : (
  | [string, undefined] 
  | [undefined, string] 
  | [string, string] 
  | [undefined, undefined]
) = (
  a[0] == undefined ?
  (
    a[1] == undefined ?
    [a[0], a[1]] :
    [a[0], a[1]]
  ) :
  (
    a[1] == undefined ?
    [a[0], a[1]] :
    [a[0], a[1]]
  )
);

//Would be nice if this satisfied the type checker
//New object literal should be okay
const c : (
  | [string, undefined] 
  | [undefined, string] 
  | [string, string] 
  | [undefined, undefined]
) = (
  [a[0], a[1]]
);

//Would be nice if this satisfied the type checker
//New object literal should be okay
const d : (
  | [string, undefined] 
  | [undefined, string] 
  | [string, string] 
  | [undefined, undefined]
) = (
  [...a]
);

Playground

@jack-williams
Copy link
Collaborator

It's covariant: [string | undefined, string | undefined] is a subtype of [string, undefined] | [undefined, string] | [string, string] | [undefined, undefined] because the set of values denoted by the former are a subset of the values denoted by the latter.

Syntax directed subtyping algorithms commonly suffer from the problem that they can't prove this relation using the normal decomposition rules, exactly because you decompose into things that try to related string | undefined to string.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Nov 7, 2019

If mutations are disallowed, I agree with you about the former being a subset of the latter.
If mutations are allowed, then I disagree.

But I guess TS pretends mutations don't happen.


Given a value x of type [string|undefined, string|undefined], you can always change the value of x[0] to a string, or change it to undefined.

But,

  • Given a value y of type [string, undefined], you cannot change y[0] to undefined.
  • Given a value y of type [undefined, string], you cannot change y[0] to string.
  • Given a value y of type [string, string], you cannot change y[0] to undefined.
  • Given a value y of type [undefined, undefined], you cannot change y[0] to string.

So, I probably should not consider mutations, since TS pretends they don't happen (in general), but it's hard for me to do so =x

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Nov 8, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.8.0 milestone Nov 8, 2019
@RyanCavanaugh
Copy link
Member

@rbuckton n.b. there's a typo (missing the [undefined, undefined] case in the OP) but this looks like a case that should have been handled by your PR to look for matching unions

@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Aug 31, 2020
@rbuckton
Copy link
Member

It looks like this was fixed by #39393, and has been working since 4.0.

@rbuckton rbuckton added the Fixed A PR has been merged for this issue label Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants