From f453728910d12e5130967a5091a2a4289f6cfc2e Mon Sep 17 00:00:00 2001 From: Aria Beingessner Date: Mon, 27 Mar 2023 11:52:08 -0400 Subject: [PATCH] normalize JS paths, implement proper tie-breaker logic, and add some minimal tests --- src/javascript.rs | 15 +++++++++++++ src/lib.rs | 30 ++++++++++++++++++++------ src/tests.rs | 55 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 7 deletions(-) create mode 100644 src/tests.rs diff --git a/src/javascript.rs b/src/javascript.rs index 981acc5..6ab2f8c 100644 --- a/src/javascript.rs +++ b/src/javascript.rs @@ -120,6 +120,21 @@ pub fn get_project(start_dir: &Utf8Path) -> Result { /// Find the workspace root given this starting dir (potentially walking up to ancestor dirs) fn workspace_root(start_dir: &Utf8Path) -> Result { + // We want a proper absolute path so we can compare paths to workspace roots easily. + // + // Also if someone starts the path with ./ we should trim that to avoid weirdness. + // Maybe we should be using proper `canonicalize` but then we'd need to canonicalize + // every path we get from random APIs to be consistent. + let start_dir = if let Ok(clean_dir) = start_dir.strip_prefix("./") { + clean_dir.to_owned() + } else { + start_dir.to_owned() + }; + let start_dir = if start_dir.is_relative() { + Utf8PathBuf::from_path_buf(std::env::current_dir().unwrap().join(start_dir)).unwrap() + } else { + start_dir + }; for path in start_dir.ancestors() { // NOTE: orogene also looks for node_modules, but we can't do anything if there's // no package.json, so we can just ignore that approach? diff --git a/src/lib.rs b/src/lib.rs index dc0c224..36c2a65 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,8 @@ pub type Result = std::result::Result; pub mod javascript; #[cfg(feature = "cargo-projects")] pub mod rust; +#[cfg(test)] +mod tests; /// Kind of workspace #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -252,16 +254,30 @@ pub struct AutoIncludes { /// Concepts of both will largely be conflated, the only distinction will be /// the top level [`WorkspaceKind`][]. pub fn get_project(start_dir: &Utf8Path) -> Option { - // FIXME: should we provide and feedback/logging here? + #![allow(clippy::vec_init_then_push)] + + let mut best_project = None; + let mut max_depth = 0; + let mut projects = vec![]; + + // FIXME: should we provide feedback/logging here? #[cfg(feature = "cargo-projects")] - if let Ok(project) = rust::get_project(start_dir) { - return Some(project); - } + projects.push(rust::get_project(start_dir)); + #[cfg(feature = "npm-projects")] - if let Ok(project) = javascript::get_project(start_dir) { - return Some(project); + projects.push(javascript::get_project(start_dir)); + + // If we find multiple projects, prefer the one deeper in the file system + // (the one closer to the start_dir). + for project in projects.into_iter().flatten() { + let depth = project.workspace_dir.ancestors().count(); + if depth > max_depth { + best_project = Some(project); + max_depth = depth; + } } - None + + best_project } /// Find auto-includeable files in a dir diff --git a/src/tests.rs b/src/tests.rs new file mode 100644 index 0000000..a96ea4f --- /dev/null +++ b/src/tests.rs @@ -0,0 +1,55 @@ +use crate::WorkspaceKind; + +#[cfg(feature = "cargo-projects")] +#[test] +fn test_self_detect() { + let project = crate::get_project("./".into()).unwrap(); + assert_eq!(project.kind, WorkspaceKind::Rust); + assert_eq!(project.package_info.len(), 1); + + let package = &project.package_info[0]; + assert_eq!(package.name, "axo-project"); + assert_eq!(package.binaries.len(), 1); + + let binary = &package.binaries[0]; + assert_eq!(binary, "axo-project"); +} + +#[cfg(feature = "cargo-projects")] +#[test] +fn test_cargo_new() { + let project = crate::get_project("tests/projects/cargo-new/src/".into()).unwrap(); + assert_eq!(project.kind, WorkspaceKind::Rust); + assert_eq!(project.package_info.len(), 1); + + let package = &project.package_info[0]; + assert_eq!(package.name, "cargo-new"); + assert_eq!(package.binaries.len(), 1); + + let binary = &package.binaries[0]; + assert_eq!(binary, "cargo-new"); +} + +#[cfg(feature = "npm-projects")] +#[test] +fn test_npm_init_legacy() { + let project = crate::get_project("tests/projects/npm-init-legacy".into()).unwrap(); + assert_eq!(project.kind, WorkspaceKind::Javascript); + assert_eq!(project.package_info.len(), 1); + + let package = &project.package_info[0]; + assert_eq!(package.name, "npm-init-legacy"); + assert_eq!(package.binaries.len(), 0); +} + +#[cfg(feature = "npm-projects")] +#[test] +fn test_npm_create_react_app() { + let project = crate::get_project("tests/projects/npm-create-react-app/src/".into()).unwrap(); + assert_eq!(project.kind, WorkspaceKind::Javascript); + assert_eq!(project.package_info.len(), 1); + + let package = &project.package_info[0]; + assert_eq!(package.name, "npm-create-react-app"); + assert_eq!(package.binaries.len(), 0); +}