Skip to content

Commit

Permalink
Use Subscript-compatible logic for __setitem__ in typeCheck.ml
Browse files Browse the repository at this point in the history
Summary:
In order to make it easy to roll out the sweeping changes when
I add a Subscript node to the AST and cut over `__getitem__` in
the parser, I first want to make sure existing uses of the
`special = true` `__getitem__` are always handled in a way that is:
- distinct from the "vanilla" `Expression.Call` case
- compatible with what Subscript will get us, which is just `base`
  and `index` (so basically, don't use the `callee` or other parts
  of the `__getitem__` `Call` node that won't exist after cutting over)

This diff makes that change in the part of `typeCheck.ml` where we
are handling subscript assignment targets: only rely on `base` and
`index` here when constructing synthetic `__setitem__` call data,
so that we'll be able to just drop in a `Subscript` match with the
same body.

Reviewed By: yangdanny97

Differential Revision: D57113190

fbshipit-source-id: c03e595a7c4a33bc910a0de32b73e57a51436e65
  • Loading branch information
stroxler authored and facebook-github-bot committed May 10, 2024
1 parent feba563 commit 03d7d3a
Showing 1 changed file with 14 additions and 13 deletions.
27 changes: 14 additions & 13 deletions source/analysis/typeCheck.ml
Original file line number Diff line number Diff line change
Expand Up @@ -5106,8 +5106,8 @@ module State (Context : Context) = struct
value =
Name (Name.Attribute { base; attribute = "__getitem__"; special = true });
_;
} as getitem_callee_expression;
arguments = [raw_key_argument];
};
arguments = [{ Call.Argument.value = index; name = None }];
} ->
let {
Resolved.errors = callee_errors;
Expand All @@ -5119,10 +5119,10 @@ module State (Context : Context) = struct
=
let setitem_callee_expression =
{
getitem_callee_expression with
value =
Node.value =
Expression.Name
(Name.Attribute { base; attribute = "__setitem__"; special = true });
Node.location;
}
in
forward_expression ~resolution setitem_callee_expression
Expand All @@ -5140,18 +5140,19 @@ module State (Context : Context) = struct
|> Option.value ~default:Type.Top;
};
attribute = { name = "__setitem__"; resolved = resolved_setitem_type };
(* TODO(T187163267) We need a placeholder expression; use the raw subscript from
the LHS for now, try to find a way of relaxing this requirement. *)
expression = getitem_callee_expression;
(* TODO(T187163267) We need a placeholder expression; use the base of the
subscript for now and eventually try to find a way of relaxing this
requirement. *)
expression = base;
}
in
(* resolve the key argument, then combine it with the value *)
let resolution_after_key_argument, base_and_callee_errors, setitem_arguments =
let updated_resolution, base_and_callee_errors, key_argument =
(* resolve the index, then combine it with the value *)
let resolution_after_index, base_and_callee_errors, setitem_arguments =
let updated_resolution, base_and_callee_errors, index_argument =
forward_argument
~resolution:resolution_after_callee
~errors:callee_errors
raw_key_argument
{ Call.Argument.value = index; name = None }
in
let value_argument =
{
Expand All @@ -5160,7 +5161,7 @@ module State (Context : Context) = struct
resolved = guide;
}
in
updated_resolution, base_and_callee_errors, [key_argument; value_argument]
updated_resolution, base_and_callee_errors, [index_argument; value_argument]
in
let target, dynamic =
if Type.is_meta resolved_setitem_type then
Expand All @@ -5177,7 +5178,7 @@ module State (Context : Context) = struct
in
let { Resolved.resolution; errors = setitem_errors; _ } =
forward_call
~resolution:resolution_after_key_argument
~resolution:resolution_after_index
~location
~errors:base_and_callee_errors
~target
Expand Down

0 comments on commit 03d7d3a

Please sign in to comment.