diff --git a/src/input.rs b/src/input.rs index bebfb05..7a56c4e 100644 --- a/src/input.rs +++ b/src/input.rs @@ -466,12 +466,13 @@ fn scan_virtual_files( &mut builder, )?, LockfileRoute::Manifest { parse, lock_names } => { - // The manifest's declared constraints are superseded by any - // sibling lockfile; the parser still receives the first - // present lockfile so it can skip redundant work. - let lock = lock_names - .iter() - .find_map(|name| files.get(&sibling(path, name))); + // The manifest's declared constraints are superseded by the + // nearest covering lockfile — a sibling, or one in an + // ancestor directory for workspace members whose lockfile + // lives at the workspace root (pnpm workspaces, npm/yarn + // workspaces). The parser still receives the first present + // lockfile so it can skip redundant work. + let lock = covering_lockfile(path, lock_names, &files); parse(path, bytes, lock, &mut builder)?; } } @@ -1021,6 +1022,28 @@ fn sibling(path: &str, name: &str) -> String { .map(|(parent, _)| format!("{parent}/{name}")) .unwrap_or_else(|| name.to_owned()) } + +/// Returns the nearest lockfile covering a manifest at `path`: its sibling +/// first, then ancestor directories outward, so a workspace member's +/// manifest is superseded by the workspace-root lockfile that resolved its +/// dependencies. +fn covering_lockfile<'a>( + path: &str, + lock_names: &[&'static str], + files: &'a BTreeMap>, +) -> Option<&'a Vec> { + let mut dir = path.rsplit_once('/').map(|(parent, _)| parent); + while let Some(current) = dir { + if let Some(lock) = lock_names + .iter() + .find_map(|name| files.get(&format!("{current}/{name}"))) + { + return Some(lock); + } + dir = current.rsplit_once('/').map(|(parent, _)| parent); + } + lock_names.iter().find_map(|name| files.get(*name)) +} fn base_name(path: &str) -> &str { path.rsplit('/').next().unwrap_or(path) } @@ -1129,7 +1152,7 @@ fn parse_package_json( /// Returns the concrete purl version for a specifier, stripping pnpm-style /// `1.2.3(integrity)` annotations, or `None` for empty values and range /// constraints (`^1.2`, `~1.2`, `>=1`, `1.*`, `a || b`, `git+https://…`). -fn concrete_version_specifier(version: &str) -> Option { +pub(crate) fn concrete_version_specifier(version: &str) -> Option { let trimmed = version.trim(); let concrete = trimmed.split('(').next().unwrap_or(trimmed).trim_end(); if concrete.is_empty() @@ -2279,6 +2302,62 @@ mod tests { assert!(inventory.components.values().any(|c| c.name == "a")); } + #[test] + fn workspace_manifest_deps_are_covered_by_ancestor_lockfile() { + // A workspace member's package.json has no sibling lockfile; the + // workspace-root pnpm-lock.yaml resolved its dependencies, so the + // manifest's declared constraints must not become specifier-versioned + // phantom components (#339). + let dir = tempdir().unwrap(); + fs::write( + dir.path().join("package.json"), + r#"{"name":"probe","private":true,"dependencies":{"is-odd":"^3.0.1"}}"#, + ) + .unwrap(); + let web = dir.path().join("apps/web"); + fs::create_dir_all(&web).unwrap(); + fs::write( + web.join("package.json"), + r#"{"name":"web","private":true,"dependencies":{"is-even":"^1.0.0"}}"#, + ) + .unwrap(); + fs::write( + dir.path().join("pnpm-lock.yaml"), + concat!( + "lockfileVersion: '9.0'\n", + "\n", + "importers:\n", + " .:\n", + " dependencies:\n", + " is-odd:\n", + " specifier: ^3.0.1\n", + " version: 3.0.1\n", + " apps/web:\n", + " dependencies:\n", + " is-even:\n", + " specifier: ^1.0.0\n", + " version: 1.0.0\n", + "\n", + "packages:\n", + "\n", + " is-odd@3.0.1:\n", + " resolution: {integrity: sha512-x}\n", + "\n", + " is-even@1.0.0:\n", + " resolution: {integrity: sha512-y}\n", + ), + ) + .unwrap(); + let inventory = scan_path(dir.path(), &config()).unwrap(); + let mut components: Vec<(&str, &str)> = inventory + .components + .values() + .map(|c| (c.name.as_str(), c.version.as_str())) + .collect(); + components.sort_unstable(); + assert_eq!(components, [("is-even", "1.0.0"), ("is-odd", "3.0.1")]); + } + #[test] fn previously_unbounded_parsers_reject_oversized_lockfiles() { let config = Config { diff --git a/src/parsers/pnpm.rs b/src/parsers/pnpm.rs index b146699..7db9a49 100644 --- a/src/parsers/pnpm.rs +++ b/src/parsers/pnpm.rs @@ -6,8 +6,8 @@ use serde_yaml::Value as Yaml; use super::npm::npm_scope; use super::{LockComponents, resolve_lock_component, split_descriptor}; use crate::input::{ - InputError, InventoryBuilder, entry_bound, malformed, malformed_msg, utf8, - yaml_expansion_within_budget, + InputError, InventoryBuilder, concrete_version_specifier, entry_bound, malformed, + malformed_msg, utf8, yaml_expansion_within_budget, }; use crate::model::Scope; @@ -88,7 +88,13 @@ pub(crate) fn parse_pnpm_lock( collect_pnpm_deps(&node, entry, &mut pending); } } - let package_names: BTreeSet = entries.keys().map(|(name, _)| name.clone()).collect(); + let mut package_versions: BTreeMap> = BTreeMap::new(); + for (name, version) in entries.keys() { + package_versions + .entry(name.clone()) + .or_default() + .insert(version.clone()); + } if let Some(importers) = importers { for importer in importers.values() { for (field, scope) in [ @@ -104,11 +110,26 @@ pub(crate) fn parse_pnpm_lock( let Some(node) = pnpm_resolved_parts(dep, spec) else { continue; }; - if !package_names.contains(node.0.as_str()) { + // pnpm 12 writes workspace-importer dependencies as bare + // specifier strings (`is-even: ^1.0.0`); a specifier is a + // constraint, not a resolved version, so it resolves + // against the locked versions of that package name. + let node = match concrete_version_specifier(&node.1) { + Some(version) if pnpm_concrete_version(&version) => node, + _ => match package_versions.get(node.0.as_str()) { + Some(versions) => (node.0, versions.iter().next().unwrap().to_string()), + None => continue, + }, + }; + if !package_versions.contains_key(node.0.as_str()) { // Importer-only dependency absent from `packages:`; // its scope comes from the importer field through the // reachability pass below. entries.insert(node.clone(), None); + package_versions + .entry(node.0.clone()) + .or_default() + .insert(node.1.clone()); } roots.push((node, scope)); } @@ -420,6 +441,16 @@ fn pnpm_valid_version(version: &str) -> bool { !version.contains('@') && !version.contains(':') && !version.contains('/') } +/// A concrete resolved version starts with a digit and has no `x`/`X` +/// wildcard segments; tags (`latest`) and partial ranges (`1.x`) are +/// specifiers that resolve against the lockfile's recorded versions. +fn pnpm_concrete_version(version: &str) -> bool { + version.chars().next().is_some_and(|c| c.is_ascii_digit()) + && !version + .split('.') + .any(|segment| segment.eq_ignore_ascii_case("x")) +} + #[cfg(test)] mod tests { use super::*; @@ -878,4 +909,100 @@ mod tests { Err(InputError::Malformed { .. }) )); } + + #[test] + fn pnpm_workspace_importer_specifiers_resolve_to_locked_versions() { + // pnpm 12 writes workspace-importer dependencies as bare specifier + // strings; they must resolve against `packages:` instead of becoming + // specifier-versioned phantom components (#339). + let dir = tempdir().unwrap(); + fs::write( + dir.path().join("pnpm-lock.yaml"), + concat!( + "---\n", + "lockfileVersion: '9.0'\n", + "\n", + "importers:\n", + " .:\n", + " dependencies:\n", + " is-odd:\n", + " specifier: ^3.0.1\n", + " version: 3.0.1\n", + " apps/web:\n", + " dependencies:\n", + " is-even: ^1.0.0\n", + " tagged: latest\n", + " ranged: 1.x\n", + " missing: ^9.9.9\n", + " devDependencies:\n", + " vitest: ^4.0.0\n", + "\n", + "---\n", + "lockfileVersion: '9.0'\n", + "\n", + "settings:\n", + " autoInstallPeers: true\n", + "\n", + "packages:\n", + "\n", + " is-odd@3.0.1:\n", + " resolution: {integrity: sha512-x}\n", + "\n", + " is-even@1.0.0:\n", + " resolution: {integrity: sha512-y}\n", + "\n", + " tagged@2.0.0:\n", + " resolution: {integrity: sha512-t}\n", + "\n", + " ranged@1.4.2:\n", + " resolution: {integrity: sha512-r}\n", + "\n", + " vitest@4.1.5:\n", + " resolution: {integrity: sha512-v}\n", + "\n", + "snapshots:\n", + "\n", + " is-odd@3.0.1: {}\n", + "\n", + " is-even@1.0.0: {}\n", + "\n", + " tagged@2.0.0: {}\n", + "\n", + " ranged@1.4.2: {}\n", + "\n", + " vitest@4.1.5: {}\n", + ), + ) + .unwrap(); + let inventory = scan_path(dir.path(), &config()).unwrap(); + let mut components: Vec<(&str, &str)> = inventory + .components + .values() + .map(|c| (c.name.as_str(), c.version.as_str())) + .collect(); + components.sort_unstable(); + // Only resolved versions appear: no specifier text becomes a version, + // and an importer specifier with no locked package yields nothing. + assert_eq!( + components, + [ + ("is-even", "1.0.0"), + ("is-odd", "3.0.1"), + ("ranged", "1.4.2"), + ("tagged", "2.0.0"), + ("vitest", "4.1.5"), + ] + ); + // Specifier-valued roots still classify their resolved package by the + // importer field that referenced them. + let scope_of = |name: &str| { + inventory + .components + .values() + .find(|c| c.name == name) + .map(|c| c.scope) + }; + assert_eq!(scope_of("vitest"), Some(Scope::Development)); + assert_eq!(scope_of("is-even"), Some(Scope::Runtime)); + } }