Skip to content

Commit

Permalink
improve member expression reasons
Browse files Browse the repository at this point in the history
Summary:
for member expressions, we have separate `prop_reason` and `expr_reason`, but were using the same `RProperty` for both in some cases, which renders like `property "foo"`. instead, switched to using `mk_expression_reason` for `expr_reason`.

then had to introduce `RMember` to preserve the "constant" hack we get from `RProperty` where all-uppercase props are treated like constants.

Reviewed By: panagosg7

Differential Revision: D15317704

fbshipit-source-id: 06a1d934d254f390319dc5c396f84ac9521c6c8c
  • Loading branch information
mroch authored and facebook-github-bot committed May 22, 2019
1 parent e4c215e commit 0f3a8cb
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 21 deletions.
37 changes: 25 additions & 12 deletions src/common/reason.ml
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ type 'loc virtual_reason_desc =
| RProperty of string option
| RPrivateProperty of string
| RShadowProperty of string
| RMember of { object_: string; property: string }
| RPropertyOf of string * 'loc virtual_reason_desc
| RPropertyIsAString of string
| RMissingProperty of string option
Expand Down Expand Up @@ -303,6 +304,7 @@ let rec map_desc_locs f = function
| RProperty _
| RPrivateProperty _
| RShadowProperty _
| RMember _
| RPropertyIsAString _
| RMissingProperty _
| RUnknownProperty _
Expand Down Expand Up @@ -669,6 +671,7 @@ let rec string_of_desc = function
| RProperty (Some x) -> spf "property `%s`" x
| RProperty None -> "computed property"
| RPrivateProperty x -> spf "property `#%s`" x
| RMember { object_; property } -> spf "`%s%s`" object_ property
| RPropertyAssignment (Some x) -> spf "assignment of property `%s`" x
| RPropertyAssignment None -> "assignment of computed property/element"
| RShadowProperty x -> spf ".%s" x
Expand Down Expand Up @@ -839,6 +842,7 @@ let is_constant_reason r =
| RIdentifier x
| RProperty (Some x)
| RPrivateProperty x
| RMember { object_ = _; property = x }
| RPropertyOf (x,_)
| RPropertyIsAString x ->
let len = String.length x in
Expand Down Expand Up @@ -1012,13 +1016,10 @@ Ast.Expression.(match x with
| Ast.Expression.Literal x -> code_desc_of_literal x
| Logical { Logical.operator; left; right } ->
do_wrap (code_desc_of_operation left (`Logical operator) right)
| Member { Member._object; property } -> Member.(
| Member { Member._object; property } ->
let o = code_desc_of_expression ~wrap:true _object in
o ^ (match property with
| PropertyIdentifier (_, { Ast.Identifier.name= x; comments= _ }) -> "." ^ x
| PropertyPrivateName (_, (_, { Ast.Identifier.name= x; comments= _ })) -> ".#" ^ x
| PropertyExpression x -> "[" ^ code_desc_of_expression ~wrap:false x ^ "]"
))
let p = code_desc_of_property ~optional:false property in
o ^ p
| MetaProperty { MetaProperty.meta = (_, { Ast.Identifier.name= o; comments= _ }); property = (_, { Ast.Identifier.name= p; comments= _ }) } ->
o ^ "." ^ p
| New { New.callee; targs; arguments } ->
Expand Down Expand Up @@ -1054,12 +1055,8 @@ Ast.Expression.(match x with
optional;
} ->
let o = code_desc_of_expression ~wrap:true _object in
o ^ Member.(match property with
| PropertyIdentifier (_, { Ast.Identifier.name= x; comments= _ }) -> (if optional then "?." else ".") ^ x
| PropertyPrivateName (_, (_, { Ast.Identifier.name= x; comments= _ })) -> (if optional then "?.#" else ".#") ^ x
| PropertyExpression x ->
(if optional then "?.[" else "[") ^ code_desc_of_expression ~wrap:false x ^ "]"
)
let p = code_desc_of_property ~optional property in
o ^ p
| Sequence { Sequence.expressions } ->
code_desc_of_expression ~wrap (List.hd (List.rev expressions))
| Super -> "super"
Expand Down Expand Up @@ -1192,6 +1189,16 @@ and code_desc_of_literal x = Ast.(match x.Literal.value with
| _ -> x.Literal.raw
)

and code_desc_of_property ~optional property =
match property with
| Ast.Expression.Member.PropertyIdentifier (_, { Ast.Identifier.name= x; comments= _ }) ->
(if optional then "?." else ".") ^ x
| Ast.Expression.Member.PropertyPrivateName (_, (_, { Ast.Identifier.name= x; comments= _ })) ->
(if optional then "?.#" else ".#") ^ x
| Ast.Expression.Member.PropertyExpression x ->
(if optional then "?.[" else "[") ^ code_desc_of_expression ~wrap:false x ^ "]"


let rec mk_expression_reason = Ast.Expression.(function
| (loc, TypeCast { TypeCast.expression; _ }) -> repos_reason loc (mk_expression_reason expression)
| (loc, Object _) -> mk_reason RObjectLit loc
Expand All @@ -1202,6 +1209,11 @@ let rec mk_expression_reason = Ast.Expression.(function
mk_reason (RStringLit "") loc
| (loc, TaggedTemplate _) -> mk_reason RTemplateString loc
| (loc, TemplateLiteral _) -> mk_reason RTemplateString loc
| (loc, Member { Member._object; property }) ->
mk_reason (RMember {
object_ = code_desc_of_expression ~wrap:true _object;
property = code_desc_of_property ~optional:false property;
}) loc
| (loc, _) as x -> mk_reason (RCode (code_desc_of_expression ~wrap:false x)) loc
)

Expand Down Expand Up @@ -1332,6 +1344,7 @@ let classification_of_reason r = match desc_of_reason ~unwrap:true r with
| RProperty _
| RPrivateProperty _
| RShadowProperty _
| RMember _
| RPropertyOf _
| RPropertyIsAString _
| RMissingProperty _
Expand Down
1 change: 1 addition & 0 deletions src/common/reason.mli
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ type 'loc virtual_reason_desc =
| RProperty of string option
| RPrivateProperty of string
| RShadowProperty of string
| RMember of { object_: string; property: string }
| RPropertyOf of string * 'loc virtual_reason_desc
| RPropertyIsAString of string
| RMissingProperty of string option
Expand Down
8 changes: 4 additions & 4 deletions src/typing/statement.ml
Original file line number Diff line number Diff line change
Expand Up @@ -4245,9 +4245,9 @@ and subscript =
Member._object;
property = Member.PropertyIdentifier (ploc, ({ Ast.Identifier.name; comments= _ } as id));
} ->
let expr_reason = mk_reason (RProperty (Some name)) loc in
let expr_reason = mk_expression_reason ex in
let prop_reason = mk_reason (RProperty (Some name)) ploc in
let use_op = Op (GetProperty (mk_expression_reason ex)) in
let use_op = Op (GetProperty expr_reason) in
begin match opt_state with
| NonOptional ->
let (_, tobj), _ as _object_ast = expression cx _object in
Expand Down Expand Up @@ -5635,7 +5635,7 @@ and predicates_of_condition cx e = Ast.(Expression.(
let (_, obj_t), _ as _object_ast = expression cx _object in

let prop_reason = mk_reason (RProperty (Some prop_name)) prop_loc in
let expr_reason = mk_reason (RProperty (Some prop_name)) expr_loc in
let expr_reason = mk_expression_reason expr in
let prop_t = match Refinement.get cx expr expr_loc with
| Some t -> t
| None ->
Expand Down Expand Up @@ -5988,7 +5988,7 @@ and predicates_of_condition cx e = Ast.(Expression.(
as `_object = foo.bar`, `prop_name = baz`, and `bar` must exist. *)
expression cx _object
in
let expr_reason = mk_reason (RProperty (Some prop_name)) loc in
let expr_reason = mk_expression_reason e in
let prop_reason = mk_reason (RProperty (Some prop_name)) prop_loc in
let t = match Refinement.get cx e loc with
| Some t -> t
Expand Down
6 changes: 3 additions & 3 deletions tests/new_react/new_react.exp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Error --------------------------------------------------------------------------------------- bad_default_props.js:15:18

Cannot use property `Component` [1] with fewer than 1 type argument.
Cannot use `React.Component` [1] with fewer than 1 type argument.

bad_default_props.js:15:18
15| class C3 extends React.Component { // error
Expand All @@ -14,7 +14,7 @@ References:

Error --------------------------------------------------------------------------------------- bad_default_props.js:20:18

Cannot use property `Component` [1] with fewer than 1 type argument.
Cannot use `React.Component` [1] with fewer than 1 type argument.

bad_default_props.js:20:18
20| class C4 extends React.Component {
Expand All @@ -28,7 +28,7 @@ References:

Error --------------------------------------------------------------------------------------------------- classes.js:7:1

Cannot extend property `Component` [1] with `Foo` because:
Cannot extend `React.Component` [1] with `Foo` because:
- property `y_` is missing in function type [2] but exists in object type [3] in the first argument of property
`setState`.
- property `y_` is missing in `State` [4] but exists in object type [3] in the first argument of property `setState`.
Expand Down
4 changes: 2 additions & 2 deletions tests/react/react.exp
Original file line number Diff line number Diff line change
Expand Up @@ -2775,7 +2775,7 @@ References:

Error ---------------------------------------------------------------------------------------------------- render.js:8:1

Cannot extend property `Component` [1] with `A` because in the return value of property `render`:
Cannot extend `React.Component` [1] with `A` because in the return value of property `render`:
- Either undefined [2] is incompatible with null [3].
- Or property `@@iterator` is missing in undefined [2] but exists in `$Iterable` [4].

Expand Down Expand Up @@ -2850,7 +2850,7 @@ References:

Error --------------------------------------------------------------------------------------------------- render.js:46:1

Cannot extend property `Component` [1] with `F` because in the return value of property `render`:
Cannot extend `React.Component` [1] with `F` because in the return value of property `render`:
- Either undefined [2] is incompatible with null [3].
- Or property `@@iterator` is missing in undefined [2] but exists in `$Iterable` [4].

Expand Down

0 comments on commit 0f3a8cb

Please sign in to comment.