track ::class in the cross-package linter

Summary:
Allow the cross-package linter to track class constants when the option `package_allow_classconst_violations` is  set to true.  The linter error message is ad-hoc for class constant accesses.

Observe that for classconst violations the linter does not check that the target file has a PackageOverride, as no errors would have been reported anyway if the file was missing a PackageOverride due to `package_allow_classconst_violations=true`.

Reviewed By: DavidSnider

Differential Revision: D94235975

fbshipit-source-id: 19798ff8cdbeec3b4ce6346e7f57b5ee392f4834

Author: vassilmladenov

hphp/hack/src/lints/lints_codes.ml

@@ -122,4 +122,6 @@ module Codes = struct
   (* let deprecated_nullsafe_on_null = 5654 *)
 
   let package_into_override = 5655
+
+  let crosspackage_classptr_reference = 5656
 end

hphp/hack/src/lints/lints_diagnostics.ml

@@ -382,3 +382,47 @@ let package_into_override
     ^ ") or gate this reference via a `package` check "
     ^ "(if this portion of the function is meant to be gated by an environment check)."
     )
+
+let crosspackage_classptr_reference
+    pos current_package target_package_before_override =
+  let current_package_name =
+    Option.value_map current_package ~default:"<none>" ~f:snd
+  in
+  let target_package_before_override_name =
+    Option.value target_package_before_override ~default:"<none>"
+  in
+  Lints.add
+    Codes.crosspackage_classptr_reference
+    Lint_error
+    pos
+    ("This is a potential cross-boundary edge from package "
+    ^ current_package_name
+    ^ " into package "
+    ^ target_package_before_override_name
+    ^ ". If this value isn't used to load the class "
+    ^ "via new $cls(), $cls::meth(), etc, please change this to a `nameof` expression. "
+    ^ "Otherwise, move the callee into the caller's package via hg mv, "
+    ^ "use `__RequirePackage` (if the whole function should only be invoked from "
+    ^ target_package_before_override_name
+    ^ ") or gate this reference via a `package` check "
+    ^ "(if this portion of the function is meant to be gated by an environment check)."
+    )
+
+(* wrapper around crosspackage_classconst_reference and package_into_override *)
+let crosspackage_linter
+    pos
+    current_package
+    target_package
+    target_package_before_override
+    classptr_reference_warning =
+  if classptr_reference_warning then
+    crosspackage_classptr_reference
+      pos
+      current_package
+      target_package_before_override
+  else
+    package_into_override
+      pos
+      current_package
+      target_package
+      target_package_before_override

hphp/hack/src/typing/typing.ml

@@ -1350,11 +1350,12 @@ end = struct
         name
     in
     List.iter access_linter_errors ~f:(fun (pos, w) ->
-        Lints_diagnostics.package_into_override
+        Lints_diagnostics.crosspackage_linter
           pos
           w.current_package
           w.target_package
-          w.target_package_before_override);
+          w.target_package_before_override
+          w.classptr_reference_warning);
     Option.iter
       ~f:(Typing_error_utils.add_typing_error ~env)
       (Typing_error.multiple_opt (other_errs @ access_errs))
@@ -3644,13 +3645,13 @@ end = struct
         | TVis.Package_access_error err ->
           Typing_error_utils.add_typing_error ~env err
         | TVis.Package_access_linter_error (pos, w) ->
-          Lints_diagnostics.package_into_override
+          Lints_diagnostics.crosspackage_linter
             pos
             w.current_package
             w.target_package
             w.target_package_before_override
+            w.classptr_reference_warning
         | TVis.Package_access_ok -> ());
-
         let ((env, ty_err_opt), ty) =
           Phase.localize_no_subst env ~ignore_errors:true const.cd_type
         in
@@ -5116,11 +5117,12 @@ end = struct
             | TVis.Package_access_error err ->
               Typing_error_utils.add_typing_error ~env err
             | TVis.Package_access_linter_error (pos, w) ->
-              Lints_diagnostics.package_into_override
+              Lints_diagnostics.crosspackage_linter
                 pos
                 w.current_package
                 w.target_package
                 w.target_package_before_override
+                w.classptr_reference_warning
             | TVis.Package_access_ok -> ())
           | Decl_entry.DoesNotExist
           | Decl_entry.NotYetAvailable ->
@@ -11717,7 +11719,7 @@ end = struct
                 `No
               else if is_const then begin
                 if Env.package_allow_classconst_violations env then
-                  `No
+                  `ClassPtrLinterOnly
                 else
                   `Yes Typing_error.Primary.Package.Class
               end else
@@ -11736,11 +11738,12 @@ end = struct
                 id
             in
             List.iter access_linter_errs ~f:(fun (pos, w) ->
-                Lints_diagnostics.package_into_override
+                Lints_diagnostics.crosspackage_linter
                   pos
                   w.current_package
                   w.target_package
-                  w.target_package_before_override);
+                  w.target_package_before_override
+                  w.classptr_reference_warning);
             List.iter ~f:(Typing_error_utils.add_typing_error ~env) access_errs
           );
 

hphp/hack/src/typing/typing_class.ml

@@ -656,11 +656,12 @@ let check_consistent_enum_inclusion
   | Typing_visibility.Package_access_error err ->
     Typing_error_utils.add_typing_error ~env err
   | Typing_visibility.Package_access_linter_error (pos, w) ->
-    Lints_diagnostics.package_into_override
+    Lints_diagnostics.crosspackage_linter
       pos
       w.current_package
       w.target_package
       w.target_package_before_override
+      w.classptr_reference_warning
   | Typing_visibility.Package_access_ok -> ());
   match (Cls.enum_type included_cls, Cls.enum_type dest_cls) with
   | (Some included_e, Some dest_e) ->

hphp/hack/src/typing/typing_packages.ml

@@ -36,6 +36,7 @@ type package_warning_info = {
   current_package: Package.pos_id option;
   target_package: Package.pos_id option;
   target_package_before_override: string option;
+  classptr_reference_warning: bool;
 }
 
 type package_error_info = {
@@ -48,6 +49,7 @@ type package_error_info = {
 
 type check_reason =
   [ `Yes of Typing_error.Primary.Package.target_symbol_spec
+  | `ClassPtrLinterOnly
   | `No
   ]
 
@@ -91,6 +93,73 @@ let package_includes current_pkg target_pkg =
       false)
   | _ -> false
 
+(* triggers a linter error if the edge introduces a new dependency from a file in a package
+ * due to another file, ingoring package rules to a file with a packageoverride; used *)
+let can_access_ignoring_package_override
+    ~(env : Typing_env_types.env)
+    ~(current_package : Package.pos_id option)
+    ~(target_package : Package.pos_id option)
+    ~(target_file : Relative_path.t)
+    ~(classptr_reference_warning : bool) =
+  let tcopt = Env.get_tcopt env in
+  let target_package_before_override =
+    Option.map
+      (Package_info.get_package_for_file
+         ~support_multifile_tests:
+           (TypecheckerOptions.package_support_multifile_tests tcopt)
+         (TypecheckerOptions.package_info tcopt)
+         (Relative_path.suffix target_file))
+      ~f:Package.get_package_name
+  in
+  let is_target_package_before_override_included =
+    match (current_package, target_package_before_override) with
+    | (Some (_, current_package_name), Some target_package_before_override_name)
+      ->
+      let target_package_before_override =
+        Env.get_package_by_name env target_package_before_override_name
+      in
+      package_includes
+        (Env.get_package_by_name env current_package_name)
+        target_package_before_override
+    | _ -> true
+  in
+  let is_target_package_before_override_loaded =
+    match target_package_before_override with
+    | Some pkg_name -> Env.is_package_loaded env pkg_name
+    | None -> true
+  in
+  let is_target_package_before_override_soft_required =
+    match
+      (Env.get_soft_package_requirement env, target_package_before_override)
+    with
+    | (Some soft_required_package_name, Some pkg_name) ->
+      let soft_required_package =
+        Env.get_package_by_name env (snd soft_required_package_name)
+      in
+      let target_package_before_override =
+        Env.get_package_by_name env pkg_name
+      in
+      package_includes soft_required_package target_package_before_override
+    | (Some _, None) -> true
+    | _ -> false
+  in
+  if
+    is_target_package_before_override_included
+    || is_target_package_before_override_loaded
+    || is_target_package_before_override_soft_required
+  then
+    `Yes
+  else
+    let warn_info =
+      {
+        current_package;
+        target_package;
+        target_package_before_override;
+        classptr_reference_warning;
+      }
+    in
+    `YesWarning warn_info
+
 let can_access_by_package_rules
     ~(env : Typing_env_types.env)
     ~(target_package_membership : Aast_defs.package_membership option)
@@ -122,55 +191,18 @@ let can_access_by_package_rules
     in
     match get_package_violation env current_pkg target_pkg with
     | None ->
-      (* There are no package errrors, but emit a warning if the edge introduces a new
-       * dependency a file in a package due to package rules and a file with a packageoverride *)
+      (* There are no package errrors, but emit a warning if the edge introduces a new dependency
+       * from a file in a package due to package rules to a file with a packageoverride *)
       (match (current_package_membership, target_package_membership) with
       | ( Some (Aast_defs.PackageConfigAssignment _),
           Some (Aast_defs.PackageOverride _) )
         when package_includes current_pkg target_pkg ->
-        let target_package_before_override =
-          let tcopt = Env.get_tcopt env in
-          Option.map
-            (Package_info.get_package_for_file
-               ~support_multifile_tests:
-                 (TypecheckerOptions.package_support_multifile_tests tcopt)
-               (TypecheckerOptions.package_info tcopt)
-               (Relative_path.suffix target_file))
-            ~f:Package.get_package_name
-        in
-        let is_target_package_before_override_loaded =
-          match target_package_before_override with
-          | Some pkg_name -> Env.is_package_loaded env pkg_name
-          | None -> true
-        in
-        let is_target_package_before_override_soft_required =
-          match
-            ( Env.get_soft_package_requirement env,
-              target_package_before_override )
-          with
-          | (Some soft_required_package_name, Some pkg_name) ->
-            let soft_required_package =
-              Env.get_package_by_name env (snd soft_required_package_name)
-            in
-            let target_package_before_override =
-              Env.get_package_by_name env pkg_name
-            in
-            package_includes
-              soft_required_package
-              target_package_before_override
-          | (Some _, None) -> true
-          | _ -> false
-        in
-        if
-          is_target_package_before_override_loaded
-          || is_target_package_before_override_soft_required
-        then
-          `Yes
-        else
-          let warn_info =
-            { current_package; target_package; target_package_before_override }
-          in
-          `YesWarning warn_info
+        can_access_ignoring_package_override
+          ~env
+          ~current_package
+          ~target_package
+          ~target_file
+          ~classptr_reference_warning:false
       | _ -> `Yes)
     | Some pkg_relationship ->
       let err_info =

hphp/hack/src/typing/typing_packages.mli

@@ -10,6 +10,7 @@ type package_warning_info = {
   current_package: Package.pos_id option;
   target_package: Package.pos_id option;
   target_package_before_override: string option;
+  classptr_reference_warning: bool;
 }
 
 type package_error_info = {
@@ -22,9 +23,23 @@ type package_error_info = {
 
 type check_reason =
   [ `Yes of Typing_error.Primary.Package.target_symbol_spec
+  | `ClassPtrLinterOnly
   | `No
   ]
 
+val get_package_profile :
+  Typing_env_types.env ->
+  Aast_defs.package_membership option ->
+  Package.t option * Package.pos_id option * string
+
+val can_access_ignoring_package_override :
+  env:Typing_env_types.env ->
+  current_package:Package.pos_id option ->
+  target_package:Package.pos_id option ->
+  target_file:Relative_path.t ->
+  classptr_reference_warning:bool ->
+  [ `Yes | `YesWarning of package_warning_info ]
+
 (** Calculate the packages of the current and target symbols using their filenames.
   * The target symbol can be accessed if:
   *    - current and target are in the same file

hphp/hack/src/typing/typing_type_integrity.ml

@@ -288,11 +288,12 @@ and check_type_integrity
            cid
        in
        List.iter linter_errs ~f:(fun (pos, w) ->
-           Lints_diagnostics.package_into_override
+           Lints_diagnostics.crosspackage_linter
              pos
              w.current_package
              w.target_package
-             w.target_package_before_override);
+             w.target_package_before_override
+             w.classptr_reference_warning);
        List.iter errs ~f:(Typing_error_utils.add_typing_error ~env));
       let tparams = Cls.tparams class_info in
       check_targs_integrity ~in_signature (Cls.pos class_info) argl tparams
@@ -310,11 +311,12 @@ and check_type_integrity
            cid
        in
        List.iter linter_errs ~f:(fun (pos, w) ->
-           Lints_diagnostics.package_into_override
+           Lints_diagnostics.crosspackage_linter
              pos
              w.current_package
              w.target_package
-             w.target_package_before_override);
+             w.target_package_before_override
+             w.classptr_reference_warning);
        List.iter errs ~f:(Typing_error_utils.add_typing_error ~env));
       check_targs_integrity ~in_signature typedef.td_pos argl typedef.td_tparams
     | Decl_entry.DoesNotExist

hphp/hack/src/typing/typing_visibility.ml

@@ -159,7 +159,7 @@ let check_package_access
     ~use_pos
     ~def_pos
     env
-    target_package
+    target_package_membership
     target_id =
   match should_check_package_boundary with
   | `No -> Package_access_ok
@@ -170,7 +170,7 @@ let check_package_access
       match
         Typing_packages.can_access_by_package_rules
           ~env
-          ~target_package_membership:target_package
+          ~target_package_membership
           ~target_pos:def_pos
           ~target_id
       with
@@ -229,6 +229,23 @@ let check_package_access
                   included_packages;
                 }))
     end
+  | `ClassPtrLinterOnly ->
+    let current_package =
+      Option.map ~f:(fun p -> p.name) (Typing_env.get_current_package env)
+    in
+    let (_, target_package, _) =
+      Typing_packages.get_package_profile env target_package_membership
+    in
+    (match
+       Typing_packages.can_access_ignoring_package_override
+         ~env
+         ~current_package
+         ~target_package
+         ~target_file:(Pos_or_decl.filename def_pos)
+         ~classptr_reference_warning:true
+     with
+    | `Yes -> Package_access_ok
+    | `YesWarning w -> Package_access_linter_error (use_pos, w))
 
 let is_visible_for_obj ~is_method ~is_receiver_interface env vis =
   let member_ty =