Reject Hack files placed directly under an implicit package family path

Summary:
# Context

An implicit package family declares a `path`, and each direct child directory
`D` of that path denotes a synthesized member package `F.D`. A file sitting
directly under the family path, with no intervening directory, therefore belongs
to no member package at all.

Nothing reported that. The file simply resolved to "no package", which silently
exempts it from cross-package enforcement in both directions. It is an edge case the original design
missed rather than a decision, so it should be an error.

# Solution

Add `Naming[2135]`, raised when a file lies directly under a family `path`.

This check is done in the lowerer, which has access to the package information needed. The check is invoked from `p_script`, which runs exactly once per file before any
definition is lowered, so a file that declares nothing is still checked. It is
deliberately independent of `__PackageOverride`: an override cannot make a
badly-placed file legal.

The error is anchored at the very start of the file as a zero-width position.
Nothing the file *contains* is at fault, so there is no text worth underlining --
and a fixed-width span would be wrong for a `.hack` file, which has no `<?hh`
pragma and therefore begins with ordinary code.

Reviewed By: madgen

Differential Revision: D114385205

fbshipit-source-id: 6f15cab900cd2065f7471a95ec94420d2b0a3f15

Author: dlreeves

hphp/hack/src/diagnostics/error_codes.ml

@@ -173,6 +173,7 @@ module Naming = struct
     | TypedOpenShapeDisallowed [@value 2132]
     | NamedVariadicTypeDisallowed [@value 2133]
     | VariadicNamedParameterDisallowed [@value 2134]
+    | ImplicitPackageFileDirectlyUnderPath [@value 2135]
   (* Add new Naming codes here! Comment out when deprecating. *)
   [@@deriving enum, show { with_path = false }]
 

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<<23a844de2b22a4423d0dc60ebaff4a51>>
+// @generated SignedSource<<aa25f2a5e885275c1761709806ae1743>>
 //
 // To regenerate this file, run:
 //   buck run @fbcode//mode/dev-nosan-lg fbcode//hphp/hack/src:oxidized_regen
@@ -147,6 +147,7 @@ pub enum Naming {
     TypedOpenShapeDisallowed = 2132,
     NamedVariadicTypeDisallowed = 2133,
     VariadicNamedParameterDisallowed = 2134,
+    ImplicitPackageFileDirectlyUnderPath = 2135,
 }
 impl TrivialDrop for Naming {}
 arena_deserializer::impl_deserialize_in_arena!(Naming);

hphp/hack/src/oxidized/manual/diagnostics_impl.rs

@@ -144,6 +144,36 @@ impl Naming {
         )
     }
 
+    /// A Hack file placed directly under an implicit package family's `path`,
+    /// with no member directory in between, belongs to no member package.
+    ///
+    /// `family_pos` points at the family's declaration in PACKAGES.toml.
+    pub fn implicit_package_file_directly_under_path(
+        pos: Pos,
+        family: &str,
+        family_pos: Pos,
+    ) -> Diagnostic {
+        UserDiagnostic::new(
+            Severity::Err,
+            Self::ImplicitPackageFileDirectlyUnderPath as isize,
+            Message(
+                pos,
+                format!(
+                    "A Hack file cannot be placed directly under the implicit package family `{}`; move it into a subdirectory so it belongs to a member package `{}.<dir>`",
+                    family, family
+                )
+                .into(),
+            ),
+            vec![Message(
+                family_pos,
+                format!("The implicit package family `{}` is declared here", family).into(),
+            )],
+            Explanation::Empty,
+            vec![],
+            vec![],
+        )
+    }
+
     pub fn bad_builtin_type(p: Pos, name: &str, correct_name: &str) -> Diagnostic {
         UserDiagnostic::new(
             Severity::Err,

hphp/hack/src/oxidized/manual/package_info_impl.rs

@@ -168,36 +168,68 @@ static MULTIFILE_PREFIX: LazyLock<regex::Regex> = LazyLock::new(|| {
     regex::Regex::new(r"^.*--").expect("should compile: the pattern is a literal")
 });
 
+/// The path rewriting every package lookup applies before prefix-matching. All
+/// lookups must go through this, so they prefix-match the same string.
+fn normalize(support_multifile_tests: bool, path: &str) -> Cow<'_, str> {
+    if support_multifile_tests {
+        MULTIFILE_PREFIX.replace(path, "")
+    } else {
+        Cow::Borrowed(path)
+    }
+}
+
 impl PackageInfo {
+    /// The most precise package whose `include_path` prefixes `path`, together
+    /// with the part of `path` lying below that `include_path`.
+    fn match_include_path<'a, 'p>(&'a self, path: &'p str) -> Option<(&'a Package, &'p str)> {
+        let (include_path, package) = self
+            .include_path_to_package_map
+            .iter()
+            .find(|(include_path, _)| path.starts_with(include_path))?;
+        Some((package, &path[include_path.len()..]))
+    }
+
     pub fn get_package_for_file(
         &self,
         support_multifile_tests: bool,
         path: &str,
     ) -> Option<Cow<'_, Package>> {
-        let path = if support_multifile_tests {
-            MULTIFILE_PREFIX.replace(path, "")
-        } else {
-            Cow::Borrowed(path)
-        };
-        let (include_path, package) = self
-            .include_path_to_package_map
-            .iter()
-            .find(|(include_path, _)| path.starts_with(include_path))?;
+        let path = normalize(support_multifile_tests, path);
+        let (package, remainder) = self.match_include_path(&path)?;
         if !package.is_implicit {
             // Common case: the stored package is returned by reference, no clone.
             return Some(Cow::Borrowed(package));
         }
-        // Implicit family match: the member directory `D` is the first path
-        // segment after the family `path`. Only direct child *directories*
-        // denote members, so a file lying directly in the family path (no `/`
-        // after the prefix) belongs to no package. The member is synthesized on
-        // demand, hence owned.
-        let remainder = &path[include_path.len()..];
+        // The member directory `D` is the first segment after the family
+        // `path`. Only child directories denote members, so a file lying
+        // directly in the family path belongs to no package.
         match remainder.split_once('/') {
             Some((dir, _)) if !dir.is_empty() => Some(Cow::Owned(synthesize_member(package, dir))),
             _ => None,
         }
     }
+
+    /// The implicit family (its positioned name) if `path` lies directly under
+    /// that family's `path`, with no member directory in between -- e.g.
+    /// `//prototypes/loose.php` under a family declared at `//prototypes/`.
+    ///
+    /// `None` means the file is legally placed: either it belongs to a member,
+    /// or it is under no family at all.
+    pub fn file_directly_under_implicit_family(
+        &self,
+        support_multifile_tests: bool,
+        path: &str,
+    ) -> Option<&PosId> {
+        let path = normalize(support_multifile_tests, path);
+        let (package, remainder) = self.match_include_path(&path)?;
+        if !package.is_implicit {
+            return None;
+        }
+        match remainder.split_once('/') {
+            Some((dir, _)) if !dir.is_empty() => None,
+            _ => Some(&package.name),
+        }
+    }
 }
 
 #[cfg(test)]

hphp/hack/src/parser/lowerer/lowerer.rs

@@ -6262,6 +6262,31 @@ fn get_current_package<'a>(env: &mut Env<'a>, node: S<'a>) -> Option<PackageMemb
     package
 }
 
+/// A file under an implicit package family's `path` must sit inside a member
+/// subdirectory; one placed directly under the family path belongs to no member
+/// package, so it is illegal.
+fn check_implicit_package_placement(env: &mut Env<'_>) {
+    let parser_options = env.parser_options;
+    let file_path = env.source_text().file_path_rc();
+    let Some(family) = parser_options
+        .package_info
+        .file_directly_under_implicit_family(
+            parser_options.package_support_multifile_tests,
+            file_path.path_str(),
+        )
+    else {
+        return;
+    };
+    let (family_pos, family_name) = (family.0.clone(), family.1.clone());
+    // Zero-width, at the start of the file: nothing the file contains is at
+    // fault, and a `.hack` file has no `<?hh` pragma to point at.
+    let file_top_pos = Pos::from_lnum_bol_offset(file_path, (1, 0, 0), (1, 0, 0));
+    raise_hh_error(
+        env,
+        Naming::implicit_package_file_directly_under_path(file_top_pos, &family_name, family_pos),
+    );
+}
+
 fn p_def<'a>(node: S<'a>, env: &mut Env<'a>) -> Result<Vec<ast::Def>> {
     let doc_comment_opt = extract_docblock(node, env);
     match &node.children {
@@ -7081,6 +7106,8 @@ fn p_program<'a>(node: S<'a>, env: &mut Env<'a>) -> ast::Program {
 }
 
 fn p_script<'a>(node: S<'a>, env: &mut Env<'a>) -> ast::Program {
+    // Once per file, so a file declaring nothing is still checked.
+    check_implicit_package_placement(env);
     match &node.children {
         Script(c) => p_program(&c.declarations, env),
         _ => {

hphp/hack/test/package/implicit_packages/dothack_file_directly_under_path.php

@@ -0,0 +1,3 @@
+//// prototypes/loose.hack
+// A `.hack` file has no `<?hh` pragma, so line 1 is ordinary code.
+const int LOOSE_HACK_C = 1;

hphp/hack/test/package/implicit_packages/dothack_file_directly_under_path.php.exp

@@ -0,0 +1,17 @@
+error: Naming[2135] A Hack file cannot be placed directly under the implicit package family `prototypes`; move it into a subdirectory so it belongs to a member package `prototypes.<dir>` [1]
+-> The implicit package family `prototypes` is declared here [2]
+
+dothack_file_directly_under_path.php--prototypes/loose.hack:1:1
+ 1 | // A `.hack` file has no `<?hh` pragma, so line 1 is ordinary code.
+   | ^ [1]
+ 2 | const int LOOSE_HACK_C = 1;
+
+PACKAGES.toml:18:20
+16 | # (prototypes.<dir>). Members are isolated from each other but all hard-include
+17 | # intern (and transitively shared) and soft-include standalone.
+18 | [implicit_packages.prototypes]
+   |                    ^^^^^^^^^^ [2]
+19 | path = "//prototypes/"
+20 | includes = ["intern", "shared"]
+
+1 error found

hphp/hack/test/package/implicit_packages/file_directly_under_path.php

@@ -0,0 +1,9 @@
+//// prototypes/loose.php
+<?hh
+// Directly under //prototypes/ with no member directory in between.
+const int LOOSE_C = 1;
+
+//// prototypes/beta/b.php
+<?hh
+// In a member directory, so this one is fine.
+const int BETA_C = 1;

hphp/hack/test/package/implicit_packages/file_directly_under_path.php.exp

@@ -0,0 +1,18 @@
+error: Naming[2135] A Hack file cannot be placed directly under the implicit package family `prototypes`; move it into a subdirectory so it belongs to a member package `prototypes.<dir>` [1]
+-> The implicit package family `prototypes` is declared here [2]
+
+file_directly_under_path.php--prototypes/loose.php:1:1
+ 1 | <?hh
+   | ^ [1]
+ 2 | // Directly under //prototypes/ with no member directory in between.
+ 3 | const int LOOSE_C = 1;
+
+PACKAGES.toml:18:20
+16 | # (prototypes.<dir>). Members are isolated from each other but all hard-include
+17 | # intern (and transitively shared) and soft-include standalone.
+18 | [implicit_packages.prototypes]
+   |                    ^^^^^^^^^^ [2]
+19 | path = "//prototypes/"
+20 | includes = ["intern", "shared"]
+
+1 error found

hphp/hack/test/package/implicit_packages/module_directly_under_path.php

@@ -0,0 +1,4 @@
+//// prototypes/mod.php
+<?hh
+// The only top-level definition is a module declaration.
+new module foo {}