Some minor changes in Luau TextRequirer (comments, naming, etc)

Author: khvzak

src/luau/require.rs

@@ -14,7 +14,7 @@ use crate::state::{callback_error_ext, Lua};
 use crate::table::Table;
 use crate::types::MaybeSend;
 
-/// An error that can occur during navigation in the Luau `require` system.
+/// An error that can occur during navigation in the Luau `require-by-string` system.
 #[cfg(any(feature = "luau", doc))]
 #[cfg_attr(docsrs, doc(cfg(feature = "luau")))]
 #[derive(Debug, Clone)]
@@ -50,7 +50,7 @@ impl From<Error> for NavigateError {
 #[cfg(feature = "luau")]
 type WriteResult = ffi::luarequire_WriteResult;
 
-/// A trait for handling modules loading and navigation in the Luau `require` system.
+/// A trait for handling modules loading and navigation in the Luau `require-by-string` system.
 #[cfg(any(feature = "luau", doc))]
 #[cfg_attr(docsrs, doc(cfg(feature = "luau")))]
 pub trait Require: MaybeSend {
@@ -103,16 +103,26 @@ impl fmt::Debug for dyn Require {
     }
 }
 
-/// The standard implementation of Luau `require` navigation.
-#[doc(hidden)]
+/// The standard implementation of Luau `require-by-string` navigation.
 #[derive(Default, Debug)]
 pub struct TextRequirer {
+    /// An absolute path to the current Luau module (not mapped to a physical file)
     abs_path: PathBuf,
+    /// A relative path to the current Luau module (not mapped to a physical file)
     rel_path: PathBuf,
-    module_path: PathBuf,
+    /// A physical path to the current Luau module, which is a file or a directory with an
+    /// `init.lua(u)` file
+    resolved_path: Option<PathBuf>,
 }
 
 impl TextRequirer {
+    /// The prefix used for chunk names in the require system.
+    /// Only chunk names starting with this prefix are allowed to be used in `require`.
+    const CHUNK_PREFIX: &str = "@";
+
+    /// The file extensions that are considered valid for Luau modules.
+    const FILE_EXTENSIONS: &[&str] = &["luau", "lua"];
+
     /// Creates a new `TextRequirer` instance.
     pub fn new() -> Self {
         Self::default()
@@ -156,44 +166,48 @@ impl TextRequirer {
         components.into_iter().collect()
     }
 
-    fn find_module(path: &Path) -> StdResult<PathBuf, NavigateError> {
+    /// Resolve a Luau module path to a physical file or directory.
+    ///
+    /// Empty directories without init files are considered valid as "intermediate" directories.
+    fn resolve_module(path: &Path) -> StdResult<Option<PathBuf>, NavigateError> {
         let mut found_path = None;
 
         if path.components().next_back() != Some(Component::Normal("init".as_ref())) {
             let current_ext = (path.extension().and_then(|s| s.to_str()))
                 .map(|s| format!("{s}."))
                 .unwrap_or_default();
-            for ext in ["luau", "lua"] {
+            for ext in Self::FILE_EXTENSIONS {
                 let candidate = path.with_extension(format!("{current_ext}{ext}"));
                 if candidate.is_file() && found_path.replace(candidate).is_some() {
                     return Err(NavigateError::Ambiguous);
                 }
             }
         }
         if path.is_dir() {
-            for component in ["init.luau", "init.lua"] {
+            for component in Self::FILE_EXTENSIONS.iter().map(|ext| format!("init.{ext}")) {
                 let candidate = path.join(component);
                 if candidate.is_file() && found_path.replace(candidate).is_some() {
                     return Err(NavigateError::Ambiguous);
                 }
             }
 
             if found_path.is_none() {
-                found_path = Some(PathBuf::new());
+                // Directories without init files are considered valid "intermediate" path
+                return Ok(None);
             }
         }
 
-        found_path.ok_or(NavigateError::NotFound)
+        Ok(Some(found_path.ok_or(NavigateError::NotFound)?))
     }
 }
 
 impl Require for TextRequirer {
     fn is_require_allowed(&self, chunk_name: &str) -> bool {
-        chunk_name.starts_with('@')
+        chunk_name.starts_with(Self::CHUNK_PREFIX)
     }
 
     fn reset(&mut self, chunk_name: &str) -> StdResult<(), NavigateError> {
-        if !chunk_name.starts_with('@') {
+        if !chunk_name.starts_with(Self::CHUNK_PREFIX) {
             return Err(NavigateError::NotFound);
         }
         let chunk_name = Self::normalize_chunk_name(&chunk_name[1..]);
@@ -205,74 +219,79 @@ impl Require for TextRequirer {
             let cwd = env::current_dir().map_err(|_| NavigateError::NotFound)?;
             self.abs_path = Self::normalize_path(&cwd.join(chunk_filename));
             self.rel_path = ([Component::CurDir, Component::Normal(chunk_filename)].into_iter()).collect();
-            self.module_path = PathBuf::new();
+            self.resolved_path = None;
 
             return Ok(());
         }
 
         if chunk_path.is_absolute() {
-            let module_path = Self::find_module(&chunk_path)?;
+            let resolved_path = Self::resolve_module(&chunk_path)?;
             self.abs_path = chunk_path.clone();
             self.rel_path = chunk_path;
-            self.module_path = module_path;
+            self.resolved_path = resolved_path;
         } else {
             // Relative path
             let cwd = env::current_dir().map_err(|_| NavigateError::NotFound)?;
             let abs_path = Self::normalize_path(&cwd.join(&chunk_path));
-            let module_path = Self::find_module(&abs_path)?;
+            let resolved_path = Self::resolve_module(&abs_path)?;
             self.abs_path = abs_path;
             self.rel_path = chunk_path;
-            self.module_path = module_path;
+            self.resolved_path = resolved_path;
         }
 
         Ok(())
     }
 
     fn jump_to_alias(&mut self, path: &str) -> StdResult<(), NavigateError> {
         let path = Self::normalize_path(path.as_ref());
-        let module_path = Self::find_module(&path)?;
+        let resolved_path = Self::resolve_module(&path)?;
 
         self.abs_path = path.clone();
         self.rel_path = path;
-        self.module_path = module_path;
+        self.resolved_path = resolved_path;
 
         Ok(())
     }
 
     fn to_parent(&mut self) -> StdResult<(), NavigateError> {
         let mut abs_path = self.abs_path.clone();
         if !abs_path.pop() {
+            // It's important to return `NotFound` if we reached the root, as it's a "recoverable" error if we
+            // cannot go beyond the root directory.
+            // Luau "require-by-string` has a special logic to search for config file to resolve aliases.
             return Err(NavigateError::NotFound);
         }
         let mut rel_parent = self.rel_path.clone();
         rel_parent.pop();
-        let module_path = Self::find_module(&abs_path)?;
+        let resolved_path = Self::resolve_module(&abs_path)?;
 
         self.abs_path = abs_path;
         self.rel_path = Self::normalize_path(&rel_parent);
-        self.module_path = module_path;
+        self.resolved_path = resolved_path;
 
         Ok(())
     }
 
     fn to_child(&mut self, name: &str) -> StdResult<(), NavigateError> {
         let abs_path = self.abs_path.join(name);
         let rel_path = self.rel_path.join(name);
-        let module_path = Self::find_module(&abs_path)?;
+        let resolved_path = Self::resolve_module(&abs_path)?;
 
         self.abs_path = abs_path;
         self.rel_path = rel_path;
-        self.module_path = module_path;
+        self.resolved_path = resolved_path;
 
         Ok(())
     }
 
     fn has_module(&self) -> bool {
-        self.module_path.is_file()
+        (self.resolved_path.as_deref())
+            .map(Path::is_file)
+            .unwrap_or(false)
     }
 
     fn cache_key(&self) -> String {
-        self.module_path.display().to_string()
+        self.resolved_path.as_deref().unwrap().display().to_string()
     }
 
     fn has_config(&self) -> bool {
@@ -285,7 +304,9 @@ impl Require for TextRequirer {
 
     fn loader(&self, lua: &Lua) -> Result<Function> {
         let name = format!("@{}", self.rel_path.display());
-        lua.load(&*self.module_path).set_name(name).into_function()
+        lua.load(self.resolved_path.as_deref().unwrap())
+            .set_name(name)
+            .into_function()
     }
 }
 
@@ -496,7 +517,7 @@ unsafe fn write_to_buffer(
 }
 
 #[cfg(feature = "luau")]
-pub fn create_require_function<R: Require + 'static>(lua: &Lua, require: R) -> Result<Function> {
+pub(super) fn create_require_function<R: Require + 'static>(lua: &Lua, require: R) -> Result<Function> {
     unsafe extern "C-unwind" fn find_current_file(state: *mut ffi::lua_State) -> c_int {
         let mut ar: ffi::lua_Debug = mem::zeroed();
         for level in 2.. {

src/prelude.rs

@@ -25,7 +25,8 @@ pub use crate::HookTriggers as LuaHookTriggers;
 #[doc(no_inline)]
 pub use crate::{
     CompileConstant as LuaCompileConstant, CoverageInfo as LuaCoverageInfo,
-    NavigateError as LuaNavigateError, Require as LuaRequire, Vector as LuaVector,
+    NavigateError as LuaNavigateError, Require as LuaRequire, TextRequirer as LuaTextRequirer,
+    Vector as LuaVector,
 };
 
 #[cfg(feature = "async")]