fix(workspace): attribute files to nearest package.json, not glob-matched intermediate dirs

Fixes #842

Author: BartWaardenburg

crates/core/src/analyze/unused_deps.rs

@@ -647,28 +647,90 @@ pub fn find_test_only_dependencies(
 }
 
 /// Check whether a package is listed in root deps or in the workspace that owns `file_path`.
+///
+/// The owning workspace is the nearest ancestor directory tracked in `ws_dep_map`.
+/// When a file lives inside a tracked workspace but also inside a nested directory
+/// that has its own `package.json` (e.g. `packages/themes/my-theme/package.json`
+/// discovered outside the glob-matched workspace set), that nested manifest is
+/// probed first. This prevents the intermediate glob-matched workspace
+/// (`packages/themes`) from masking the actual owning package's declared deps.
 pub fn is_package_listed_for_file(
     file_path: &Path,
     package_name: &str,
     root_deps: &FxHashSet<String>,
     ws_dep_map: &[(PathBuf, FxHashSet<String>)],
 ) -> bool {
-    if let Some(ws_deps) = owning_workspace_deps(file_path, ws_dep_map) {
-        return ws_deps.contains(package_name);
+    if let Some(ws_root) = owning_workspace_root(file_path, ws_dep_map) {
+        // Walk from the file's parent up to (but not past) the matched workspace root.
+        // If a closer package.json exists, use its deps for the check so that nested
+        // packages not declared as workspace entries are still attributed correctly.
+        if let Some(deps) = nearest_undeclared_package_json_deps(file_path, ws_root, ws_dep_map) {
+            return deps.contains(package_name);
+        }
+
+        // Fall back to the matched workspace's declared dep set.
+        if let Some(ws_deps) = ws_dep_map
+            .iter()
+            .find(|(r, _)| r == ws_root)
+            .map(|(_, d)| d)
+        {
+            return ws_deps.contains(package_name);
+        }
     }
 
     root_deps.contains(package_name)
 }
 
-fn owning_workspace_deps<'a>(
+/// Return the root path of the deepest tracked workspace that is an ancestor of `file_path`.
+fn owning_workspace_root<'a>(
     file_path: &Path,
     ws_dep_map: &'a [(PathBuf, FxHashSet<String>)],
-) -> Option<&'a FxHashSet<String>> {
+) -> Option<&'a PathBuf> {
     ws_dep_map
         .iter()
         .filter(|(ws_root, _)| file_path.starts_with(ws_root))
         .max_by_key(|(ws_root, _)| ws_root.components().count())
-        .map(|(_, ws_deps)| ws_deps)
+        .map(|(ws_root, _)| ws_root)
+}
+
+/// Walk ancestor directories of `file_path` from the file's parent down to
+/// (but not past) `workspace_root`, looking for a `package.json` that is NOT
+/// already tracked in `ws_dep_map`.
+///
+/// Returns the loaded dependency set when such a closer manifest is found.
+/// This handles the case where a glob like `./packages/*` discovers
+/// `packages/themes` as a workspace, but `packages/themes/my-theme` has its
+/// own `package.json` that was not expanded by the glob. The nested manifest
+/// is the actual owning package for any file beneath it.
+fn nearest_undeclared_package_json_deps(
+    file_path: &Path,
+    workspace_root: &Path,
+    ws_dep_map: &[(PathBuf, FxHashSet<String>)],
+) -> Option<FxHashSet<String>> {
+    let file_dir = file_path.parent()?;
+    for ancestor in file_dir.ancestors() {
+        // Stop once we reach or pass the matched workspace root.
+        if ancestor == workspace_root {
+            break;
+        }
+        if !ancestor.starts_with(workspace_root) {
+            break;
+        }
+        // Skip directories that are already tracked in ws_dep_map; those are
+        // handled by the normal max-depth lookup.
+        if ws_dep_map.iter().any(|(r, _)| r == ancestor) {
+            break;
+        }
+        let pkg_path = ancestor.join("package.json");
+        if let Ok(pkg) = PackageJson::load(&pkg_path) {
+            let mut deps: FxHashSet<String> = pkg.all_dependency_names().into_iter().collect();
+            if let Some(name) = pkg.name {
+                deps.insert(name);
+            }
+            return Some(deps);
+        }
+    }
+    None
 }
 
 /// Check if a corresponding `@types/<package>` is listed in dependencies.

crates/core/src/analyze/unused_deps_tests/unlisted_deps.rs

@@ -931,3 +931,79 @@ fn workspace_import_case(
         workspaces,
     }
 }
+
+/// Regression test for issue #842.
+///
+/// Fixture layout:
+///   root/
+///     package.json                          (workspaces: `["./packages/*"]`)
+///     packages/
+///       themes/
+///         package.json                      (name: "themes", no dep on "chroma-js")
+///         my-theme/
+///           package.json                    (name: "@scope/my-theme", dep: "chroma-js")
+///           src/
+///             index.ts                      <-- file being checked
+///
+/// The glob `./packages/*` discovers `packages/themes` as a workspace but does NOT
+/// discover `packages/themes/my-theme` (one level too deep). So `ws_dep_map` has
+/// `packages/themes` but not `packages/themes/my-theme`.
+///
+/// Before the fix, `is_package_listed_for_file` attributed `packages/themes/my-theme/src/index.ts`
+/// to `packages/themes` and reported "chroma-js" as an unlisted dependency because it is not
+/// in `themes/package.json`.
+///
+/// After the fix, the function walks up from the file and finds
+/// `packages/themes/my-theme/package.json`, uses its deps, and correctly reports
+/// "chroma-js" as listed.
+#[cfg_attr(miri, ignore)]
+#[test]
+fn is_package_listed_attributes_file_to_nearest_package_json_not_glob_matched_intermediate() {
+    let tmp = tempfile::tempdir().expect("create temp dir");
+    let root = tmp.path();
+
+    // packages/themes/package.json (intermediate, glob-matched workspace, no chroma-js dep)
+    let themes_dir = root.join("packages").join("themes");
+    std::fs::create_dir_all(&themes_dir).expect("create themes dir");
+    std::fs::write(
+        themes_dir.join("package.json"),
+        r#"{"name": "themes", "dependencies": {}}"#,
+    )
+    .expect("write themes package.json");
+
+    // packages/themes/my-theme/package.json (real nested package, declares chroma-js)
+    let my_theme_dir = themes_dir.join("my-theme");
+    std::fs::create_dir_all(my_theme_dir.join("src")).expect("create my-theme/src dir");
+    std::fs::write(
+        my_theme_dir.join("package.json"),
+        r#"{"name": "@scope/my-theme", "dependencies": {"chroma-js": "^2.0.0"}}"#,
+    )
+    .expect("write my-theme package.json");
+
+    // The source file that imports chroma-js.
+    let source_file = my_theme_dir.join("src").join("index.ts");
+    std::fs::write(&source_file, "import chroma from 'chroma-js';").expect("write source file");
+
+    // ws_dep_map mirrors what find_unlisted_dependencies builds when the glob
+    // `./packages/*` discovers `packages/themes` but not `packages/themes/my-theme`.
+    let mut themes_deps: FxHashSet<String> = FxHashSet::default();
+    themes_deps.insert("themes".to_string()); // no chroma-js
+    let ws_dep_map: Vec<(PathBuf, FxHashSet<String>)> = vec![(themes_dir, themes_deps)];
+
+    let root_deps: FxHashSet<String> = FxHashSet::default();
+
+    // chroma-js is declared in my-theme/package.json but NOT in themes/package.json.
+    // Without the fix, this would return false (false unlisted-dependency).
+    // With the fix, it should return true.
+    assert!(
+        is_package_listed_for_file(&source_file, "chroma-js", &root_deps, &ws_dep_map),
+        "dep declared in a nested package.json below the glob-matched intermediate workspace \
+         must be found; file should be attributed to the nearest ancestor package.json"
+    );
+
+    // A dep that is truly unlisted (not in any package.json) should still be flagged.
+    assert!(
+        !is_package_listed_for_file(&source_file, "lodash", &root_deps, &ws_dep_map),
+        "dep not declared in any ancestor package.json should still be reported as unlisted"
+    );
+}