From 75cbbb090b193dd82c3f91de7895037ff3bf1b8b Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 8 Jul 2024 15:11:51 -0700 Subject: [PATCH 1/7] Allow mod path to be directory --- src/compiler.rs | 178 ++++++++++++++++++++++++++++++++++++++++------- src/error.rs | 4 +- tests/modules.rs | 17 +++++ 3 files changed, 170 insertions(+), 29 deletions(-) diff --git a/src/compiler.rs b/src/compiler.rs index a977be586c..3e98644e98 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -43,17 +43,12 @@ impl Compiler { } => { let parent = current.path.parent().unwrap(); - let import = if let Some(relative) = relative { - let path = parent.join(Self::expand_tilde(&relative.cooked)?); + let relative = relative + .as_ref() + .map(|relative| Self::expand_tilde(&relative.cooked)) + .transpose()?; - if path.is_file() { - Some(path) - } else { - None - } - } else { - Self::find_module_file(parent, *name)? - }; + let import = Self::find_module_file(parent, *name, relative.as_deref())?; if let Some(import) = import { if current.file_path.contains(&import) { @@ -111,19 +106,65 @@ impl Compiler { }) } - fn find_module_file<'src>(parent: &Path, module: Name<'src>) -> RunResult<'src, Option> { - let mut candidates = vec![format!("{module}.just"), format!("{module}/mod.just")] - .into_iter() - .filter(|path| parent.join(path).is_file()) - .collect::>(); + fn find_module_file<'src>( + parent: &Path, + module: Name<'src>, + path: Option<&Path>, + ) -> RunResult<'src, Option> { + let mut candidates = Vec::new(); + + if let Some(path) = path { + let full = parent.join(path); + + if full.is_file() { + return Ok(Some(full)); + } + + candidates.push((path.join("mod.just"), true)); + + for name in search::JUSTFILE_NAMES { + candidates.push((path.join(name), false)); + } + } else { + candidates.push((format!("{module}.just").into(), true)); + candidates.push((format!("{module}/mod.just").into(), true)); + + for name in search::JUSTFILE_NAMES { + candidates.push((format!("{module}/{name}").into(), false)); + } + } + + let mut grouped = BTreeMap::>::new(); + + for (candidate, case_sensitive) in candidates { + let full = parent.join(candidate).lexiclean(); + let dir = full.parent().unwrap(); - let directory = parent.join(module.lexeme()); + grouped + .entry(dir.into()) + .or_default() + .push((full, case_sensitive)); + } + + let mut found = Vec::new(); - if directory.exists() { - let entries = fs::read_dir(&directory).map_err(|io_error| SearchError::Io { - io_error, - directory: directory.clone(), - })?; + for (directory, candidates) in grouped { + let entries = match fs::read_dir(&directory) { + Ok(entries) => entries, + Err(io_error) => { + if io_error.kind() == io::ErrorKind::NotFound { + continue; + } + + return Err( + SearchError::Io { + io_error, + directory, + } + .into(), + ); + } + }; for entry in entries { let entry = entry.map_err(|io_error| SearchError::Io { @@ -132,20 +173,31 @@ impl Compiler { })?; if let Some(name) = entry.file_name().to_str() { - for justfile_name in search::JUSTFILE_NAMES { - if name.eq_ignore_ascii_case(justfile_name) { - candidates.push(format!("{module}/{name}")); + for (candidate, case_sensitive) in &candidates { + let candidate_name = candidate.file_name().unwrap().to_str().unwrap(); + + let eq = if *case_sensitive { + name == candidate_name + } else { + name.eq_ignore_ascii_case(candidate_name) + }; + + if eq { + found.push(candidate.parent().unwrap().join(name)); } } } } } - match candidates.as_slice() { + match found.as_slice() { [] => Ok(None), - [file] => Ok(Some(parent.join(file).lexiclean())), + [file] => Ok(Some(file.into())), found => Err(Error::AmbiguousModuleFile { - found: found.into(), + found: found + .iter() + .map(|found| found.strip_prefix(parent).unwrap().to_owned()) + .collect(), module, }), } @@ -242,4 +294,76 @@ recipe_b: recipe_c import == tmp.path().join("justfile").lexiclean() ); } + + #[test] + fn find_module_file() { + #[track_caller] + fn case(path: Option<&str>, files: &[&str], expected: Result, &[&str]>) { + let module = Name { + token: Token { + column: 0, + kind: TokenKind::Identifier, + length: 3, + line: 0, + offset: 0, + path: Path::new(""), + src: "foo", + }, + }; + + let tempdir = tempfile::tempdir().unwrap(); + + for file in files { + if let Some(parent) = Path::new(file).parent() { + fs::create_dir_all(tempdir.path().join(parent)).unwrap(); + } + + fs::write(tempdir.path().join(file), "").unwrap(); + } + + let actual = Compiler::find_module_file(tempdir.path(), module, path.map(Path::new)); + + match expected { + Err(expected) => match actual.unwrap_err() { + Error::AmbiguousModuleFile { found, .. } => { + assert_eq!( + found, + expected.iter().map(Into::into).collect::>() + ); + } + _ => panic!("unexpected error"), + }, + Ok(Some(expected)) => assert_eq!(actual.unwrap().unwrap(), tempdir.path().join(expected)), + Ok(None) => assert_eq!(actual.unwrap(), None), + } + } + + case(None, &["foo.just"], Ok(Some("foo.just"))); + case(None, &["FOO.just"], Ok(None)); + case(None, &["foo/mod.just"], Ok(Some("foo/mod.just"))); + case(None, &["foo/MOD.just"], Ok(None)); + case(None, &["foo/justfile"], Ok(Some("foo/justfile"))); + case(None, &["foo/JUSTFILE"], Ok(Some("foo/JUSTFILE"))); + case(None, &["foo/.justfile"], Ok(Some("foo/.justfile"))); + case(None, &["foo/.JUSTFILE"], Ok(Some("foo/.JUSTFILE"))); + case( + None, + &["foo/justfile", "foo/.justfile"], + Err(&["foo/justfile", "foo/.justfile"]), + ); + case(None, &["foo/JUSTFILE"], Ok(Some("foo/JUSTFILE"))); + + case(Some("bar"), &["bar"], Ok(Some("bar"))); + case(Some("bar"), &["bar/mod.just"], Ok(Some("bar/mod.just"))); + case(Some("bar"), &["bar/justfile"], Ok(Some("bar/justfile"))); + case(Some("bar"), &["bar/JUSTFILE"], Ok(Some("bar/JUSTFILE"))); + case(Some("bar"), &["bar/.justfile"], Ok(Some("bar/.justfile"))); + case(Some("bar"), &["bar/.JUSTFILE"], Ok(Some("bar/.JUSTFILE"))); + + case( + Some("bar"), + &["bar/justfile", "bar/mod.just"], + Err(&["bar/justfile", "bar/mod.just"]), + ); + } } diff --git a/src/error.rs b/src/error.rs index d2e8704f2c..563f21b060 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,7 +4,7 @@ use super::*; pub(crate) enum Error<'src> { AmbiguousModuleFile { module: Name<'src>, - found: Vec, + found: Vec, }, ArgumentCountMismatch { recipe: &'src str, @@ -262,7 +262,7 @@ impl<'src> ColorDisplay for Error<'src> { AmbiguousModuleFile { module, found } => write!(f, "Found multiple source files for module `{module}`: {}", - List::and_ticked(found), + List::and_ticked(found.iter().map(|path| path.display())), )?, ArgumentCountMismatch { recipe, found, min, max, .. } => { let count = Count("argument", *found); diff --git a/tests/modules.rs b/tests/modules.rs index 178394208f..2fcc11a603 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -564,6 +564,23 @@ fn modules_may_specify_path() { .run(); } +#[test] +fn modules_may_specify_path_to_directory() { + Test::new() + .write("commands/bar/mod.just", "foo:\n @echo FOO") + .justfile( + " + mod foo 'commands/bar' + ", + ) + .test_round_trip(false) + .arg("--unstable") + .arg("foo") + .arg("foo") + .stdout("FOO\n") + .run(); +} + #[test] fn modules_with_paths_are_dumped_correctly() { Test::new() From fe8ece7e2af496b16c8bceed4abe3ed40f01e5e4 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 8 Jul 2024 15:13:27 -0700 Subject: [PATCH 2/7] Adapt --- README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 1f5d6ad089..d8e1a0afa1 100644 --- a/README.md +++ b/README.md @@ -3204,7 +3204,10 @@ mod foo 'PATH' Which loads the module's source file from `PATH`, instead of from the usual locations. A leading `~/` in `PATH` is replaced with the current user's home -directory. +directory. `PATH` may point to the module source file itself, or to a directory +containing the module source file with the name `mod.just`, `justfile`, or +`.justfile`. In the latter two cases, the module file may have any +capitalization. Environment files are only loaded for the root justfile, and loaded environment variables are available in submodules. Settings in submodules that affect From f055462d3a1f39217d56ef13f935aa6b31666b17 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 8 Jul 2024 15:18:00 -0700 Subject: [PATCH 3/7] Amend --- src/compiler.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/compiler.rs b/src/compiler.rs index 3e98644e98..0e722e5993 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -137,13 +137,11 @@ impl Compiler { let mut grouped = BTreeMap::>::new(); for (candidate, case_sensitive) in candidates { - let full = parent.join(candidate).lexiclean(); - let dir = full.parent().unwrap(); - + let candidate = parent.join(candidate).lexiclean(); grouped - .entry(dir.into()) + .entry(candidate.parent().unwrap().into()) .or_default() - .push((full, case_sensitive)); + .push((candidate, case_sensitive)); } let mut found = Vec::new(); @@ -190,16 +188,16 @@ impl Compiler { } } - match found.as_slice() { - [] => Ok(None), - [file] => Ok(Some(file.into())), - found => Err(Error::AmbiguousModuleFile { + if found.len() > 1 { + Err(Error::AmbiguousModuleFile { found: found - .iter() - .map(|found| found.strip_prefix(parent).unwrap().to_owned()) + .into_iter() + .map(|found| found.strip_prefix(parent).unwrap().into()) .collect(), module, - }), + }) + } else { + Ok(found.into_iter().next()) } } From d326183b98963c3859ceb57746065ba60ee12032 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 8 Jul 2024 15:25:44 -0700 Subject: [PATCH 4/7] Adjust --- src/compiler.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/compiler.rs b/src/compiler.rs index 0e722e5993..bd054ddaef 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -326,12 +326,20 @@ recipe_b: recipe_c Error::AmbiguousModuleFile { found, .. } => { assert_eq!( found, - expected.iter().map(Into::into).collect::>() + expected + .iter() + .map(|expected| expected.replace('/', std::path::MAIN_SEPARATOR_STR).into()) + .collect::>() ); } _ => panic!("unexpected error"), }, - Ok(Some(expected)) => assert_eq!(actual.unwrap().unwrap(), tempdir.path().join(expected)), + Ok(Some(expected)) => assert_eq!( + actual.unwrap().unwrap(), + tempdir + .path() + .join(expected.replace('/', std::path::MAIN_SEPARATOR_STR)) + ), Ok(None) => assert_eq!(actual.unwrap(), None), } } From 49b0f8151fc0f25de329a23eec0e1f4b3fc6ced5 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 8 Jul 2024 15:31:41 -0700 Subject: [PATCH 5/7] Revise --- src/compiler.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/compiler.rs b/src/compiler.rs index bd054ddaef..119a0af55e 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -189,6 +189,7 @@ impl Compiler { } if found.len() > 1 { + found.sort(); Err(Error::AmbiguousModuleFile { found: found .into_iter() @@ -354,8 +355,8 @@ recipe_b: recipe_c case(None, &["foo/.JUSTFILE"], Ok(Some("foo/.JUSTFILE"))); case( None, - &["foo/justfile", "foo/.justfile"], - Err(&["foo/justfile", "foo/.justfile"]), + &["foo/.justfile", "foo/justfile"], + Err(&["foo/.justfile", "foo/justfile"]), ); case(None, &["foo/JUSTFILE"], Ok(Some("foo/JUSTFILE"))); From f494354f46b0ef2bf792a551c4f6dcc321d112d8 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 8 Jul 2024 15:33:24 -0700 Subject: [PATCH 6/7] Revise --- tests/modules.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/modules.rs b/tests/modules.rs index 2fcc11a603..4a8ffe6a77 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -439,7 +439,7 @@ fn modules_require_unambiguous_file() { .status(EXIT_FAILURE) .stderr( " - error: Found multiple source files for module `foo`: `foo.just` and `foo/justfile` + error: Found multiple source files for module `foo`: `foo/justfile` and `foo.just` ——▶ justfile:1:5 │ 1 │ mod foo From 1d637e93cb4ea3001e62f9255fabad936b88ea8d Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 8 Jul 2024 15:36:23 -0700 Subject: [PATCH 7/7] Modify --- tests/lib.rs | 2 +- tests/modules.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/lib.rs b/tests/lib.rs index 932a6c34d6..072574bdf8 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -19,7 +19,7 @@ pub(crate) use { fs, io::Write, iter, - path::{Path, PathBuf, MAIN_SEPARATOR}, + path::{Path, PathBuf, MAIN_SEPARATOR, MAIN_SEPARATOR_STR}, process::{Command, Stdio}, str, }, diff --git a/tests/modules.rs b/tests/modules.rs index 4a8ffe6a77..46a06bcaa4 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -444,7 +444,8 @@ fn modules_require_unambiguous_file() { │ 1 │ mod foo │ ^^^ - ", + " + .replace('/', MAIN_SEPARATOR_STR), ) .run(); }