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

$ObjMapi, $ObjMap and $TupleMap allow invalid writes #2674

Closed
vkurchatkin opened this issue Oct 23, 2016 · 8 comments
Closed

$ObjMapi, $ObjMap and $TupleMap allow invalid writes #2674

vkurchatkin opened this issue Oct 23, 2016 · 8 comments
Labels

Comments

@vkurchatkin
Copy link
Contributor

Example 1:

type A = $ObjMapi<{ FOO: null }, <K>(k:K) => K>
declare var a: A;

(a.FOO : 'FOO'); // ok
(a.FOO : 'BAR'); // error
a.FOO = 'BAR'; // no error

Example 2:

type A = $ObjMap<{ FOO: null }, <K>(k:K) => 'FOO'>
declare var a: A;

(a.FOO : 'FOO'); // ok
(a.FOO : 'BAR'); // error
a.FOO = 'BAR'; // no error

Example 3:

type A = $TupleMap<[mixed, mixed], <K>(k:K) => 'FOO'>
declare var a: A;

(a[0] : 'FOO'); // ok
(a[0]: 'BAR'); // error
a[0] = 'BAR'; // no error

See: gcanti/flow-io#11

/cc @gcanti

@gcanti
Copy link

gcanti commented Oct 23, 2016

This is pretty bad for libraries like validated or flow-io which rely on $ObjMap in order to build and extract a static type from a runtime value representing a schema.

/cc @andreypopp

@vkurchatkin
Copy link
Contributor Author

vkurchatkin commented Oct 23, 2016

This affects only setting properties and literal assignment:

var a: A = { FOO: 'BAR' }; // no error, but should be

but:

declare var b: { FOO: 'BAR' };
var a: A = b; // an error, as expected

and:

type A = $ObjMap<{ FOO: null }, <K>(k:K) => 'FOO'>

declare var b: { FOO: number };
var a: A = b; // an error, as expected

but:

declare var b: { FOO: string };
var a: A = b; // no error, but should be

@samwgoldman
Copy link
Member

You're right, this is a problem. The cause in the implementation is that we use the existing mechanism for function calls to perform the mapping. This is convenient, but inherits an unfortunate behavior:

declare function f(): string;
var x = f();
if (true) x = 0; // cond. assign to avoid definite assignment refinement
(x: void); // errors: number|string ~> void

Calling a function returns a new, open tvar with the function return type as a lower bound. This doesn't prevent more lower bounds from flowing in, but all inflowing lower bounds are checked against any uses, so it's not totally unsound.

Just explaining this behavior, not excusing it. We should work to make the generate type more precise, because unlike function calls, these types come from annotations where people rightfully expect to establish an exact constraint, not just the lower bound side of one.

@vkurchatkin
Copy link
Contributor Author

vkurchatkin commented Oct 23, 2016

@samwgoldman I though it was the case, but how does it explain this:

type A = $ObjMapi<{ FOO: null }, <K>(k:K) => K>
declare var a: A;

(a.FOO : 'FOO'); // ok
a.FOO = 'BAR'; // no error, a.FOO should be 'FOO' | 'BAR'
(a.FOO : 'FOO'); // error

@samwgoldman
Copy link
Member

That's caused by definite assignment analysis. On that line the value is definitely BAR. Wrap in a conditional or otherwise havoc the refinements to get the union.

@tkuminecz
Copy link

Hey, just wanted to see if there were any plans for this. Would be pretty cool to have this working properly!

@calebmer
Copy link
Contributor

These bugs were fixed a couple versions back. Sorry for not closing this sooner 😊

@gcanti
Copy link

gcanti commented Nov 17, 2017

This is a great news, thanks @calebmer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants