Distinguish Singleton and Caching pattern

Summary: This diff distinguishes Caching pattern from Singleton pattern. Essentially, both patterns writes to (or say, initializes) a global variable when its value is null or does not exist. Yet, it is desired to distinguish if the written global variable is a collection or not: if the assignment directly writes to a collection (e.g. `Foo::$dic[$key] = $value`), then we assume it's a caching; otherwise, it is a singleton (e.g. `Foo::$bar = $x` or `(Foo::$dic[$key])->prop = $y`).

Reviewed By: lavenzg

Differential Revision: D40872670

fbshipit-source-id: 0ec2e66767ef3d1fe241749e4de9607c88222a3e

Author: Chaoqiang Deng

hphp/hack/src/typing/tast_check/global_access_check.ml

@@ -43,6 +43,7 @@ module GlobalAccessCheck = Error_codes.GlobalAccessCheck
 (* Recognize common patterns for global access (only writes for now). *)
 type global_access_pattern =
   | Singleton (* Write to a global variable whose value is null *)
+  | Caching (* Write to a global collection when the element is null or does not exist *)
   | CounterIncrement (* Increase a global variable by 1 or -1 *)
   | WriteEmptyOrNull (* Assign null or empty string to a global variable *)
   | WriteLiteral (* Assign a literal to a global variable *)
@@ -851,17 +852,25 @@ let visitor =
         let () = self#on_expr (env, (ctx, fun_name)) re in
         let re_ty = Tast_env.print_ty env (Tast.get_type re) in
         let le_global_opt = get_global_vars_from_expr env ctx le in
+        (* When write to a global variable whose value is null or does not exist:
+           if the written variable is in a collection (e.g. $global[$key] = $val),
+           then it's asumed to be caching; otherwise, it's a singleton. *)
+        let singleton_or_caching =
+          match le with
+          | (_, _, Array_get _) -> Caching
+          | _ -> Singleton
+        in
         let le_pattern =
           match (le_global_opt, bop_opt) with
           | (None, _) -> NoPattern
-          | (Some _, Some Ast_defs.QuestionQuestion) -> Singleton
+          | (Some _, Some Ast_defs.QuestionQuestion) -> singleton_or_caching
           | (Some le_global, _) ->
             if
               SSet.exists
                 (fun v -> SSet.mem v !(ctx.null_global_var_set))
                 le_global
             then
-              Singleton
+              singleton_or_caching
             else
               NoPattern
         in

hphp/hack/test/typecheck/global_access_check/test_pattern.php

@@ -45,6 +45,6 @@ public function test_method_call(): void {
     Foo::$static_dict = HH\Lib\Dict\merge(Foo::$static_dict, dict["0" => 0]); // WriteLiteral,WriteGlobalToGlobal
     Foo::$static_dict = HH\Lib\Dict\merge(Foo::$static_dict, dict["0" => (int)call_mixed(0)]); // NoPattern (call_mixed is unknown)
     Foo::$static_dict = dict[]; // WriteEmptyOrNUll
-    Foo::$static_dict["1"] ??= 1; // Singleton
+    Foo::$static_dict["1"] ??= 1; // Caching
   }
 }

hphp/hack/test/typecheck/global_access_check/test_pattern.php.exp

@@ -47,4 +47,4 @@ File "test_pattern.php", line 46, characters 43-59:
 File "test_pattern.php", line 47, characters 5-30:
 [Test::test_method_call]{Foo::$static_dict}(dict<nothing, nothing>)<WriteEmptyOrNull> A global variable is definitely written. (Other[11001])
 File "test_pattern.php", line 48, characters 5-32:
-[Test::test_method_call]{Foo::$static_dict}(int)<Singleton,WriteLiteral> A global variable is definitely written. (Other[11001])
+[Test::test_method_call]{Foo::$static_dict}(int)<Caching,WriteLiteral> A global variable is definitely written. (Other[11001])

hphp/hack/test/typecheck/global_access_check/test_singleton_write_collection.php

@@ -25,14 +25,14 @@ public static function get_dict_value(string $key): Bar {
     $value = idx(self::$static_dic, $key);
     if ($value === null) {
       $value = new Bar();
-      self::$static_dic[$key] = $value; // Singleton
+      self::$static_dic[$key] = $value; // Caching
     }
     return $value;
   }
 
   public static function get_dict_value2(string $key): Bar {
     if (!HH\Lib\C\contains_key(self::$static_dic, $key)) {
-      self::$static_dic[$key] = new Bar();  // Singleton
+      self::$static_dic[$key] = new Bar();  // Caching
     }
     return self::$static_dic[$key];
   }
@@ -41,7 +41,7 @@ public static function get_shape_value(): Bar {
     $value = self::$static_shape["field1"];
     if ($value === null) {
       $value = new Bar();
-      self::$static_shape["field1"] = $value; // Singleton
+      self::$static_shape["field1"] = $value; // Caching
     }
     return $value;
   }
@@ -50,7 +50,7 @@ public static function get_dict_value_3(string $key): Bar {
     $value = arbitrary_func(self::$static_dic, $key);
     if ($value === null) {
       $value = new Bar();
-      self::$static_dic[$key] = $value; // Not a singleton
+      self::$static_dic[$key] = $value; // Not caching
     }
     return $value;
   }
@@ -59,7 +59,7 @@ public static function get_dict_value_4(string $key): Bar {
     $exist_key = HH\Lib\C\contains_key(self::$static_dic, $key);
 
     if (!$exist_key) {
-      self::$static_dic[$key] = new Bar();  // FN: singleton not recognized
+      self::$static_dic[$key] = new Bar();  // FN: caching not recognized
     }
     return self::$static_dic[$key];
   }

hphp/hack/test/typecheck/global_access_check/test_singleton_write_collection.php.exp

@@ -3,23 +3,23 @@ File "test_singleton_write_collection.php", line 25, characters 18-34:
 File "test_singleton_write_collection.php", line 25, characters 18-34:
 [Foo::get_dict_value]{Foo::$static_dic}(dict<string, Bar>) A global variable is possibly written via function call. (Other[11003])
 File "test_singleton_write_collection.php", line 28, characters 7-38:
-[Foo::get_dict_value]{Foo::$static_dic}(Bar)<Singleton> A global variable is definitely written. (Other[11001])
+[Foo::get_dict_value]{Foo::$static_dic}(Bar)<Caching> A global variable is definitely written. (Other[11001])
 File "test_singleton_write_collection.php", line 30, characters 12-17:
 [Foo::get_dict_value]{Foo::$static_dic}(Bar) A global variable is possibly written via function call. (Other[11003])
 File "test_singleton_write_collection.php", line 34, characters 32-48:
 [Foo::get_dict_value2]{Foo::$static_dic}(dict<string, Bar>) A global variable is definitely read. (Other[11004])
 File "test_singleton_write_collection.php", line 34, characters 32-48:
 [Foo::get_dict_value2]{Foo::$static_dic}(dict<string, Bar>) A global variable is possibly written via function call. (Other[11003])
 File "test_singleton_write_collection.php", line 35, characters 7-41:
-[Foo::get_dict_value2]{Foo::$static_dic}(Bar)<Singleton> A global variable is definitely written. (Other[11001])
+[Foo::get_dict_value2]{Foo::$static_dic}(Bar)<Caching> A global variable is definitely written. (Other[11001])
 File "test_singleton_write_collection.php", line 37, characters 12-28:
 [Foo::get_dict_value2]{Foo::$static_dic}(dict<string, Bar>) A global variable is definitely read. (Other[11004])
 File "test_singleton_write_collection.php", line 37, characters 12-34:
 [Foo::get_dict_value2]{Foo::$static_dic}(Bar) A global variable is possibly written via function call. (Other[11003])
 File "test_singleton_write_collection.php", line 41, characters 14-32:
 [Foo::get_shape_value]{Foo::$static_shape}(shape('field1' => ?Bar, 'field2' => ?Bar)) A global variable is definitely read. (Other[11004])
 File "test_singleton_write_collection.php", line 44, characters 7-44:
-[Foo::get_shape_value]{Foo::$static_shape}(Bar)<Singleton> A global variable is definitely written. (Other[11001])
+[Foo::get_shape_value]{Foo::$static_shape}(Bar)<Caching> A global variable is definitely written. (Other[11001])
 File "test_singleton_write_collection.php", line 46, characters 12-17:
 [Foo::get_shape_value]{Foo::$static_shape}(Bar) A global variable is possibly written via function call. (Other[11003])
 File "test_singleton_write_collection.php", line 50, characters 29-45: