feat: required private field for yarn workspaces

nokazn · Feb 14, 2024 · c968355 · c968355
1 parent ae7a91a
commit c968355
Show file tree

Hide file tree

Showing 8 changed files with 94 additions and 27 deletions.
diff --git a/src/errors.rs b/src/errors.rs
@@ -1,27 +1,36 @@
-use glob::PatternError;
 use std::{fmt::Debug, path::PathBuf};
 use thiserror::Error;
 
 use crate::utils::path::to_absolute_path;
 
-#[derive(Debug, Error)]
+#[derive(Debug, Error, PartialEq)]
 pub enum Error {
   #[error("Cannot access to a file or a directory: `{}`", stringify_path(&Paths::One(.0.to_path_buf())))]
   NotAccessibleError(PathBuf),
+
   #[error("No such a file or a directory: `{}`", stringify_path(.0))]
   NoEntryError(Paths),
+
   #[error("No lockfile at: `{}`", stringify_path(&Paths::One(.0.to_path_buf())))]
   NoLockfileError(PathBuf),
+
   #[error("Invalid workspace: `{}`", stringify_path(&Paths::One(.0.to_path_buf())))]
   InvalidWorkspaceError(PathBuf),
-  #[error("\"name\" and \"version\" are missing in: `{}`", stringify_path(&Paths::One(.0.to_path_buf())))]
+
+  #[error("\"name\" or \"version\" are missing in: `{}`", stringify_path(&Paths::One(.0.to_path_buf())))]
   InvalidPackageJsonFieldsForYarnError(PathBuf),
+
+  #[error("\"private\" should be set to `true`: `{}`", stringify_path(&Paths::One(.0.to_path_buf())))]
+  InvalidPackageJsonPrivateForYarnError(PathBuf),
+
   #[error("\"name\" is missing in: `{}`", stringify_path(&Paths::One(.0.to_path_buf())))]
   InvalidPackageJsonFieldsForBunError(PathBuf),
+
   #[error("Failed to parse: `{}`", stringify_path(.0))]
   ParseError(Paths),
+
   #[error("Invalid glob pattern: {:?}", .0)]
-  InvalidGlobPatternError(PatternError),
+  InvalidGlobPatternError(&'static str),
 }
 
 impl Error {
@@ -50,12 +59,13 @@ impl Error {
   }
 }
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 pub enum Paths {
   One(PathBuf),
   Multiple(Vec<PathBuf>),
 }
 
+/// convert to stringified absolute path
 fn stringify_path(paths: &Paths) -> String {
   match paths {
     Paths::One(path) => to_absolute_path(path)

diff --git a/src/package_json.rs b/src/package_json.rs
@@ -185,12 +185,15 @@ impl Hashable for ProjectRoot {
 impl ProjectRoot {
   pub fn new<T: AsRef<Path>>(base_dir: T, kind: PackageManagerKind) -> Result<Self, Error> {
     let original = PackageJson::new(&base_dir)?;
-    Ok(Self {
-      original: original.clone(),
-      kind,
-      root: PackageDependencies::new(original.clone()),
-      workspaces: Self::resolve_workspaces(base_dir, kind, original.workspaces),
-    })
+    Ok(
+      Self {
+        original: original.clone(),
+        kind,
+        root: PackageDependencies::new(original.clone()),
+        workspaces: Self::resolve_workspaces(&base_dir, kind, original.workspaces),
+      }
+      .validate_package_json_fields(&base_dir),
+    )?
   }
 
   fn resolve_workspaces<T: AsRef<Path>>(
@@ -215,6 +218,22 @@ impl ProjectRoot {
     workspace_map
   }
 
+  fn validate_package_json_fields<T: AsRef<Path>>(self, base_dir: T) -> Result<Self, Error> {
+    match self.kind {
+      PackageManagerKind::Yarn
+        if self.original.workspaces.clone().unwrap_or_default().len() > 0
+          && !self.original.private.unwrap_or_default() =>
+      {
+        Err(
+          Error::InvalidPackageJsonPrivateForYarnError(to_package_json_path(&base_dir))
+            .log_error(None),
+        )
+      }
+      _ => Ok(self),
+    }
+  }
+}
+
 #[cfg(test)]
 mod tests {
   use tempfile::TempDir;
@@ -294,16 +313,21 @@ mod tests {
 
   struct NewTestCase {
     input: (PathBuf, PackageManagerKind),
-    expected: ProjectRoot,
+    expected: Result<ProjectRoot, Error>,
   }
 
   fn test_new_each(case: NewTestCase) {
     let base_dir = case.input.0;
-    let project_root = ProjectRoot::new(base_dir.clone(), case.input.1).unwrap();
-    assert_eq!(project_root.original, case.expected.original);
-    assert_eq!(project_root.kind, case.expected.kind);
-    assert_eq!(project_root.root, case.expected.root);
-    assert_eq!(project_root.workspaces, case.expected.workspaces);
+    let project_root = ProjectRoot::new(base_dir.clone(), case.input.1);
+    if let Ok(expected) = case.expected {
+      let project_root = project_root.unwrap();
+      assert_eq!(project_root.original, expected.original);
+      assert_eq!(project_root.kind, expected.kind);
+      assert_eq!(project_root.root, expected.root);
+      assert_eq!(project_root.workspaces, expected.workspaces);
+    } else {
+      assert_eq!(project_root.unwrap_err(), case.expected.unwrap_err());
+    }
   }
 
   test_each_serial!(
@@ -318,7 +342,7 @@ mod tests {
         let dev_dependencies = btree_map!(
           String::from("typescript") => String::from("^5.3.3"),
         );
-        ProjectRoot {
+        Ok(ProjectRoot {
           original: PackageJson {
             workspaces: Some(vec![
               String::from("packages/*"),
@@ -338,7 +362,7 @@ mod tests {
               ..Default::default()
             },
           ),
-        }
+        })
       }
     },
     "yarn" => NewTestCase {
@@ -350,7 +374,7 @@ mod tests {
         let dev_dependencies = btree_map!(
           String::from("typescript") => String::from("^5.3.3"),
         );
-        ProjectRoot {
+        Ok(ProjectRoot {
           original: PackageJson {
             workspaces: Some(vec![String::from("packages/*"), String::from("!packages/c")]),
             private: Some(true),
@@ -384,9 +408,16 @@ mod tests {
               ..Default::default()
             },
           ),
-        }
+        })
       }
     },
+    "yarn_private_false" => NewTestCase {
+      input: (
+        PathBuf::from("tests/fixtures/workspaces/yarn_private_false"),
+        PackageManagerKind::Yarn,
+      ),
+      expected:Err(Error::InvalidPackageJsonPrivateForYarnError(PathBuf::from("tests/fixtures/workspaces/yarn_private_false/package.json")))
+    },
     "pnpm" => NewTestCase {
       input: (
         PathBuf::from("tests/fixtures/workspaces/pnpm"),
@@ -396,7 +427,7 @@ mod tests {
         let dev_dependencies = btree_map!(
           String::from("typescript") => String::from("^5.3.3"),
         );
-        ProjectRoot {
+        Ok(ProjectRoot {
           original: PackageJson {
             devDependencies: Some(dev_dependencies.clone()),
             ..Default::default()
@@ -416,7 +447,7 @@ mod tests {
               ..Default::default()
             },
           ),
-        }
+        })
       }
     },
     "bun" => NewTestCase {
@@ -428,7 +459,7 @@ mod tests {
         let dev_dependencies = btree_map!(
           String::from("typescript") => String::from("^5.3.3"),
         );
-        ProjectRoot {
+        Ok(ProjectRoot {
           original: PackageJson {
             workspaces: Some(vec![String::from("packages/*")]),
             devDependencies: Some(dev_dependencies.clone()),
@@ -459,7 +490,7 @@ mod tests {
               ..Default::default()
             },
           ),
-        }
+        })
       }
     },
   );

diff --git a/src/utils/glob.rs b/src/utils/glob.rs
@@ -2,7 +2,7 @@ use glob::glob;
 use itertools::Itertools;
 use std::path::PathBuf;
 
-use crate::errors::{Error, Paths};
+use crate::errors::Error;
 use crate::utils::path::{run_in_base_dir, to_absolute_path};
 
 const NEGATE: char = '!';
@@ -61,7 +61,7 @@ fn resolve_glob(pattern: String, enable_negate: bool) -> (Option<Vec<PathBuf>>,
       (Some(entries), negate)
     }
     Err(error) => {
-      Error::InvalidGlobPatternError(error).log_warn(None);
+      Error::InvalidGlobPatternError(error.msg).log_warn(None);
       (None, negate)
     }
   }

diff --git a/tests/fixtures/workspaces/yarn_private_false/package.json b/tests/fixtures/workspaces/yarn_private_false/package.json
@@ -0,0 +1,10 @@
+{
+  "workspaces": [
+    "packages/*",
+    "!packages/c"
+  ],
+  "private": false,
+  "devDependencies": {
+    "typescript": "^5.3.3"
+  }
+}
diff --git a/tests/fixtures/workspaces/yarn_private_false/packages/a/package.json b/tests/fixtures/workspaces/yarn_private_false/packages/a/package.json
@@ -0,0 +1,4 @@
+{
+  "name": "@yarn/a",
+  "version": "0.1.0"
+}
diff --git a/tests/fixtures/workspaces/yarn_private_false/packages/b/.gitkeep b/tests/fixtures/workspaces/yarn_private_false/packages/b/.gitkeep
diff --git a/tests/fixtures/workspaces/yarn_private_false/packages/c/package.json b/tests/fixtures/workspaces/yarn_private_false/packages/c/package.json
@@ -0,0 +1,4 @@
+{
+  "name": "@yarn/c",
+  "version": "0.0.0"
+}
diff --git a/tests/fixtures/workspaces/yarn_private_false/yarn.lock b/tests/fixtures/workspaces/yarn_private_false/yarn.lock
@@ -0,0 +1,8 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+
+
+typescript@^5.3.3:
+  version "5.3.3"
+  resolved "https://registry.yarnpkg.com/typescript/-/typescript-5.3.3.tgz#b3ce6ba258e72e6305ba66f5c9b452aaee3ffe37"
+  integrity sha512-pXWcraxM0uxAS+tN0AG/BF2TyqmHO014Z070UsJ+pFvYuRSq8KH8DmWpnbXe0pEPDHXZV3FcAbJkijJ5oNEnWw==