Error on nameof of a strict-isolation package's class

Summary:
# Context

`nameof Foo` yields the string name of `Foo` without requiring the package of `Foo` to be loaded, so it is normally exempt from the package-boundary check (`inside_nameof` forces the check off in `class_expr`). Strict-isolation packages (`enable_strict_isolation = true`) are the exception: their presence must not be statically referenced from outside, so even a `nameof` of one of their classes is treated as a real cross-package reference.

# Solution

At the `nameof` typing site, when the target class belongs to a strict-isolation package, run the ordinary package-boundary check (`Typing_visibility.check_package_access`) and report any violation as a hard error, exactly as a normal reference would: a cross-package `nameof` is `Typing[4472]` (cross-package access), and a `nameof` of a class on the package excluded path is `Typing[4518]` (excluded-path access). Same-package references, and targets that are not strict-isolation packages, produce nothing -- matching the other checks in this stack.

# Tests

New verify tests under `test/package/strict_isolation/`: a cross-package `nameof` into a strict-isolation package errors (`Typing[4472]`); a `nameof` of a class defined on an excluded path of the package errors (`Typing[4518]`, exercising the `ExcludedPathAccess` branch); the same reference within its own package, and a `nameof` of a non-strict-isolation package, produce nothing.

Reviewed By: madgen

Differential Revision: D112642508

fbshipit-source-id: 3cfd7c7c8f22a88b4ffa3a0774fa4cc863cf651c

Author: dlreeves

hphp/hack/src/typing/typing.ml

@@ -11952,16 +11952,35 @@ end = struct
         | Expr.Check_all
         | Expr.Skip_package ->
           let should_check_package_boundary =
+            (* Some reference contexts are exempt from the package boundary check
+               by default (nameof, catch, ::class), but a strict-isolation target
+               opts them back in: its classes must not be statically referenced
+               from outside its package. Reuses the already-fetched decl, so no
+               extra heap access. *)
+            let exempt_unless_strict_isolation spec =
+              if
+                Typing_packages.is_strict_isolation_target
+                  env
+                  (Cls.get_package class_)
+              then
+                `Yes spec
+              else
+                `No
+            in
             if
-              inside_nameof
-              || is_attribute
-              || is_catch
+              is_attribute
               ||
               match attribute_check_policy with
               | Expr.Skip_package -> true
               | _ -> false
             then
               `No
+            else if is_catch then
+              (* `catch (C $e)` references the exception class C. *)
+              exempt_unless_strict_isolation Typing_error.Primary.Package.Class
+            else if inside_nameof then
+              (* `nameof C` yields C's name without loading its package. *)
+              exempt_unless_strict_isolation Typing_error.Primary.Package.Class
             else if is_const then begin
               if Env.package_allow_classconst_violations env then
                 if is_classptr then

hphp/hack/test/package/strict_isolation/catch_cross_package_forbidden.php

@@ -0,0 +1,14 @@
+//// isolated/exn.php
+<?hh
+// An exception class in the strict-isolation package `isolated`.
+class IsolatedException extends Exception {}
+
+//// intern/use.php
+<?hh
+// `intern` does not include `isolated`; catching its exception statically
+// references the strict-isolation package, which is a hard package violation.
+function use_isolated_catch(): void {
+  try {
+  } catch (IsolatedException $_e) {
+  }
+}

hphp/hack/test/package/strict_isolation/catch_cross_package_forbidden.php.exp

@@ -0,0 +1,39 @@
+error: Typing[4472] Cannot access class `IsolatedException` defined in package `isolated` which may not be available [1]
+-> `IsolatedException` is defined in catch_cross_package_forbidden.php--isolated/exn.php [2]
+-> `IsolatedException` belongs to package `isolated` by this package definition [3]
+-> package `intern` is available because the current file belongs to it by this package definition [4]
+-> `intern` includes `shared` via its definition [5]
+
+catch_cross_package_forbidden.php--intern/use.php:6:12
+ 4 | function use_isolated_catch(): void {
+ 5 |   try {
+ 6 |   } catch (IsolatedException $_e) {
+   |            ^^^^^^^^^^^^^^^^^ [1]
+ 7 |   }
+ 8 | }
+
+catch_cross_package_forbidden.php--isolated/exn.php:3:7
+ 1 | <?hh
+ 2 | // An exception class in the strict-isolation package `isolated`.
+ 3 | class IsolatedException extends Exception {}
+   |       ^^^^^^^^^^^^^^^^^ [2]
+
+PACKAGES.toml:14:11
+ 4 | include_paths = ["//shared/"]
+ 5 | 
+ 6 | [packages.intern]
+   |           ^^^^^^ [4]
+ 7 | include_paths = ["//intern/"]
+ 8 | includes = ["shared"]
+   |             ^^^^^^^^ [5]
+ 9 | 
+10 | # A package that opts into strict isolation. Its presence may not be dynamically
+11 | # observed (no `package` expression / `__RequirePackage`), and
+12 | # package_exclude_patterns (e.g. __tests__) do not grant a typecheck exemption
+13 | # for references into it.
+14 | [packages.isolated]
+   |           ^^^^^^^^ [3]
+15 | include_paths = ["//isolated/"]
+16 | includes = ["intern", "shared"]
+
+1 error found

hphp/hack/test/package/strict_isolation/catch_same_package_ok.php

@@ -0,0 +1,13 @@
+//// isolated/exn.php
+<?hh
+// An exception class in the strict-isolation package `isolated`.
+class IsolatedException2 extends Exception {}
+
+//// isolated/use.php
+<?hh
+// Same package (`isolated`) may catch its own exception: no violation.
+function use_within_isolated(): void {
+  try {
+  } catch (IsolatedException2 $_e) {
+  }
+}

hphp/hack/test/package/strict_isolation/catch_same_package_ok.php.exp

@@ -0,0 +1 @@
+No errors

hphp/hack/test/package/strict_isolation/nameof_cross_package_error.php

@@ -0,0 +1,13 @@
+//// isolated/c.php
+<?hh
+// A class in the strict-isolation package `isolated`.
+class IsolatedNameof {}
+
+//// intern/use.php
+<?hh
+// `intern` does not include `isolated`. Even though `nameof` does not require the
+// package to be loaded, a strict-isolation package may not be statically
+// referenced from outside, so this is a hard package-boundary error.
+function use_isolated_nameof(): void {
+  $_ = nameof IsolatedNameof;
+}

hphp/hack/test/package/strict_isolation/nameof_cross_package_error.php.exp

@@ -0,0 +1,38 @@
+error: Typing[4472] Cannot access class `IsolatedNameof` defined in package `isolated` which may not be available [1]
+-> `IsolatedNameof` is defined in nameof_cross_package_error.php--isolated/c.php [2]
+-> `IsolatedNameof` belongs to package `isolated` by this package definition [3]
+-> package `intern` is available because the current file belongs to it by this package definition [4]
+-> `intern` includes `shared` via its definition [5]
+
+nameof_cross_package_error.php--intern/use.php:6:15
+ 4 | // referenced from outside, so this is a hard package-boundary error.
+ 5 | function use_isolated_nameof(): void {
+ 6 |   $_ = nameof IsolatedNameof;
+   |               ^^^^^^^^^^^^^^ [1]
+ 7 | }
+
+nameof_cross_package_error.php--isolated/c.php:3:7
+ 1 | <?hh
+ 2 | // A class in the strict-isolation package `isolated`.
+ 3 | class IsolatedNameof {}
+   |       ^^^^^^^^^^^^^^ [2]
+
+PACKAGES.toml:14:11
+ 4 | include_paths = ["//shared/"]
+ 5 | 
+ 6 | [packages.intern]
+   |           ^^^^^^ [4]
+ 7 | include_paths = ["//intern/"]
+ 8 | includes = ["shared"]
+   |             ^^^^^^^^ [5]
+ 9 | 
+10 | # A package that opts into strict isolation. Its presence may not be dynamically
+11 | # observed (no `package` expression / `__RequirePackage`), and
+12 | # package_exclude_patterns (e.g. __tests__) do not grant a typecheck exemption
+13 | # for references into it.
+14 | [packages.isolated]
+   |           ^^^^^^^^ [3]
+15 | include_paths = ["//isolated/"]
+16 | includes = ["intern", "shared"]
+
+1 error found

hphp/hack/test/package/strict_isolation/nameof_excluded_path_error.php

@@ -0,0 +1,15 @@
+//// isolated/__tests__/helper.php
+<?hh
+// A class on an excluded path (__tests__) within the strict-isolation package
+// `isolated`.
+class IsolatedTestOnlyNameof {}
+
+//// isolated/use.php
+<?hh
+// Non-excluded code in `isolated` referencing its own excluded-path class via
+// `nameof` is an error too: the class is deployment-only (an excluded path), so
+// the reference is forbidden even within the same package. This exercises the
+// `ExcludedPathAccess` branch of the nameof check.
+function use_isolated_excluded_nameof(): void {
+  $_ = nameof IsolatedTestOnlyNameof;
+}

hphp/hack/test/package/strict_isolation/nameof_excluded_path_error.php.exp

@@ -0,0 +1,26 @@
+error: Typing[4518] Cannot access class `IsolatedTestOnlyNameof`: it is defined on an excluded path of the strict-isolation package `isolated`, and code outside the excluded path may not reference it [1]
+-> `IsolatedTestOnlyNameof` is defined here, in nameof_excluded_path_error.php--isolated/__tests__/helper.php -- an excluded path (e.g. matching `package_exclude_patterns`) [2]
+-> package `isolated` is a strict-isolation package (declared here); code on an excluded path is deployment-only and may not be referenced from outside that path [3]
+
+nameof_excluded_path_error.php--isolated/use.php:7:15
+ 5 | // `ExcludedPathAccess` branch of the nameof check.
+ 6 | function use_isolated_excluded_nameof(): void {
+ 7 |   $_ = nameof IsolatedTestOnlyNameof;
+   |               ^^^^^^^^^^^^^^^^^^^^^^ [1]
+ 8 | }
+
+nameof_excluded_path_error.php--isolated/__tests__/helper.php:4:7
+ 2 | // A class on an excluded path (__tests__) within the strict-isolation package
+ 3 | // `isolated`.
+ 4 | class IsolatedTestOnlyNameof {}
+   |       ^^^^^^^^^^^^^^^^^^^^^^ [2]
+
+PACKAGES.toml:14:11
+12 | # package_exclude_patterns (e.g. __tests__) do not grant a typecheck exemption
+13 | # for references into it.
+14 | [packages.isolated]
+   |           ^^^^^^^^ [3]
+15 | include_paths = ["//isolated/"]
+16 | includes = ["intern", "shared"]
+
+1 error found

hphp/hack/test/package/strict_isolation/nameof_non_strict_no_warning.php

@@ -0,0 +1,12 @@
+//// standalone/c.php
+<?hh
+// A class in `standalone`, which does NOT opt into strict isolation.
+class StandaloneNameof {}
+
+//// intern/use.php
+<?hh
+// `intern` does not include `standalone`, but `standalone` is not a
+// strict-isolation package, so `nameof` on it produces no warning.
+function use_standalone_nameof(): void {
+  $_ = nameof StandaloneNameof;
+}