Skip to content

Commit

Permalink
[AST] remove redundant Expression.Member.computed field
Browse files Browse the repository at this point in the history
Summary:
a member expression is computed if its property is a `PropertyExpression` vs a `PropertyIdentifier`. a separate `computed` field is redundant and allows bogus ASTs.

this flag is useful in estree to differentiate between `foo.bar` and `foo[bar]`, since the property is an identifier either way, but the ocaml ast uses a variant instead.

Reviewed By: nmote

Differential Revision: D12924869

fbshipit-source-id: 0b72b99b59c703f3d9a607c0c650d0568216fd39
  • Loading branch information
mroch authored and facebook-github-bot committed Nov 6, 2018
1 parent 75978ab commit a8f8e00
Show file tree
Hide file tree
Showing 18 changed files with 40 additions and 95 deletions.
4 changes: 2 additions & 2 deletions src/common/reason.ml
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,7 @@ 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; computed = _ } -> Member.(
| Member { Member._object; property } -> Member.(
let o = code_desc_of_expression ~wrap:true _object in
o ^ (match property with
| PropertyIdentifier (_, x) -> "." ^ x
Expand Down Expand Up @@ -868,7 +868,7 @@ Ast.Expression.(match x with
(if optional then "?." else "") ^
targ_string ^ arg_string
| OptionalMember { OptionalMember.
member = { Member._object; property; computed = _ };
member = { Member._object; property };
optional;
} ->
let o = code_desc_of_expression ~wrap:true _object in
Expand Down
15 changes: 7 additions & 8 deletions src/parser/estree_translator.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1479,16 +1479,15 @@ end with type t = Impl.t) = struct
"arguments", array_of_list expression_or_spread call.arguments;
])

and member_node_properties member = Expression.Member.(
let property = match member.property with
| PropertyIdentifier id -> identifier id
| PropertyPrivateName name -> private_name name
| PropertyExpression expr -> expression expr
and member_node_properties { Expression.Member._object; property } =
let property, computed = match property with
| Expression.Member.PropertyIdentifier id -> identifier id, false
| Expression.Member.PropertyPrivateName name -> private_name name, false
| Expression.Member.PropertyExpression expr -> expression expr, true
in
[
"object", expression member._object;
"object", expression _object;
"property", property;
"computed", bool member.computed;
"computed", bool computed;
]
)
end
2 changes: 0 additions & 2 deletions src/parser/expression_parser.ml
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,6 @@ module Expression
let member = Expression.Member.({
_object = as_expression env left;
property = PropertyExpression expr;
computed = true;
}) in
let member = if in_optional_chain
then Expression.(OptionalMember { OptionalMember.
Expand Down Expand Up @@ -737,7 +736,6 @@ module Expression
let member = Expression.Member.({
_object = as_expression env left;
property;
computed = false;
}) in
let member = if in_optional_chain
then Expression.(OptionalMember { OptionalMember.
Expand Down
1 change: 0 additions & 1 deletion src/parser/flow_ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,6 @@ and Expression : sig
and ('M, 'T) t = {
_object: ('M, 'T) Expression.t;
property: ('M, 'T) property;
computed: bool;
}

[@@deriving show]
Expand Down
11 changes: 1 addition & 10 deletions src/parser_utils/ast_builder.ml
Original file line number Diff line number Diff line change
Expand Up @@ -344,22 +344,13 @@ module Expressions = struct
{ Member.
_object;
property = Member.PropertyIdentifier (Loc.none, property);
computed = false;
}

(* _object[property] *)
let member_computed ~property _object =
{ Member.
_object;
property = Member.PropertyIdentifier (Loc.none, property);
computed = true;
}

let member_computed_expr ~property _object =
{ Member.
_object;
property = Member.PropertyExpression property;
computed = true;
}

let member_expression expr =
Expand All @@ -369,7 +360,7 @@ module Expressions = struct
member_expression (member obj ~property: name)

let member_expression_computed_string obj (str: string) =
member_expression (member_computed_expr obj
member_expression (member_computed obj
~property:(literal (Literals.string str)))

let optional_member_expression ~optional expr =
Expand Down
2 changes: 1 addition & 1 deletion src/parser_utils/file_sig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ module Make
method! member loc (expr: (L.t, L.t) Ast.Expression.Member.t) =
let open Ast.Expression in
let open Ast.Expression.Member in
let { _object; property; computed = _ } = expr in
let { _object; property } = expr in
(* Strip the loc to simplify the patterns *)
let _, _object = _object in
(* This gets called when patterns like `module.id` appear on the LHS of an
Expand Down
9 changes: 3 additions & 6 deletions src/parser_utils/flow_ast_differ.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1091,13 +1091,10 @@ let program (algo : diff_algorithm)
and member (member1: (Loc.t, Loc.t) Ast.Expression.Member.t)
(member2: (Loc.t, Loc.t) Ast.Expression.Member.t): node change list option =
let open Ast.Expression.Member in
let { _object = obj1; property = prop1; computed = computed1 } = member1 in
let { _object = obj2; property = prop2; computed = computed2 } = member2 in
let { _object = obj1; property = prop1 } = member1 in
let { _object = obj2; property = prop2 } = member2 in
let obj = Some (diff_if_changed expression obj1 obj2) in
let prop = if computed1 != computed2 then
None
else
diff_if_changed_ret_opt member_property prop1 prop2 in
let prop = diff_if_changed_ret_opt member_property prop1 prop2 in
join_diff_list [obj; prop]

and member_property (prop1: (Loc.t, Loc.t) Ast.Expression.Member.property)
Expand Down
4 changes: 2 additions & 2 deletions src/parser_utils/flow_ast_mapper.ml
Original file line number Diff line number Diff line change
Expand Up @@ -967,11 +967,11 @@ class ['loc] mapper = object(this)

method member _loc (expr: ('loc, 'loc) Flow_ast.Expression.Member.t) =
let open Flow_ast.Expression.Member in
let { _object; property; computed = _ } = expr in
let { _object; property } = expr in
let _object' = this#expression _object in
let property' = this#member_property property in
if _object == _object' && property == property' then expr
else { expr with _object = _object'; property = property' }
else { _object = _object'; property = property' }

method optional_member loc (expr: ('loc, 'loc) Flow_ast.Expression.OptionalMember.t) =
let open Flow_ast.Expression.OptionalMember in
Expand Down
4 changes: 2 additions & 2 deletions src/parser_utils/flow_polymorphic_ast_mapper.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1027,10 +1027,10 @@ class virtual ['M, 'T, 'N, 'U] mapper = object(this)

method member (expr: ('M, 'T) Ast.Expression.Member.t) : ('N, 'U) Ast.Expression.Member.t =
let open Ast.Expression.Member in
let { _object; property; computed } = expr in
let { _object; property } = expr in
let _object' = this#expression _object in
let property' = this#member_property property in
{ _object = _object'; property = property'; computed }
{ _object = _object'; property = property' }

method optional_member (expr: ('M, 'T) Ast.Expression.OptionalMember.t)
: ('N, 'U) Ast.Expression.OptionalMember.t =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ let tests = "js_layout_generator" >::: [

(* x()[y] *)
let computed = E.member_expression (
E.member_computed (E.call x) ~property:"y"
E.member_computed (E.call x) ~property:(E.identifier "y")
) in
assert_expression ~ctxt "x()[y]" computed;

Expand Down
7 changes: 6 additions & 1 deletion src/parser_utils/output/js_layout_generator.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,12 @@ and literal { Ast.Literal.raw; value; } =
| _ -> Atom raw

and member ?(optional=false) ~precedence ~ctxt member_node =
let { Ast.Expression.Member._object; property; computed } = member_node in
let { Ast.Expression.Member._object; property } = member_node in
let computed = match property with
| Ast.Expression.Member.PropertyExpression _ -> true
| Ast.Expression.Member.PropertyIdentifier _
| Ast.Expression.Member.PropertyPrivateName _ -> false
in
let ldelim, rdelim = begin match computed, optional with
| false, false -> Atom ".", Empty
| false, true -> Atom "?.", Empty
Expand Down
2 changes: 1 addition & 1 deletion src/parser_utils/signature_builder_generate.ml
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ module Eval(Env: Signature_builder_verify.EvalEnv) = struct

and member stuff =
let open Ast.Expression.Member in
let { _object; property; computed = _ } = stuff in
let { _object; property } = stuff in
let path_loc, t = ref_expr _object in
let name = match property with
| PropertyIdentifier (loc, x) -> loc, x
Expand Down
1 change: 0 additions & 1 deletion src/parsing/parsing_service_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ let parse_json_file ~fail content file =
let module_exports = loc_none, Expression.(Member { Member.
_object = loc_none, Identifier (loc_none, "module");
property = Member.PropertyIdentifier (loc_none, "exports");
computed = false;
}) in
let loc = fst expr in
let statement =
Expand Down
3 changes: 0 additions & 3 deletions src/typing/destructuring.ml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ let destructuring cx ~expr ~f = Ast.Pattern.(
raw = string_of_int i;
}
);
computed = true;
}))
) in
let refinement = Option.bind init (fun init ->
Expand Down Expand Up @@ -100,7 +99,6 @@ let destructuring cx ~expr ~f = Ast.Pattern.(
loc, Ast.Expression.(Member Member.({
_object = init;
property = PropertyIdentifier (loc, name);
computed = false;
}))
) in
let refinement = Option.bind init (fun init ->
Expand Down Expand Up @@ -142,7 +140,6 @@ let destructuring cx ~expr ~f = Ast.Pattern.(
loc, Ast.Expression.(Member Member.({
_object = init;
property = PropertyExpression key;
computed = true;
}))
) in
let refinement = Option.bind init (fun init ->
Expand Down
Loading

0 comments on commit a8f8e00

Please sign in to comment.