fix(lockfile): error if a referenced package id doesn't exist in list of packages (#16509)

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>

cli/npm/resolution.rs

@@ -372,36 +372,45 @@ impl NpmResolutionSnapshot {
     lockfile: Arc<Mutex<Lockfile>>,
     api: &NpmRegistryApi,
   ) -> Result<Self, AnyError> {
-    let mut package_reqs = HashMap::new();
+    let mut package_reqs: HashMap<NpmPackageReq, NpmVersion>;
-    let mut packages_by_name: HashMap<String, Vec<NpmVersion>> = HashMap::new();
+    let mut packages_by_name: HashMap<String, Vec<NpmVersion>>;
-    let mut packages = HashMap::new();
+    let mut packages: HashMap<NpmPackageId, NpmResolutionPackage>;
 
     {
       let lockfile = lockfile.lock();
 
+      // pre-allocate collections
+      package_reqs =
+        HashMap::with_capacity(lockfile.content.npm.specifiers.len());
+      packages = HashMap::with_capacity(lockfile.content.npm.packages.len());
+      packages_by_name =
+        HashMap::with_capacity(lockfile.content.npm.packages.len()); // close enough
+      let mut verify_ids =
+        HashSet::with_capacity(lockfile.content.npm.packages.len());
+
+      // collect the specifiers to version mappings
       for (key, value) in &lockfile.content.npm.specifiers {
         let reference = NpmPackageReference::from_str(&format!("npm:{}", key))
           .with_context(|| format!("Unable to parse npm specifier: {}", key))?;
         let package_id = NpmPackageId::deserialize_from_lock_file(value)?;
         package_reqs.insert(reference.req, package_id.version.clone());
+        verify_ids.insert(package_id.clone());
       }
 
+      // then the packages
       for (key, value) in &lockfile.content.npm.packages {
         let package_id = NpmPackageId::deserialize_from_lock_file(key)?;
         let mut dependencies = HashMap::default();
 
-        for (name, specifier) in &value.dependencies {
-          dependencies.insert(
-            name.to_string(),
-            NpmPackageId::deserialize_from_lock_file(specifier)?,
-          );
-        }
-
-        for (name, id) in &dependencies {
         packages_by_name
-            .entry(name.to_string())
+          .entry(package_id.name.to_string())
           .or_default()
-            .push(id.version.clone());
+          .push(package_id.version.clone());
+
+        for (name, specifier) in &value.dependencies {
+          let dep_id = NpmPackageId::deserialize_from_lock_file(specifier)?;
+          dependencies.insert(name.to_string(), dep_id.clone());
+          verify_ids.insert(dep_id);
         }
 
         let package = NpmResolutionPackage {
@@ -415,32 +424,49 @@ impl NpmResolutionSnapshot {
           dependencies,
         };
 
-        packages.insert(package_id.clone(), package);
+        packages.insert(package_id, package);
+      }
+
+      // verify that all these ids exist in packages
+      for id in &verify_ids {
+        if !packages.contains_key(id) {
+          bail!(
+            "the lockfile ({}) is corrupt. You can recreate it with --lock-write",
+            lockfile.filename.display(),
+          );
+        }
       }
     }
 
     let mut unresolved_tasks = Vec::with_capacity(packages_by_name.len());
 
-    for package_id in packages.keys() {
+    // cache the package names in parallel in the registry api
-      let package_id = package_id.clone();
+    for package_name in packages_by_name.keys() {
+      let package_name = package_name.clone();
       let api = api.clone();
       unresolved_tasks.push(tokio::task::spawn(async move {
-        let info = api.package_info(&package_id.name).await?;
+        api.package_info(&package_name).await?;
-        Result::<_, AnyError>::Ok((package_id, info))
+        Result::<_, AnyError>::Ok(())
       }));
     }
     for result in futures::future::join_all(unresolved_tasks).await {
-      let (package_id, info) = result??;
+      result??;
+    }
 
-      let version_and_info = get_resolved_package_version_and_info(
+    // ensure the dist is set for each package
-        &package_id.name,
+    for package in packages.values_mut() {
-        &NpmVersionReq::parse(&package_id.version.to_string()).unwrap(),
+      // this will read from the memory cache now
-        info,
+      let package_info = api.package_info(&package.id.name).await?;
-        None,
+      let version_info = match package_info
-      )?;
+        .versions
-
+        .get(&package.id.version.to_string())
-      let package = packages.get_mut(&package_id).unwrap();
+      {
-      package.dist = version_and_info.info.dist;
+        Some(version_info) => version_info,
+        None => {
+          bail!("could not find '{}' specified in the lockfile. Maybe try again with --reload", package.id);
+        }
+      };
+      package.dist = version_info.dist.clone();
     }
 
     Ok(Self {

cli/tests/integration/npm_tests.rs

@@ -783,6 +783,221 @@ itest!(info_cli_chalk_json {
   http_server: true,
 });
 
+#[test]
+fn lock_file_missing_top_level_package() {
+  let _server = http_server();
+
+  let deno_dir = util::new_deno_dir();
+  let temp_dir = util::TempDir::new();
+
+  // write empty config file
+  temp_dir.write("deno.json", "{}");
+
+  // Lock file that is automatically picked up has been intentionally broken,
+  // by removing "cowsay" package from it. This test ensures that npm resolver
+  // snapshot can be successfully hydrated in such situation
+  let lock_file_content = r#"{
+    "version": "2",
+    "remote": {},
+    "npm": {
+      "specifiers": { "cowsay": "cowsay@1.5.0" },
+      "packages": {
+        "ansi-regex@3.0.1": {
+          "integrity": "sha512-+O9Jct8wf++lXxxFc4hc8LsjaSq0HFzzL7cVsw8pRDIPdjKD2mT4ytDZlLuSBZ4cLKZFXIrMGO7DbQCtMJJMKw==",
+          "dependencies": {}
+        },
+        "ansi-regex@5.0.1": {
+          "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==",
+          "dependencies": {}
+        },
+        "ansi-styles@4.3.0": {
+          "integrity": "sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==",
+          "dependencies": { "color-convert": "color-convert@2.0.1" }
+        },
+        "camelcase@5.3.1": {
+          "integrity": "sha512-L28STB170nwWS63UjtlEOE3dldQApaJXZkOI1uMFfzf3rRuPegHaHesyee+YxQ+W6SvRDQV6UrdOdRiR153wJg==",
+          "dependencies": {}
+        },
+        "cliui@6.0.0": {
+          "integrity": "sha512-t6wbgtoCXvAzst7QgXxJYqPt0usEfbgQdftEPbLL/cvv6HPE5VgvqCuAIDR0NgU52ds6rFwqrgakNLrHEjCbrQ==",
+          "dependencies": {
+            "string-width": "string-width@4.2.3",
+            "strip-ansi": "strip-ansi@6.0.1",
+            "wrap-ansi": "wrap-ansi@6.2.0"
+          }
+        },
+        "color-convert@2.0.1": {
+          "integrity": "sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ==",
+          "dependencies": { "color-name": "color-name@1.1.4" }
+        },
+        "color-name@1.1.4": {
+          "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==",
+          "dependencies": {}
+        },
+        "decamelize@1.2.0": {
+          "integrity": "sha512-z2S+W9X73hAUUki+N+9Za2lBlun89zigOyGrsax+KUQ6wKW4ZoWpEYBkGhQjwAjjDCkWxhY0VKEhk8wzY7F5cA==",
+          "dependencies": {}
+        },
+        "emoji-regex@8.0.0": {
+          "integrity": "sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A==",
+          "dependencies": {}
+        },
+        "find-up@4.1.0": {
+          "integrity": "sha512-PpOwAdQ/YlXQ2vj8a3h8IipDuYRi3wceVQQGYWxNINccq40Anw7BlsEXCMbt1Zt+OLA6Fq9suIpIWD0OsnISlw==",
+          "dependencies": {
+            "locate-path": "locate-path@5.0.0",
+            "path-exists": "path-exists@4.0.0"
+          }
+        },
+        "get-caller-file@2.0.5": {
+          "integrity": "sha512-DyFP3BM/3YHTQOCUL/w0OZHR0lpKeGrxotcHWcqNEdnltqFwXVfhEBQ94eIo34AfQpo0rGki4cyIiftY06h2Fg==",
+          "dependencies": {}
+        },
+        "get-stdin@8.0.0": {
+          "integrity": "sha512-sY22aA6xchAzprjyqmSEQv4UbAAzRN0L2dQB0NlN5acTTK9Don6nhoc3eAbUnpZiCANAMfd/+40kVdKfFygohg==",
+          "dependencies": {}
+        },
+        "is-fullwidth-code-point@2.0.0": {
+          "integrity": "sha512-VHskAKYM8RfSFXwee5t5cbN5PZeq1Wrh6qd5bkyiXIf6UQcN6w/A0eXM9r6t8d+GYOh+o6ZhiEnb88LN/Y8m2w==",
+          "dependencies": {}
+        },
+        "is-fullwidth-code-point@3.0.0": {
+          "integrity": "sha512-zymm5+u+sCsSWyD9qNaejV3DFvhCKclKdizYaJUuHA83RLjb7nSuGnddCHGv0hk+KY7BMAlsWeK4Ueg6EV6XQg==",
+          "dependencies": {}
+        },
+        "locate-path@5.0.0": {
+          "integrity": "sha512-t7hw9pI+WvuwNJXwk5zVHpyhIqzg2qTlklJOf0mVxGSbe3Fp2VieZcduNYjaLDoy6p9uGpQEGWG87WpMKlNq8g==",
+          "dependencies": { "p-locate": "p-locate@4.1.0" }
+        },
+        "p-limit@2.3.0": {
+          "integrity": "sha512-//88mFWSJx8lxCzwdAABTJL2MyWB12+eIY7MDL2SqLmAkeKU9qxRvWuSyTjm3FUmpBEMuFfckAIqEaVGUDxb6w==",
+          "dependencies": { "p-try": "p-try@2.2.0" }
+        },
+        "p-locate@4.1.0": {
+          "integrity": "sha512-R79ZZ/0wAxKGu3oYMlz8jy/kbhsNrS7SKZ7PxEHBgJ5+F2mtFW2fK2cOtBh1cHYkQsbzFV7I+EoRKe6Yt0oK7A==",
+          "dependencies": { "p-limit": "p-limit@2.3.0" }
+        },
+        "p-try@2.2.0": {
+          "integrity": "sha512-R4nPAVTAU0B9D35/Gk3uJf/7XYbQcyohSKdvAxIRSNghFl4e71hVoGnBNQz9cWaXxO2I10KTC+3jMdvvoKw6dQ==",
+          "dependencies": {}
+        },
+        "path-exists@4.0.0": {
+          "integrity": "sha512-ak9Qy5Q7jYb2Wwcey5Fpvg2KoAc/ZIhLSLOSBmRmygPsGwkVVt0fZa0qrtMz+m6tJTAHfZQ8FnmB4MG4LWy7/w==",
+          "dependencies": {}
+        },
+        "require-directory@2.1.1": {
+          "integrity": "sha512-fGxEI7+wsG9xrvdjsrlmL22OMTTiHRwAMroiEeMgq8gzoLC/PQr7RsRDSTLUg/bZAZtF+TVIkHc6/4RIKrui+Q==",
+          "dependencies": {}
+        },
+        "require-main-filename@2.0.0": {
+          "integrity": "sha512-NKN5kMDylKuldxYLSUfrbo5Tuzh4hd+2E8NPPX02mZtn1VuREQToYe/ZdlJy+J3uCpfaiGF05e7B8W0iXbQHmg==",
+          "dependencies": {}
+        },
+        "set-blocking@2.0.0": {
+          "integrity": "sha512-KiKBS8AnWGEyLzofFfmvKwpdPzqiy16LvQfK3yv/fVH7Bj13/wl3JSR1J+rfgRE9q7xUJK4qvgS8raSOeLUehw==",
+          "dependencies": {}
+        },
+        "string-width@2.1.1": {
+          "integrity": "sha512-nOqH59deCq9SRHlxq1Aw85Jnt4w6KvLKqWVik6oA9ZklXLNIOlqg4F2yrT1MVaTjAqvVwdfeZ7w7aCvJD7ugkw==",
+          "dependencies": {
+            "is-fullwidth-code-point": "is-fullwidth-code-point@2.0.0",
+            "strip-ansi": "strip-ansi@4.0.0"
+          }
+        },
+        "string-width@4.2.3": {
+          "integrity": "sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==",
+          "dependencies": {
+            "emoji-regex": "emoji-regex@8.0.0",
+            "is-fullwidth-code-point": "is-fullwidth-code-point@3.0.0",
+            "strip-ansi": "strip-ansi@6.0.1"
+          }
+        },
+        "strip-ansi@4.0.0": {
+          "integrity": "sha512-4XaJ2zQdCzROZDivEVIDPkcQn8LMFSa8kj8Gxb/Lnwzv9A8VctNZ+lfivC/sV3ivW8ElJTERXZoPBRrZKkNKow==",
+          "dependencies": { "ansi-regex": "ansi-regex@3.0.1" }
+        },
+        "strip-ansi@6.0.1": {
+          "integrity": "sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==",
+          "dependencies": { "ansi-regex": "ansi-regex@5.0.1" }
+        },
+        "strip-final-newline@2.0.0": {
+          "integrity": "sha512-BrpvfNAE3dcvq7ll3xVumzjKjZQ5tI1sEUIKr3Uoks0XUl45St3FlatVqef9prk4jRDzhW6WZg+3bk93y6pLjA==",
+          "dependencies": {}
+        },
+        "which-module@2.0.0": {
+          "integrity": "sha512-B+enWhmw6cjfVC7kS8Pj9pCrKSc5txArRyaYGe088shv/FGWH+0Rjx/xPgtsWfsUtS27FkP697E4DDhgrgoc0Q==",
+          "dependencies": {}
+        },
+        "wrap-ansi@6.2.0": {
+          "integrity": "sha512-r6lPcBGxZXlIcymEu7InxDMhdW0KDxpLgoFLcguasxCaJ/SOIZwINatK9KY/tf+ZrlywOKU0UDj3ATXUBfxJXA==",
+          "dependencies": {
+            "ansi-styles": "ansi-styles@4.3.0",
+            "string-width": "string-width@4.2.3",
+            "strip-ansi": "strip-ansi@6.0.1"
+          }
+        },
+        "y18n@4.0.3": {
+          "integrity": "sha512-JKhqTOwSrqNA1NY5lSztJ1GrBiUodLMmIZuLiDaMRJ+itFd+ABVE8XBjOvIWL+rSqNDC74LCSFmlb/U4UZ4hJQ==",
+          "dependencies": {}
+        },
+        "yargs-parser@18.1.3": {
+          "integrity": "sha512-o50j0JeToy/4K6OZcaQmW6lyXXKhq7csREXcDwk2omFPJEwUNOVtJKvmDr9EI1fAJZUyZcRF7kxGBWmRXudrCQ==",
+          "dependencies": {
+            "camelcase": "camelcase@5.3.1",
+            "decamelize": "decamelize@1.2.0"
+          }
+        },
+        "yargs@15.4.1": {
+          "integrity": "sha512-aePbxDmcYW++PaqBsJ+HYUFwCdv4LVvdnhBy78E57PIor8/OVvhMrADFFEDh8DHDFRv/O9i3lPhsENjO7QX0+A==",
+          "dependencies": {
+            "cliui": "cliui@6.0.0",
+            "decamelize": "decamelize@1.2.0",
+            "find-up": "find-up@4.1.0",
+            "get-caller-file": "get-caller-file@2.0.5",
+            "require-directory": "require-directory@2.1.1",
+            "require-main-filename": "require-main-filename@2.0.0",
+            "set-blocking": "set-blocking@2.0.0",
+            "string-width": "string-width@4.2.3",
+            "which-module": "which-module@2.0.0",
+            "y18n": "y18n@4.0.3",
+            "yargs-parser": "yargs-parser@18.1.3"
+          }
+        }
+      }
+    }
+  }
+  "#;
+  temp_dir.write("deno.lock", lock_file_content);
+  let main_contents = r#"
+  import cowsay from "npm:cowsay";
+  console.log(cowsay);
+  "#;
+  temp_dir.write("main.ts", main_contents);
+
+  let deno = util::deno_cmd_with_deno_dir(&deno_dir)
+    .current_dir(temp_dir.path())
+    .arg("run")
+    .arg("--unstable")
+    .arg("--quiet")
+    .arg("--lock")
+    .arg("deno.lock")
+    .arg("-A")
+    .arg("main.ts")
+    .envs(env_vars())
+    .stdout(Stdio::piped())
+    .stderr(Stdio::piped())
+    .spawn()
+    .unwrap();
+  let output = deno.wait_with_output().unwrap();
+  assert!(!output.status.success());
+
+  let stderr = String::from_utf8(output.stderr).unwrap();
+  assert_eq!(
+    stderr,
+    "error: the lockfile (deno.lock) is corrupt. You can recreate it with --lock-write\n"
+  );
+}
+
 fn env_vars_no_sync_download() -> Vec<(String, String)> {
   vec![
     ("DENO_NODE_COMPAT_URL".to_string(), util::std_file_url()),