Hard-ban unset on non-array-access expressions

Summary: Add unfixmeable `Typing[4521]` for `unset(e)` when `e` is not an array access. Keep the existing container-type diagnostic for indexed accesses and cover local and property targets.

Reviewed By: fangyi-zhou

Differential Revision: D116917392

fbshipit-source-id: e3c2ad999de256d65fc4092b3d872f16070a05ee

Author: Andrew Kennedy

hphp/hack/src/diagnostics/error_codes.ml

@@ -816,6 +816,7 @@ module Typing = struct
     | StrictIsolationExcludedPathAccess [@value 4518]
     | EnumDuplicateValue [@value 4519]
     | EnumUncheckableValue [@value 4520]
+    | InvalidUnsetTarget [@value 4521]
   (* Add new Typing codes here! Comment out when deprecating. *)
   [@@deriving enum, show { with_path = false }]
 

hphp/hack/src/hackc/test/infer/member_op.hack

@@ -229,20 +229,6 @@ function mop_basel_unset_ei(dict<int, int> $a): void {
   unset($a[5]);
 }
 
-// TEST-CHECK-BAL: define $root.mop_basel_unset_pt
-// CHECK: define $root.mop_basel_unset_pt($this: *void, $a: *C) : *void {
-// CHECK: #b0:
-// CHECK: // .column 1
-// CHECK:   n0: *HackMixed = load &$a
-// CHECK:   store n0.?.foo <- null: *HackMixed
-// CHECK: // .column 2
-// CHECK:   ret null
-// CHECK: }
-function mop_basel_unset_pt(C $a): void {
-  /* HH_FIXME[4135] Allow unset */
-  unset($a->foo);
-}
-
 // TEST-CHECK-BAL: define $root.mop_basec_querym_cget(
 // CHECK: define $root.mop_basec_querym_cget($this: *void, $d: .notnull *HackDict) : .notnull *HackInt {
 // CHECK: #b0:

hphp/hack/src/oxidized/gen/error_codes.rs

@@ -3,7 +3,7 @@
 // This source code is licensed under the MIT license found in the
 // LICENSE file in the "hack" directory of this source tree.
 //
-// @generated SignedSource<<7ef8934272d5c4675b44238825a34e3f>>
+// @generated SignedSource<<23a844de2b22a4423d0dc60ebaff4a51>>
 //
 // To regenerate this file, run:
 //   buck run @fbcode//mode/dev-nosan-lg fbcode//hphp/hack/src:oxidized_regen
@@ -587,6 +587,7 @@ pub enum Typing {
     StrictIsolationExcludedPathAccess = 4518,
     EnumDuplicateValue = 4519,
     EnumUncheckableValue = 4520,
+    InvalidUnsetTarget = 4521,
 }
 impl TrivialDrop for Typing {}
 arena_deserializer::impl_deserialize_in_arena!(Typing);

hphp/hack/src/typing/typing.ml

@@ -6168,6 +6168,18 @@ end = struct
             @@ Some
                  Typing_error.(
                    primary @@ Primary.Unset_nonidx_in_strict { pos = p; reason })
+          | ([Aast_defs.Anormal (_, _, Array_get (_, None))], None) ->
+            let ty_err =
+              Typing_error.(
+                primary
+                @@ Primary.Unset_nonidx_in_strict { pos = p; reason = lazy [] })
+            in
+            (env, Some ty_err)
+          | ([Aast_defs.Anormal _], None) ->
+            let ty_err =
+              Typing_error.(primary @@ Primary.Invalid_unset_target p)
+            in
+            (env, Some ty_err)
           | _ ->
             let ty_err =
               Typing_error.(

hphp/hack/src/typing/typing_error.ml

@@ -683,6 +683,7 @@ module Primary = struct
         pos: Pos.t;
         reason: Pos_or_decl.t Message.t list Lazy.t;
       }
+    | Invalid_unset_target of Pos.t
     | Nullable_cast of {
         pos: Pos.t;
         ty_pos: Pos_or_decl.t;

hphp/hack/src/typing/typing_error.mli

@@ -642,6 +642,7 @@ module Primary : sig
         pos: Pos.t;
         reason: Pos_or_decl.t Message.t list Lazy.t;
       }
+    | Invalid_unset_target of Pos.t
     | Nullable_cast of {
         pos: Pos.t;
         ty_pos: Pos_or_decl.t;

hphp/hack/src/typing/typing_error_utils.ml

@@ -4288,6 +4288,12 @@ end = struct
       ~reasons
       ()
 
+  let invalid_unset_target pos =
+    create
+      ~code:Error_code.InvalidUnsetTarget
+      ~claim:(lazy (pos, "The argument to `unset` must be an array access."))
+      ()
+
   let unpacking_disallowed_builtin_function pos name =
     create
       ~code:Error_code.UnpackingDisallowed
@@ -5099,6 +5105,7 @@ end = struct
     | Invalid_substring { pos; ty_name } -> invalid_substring pos ty_name
     | Unset_nonidx_in_strict { pos; reason } ->
       unset_nonidx_in_strict pos reason
+    | Invalid_unset_target pos -> invalid_unset_target pos
     | Nullable_cast { pos; ty_pos; ty_name } -> nullable_cast pos ty_pos ty_name
     | Hh_expect { pos; equivalent } -> hh_expect pos equivalent
     | Null_member { pos; obj_pos_opt; ctxt; kind; member_name; reason } ->

hphp/hack/test/tast/pseudofunctions.php.exp

@@ -1,6 +1,6 @@
 Errors:
   [16:3-12] `isset` tends to hide errors due to variable typos and so is limited to dynamic checks in `strict` mode
-  [17:3-13] In `strict` mode, `unset` is banned except on dynamic, darray, keyset, or dict indexing
+  [17:3-13] The argument to `unset` must be an array access.
 [(Class
     { c_span = [3:1-6:2]; c_annotation = (); c_mode = Mstrict; c_final = false; 
       c_is_xhp = false; c_has_xhp_keyword = false; c_kind = (Cclass Concrete); 

hphp/hack/test/typecheck/argument_unpacking/unpack_call10_unset.php.exp

@@ -5,6 +5,6 @@ An expression is expected here. (Parsing[1002])
 ERROR: File "unpack_call10_unset.php", line 6, characters 9-11:
 Unbound name: `...` (a global constant) (Naming[2049])
 ERROR: File "unpack_call10_unset.php", line 6, characters 3-18:
-In `strict` mode, `unset` is banned except on dynamic, darray, keyset, or dict indexing (Typing[4135])
+The argument to `unset` must be an array access. (Typing[4521])
 ERROR: File "unpack_call10_unset.php", line 6, characters 9-11:
 Unbound global constant (Typing) (Typing[4106])

hphp/hack/test/typecheck/const_attribute/static/unset_const_on_instance.php.exp

@@ -1,4 +1,4 @@
 ERROR: File "unset_const_on_instance.php", line 10, characters 9-13:
 You cannot use this syntax in an `unset()` expression. (Parsing[1002])
 ERROR: File "unset_const_on_instance.php", line 10, characters 3-15:
-In `strict` mode, `unset` is banned except on dynamic, darray, keyset, or dict indexing (Typing[4135])
+The argument to `unset` must be an array access. (Typing[4521])