Refactor CLI file resolution to be more in line with rbs + use std path instead of str

filiptibell · filiptibell · commit d7c3419023bd · 2025-07-16T20:25:11.000+02:00
diff --git a/crates/lune-utils/src/path/luau.rs b/crates/lune-utils/src/path/luau.rs
@@ -106,9 +106,22 @@ impl AsRef<Path> for LuauFilePath {
     }
 }
 
+impl fmt::Display for LuauFilePath {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Directory(path) | Self::File(path) => path.display().fmt(f),
+        }
+    }
+}
+
 /**
-    A resolved module path for Luau, containing both the source module path,
-    and the corresponding target path on the current filesystem.
+    A resolved module path for Luau, containing both:
+
+    - The **source** Luau module path.
+    - The **target** filesystem path.
+
+    Note the separation here - the source is not necessarily a valid filesystem path,
+    and the target is not necessarily a valid Luau module path for require-by-string.
 
     See [`LuauFilePath`] and [`LuauModulePath::resolve`] for more information.
 */
@@ -121,6 +134,38 @@ pub struct LuauModulePath {
 }
 
 impl LuauModulePath {
+    /**
+        Strips Luau file extensions and potential init segments from a given path.
+
+        This is the opposite operation of [`LuauModulePath::resolve`] and is generally
+        useful for converting between paths in a CLI or other similar use cases - but
+        should *never* be used to implement `require` resolution.
+
+        Does not use any filesystem calls and will not panic.
+    */
+    #[must_use]
+    pub fn strip(path: impl Into<PathBuf>) -> PathBuf {
+        let mut path: PathBuf = path.into();
+
+        if path
+            .extension()
+            .and_then(|e| e.to_str())
+            .is_some_and(|e| FILE_EXTENSIONS.contains(&e))
+        {
+            path = path.with_extension("");
+        }
+
+        if path
+            .file_name()
+            .and_then(|e| e.to_str())
+            .is_some_and(|f| f == FILE_NAME_INIT)
+        {
+            path.pop();
+        }
+
+        path
+    }
+
     /**
         Resolves an existing file or directory path for the given *module* path.
 
@@ -146,30 +191,24 @@ impl LuauModulePath {
     }
 
     /**
-        Returns the source module path.
+        Returns the source Luau module path.
     */
     #[must_use]
     pub fn source(&self) -> &Path {
         &self.source
     }
 
     /**
-        Returns the target file path.
+        Returns the target filesystem file path.
     */
     #[must_use]
     pub fn target(&self) -> &LuauFilePath {
         &self.target
     }
 }
 
-impl AsRef<Path> for LuauModulePath {
-    fn as_ref(&self) -> &Path {
-        self.target.as_ref()
-    }
-}
-
 impl fmt::Display for LuauModulePath {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.as_ref().display().fmt(f)
+        self.source().display().fmt(f)
     }
 }
diff --git a/crates/lune/src/cli/utils/files.rs b/crates/lune/src/cli/utils/files.rs
@@ -1,127 +1,54 @@
-use std::{
-    fs::Metadata,
-    path::{MAIN_SEPARATOR, PathBuf},
-    sync::LazyLock,
-};
+use std::path::{Path, PathBuf};
 
-use anyhow::{Result, anyhow, bail};
+use anyhow::{Context, Result, anyhow};
 use console::style;
 use directories::UserDirs;
 
-const LUNE_COMMENT_PREFIX: &str = "-->";
+use lune_utils::path::{LuauFilePath, LuauModulePath, get_current_dir};
 
-static ERR_MESSAGE_HELP_NOTE: LazyLock<String> = LazyLock::new(|| {
-    format!(
-        "To run this file, either:\n{}\n{}",
-        format_args!(
-            "{} rename it to use a {} or {} extension",
-            style("-").dim(),
-            style(".luau").blue(),
-            style(".lua").blue()
-        ),
-        format_args!(
-            "{} pass it as an absolute path instead of relative",
-            style("-").dim()
-        ),
-    )
-});
+const LUNE_COMMENT_PREFIX: &str = "-->";
 
 /**
-    Discovers a script file path based on a given script name.
-
-    Script discovery is done in several steps here for the best possible user experience:
-
-    1. If we got a file that definitely exists, make sure it is either
-        - using an absolute path
-        - has the lua or luau extension
-    2. If we got a directory, check if it has an `init` file to use, and if it doesn't, let the user know
-    3. If we got an absolute path, don't check any extensions, just let the user know it didn't exist
-    4. If we got a relative path with no extension, also look for a file with a lua or luau extension
-    5. No other options left, the file simply did not exist
+    Discovers a script file path based on a given module path *or* file path.
 
-    This behavior ensures that users can do pretty much whatever they want if they pass in an absolute
-    path, and that they then have control over script discovery behavior, whereas if they pass in
-    a relative path we will instead try to be as permissive as possible for user-friendliness
+    See the documentation for [`LuauModulePath`] for more information about
+    what a module path vs a script path is.
 */
-pub fn discover_script_path(path: impl AsRef<str>, in_home_dir: bool) -> Result<PathBuf> {
-    // NOTE: We don't actually support any platforms without home directories,
-    // but just in case the user has some strange configuration and it cannot
-    // be found we should at least throw a nice error instead of panicking
-    let path = path.as_ref();
-    let file_path = if in_home_dir {
-        match UserDirs::new() {
-            Some(dirs) => dirs.home_dir().join(path),
-            None => {
-                bail!(
-                    "No file was found at {}\nThe home directory does not exist",
-                    style(path).yellow()
-                )
-            }
-        }
+pub fn discover_script_path(path: impl Into<PathBuf>, in_home_dir: bool) -> Result<PathBuf> {
+    // First, for legacy compatibility, we will strip any lua/luau file extension,
+    // and if the entire file stem is simply "init", we will get rid of that too
+    // This lets users pass "dir/init.luau" and have it resolve to simply "dir",
+    // which is a valid luau module path, while "dir/init.luau" is not
+    let path = LuauModulePath::strip(path);
+
+    // If we got an absolute path, we should not modify it,
+    // otherwise we should either resolve against home or cwd
+    let path = if path.is_absolute() {
+        path
+    } else if in_home_dir {
+        UserDirs::new()
+            .context("Missing home directory")?
+            .home_dir()
+            .join(path)
     } else {
-        PathBuf::from(path)
+        get_current_dir().join(path)
     };
-    // NOTE: We use metadata directly here to try to
-    // avoid accessing the file path more than once
-    let file_meta = file_path.metadata();
-    let is_file = file_meta.as_ref().is_ok_and(Metadata::is_file);
-    let is_dir = file_meta.as_ref().is_ok_and(Metadata::is_dir);
-    let is_abs = file_path.is_absolute();
-    let ext = file_path.extension();
-    if is_file {
-        if is_abs {
-            Ok(file_path)
-        } else if let Some(ext) = file_path.extension() {
-            match ext {
-                e if e == "lua" || e == "luau" => Ok(file_path),
-                _ => Err(anyhow!(
-                    "A file was found at {} but it uses the '{}' file extension\n{}",
-                    style(file_path.display()).green(),
-                    style(ext.to_string_lossy()).blue(),
-                    *ERR_MESSAGE_HELP_NOTE
-                )),
-            }
-        } else {
-            Err(anyhow!(
-                "A file was found at {} but it has no file extension\n{}",
-                style(file_path.display()).green(),
-                *ERR_MESSAGE_HELP_NOTE
-            ))
-        }
-    } else if is_dir {
-        match (
-            discover_script_path(format!("{path}/init.luau"), in_home_dir),
-            discover_script_path(format!("{path}/init.lua"), in_home_dir),
-        ) {
-            (Ok(path), _) | (_, Ok(path)) => Ok(path),
-            _ => Err(anyhow!(
-                "No file was found at {}, found a directory without an init file",
-                style(file_path.display()).yellow()
+
+    // The rest of the logic should follow Luau module path resolution rules
+    match LuauModulePath::resolve(&path) {
+        Err(e) => Err(anyhow!(
+            "Failed to resolve script at path {} ({})",
+            style(path.display()).yellow(),
+            style(format!("{e:?}")).red()
+        )),
+        Ok(m) => match m.target() {
+            LuauFilePath::File(f) => Ok(f.clone()),
+            LuauFilePath::Directory(_) => Err(anyhow!(
+                "Failed to resolve script at path {}\
+                \nThe path is a directory without an init file",
+                style(path.display()).yellow()
             )),
-        }
-    } else if is_abs && !in_home_dir {
-        Err(anyhow!(
-            "No file was found at {}",
-            style(file_path.display()).yellow()
-        ))
-    } else if ext.is_none() {
-        let file_path_lua = file_path.with_extension("lua");
-        let file_path_luau = file_path.with_extension("luau");
-        if file_path_lua.is_file() {
-            Ok(file_path_lua)
-        } else if file_path_luau.is_file() {
-            Ok(file_path_luau)
-        } else {
-            Err(anyhow!(
-                "No file was found at {}",
-                style(file_path.display()).yellow()
-            ))
-        }
-    } else {
-        Err(anyhow!(
-            "No file was found at {}",
-            style(file_path.display()).yellow()
-        ))
+        },
     }
 }
 
@@ -134,22 +61,24 @@ pub fn discover_script_path(path: impl AsRef<str>, in_home_dir: bool) -> Result<
 
     Behavior is otherwise exactly the same as for `discover_script_file_path`.
 */
-pub fn discover_script_path_including_lune_dirs(path: &str) -> Result<PathBuf> {
+pub fn discover_script_path_including_lune_dirs(path: impl AsRef<Path>) -> Result<PathBuf> {
+    let path: &Path = path.as_ref();
     match discover_script_path(path, false) {
         Ok(path) => Ok(path),
         Err(e) => {
             // If we got any absolute path it means the user has also
             // told us to not look in any special relative directories
             // so we should error right away with the first err message
-            if PathBuf::from(path).is_absolute() {
+            if path.is_absolute() {
                 return Err(e);
             }
+
             // Otherwise we take a look in relative lune and .lune
             // directories + the home directory for the current user
-            let res = discover_script_path(format!("lune{MAIN_SEPARATOR}{path}"), false)
-                .or_else(|_| discover_script_path(format!(".lune{MAIN_SEPARATOR}{path}"), false))
-                .or_else(|_| discover_script_path(format!("lune{MAIN_SEPARATOR}{path}"), true))
-                .or_else(|_| discover_script_path(format!(".lune{MAIN_SEPARATOR}{path}"), true));
+            let res = discover_script_path(Path::new("lune").join(path), false)
+                .or_else(|_| discover_script_path(Path::new(".lune").join(path), false))
+                .or_else(|_| discover_script_path(Path::new("lune").join(path), true))
+                .or_else(|_| discover_script_path(Path::new(".lune").join(path), true));
 
             match res {
                 // NOTE: The first error message is generally more
diff --git a/crates/lune/src/rt/runtime.rs b/crates/lune/src/rt/runtime.rs
@@ -10,7 +10,10 @@ use std::{
 };
 
 use async_fs as fs;
-use lune_utils::process::{ProcessArgs, ProcessEnv, ProcessJitEnablement};
+use lune_utils::{
+    path::{LuauModulePath, constants::FILE_CHUNK_PREFIX},
+    process::{ProcessArgs, ProcessEnv, ProcessJitEnablement},
+};
 use mlua::prelude::*;
 use mlua_luau_scheduler::{Functions, Scheduler};
 
@@ -257,7 +260,9 @@ impl Runtime {
     /**
         Runs some kind of custom input, inside of the current runtime.
 
-        For any input that is a real file path, [`run_file`] should be used instead.
+        For any input that is a real module or file path, [`run_file`] should
+        be used instead, since when using this method, any file requires will
+        fail. Requires for standard libraries and custom libraries will work.
 
         # Errors
 
@@ -275,7 +280,7 @@ impl Runtime {
     }
 
     /**
-        Runs a file at the given file path, inside of the current runtime.
+        Runs a file at the given file or module path, inside of the current runtime.
 
         # Errors
 
@@ -288,16 +293,40 @@ impl Runtime {
         &mut self,
         path: impl Into<PathBuf>,
     ) -> RuntimeResult<RuntimeReturnValues> {
-        let path: PathBuf = path.into();
-        let contents = fs::read(&path).await.into_lua_err().context(format!(
-            "Failed to read file at path \"{}\"",
-            path.display()
-        ))?;
-
-        // For calls to `require` to resolve properly, we must convert the file
-        // path to the respective "module" path according to require-by-string
-        let module_path = remove_lua_luau_ext(path);
-        let module_name = format!("@{}", module_path.display());
+        /*
+            For calls to `require` to resolve properly, we must:
+
+            1. Strip any lua/luau extensions, as well as "init" file
+               segments from the path.
+            2. Resolve any given file path to the respective "module"
+               path according to the require-by-string specification.
+
+            After doing this, we should end up with both:
+
+            - A source (module path)
+            - A target (file path)
+
+            If the given path was already a valid module path,
+            this should be a no-op.
+        */
+        let module_or_file_path = LuauModulePath::strip(path);
+        let module_path = LuauModulePath::resolve(&module_or_file_path)
+            .map_err(|e| LuaError::external(format!("{e:?}")))
+            .with_context(|_| {
+                format!(
+                    "Failed to read file at path \"{}\"",
+                    module_or_file_path.display()
+                )
+            })?;
+
+        let contents = fs::read(module_path.target())
+            .await
+            .into_lua_err()
+            .with_context(|_| {
+                format!("Failed to read file at path \"{}\"", module_path.target())
+            })?;
+
+        let module_name = format!("{FILE_CHUNK_PREFIX}{module_path}");
         let module_contents = strip_shebang(contents);
 
         self.run_inner(module_name, module_contents).await
@@ -366,14 +395,6 @@ impl Runtime {
     }
 }
 
-fn remove_lua_luau_ext(path: impl Into<PathBuf>) -> PathBuf {
-    let path: PathBuf = path.into();
-    match path.extension().and_then(|e| e.to_str()) {
-        Some("lua" | "luau") => path.with_extension(""),
-        _ => path,
-    }
-}
-
 fn strip_shebang(mut contents: Vec<u8>) -> Vec<u8> {
     if contents.starts_with(b"#!") {
         if let Some(first_newline_idx) = contents
