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

Number literal object keys typechecking #7593

Closed
wants to merge 10 commits into from
5 changes: 5 additions & 0 deletions src/typing/debug_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1290,6 +1290,10 @@ and json_of_propref_impl json_cx = Hh_json.(function
"reason", json_of_reason ~strip_root:json_cx.strip_root ~offset_table:None r;
"name", JSON_String x;
]
| NamedNum (r, x, _) -> JSON_Object [
"reason", json_of_reason ~strip_root:json_cx.strip_root ~offset_table:None r;
"name", JSON_String x;
]
| Computed t -> JSON_Object [
"elem", _json_of_t json_cx t
]
Expand Down Expand Up @@ -1910,6 +1914,7 @@ and dump_use_t_ (depth, tvars) cx t =

let propref = function
| Named (r, x) -> spf "%S %s" (dump_reason cx r) x
| NamedNum (r, x, _) -> spf "%S %s" (dump_reason cx r) x
| Computed t -> kid t
in

Expand Down
68 changes: 60 additions & 8 deletions src/typing/flow_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -4573,6 +4573,30 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace =
rec_flow cx trace (DefT (r1, trust, ArrT (TupleAT (t1, ts1))), u)
end

(* Tuples can flow to numeric objects *)
| DefT (r1, trust, ArrT (TupleAT (_, ts1))),
UseT (use_op, DefT (r2, _, ObjT o)) ->
let fresh = (desc_of_reason r1) = RArrayLit in
let fields = Core_list.foldi ts1 ~f:(fun i map value ->
let key = (Dtoa.ecma_string_of_float (float_of_int i)) in
let prop = Context.get_prop cx o.props_tmap key in
let use_op = Frame (TupleElementCompatibility {
n = i + 1;
lower = r1;
upper = r2;
}, use_op) in
let value = match prop with
| Some field ->
(match Property.read_t field with
| Some t2 -> flow_to_mutable_child cx trace use_op fresh value t2;
| None -> ());
field
| None -> Field (None, value, Neutral) in
SMap.add key value map
) ~init:SMap.empty in
let props_tmap = Context.make_property_map cx fields in
rec_flow cx trace (DefT (r1, trust, ObjT {o with props_tmap;}), u)

(* Read only arrays are the super type of all tuples and arrays *)
| DefT (r1, _, ArrT (ArrayAT (t1, _) | TupleAT (t1, _) | ROArrayAT (t1))),
UseT (use_op, DefT (r2, _, ArrT (ROArrayAT (t2)))) ->
Expand Down Expand Up @@ -5271,6 +5295,7 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace =
when flags.frozen ->
let reason_prop, prop = match prop with
| Named (r, prop) -> r, Some prop
| NamedNum (r, prop, _) -> r, Some prop
| Computed t -> reason_of_t t, None
in
add_output cx ~trace (Error_message.EPropNotWritable {
Expand Down Expand Up @@ -5351,12 +5376,17 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace =
let action = CallElem (reason_call, ft) in
rec_flow cx trace (key, ElemT (unknown_use, reason_lookup, l, action))


| _, ElemT (use_op, reason_op, (DefT (_, _, ObjT _) as obj), action) ->
| _, ElemT (use_op, reason_op, (DefT (_, _, ObjT { props_tmap = _; _ }) as obj), action) ->
let propref = match l with
| DefT (reason_x, _, StrT (Literal (_, x))) ->
let reason_prop = replace_reason_const (RProperty (Some x)) reason_x in
Named (reason_prop, x)
let reason_prop = replace_reason_const (RProperty (Some x)) reason_x in
Named (reason_prop, x)
| DefT (reason_x, _, NumT (Literal (_, (index, _)))) ->
let name = Dtoa.ecma_string_of_float index in
let reason_prop = replace_reason_const (RProperty (Some name)) reason_x in
(* Probably should be different type constructor *)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this broke {[key: number]: any} reads,

declare var e: { [key: number]: any };

e[0]; // string `0` [1] is incompatible with  number [2].Flow(InferError)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this #2293 (comment) applies to this also, so probably I fixed it wrong

(* if Context.has_prop cx mapr x then Named (reason_prop, x) else Computed l *)
NamedNum (reason_prop, name, int_of_float index)
| _ -> Computed l
in
(match action with
Expand Down Expand Up @@ -6597,7 +6627,11 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace =
| _, LookupT (_, _, _, propref, lookup_action) ->
let use_op = use_op_of_lookup_action lookup_action in
add_output cx ~trace (Error_message.EIncompatibleProp {
prop = (match propref with Named (_, name) -> Some name | Computed _ -> None);
prop = (match propref with
| Named (_, name) -> Some name
| NamedNum (_, name, _) -> Some name
| Computed _ -> None
);
reason_prop = reason_of_propref propref;
reason_obj = reason_of_t l;
special = error_message_kind_of_lower l;
Expand Down Expand Up @@ -8861,6 +8895,7 @@ and set_prop cx ?(wr_ctx=Normal) trace ~use_op
and get_obj_prop cx trace o propref reason_op =
let named_prop = match propref with
| Named (_, x) -> Context.get_prop cx o.props_tmap x
| NamedNum (_, x, _) -> Context.get_prop cx o.props_tmap x
| Computed _ -> None
in
match propref, named_prop, o.dict_t with
Expand All @@ -8872,6 +8907,11 @@ and get_obj_prop cx trace o propref reason_op =
(* Dictionaries match all property reads *)
rec_flow_t cx trace (string_key x reason_op, key);
Some (Field (None, value, dict_polarity))
| NamedNum (_, x, index), None, Some { key; value; dict_polarity; _ }
when not (is_dictionary_exempt x) ->
(* Dictionaries match all property reads *)
rec_flow_t cx trace (number_key index x reason_op, key);
Some (Field (None, value, dict_polarity))
| Computed k, None, Some { key; value; dict_polarity; _ } ->
rec_flow_t cx trace (k, key);
Some (Field (None, value, dict_polarity))
Expand All @@ -8885,7 +8925,8 @@ and read_obj_prop cx trace ~use_op o propref reason_obj reason_op tout =
perform_lookup_action cx trace propref p reason_obj reason_op action
| None ->
match propref with
| Named _ ->
| Named _
| NamedNum _ ->
let strict =
if Obj_type.sealed_in_op reason_op o.flags.sealed
then Strict reason_obj
Expand Down Expand Up @@ -8921,7 +8962,8 @@ and writelike_obj_prop cx trace ~use_op o propref reason_obj reason_op prop_t ac
perform_lookup_action cx trace propref p reason_obj reason_op action
| None ->
match propref with
| Named (reason_prop, prop) ->
| Named (reason_prop, prop)
| NamedNum (reason_prop, prop, _) ->
let sealed = Obj_type.sealed_in_op reason_op o.flags.sealed in
if sealed && o.flags.exact
then
Expand Down Expand Up @@ -10871,6 +10913,7 @@ and perform_lookup_action cx trace propref p lreason ureason = function
| None ->
let reason_prop, prop_name = match propref with
| Named (r, x) -> r, Some x
| NamedNum (r, x, _) -> r, Some x
| Computed t -> reason_of_t t, None
in
let msg = Error_message.EPropNotReadable { reason_prop; prop_name; use_op } in
Expand All @@ -10896,6 +10939,7 @@ and perform_lookup_action cx trace propref p lreason ureason = function
| None ->
let reason_prop, prop_name = match propref with
| Named (r, x) -> r, Some x
| NamedNum (r, x, _) -> r, Some x
| Computed t -> reason_of_t t, None
in
add_output cx ~trace (Error_message.EPropNotReadable { reason_prop; prop_name; use_op })
Expand All @@ -10915,6 +10959,10 @@ and string_key s reason =
let key_reason = replace_reason_const (RPropertyIsAString s) reason in
DefT (key_reason, bogus_trust (), StrT (Literal (None, s)))

and number_key x raw reason =
let key_reason = replace_reason_const (RPropertyIsAString raw) reason in
DefT (key_reason, bogus_trust (), NumT (Literal (None, (float_of_int x, raw))))

(* builtins, contd. *)

and get_builtin cx ?trace x reason =
Expand Down Expand Up @@ -11219,7 +11267,11 @@ and flow_opt_p cx ?trace ~use_op ~report_polarity lreason ureason propref =
unify_opt cx ?trace ~use_op lt ut
(* directional cases *)
| lp, up ->
let x = match propref with Named (_, x) -> Some x | Computed _ -> None in
let x = match propref with
| Named (_, x) -> Some x
| NamedNum (_, x, _) -> Some x
| Computed _ -> None
in
(match Property.read_t lp, Property.read_t up with
| Some lt, Some ut ->
flow_opt cx ?trace (lt, UseT (use_op, ut))
Expand Down
19 changes: 19 additions & 0 deletions src/typing/statement.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2521,6 +2521,25 @@ and object_prop cx map prop = Ast.Expression.Object.(
shorthand
})

(* named number literal prop *)
| Property (prop_loc, Property.Init {
key =
Property.Literal (loc, {
Ast.Literal.value = Ast.Literal.Number name;
_;
}) as key;
value = v; shorthand; }) ->
let name = Dtoa.ecma_string_of_float name in
let (_, t), _ as v = expression cx v in
let id_info = name, t, Type_table.Other in
Type_table.set_info loc id_info (Context.type_table cx);
Properties.add_field name Neutral (Some loc) t map,
Property (prop_loc, Property.Init {
key = translate_identifier_or_literal_key t key;
value = v;
shorthand
})

(* named method *)
| Property (prop_loc, Property.Method {
key =
Expand Down
3 changes: 3 additions & 0 deletions src/typing/type.ml
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,7 @@ module rec TypeTerm : sig

and propref =
| Named of reason * name
| NamedNum of reason * name * index
| Computed of t

and sealtype =
Expand Down Expand Up @@ -3372,10 +3373,12 @@ let rec string_of_predicate = function

let name_of_propref = function
| Named (_, x) -> Some x
| NamedNum (_, x, _) -> Some x
| Computed _ -> None

let reason_of_propref = function
| Named (r, _) -> r
| NamedNum (r, _, _) -> r
| Computed t -> reason_of_t t

and extract_setter_type = function
Expand Down
21 changes: 21 additions & 0 deletions src/typing/type_annotation.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,27 @@ and convert_object =
let polarity = if _method then Polarity.Positive else polarity variance in
Acc.add_prop (Properties.add_field name polarity (Some loc) t) acc,
prop_ast t
| Ast.Expression.Object.Property.Literal
(loc, { Ast.Literal.value = Ast.Literal.Number name; _ }) ->
let name = Dtoa.ecma_string_of_float name in
Type_inference_hooks_js.dispatch_obj_prop_decl_hook cx name loc;
let (_, t), _ as value_ast = convert cx tparams_map value in
let prop_ast t = { prop with Object.Property.
key = begin match key with
| Ast.Expression.Object.Property.Literal (_, lit) ->
Ast.Expression.Object.Property.Literal ((loc, t), lit)
| Ast.Expression.Object.Property.Identifier (_loc, { Ast.Identifier.comments; _ }) ->
Ast.Expression.Object.Property.Identifier ((loc, t), mk_commented_ident t comments name)
| _ -> assert_false "branch invariant"
end;
value = Object.Property.Init value_ast;
} in
let t = if optional then Type.optional t else t in
let id_info = name, t, Type_table.Other in
Type_table.set_info loc id_info (Context.type_table cx);
let polarity = if _method then Positive else polarity variance in
Acc.add_prop (Properties.add_field name polarity (Some loc) t) acc,
prop_ast t
| Ast.Expression.Object.Property.Literal (loc, _)
| Ast.Expression.Object.Property.PrivateName (loc, _)
| Ast.Expression.Object.Property.Computed (loc, _)
Expand Down
1 change: 1 addition & 0 deletions src/typing/type_mapper.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,7 @@ class virtual ['a] t_with_uses = object(self)
method prop_ref cx map_cx t =
match t with
| Named _ -> t
| NamedNum _ -> t
| Computed t' ->
let t'' = self#type_ cx map_cx t' in
if t'' == t' then t
Expand Down
1 change: 1 addition & 0 deletions src/typing/type_visitor.ml
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,7 @@ class ['a] t = object(self)

method private propref cx acc = function
| Named _ -> acc
| NamedNum _ -> acc
| Computed t -> self#type_ cx pole_TODO acc t

method private class_binding cx acc { class_private_fields; class_private_static_fields; _ } =
Expand Down
16 changes: 15 additions & 1 deletion tests/dictionary/dictionary.exp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,20 @@ References:
^^^^^^^ [2]


Error --------------------------------------------------------------------------------------------- dictionary.js:104:10

Cannot assign `"not-x"` to `o[0]` because string [1] is incompatible with `X` [2].

dictionary.js:104:10
104| o[0] = "not-x"; // error: string ~> X
^^^^^^^ [1]

References:
dictionary.js:102:27
102| o: {[k:number]:any, "0":X},
^ [2]


Error --------------------------------------------------------------------------------------------- dictionary.js:110:34

Cannot assign `x` to `a` because `B` [1] is incompatible with `A` [2] in the indexer property of array element.
Expand Down Expand Up @@ -1041,4 +1055,4 @@ References:



Found 61 errors
Found 62 errors
10 changes: 5 additions & 5 deletions tests/dictionary/dictionary.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ function object_prototype(
return o; // ok
}

// **UNSOUND**
// Because we support non-string props w/ bracket notation, it's possible to
// write into a declared prop unsoundly.
function unsound_string_conversion_alias_declared_prop(
//
//
//
function string_conversion_alias_declared_prop(
o: {[k:number]:any, "0":X},
) {
o[0] = "not-x"; // a["0"] no longer X
o[0] = "not-x"; // error: string ~> X
}

function unification_dict_values_invariant(
Expand Down
52 changes: 37 additions & 15 deletions tests/indexer/A.js
Original file line number Diff line number Diff line change
@@ -1,39 +1,61 @@
//@flow

// No indexer should be fine
function foo0(): {} {
return { foo: "bar" }
return { foo: "bar" };
}

// Matching indexer should be fine
function foo1(): {[key: string]: string} {
return { foo: "bar" }
function foo1(): { [key: string]: string } {
return { foo: "bar" };
}

// Indexer with different key type is an error when it matches
function foo2(): {[key: number]: string} {
return { foo: "bar" }
function foo2(): { [key: number]: string } {
return { foo: "bar" };
}

// Matching indexer with different value type is an error
function foo3(): {[key: string]: number} {
return { foo: "bar" }
function foo3(): { [key: string]: number } {
return { foo: "bar" };
}

// Indexer with different key type and different value type is twice an error
function foo4(): {[key: number]: number} {
return { foo: "bar" }
function foo4(): { [key: number]: number } {
return { foo: "bar" };
}

// If key exists in object type then indexer is not matched
function foo5(): {[key: string]: number; foo: string} {
return { foo: "bar" }
function foo5(): { [key: string]: number, foo: string } {
return { foo: "bar" };
}

// If key exists in object type then indexer is not matched
function foo6(): {[key: number]: number; foo: string} {
return { foo: "bar" }
function foo6(): { [key: number]: number, foo: string } {
return { foo: "bar" };
}

// Should still complain about mistyped properties
function foo7(): {[key: string]: number; foo: number} {
return { foo: "bar" }
function foo7(): { [key: string]: number, foo: number } {
return { foo: "bar" };
}

// TODO: ok
function foo8(): { [key: number]: string } {
return { 0: "bar" };
}

// TODO: error
function foo9(): { [key: string]: string } {
return { 0: "bar" };
}

// TODO: error
function foo9(): { [key: string]: string } {
return { 0: "bar" };
}

// TODO: ok
function foo10(): { [key: string | number]: string } {
return { 1: "nice", a: "wtf" };
}
Loading