Skip to content

Commit 1d9d645

Browse files
dlreevesmeta-codesync[bot]
authored andcommitted
Enforce package rules on ::class expressions for strict-isolation packages
Summary: # Context `Foo::class` expressions do not produce package-boundary errors: under the `package_allow_classconst_violations` migration carve-out they are downgraded to a classptr lint (`ClassPtrLinterOnly`) so intern/prod separation V1 can roll out without breaking `::class` callers. For a package that opts into strict isolation (`enable_strict_isolation = true`), a `::class` reference from a package that does not include it should be a hard violation, not just a lint. # Solution In the `class_expr` mode selector (the single site that classifies class-id references), when the reference is a `::class` (`is_classptr`) and the target class's package has strict isolation enabled, the boundary reason is upgraded from `ClassPtrLinterOnly` to a hard `Yes Class` error (reusing cross-package error code 4472). The strict-isolation lookup happens only on the `::class` branch, so the common class-reference path is unaffected. Non-strict targets keep the existing classptr lint behavior. # Tests New verify tests under `test/package/strict_isolation/`: a `::class` reference to a strict-isolation class from another package is a violation; the same reference from within its own package is allowed. Reviewed By: madgen Differential Revision: D112642501 fbshipit-source-id: eccd94e20cc9358c14bb8256e6d068896e869a18
1 parent acdcb7f commit 1d9d645

5 files changed

Lines changed: 74 additions & 1 deletion

File tree

hphp/hack/src/typing/typing.ml

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11965,7 +11965,17 @@ end = struct
1196511965
else if is_const then begin
1196611966
if Env.package_allow_classconst_violations env then
1196711967
if is_classptr then
11968-
`ClassPtrLinterOnly
11968+
(* `Foo::class` is normally only a migration-era classptr lint,
11969+
but a strict-isolation target makes it a hard package
11970+
violation. *)
11971+
if
11972+
Typing_packages.is_strict_isolation_target
11973+
env
11974+
(Cls.get_package class_)
11975+
then
11976+
`Yes Typing_error.Primary.Package.Class
11977+
else
11978+
`ClassPtrLinterOnly
1196911979
else
1197011980
(* Non-::class constants: skip the class-level check
1197111981
here and let class_const handle it *)
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//// isolated/c.php
2+
<?hh
3+
// A class in the strict-isolation package `isolated`.
4+
class IsolatedClassPtr {}
5+
6+
//// intern/use.php
7+
<?hh
8+
// `intern` does not include `isolated`. A `::class` reference to its class is a
9+
// package violation for a strict-isolation package (normally only a classptr
10+
// lint under the migration carve-out).
11+
function use_isolated_classptr(): void {
12+
$_ = IsolatedClassPtr::class;
13+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
error: Typing[4472] Cannot access class `IsolatedClassPtr` defined in package `isolated` which may not be available [1]
2+
-> `IsolatedClassPtr` is defined in classptr_cross_package_forbidden.php--isolated/c.php [2]
3+
-> `IsolatedClassPtr` belongs to package `isolated` by this package definition [3]
4+
-> package `intern` is available because the current file belongs to it by this package definition [4]
5+
-> `intern` includes `shared` via its definition [5]
6+
7+
classptr_cross_package_forbidden.php--intern/use.php:6:8
8+
4 | // lint under the migration carve-out).
9+
5 | function use_isolated_classptr(): void {
10+
6 | $_ = IsolatedClassPtr::class;
11+
| ^^^^^^^^^^^^^^^^ [1]
12+
7 | }
13+
14+
classptr_cross_package_forbidden.php--isolated/c.php:3:7
15+
1 | <?hh
16+
2 | // A class in the strict-isolation package `isolated`.
17+
3 | class IsolatedClassPtr {}
18+
| ^^^^^^^^^^^^^^^^ [2]
19+
20+
PACKAGES.toml:14:11
21+
4 | include_paths = ["//shared/"]
22+
5 |
23+
6 | [packages.intern]
24+
| ^^^^^^ [4]
25+
7 | include_paths = ["//intern/"]
26+
8 | includes = ["shared"]
27+
| ^^^^^^^^ [5]
28+
9 |
29+
10 | # A package that opts into strict isolation. Its presence may not be dynamically
30+
11 | # observed (no `package` expression / `__RequirePackage`), and
31+
12 | # package_exclude_patterns (e.g. __tests__) do not grant a typecheck exemption
32+
13 | # for references into it.
33+
14 | [packages.isolated]
34+
| ^^^^^^^^ [3]
35+
15 | include_paths = ["//isolated/"]
36+
16 | includes = ["intern", "shared"]
37+
38+
1 error found
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//// isolated/c.php
2+
<?hh
3+
// A class in the strict-isolation package `isolated`.
4+
class IsolatedClassPtr2 {}
5+
6+
//// isolated/use.php
7+
<?hh
8+
// Same package (`isolated`) may take a `::class` reference to its own class.
9+
function use_within_isolated(): void {
10+
$_ = IsolatedClassPtr2::class;
11+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
No errors

0 commit comments

Comments
 (0)