Skip to content

Unsafe type mismatch of destructuring assignment #6125

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

Closed
falsandtru opened this issue Dec 17, 2015 · 5 comments
Closed

Unsafe type mismatch of destructuring assignment #6125

falsandtru opened this issue Dec 17, 2015 · 5 comments
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead

Comments

@falsandtru
Copy link
Contributor

A following valid destructuring assignment always occurs a TypeError.

// typescript
var {x: {y}}: {x?: {y?}} = {}; // should be a compile error
// emit
var y = {}.x.y; // TypeError: (intermediate value).x is undefined
// javascript on firefox
var {x:{y}} = {}; // TypeError: can't convert undefined to object

Destructuring assignment should constrain type declarations.

// expects
var {x: {y}}: {x: {y}} = {x: {y: 0}}; // ok
var {x: {y}}: {x?: {y?}} = {}; // error
var {x: {y = 0}}: {x?: {y?}} = {}; // error
var {x: {y} = {y: 0}}: {x?: {y?}} = {}; // error
var {x: {y = 0} = {}}: {x?: {y?}} = {}; // error
var {x: {y = 0} = {y: 0}}: {x?: {y?}} = {}; // ok

Destructuring assignments violate a following type constraint.

var options: {x?: {y?}};
var required: {x: {y}} = options; // compile error, but destructuring assignments are not.
var {x: {y}}: {x?: {y?}} = {}; // should be a compile error

Destructuring assignments have a own implicit type but TypeScript breaks it type improperly.

@vladima
Copy link
Contributor

vladima commented Dec 18, 2015

I'm not sure that I'm following you here. In all cases that you claim as illegal you've explicitly stated that properties are optional so compiler does not try to enforce their presence. I.e.

var a: {x?: {y?}} = {}; // ok since x and y are optional
var {x: {y}}: {x?: {y?}} = a; // ok - types match

if optionality is removed then compiler will error since some fields on the left hand side are optional when they should be required

@DanielRosenwasser DanielRosenwasser added the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Dec 18, 2015
@falsandtru
Copy link
Contributor Author

I think, implicit type of ({x: {y}} = o) should be :{x: {y}} = {}, not :{x?: {y?}} = {}.

var o: {x?:{y?}}, y;
({x: {y}} = o); // valid, but should be an compile error

@DanielRosenwasser
Copy link
Member

Why would that be an error, but not the following?

var o: {
    x?: {
        y?: any
    }
};

y = o.x.y;

@falsandtru
Copy link
Contributor Author

Because type of destructuring assignment should not be an optional.

Your code example will be an error but right because it accepted by an explicit type.

var o: {
    x?: {
        y?: any
    }
};

y = o.x.y;

But it does not have my suggested problem.

var o: {x?:{y?}}, y;
({x: {y}} = o); // valid, but should be an compile error

If destructuring assignment has a implicit type, it will be a not optional type because a following code always throw TypeError. Destructuring assignment requires the accessibility of target element.

// typescript
var {x: {y}}: {x?: {y?}} = {}; // should be a compile error
// emit
var y = {}.x.y; // TypeError: (intermediate value).x is undefined

It error can escape manually in your code:

var o: {
    x?: {
        y?: any
    }
};

if (o.x) {
  y = o.x.y;
}

Destructuring assignment is not:

var y = {}.x.y; // cannot fix this generated incorrect code

Therefore destructuring assignment type must be a not optional type:

var {x: {y}}: {x: {y}} = ...;

however, accept an optional type if destructuring assignment has default value:

var o: {x?: {y?}}, y;
({x: {y = 0} = {y: 0}} = o); // ok

or should generate a executable valid code:

// typescript
var {x: {y}}: {x?: {y?}} = {};
// emit
var tmp = {};
var y = tmp.x ? tmp.x.y : void 0;

Points

  1. Destructuring assignment type is not optional if TypeScript generate an invalid code:

    var {x: {y}} = ...; // left-hand type is `{x: {y: any}}`, not `{x?: {y?}}`

    because destructuring assignment requires the accessibility of target element:

    var y = {}.x.y; // TypeError: (intermediate value).x is undefined
  2. An optional type cannot assign to a not optional type:

    var options: {x?: {y?}};
    var required: {x: {y}} = options; // compile error
  3. Therefore destructuring assignment cannot declare the explicit optional type:

    var {x: {y}}: {x?: {y?}} = ...; // should be an invalid explicit type declaration error

    also:

    var o: {x?: {y?}}, y;
    ({x: {y}} = o); // should be an error because `{x?: {y?}}` type cannot assign to `{x: {y}}` type

    however:

    var o: {x?: {y?}}, y;
    ({x: {y = 0} = {y: 0}} = o); // ok
  4. Or, generate a valid code:

    // emit
    var tmp = {};
    var y = tmp.x ? tmp.x.y : void 0;

What do you think about a requirement of accessibility in destructuring assignment?

@falsandtru
Copy link
Contributor Author

These are a same processing:

// use destructuring assignment
var {x: {y}}: {x?: {y?}} = {}; // runtime error, explicit type declaration erase the type checking and safety
// use function
var y;
f({}); // compile error, before the runtime error
function f(o: {x: {y: any}}) {
    y = o.x.y;
}

f function does not accept the implicit type cast of own parameter type because f function does not work if anyone cast own parameter type. But destructuring assignment accept the implicit(not explicit) type cast of own type for the acceptance of right-hand type, and it occurs an error. An explicit type declaration of destructuring assignment erase the type checking and safety. This is nonsense.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead
Projects
None yet
Development

No branches or pull requests

3 participants