feat: detect phantom dependencies in executable code, not only in types

README.md

@@ -95,12 +95,26 @@ Findings come in two kinds:
   often a bundling artifact or an optional integration, so these deserve a look
   before you act on them.
 
+Three things reliably produce findings that are true of a file but not of the
+package: bundled output that still names the modules it inlined, generator
+templates describing what a *generated* project needs, and tests that ship in
+the tarball. Reading the origin and the severity together usually separates
+them.
+
 ## What it looks at
 
-Shipped `.d.ts`, `.d.mts` and `.d.cts` files, parsed with
+Every file a package ships as behaviour or as types — `.js`, `.mjs`, `.cjs`,
+`.jsx`, `.ts`, `.mts`, `.cts`, `.tsx` and the `.d.ts` family — parsed with
 [oxc](https://oxc.rs). Every position that names a module counts: `import` and
-`export … from`, `export *`, dynamic `import()`, type-position `import("pkg")`,
-`import x = require("pkg")`, and `/// <reference types="pkg" />`.
+`export … from`, `export *`, `require()` and `require.resolve()`, dynamic
+`import()`, type-position `import("pkg")`, `import x = require("pkg")`, and
+`/// <reference types="pkg" />`.
+
+Findings say where the reference was found. A dependency reached from executable
+code breaks the program when it is missing. One reached only from declarations
+breaks type checking instead — which is why no runtime detector can see it, and
+why Yarn's Plug'n'Play, which finds undeclared dependencies by failing at
+runtime, has no entry for most of them.
 
 Parsing rather than pattern matching is what keeps the results honest — a
 specifier mentioned in a JSDoc example is not a dependency, and a scanner built

src/analyze.rs

@@ -5,28 +5,32 @@ use crate::{
     classify::missing_package,
     manifest::Manifest,
     report::{Finding, PackageReport, Severity},
-    scan,
+    scan::{self, Origin},
 };
 use anyhow::Result;
 use std::{
-    collections::{BTreeSet, HashSet},
+    collections::{BTreeMap, BTreeSet, HashSet},
     path::{Path, PathBuf},
 };
 use walkdir::WalkDir;
 
+/// Extensions worth reading: what the package ships as behaviour, and what it
+/// ships as types.
+const SCANNED_EXTENSIONS: &[&str] = &[".js", ".mjs", ".cjs", ".jsx", ".ts", ".mts", ".cts", ".tsx"];
+
 /// What one installed package reaches for but never declared, or `None` when it
-/// declared everything its declaration files name.
+/// declared everything its shipped files name.
 pub fn analyze(package_dir: &Path) -> Result<Option<PackageReport>> {
     let manifest = Manifest::read(package_dir)?;
     let declared: HashSet<&str> = manifest.reachable().collect();
 
-    let mut referenced = BTreeSet::new();
+    let mut referenced: BTreeSet<(scan::Requirement, Origin)> = BTreeSet::new();
     let mut unparsed = Vec::new();
-    for file in declaration_files(package_dir) {
+    for file in scannable_files(package_dir) {
         let Ok(source) = std::fs::read_to_string(&file) else {
             continue;
         };
-        match scan::specifiers(&source) {
+        match scan::specifiers(&source, &file) {
             Ok(found) => referenced.extend(found),
             Err(reason) => unparsed.push((file, reason)),
         }
@@ -35,19 +39,27 @@ pub fn analyze(package_dir: &Path) -> Result<Option<PackageReport>> {
         eprintln!("warning: could not parse {}: {reason}", file.display());
     }
 
-    let findings: Vec<Finding> = referenced
-        .iter()
-        .filter_map(|requirement| missing_package(requirement, &declared))
-        .collect::<BTreeSet<_>>()
+    // Classified per reference rather than per name: the same package can be
+    // satisfied in a type position by `@types/` and still be missing at run
+    // time, and it is the run-time reference that has to win.
+    let mut missing: BTreeMap<String, Origin> = BTreeMap::new();
+    for (requirement, origin) in &referenced {
+        if let Some(name) = missing_package(requirement, *origin, &declared) {
+            missing.entry(name).and_modify(|seen| *seen = seen.merged(*origin)).or_insert(*origin);
+        }
+    }
+
+    let findings: Vec<Finding> = missing
         .into_iter()
-        .map(|name| Finding {
+        .map(|(name, origin)| Finding {
             declared_range: manifest.dev_dependencies.get(&name).cloned(),
             severity: if manifest.dev_dependencies.contains_key(&name) {
                 Severity::DevDependency
             } else {
                 Severity::Undeclared
             },
             dependency: name,
+            origin,
         })
         .collect();
 
@@ -57,9 +69,9 @@ pub fn analyze(package_dir: &Path) -> Result<Option<PackageReport>> {
     Ok(Some(PackageReport { package: manifest.id(), findings }))
 }
 
-/// The declaration files a package ships. Anything under a nested
-/// `node_modules` belongs to a bundled dependency, not to the package itself.
-pub fn declaration_files(package_dir: &Path) -> impl Iterator<Item = PathBuf> {
+/// The files a package ships. Anything under a nested `node_modules` belongs to
+/// a bundled dependency, not to the package itself.
+pub fn scannable_files(package_dir: &Path) -> impl Iterator<Item = PathBuf> {
     WalkDir::new(package_dir)
         .follow_links(false)
         .into_iter()
@@ -69,6 +81,6 @@ pub fn declaration_files(package_dir: &Path) -> impl Iterator<Item = PathBuf> {
         .map(walkdir::DirEntry::into_path)
         .filter(|path| {
             let name = path.file_name().and_then(|name| name.to_str()).unwrap_or_default();
-            name.ends_with(".d.ts") || name.ends_with(".d.mts") || name.ends_with(".d.cts")
+            SCANNED_EXTENSIONS.iter().any(|extension| name.ends_with(extension))
         })
 }

src/analyze/tests.rs

@@ -1,9 +1,10 @@
 #![cfg(unix)]
 
 use crate::{
-    analyze::{analyze, declaration_files},
+    analyze::{analyze, scannable_files},
     fixtures::global_virtual_store,
     report::Severity,
+    scan::Origin,
 };
 
 #[test]
@@ -12,10 +13,21 @@ fn reports_a_dev_dependency_that_leaks_into_declarations() {
     let app = root.path().join("store/app/node_modules/app");
     let report = analyze(&app).unwrap().expect("app should have a finding");
     assert_eq!(report.package, "app@1.0.0");
-    assert_eq!(report.findings.len(), 1);
-    assert_eq!(report.findings[0].dependency, "ghost");
-    assert_eq!(report.findings[0].severity, Severity::DevDependency);
-    assert_eq!(report.findings[0].declared_range.as_deref(), Some("^1"));
+    let ghost = report.findings.iter().find(|f| f.dependency == "ghost").expect("ghost");
+    assert_eq!(ghost.severity, Severity::DevDependency);
+    assert_eq!(ghost.declared_range.as_deref(), Some("^1"));
+    assert_eq!(ghost.origin, Origin::Types);
+}
+
+#[test]
+fn reports_a_dependency_required_from_executable_code() {
+    let root = global_virtual_store();
+    let app = root.path().join("store/app/node_modules/app");
+    let report = analyze(&app).unwrap().expect("app should have findings");
+    let runtime =
+        report.findings.iter().find(|f| f.dependency == "runtime-ghost").expect("runtime-ghost");
+    assert_eq!(runtime.severity, Severity::DevDependency);
+    assert_eq!(runtime.origin, Origin::Runtime);
 }
 
 #[test]
@@ -26,10 +38,10 @@ fn a_package_that_declares_what_it_imports_is_not_reported() {
 }
 
 #[test]
-fn declarations_bundled_under_node_modules_are_not_the_package_own() {
+fn files_bundled_under_node_modules_are_not_the_package_own() {
     let root = global_virtual_store();
     let app = root.path().join("store/app/node_modules/app");
-    let scanned: Vec<_> = declaration_files(&app).collect();
-    assert_eq!(scanned.len(), 1, "{scanned:?}");
-    assert!(scanned[0].ends_with("app/index.d.ts"), "{scanned:?}");
+    let scanned: Vec<_> = scannable_files(&app).collect();
+    assert_eq!(scanned.len(), 2, "{scanned:?}");
+    assert!(scanned.iter().all(|path| !path.to_string_lossy().contains("vendored")), "{scanned:?}");
 }

src/classify.rs

@@ -1,19 +1,25 @@
 #[cfg(test)]
 mod tests;
 
-use crate::scan::Requirement;
+use crate::scan::{Origin, Requirement};
 use std::collections::HashSet;
 
 /// The package that has to be installed for a requirement to resolve, or `None`
 /// when the package already declared something that satisfies it.
-pub fn missing_package(requirement: &Requirement, declared: &HashSet<&str>) -> Option<String> {
+pub fn missing_package(
+    requirement: &Requirement,
+    origin: Origin,
+    declared: &HashSet<&str>,
+) -> Option<String> {
     match requirement {
-        // A bare specifier can also be satisfied by a types package that declares
-        // the module ambiently, the way `@types/estree` declares `estree`.
+        // In a type position a bare specifier can be satisfied by a types
+        // package that declares the module ambiently, the way `@types/estree`
+        // declares `estree`. Executing code needs the package itself: a
+        // declaration file carries no implementation.
         Requirement::Module(specifier) => {
             let name = package_name(specifier)?;
-            let satisfied =
-                declared.contains(name) || declared.contains(types_package(name).as_str());
+            let satisfied = declared.contains(name)
+                || (origin == Origin::Types && declared.contains(types_package(name).as_str()));
             (!satisfied).then(|| name.to_string())
         }
         // `/// <reference types="x" />` is satisfied by `@types/x` or by `x`

src/classify/tests.rs

@@ -1,6 +1,6 @@
 use crate::{
     classify::{missing_package, package_name, types_package},
-    scan::Requirement,
+    scan::{Origin, Requirement},
 };
 use std::collections::HashSet;
 
@@ -49,19 +49,38 @@ fn maps_a_name_to_its_types_package() {
 #[test]
 fn an_import_is_satisfied_by_the_types_package_that_declares_it() {
     let requirement = Requirement::Module("estree".to_string());
-    assert_eq!(missing_package(&requirement, &declared(&["@types/estree"])), None);
-    assert_eq!(missing_package(&requirement, &declared(&[])), Some("estree".to_string()));
+    assert_eq!(missing_package(&requirement, Origin::Types, &declared(&["@types/estree"])), None);
+    assert_eq!(
+        missing_package(&requirement, Origin::Types, &declared(&[])),
+        Some("estree".to_string()),
+    );
 }
 
 #[test]
 fn a_types_reference_is_satisfied_by_either_spelling() {
     let requirement = Requirement::TypesReference("node".to_string());
-    assert_eq!(missing_package(&requirement, &declared(&["@types/node"])), None);
-    assert_eq!(missing_package(&requirement, &declared(&["node"])), None);
+    assert_eq!(missing_package(&requirement, Origin::Types, &declared(&["@types/node"])), None);
+    assert_eq!(missing_package(&requirement, Origin::Types, &declared(&["node"])), None);
 }
 
 #[test]
 fn an_unsatisfied_types_reference_is_reported_under_the_types_name() {
     let requirement = Requirement::TypesReference("estree".to_string());
-    assert_eq!(missing_package(&requirement, &declared(&[])), Some("@types/estree".to_string()));
+    assert_eq!(
+        missing_package(&requirement, Origin::Types, &declared(&[])),
+        Some("@types/estree".to_string()),
+    );
+}
+
+#[test]
+fn a_types_package_does_not_satisfy_a_runtime_reference() {
+    // `@types/lodash` carries no implementation, so requiring `lodash` at run
+    // time still needs `lodash` even though the type position was satisfied.
+    let requirement = Requirement::Module("lodash".to_string());
+    assert_eq!(missing_package(&requirement, Origin::Types, &declared(&["@types/lodash"])), None);
+    assert_eq!(
+        missing_package(&requirement, Origin::Runtime, &declared(&["@types/lodash"])),
+        Some("lodash".to_string()),
+    );
+    assert_eq!(missing_package(&requirement, Origin::Runtime, &declared(&["lodash"])), None);
 }

src/fixtures.rs

@@ -13,8 +13,9 @@ pub fn manifest(name: &str, extra: &str) -> String {
 /// store produces: `node_modules` holds links, and each store entry holds the
 /// package next to the dependencies it may reach.
 ///
-/// `app` leaks a devDependency into its declarations and bundles a copy of
-/// something else; `helper` declares the dependency it imports.
+/// `app` leaks one devDependency into its declarations and requires another
+/// from executable code, and bundles a copy of something else; `helper`
+/// declares the dependency it imports.
 #[cfg(unix)]
 pub fn global_virtual_store() -> tempfile::TempDir {
     use std::os::unix::fs::symlink;
@@ -24,8 +25,12 @@ pub fn global_virtual_store() -> tempfile::TempDir {
     let store = root.path().join("store");
 
     let app = store.join("app/node_modules/app");
-    write(&app.join("package.json"), &manifest("app", r#","devDependencies":{"ghost":"^1"}"#));
+    write(
+        &app.join("package.json"),
+        &manifest("app", r#","devDependencies":{"ghost":"^1","runtime-ghost":"^3"}"#),
+    );
     write(&app.join("index.d.ts"), "import type { G } from 'ghost';\nexport type { G };\n");
+    write(&app.join("index.js"), "const r = require('runtime-ghost');\nmodule.exports = r;\n");
     write(&app.join("node_modules/vendored/index.d.ts"), "import 'not-yours';\n");
 
     let helper = store.join("helper/node_modules/helper");

src/report.rs

@@ -1,3 +1,4 @@
+use crate::scan::Origin;
 use anyhow::Result;
 use serde::Serialize;
 use std::fmt::Write as _;
@@ -18,6 +19,7 @@ pub struct Finding {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub declared_range: Option<String>,
     pub severity: Severity,
+    pub origin: Origin,
 }
 
 /// A dependency the package build-depends on but ships references to is a far
@@ -36,7 +38,7 @@ pub fn as_json(reports: &[PackageReport]) -> Result<String> {
 
 pub fn as_text(reports: &[PackageReport]) -> String {
     if reports.is_empty() {
-        return "No undeclared dependencies found in shipped declaration files.\n".to_string();
+        return "No undeclared dependencies found in shipped files.\n".to_string();
     }
 
     let mut out = String::new();
@@ -47,7 +49,12 @@ pub fn as_text(reports: &[PackageReport]) -> String {
                 Severity::DevDependency => "declared as a devDependency",
                 Severity::Undeclared => "not in the manifest at all",
             };
-            let _ = writeln!(out, "  {} — {note}", finding.dependency);
+            let where_from = match finding.origin {
+                Origin::Runtime => "code",
+                Origin::Types => "types",
+                Origin::Both => "code and types",
+            };
+            let _ = writeln!(out, "  {} — {note}, used in {where_from}", finding.dependency);
         }
         out.push('\n');
     }

src/report/tests.rs

@@ -1,4 +1,4 @@
-use crate::report::{as_package_extensions, as_text, Finding, PackageReport, Severity};
+use crate::report::{as_package_extensions, as_text, Finding, Origin, PackageReport, Severity};
 use yaml_rust2::YamlLoader;
 
 fn medplum() -> Vec<PackageReport> {
@@ -8,6 +8,7 @@ fn medplum() -> Vec<PackageReport> {
             dependency: "@medplum/fhirtypes".to_string(),
             declared_range: Some("5.1.15".to_string()),
             severity: Severity::DevDependency,
+            origin: Origin::Types,
         }],
     }]
 }
@@ -62,15 +63,17 @@ fn text_separates_the_two_kinds_of_finding() {
                 dependency: "known".to_string(),
                 declared_range: Some("^1".to_string()),
                 severity: Severity::DevDependency,
+                origin: Origin::Types,
             },
             Finding {
                 dependency: "unknown".to_string(),
                 declared_range: None,
                 severity: Severity::Undeclared,
+                origin: Origin::Runtime,
             },
         ],
     }];
     let rendered = as_text(&reports);
-    assert!(rendered.contains("known — declared as a devDependency"), "{rendered}");
-    assert!(rendered.contains("unknown — not in the manifest at all"), "{rendered}");
+    assert!(rendered.contains("known — declared as a devDependency, used in types"), "{rendered}");
+    assert!(rendered.contains("unknown — not in the manifest at all, used in code"), "{rendered}");
 }