refactor: remove deno_graph::Locker usage (#16877)

This is just a straight refactor and doesn't make any improvements to
the code that could now be made.

Closes #16493

Cargo.lock

@@ -982,9 +982,9 @@ dependencies = [
 
 [[package]]
 name = "deno_doc"
-version = "0.50.0"
+version = "0.51.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "dc40beda42f25219236ea7a517c63c6ad57375cba5ecf1a00aa4a21a0c294f90"
+checksum = "7a5f0f24f690e9c0c1d22fe9c9da68b65d7378a5c10afe4a61398134eb031e21"
 dependencies = [
  "cfg-if",
  "deno_ast",
@@ -1000,9 +1000,9 @@ dependencies = [
 
 [[package]]
 name = "deno_emit"
-version = "0.11.0"
+version = "0.12.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7be76420d8eaac9d82295eb51cb2c0aebd326d305a2ecbeab6582343fb1b743c"
+checksum = "c721cb4e2ca7d94702f6987c2050aedfd270d18f87020080e396865a65dd957e"
 dependencies = [
  "anyhow",
  "base64",
@@ -1063,19 +1063,18 @@ dependencies = [
 
 [[package]]
 name = "deno_graph"
-version = "0.38.0"
+version = "0.39.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "65eb6b2223b71a759b12bc21ee35842193d3703157950d1411c0243239f072eb"
+checksum = "87b3758993b62cf234fac6e922b2217aac6c3086d6d9a4fa36ddf7779abb0890"
 dependencies = [
  "anyhow",
  "cfg-if",
  "data-url",
  "deno_ast",
  "futures",
- "lazy_static",
+ "once_cell",
  "parking_lot 0.12.1",
  "regex",
- "ring",
  "serde",
  "serde_json",
  "sourcemap",
@@ -1660,9 +1659,9 @@ dependencies = [
 
 [[package]]
 name = "eszip"
-version = "0.30.0"
+version = "0.31.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1fdf9b31295c768c806cbc0d08d98868b4382684b002d216a16eb661bb8e8575"
+checksum = "d8119eb19b5b7f9c6b6da550781249bb05562fbdadb10f80f0a3afb96dde4944"
 dependencies = [
  "anyhow",
  "base64",
@@ -3024,9 +3023,9 @@ dependencies = [
 
 [[package]]
 name = "once_cell"
-version = "1.14.0"
+version = "1.16.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2f7254b99e31cad77da24b08ebf628882739a608578bb1bcdfc1f9c21260d7c0"
+checksum = "86f0b0d4bf799edbc74508c1e8bf170ff5f41238e5f8225603ca7caaae2b7860"
 
 [[package]]
 name = "opaque-debug"

Cargo.toml

@@ -89,7 +89,7 @@ libc = "=0.2.126"
 log = "=0.4.17"
 lzzzz = "1.0"
 notify = "=5.0.0"
-once_cell = "=1.14.0"
+once_cell = "=1.16.0"
 os_pipe = "=1.0.1"
 parking_lot = "0.12.0"
 percent-encoding = "=2.2.0"

cli/Cargo.toml

@@ -43,9 +43,9 @@ winres.workspace = true
 [dependencies]
 deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_graph", "module_specifier", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] }
 deno_core.workspace = true
-deno_doc = "0.50.0"
-deno_emit = "0.11.0"
-deno_graph = "0.38.0"
+deno_doc = "0.51.0"
+deno_emit = "0.12.0"
+deno_graph = "0.39.0"
 deno_lint = { version = "0.35.0", features = ["docs"] }
 deno_runtime.workspace = true
 deno_task_shell = "0.8.1"
@@ -66,7 +66,7 @@ dprint-plugin-markdown = "=0.14.3"
 dprint-plugin-typescript = "=0.78.0"
 encoding_rs.workspace = true
 env_logger = "=0.9.0"
-eszip = "=0.30.0"
+eszip = "=0.31.0"
 fancy-regex = "=0.10.0"
 flate2.workspace = true
 http.workspace = true

cli/args/lockfile.rs

@@ -2,18 +2,13 @@
 
 use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
-use deno_core::parking_lot::Mutex;
 use deno_core::serde::Deserialize;
 use deno_core::serde::Serialize;
 use deno_core::serde_json;
-use deno_core::ModuleSpecifier;
 use log::debug;
-use std::cell::RefCell;
 use std::collections::BTreeMap;
 use std::io::Write;
 use std::path::PathBuf;
-use std::rc::Rc;
-use std::sync::Arc;
 
 use crate::args::ConfigFile;
 use crate::npm::NpmPackageId;
@@ -96,15 +91,6 @@ pub struct Lockfile {
 }
 
 impl Lockfile {
-  pub fn as_maybe_locker(
-    lockfile: Option<Arc<Mutex<Lockfile>>>,
-  ) -> Option<Rc<RefCell<dyn deno_graph::source::Locker>>> {
-    lockfile.as_ref().map(|lf| {
-      Rc::new(RefCell::new(Locker(Some(lf.clone()))))
-        as Rc<RefCell<dyn deno_graph::source::Locker>>
-    })
-  }
-
   pub fn discover(
     flags: &Flags,
     maybe_config_file: Option<&ConfigFile>,
@@ -342,33 +328,6 @@ Use \"--lock-write\" flag to regenerate the lockfile at \"{}\".",
   }
 }
 
-#[derive(Debug)]
-pub struct Locker(Option<Arc<Mutex<Lockfile>>>);
-
-impl deno_graph::source::Locker for Locker {
-  fn check_or_insert(
-    &mut self,
-    specifier: &ModuleSpecifier,
-    source: &str,
-  ) -> bool {
-    if let Some(lock_file) = &self.0 {
-      let mut lock_file = lock_file.lock();
-      lock_file.check_or_insert_remote(specifier.as_str(), source)
-    } else {
-      true
-    }
-  }
-
-  fn get_checksum(&self, content: &str) -> String {
-    util::checksum::gen(&[content.as_bytes()])
-  }
-
-  fn get_filename(&self) -> Option<String> {
-    let lock_file = self.0.as_ref()?.lock();
-    lock_file.filename.to_str().map(|s| s.to_string())
-  }
-}
-
 #[cfg(test)]
 mod tests {
   use super::*;

cli/errors.rs

@@ -26,8 +26,7 @@ fn get_diagnostic_class(_: &Diagnostic) -> &'static str {
 fn get_module_graph_error_class(err: &ModuleGraphError) -> &'static str {
   match err {
     ModuleGraphError::LoadingErr(_, err) => get_error_class_name(err.as_ref()),
-    ModuleGraphError::InvalidSource(_, _)
-    | ModuleGraphError::InvalidTypeAssertion { .. } => "SyntaxError",
+    ModuleGraphError::InvalidTypeAssertion { .. } => "SyntaxError",
     ModuleGraphError::ParseErr(_, diagnostic) => {
       get_diagnostic_class(diagnostic)
     }

cli/graph_util.rs

@@ -1,5 +1,6 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
 
+use crate::args::Lockfile;
 use crate::args::TsTypeLib;
 use crate::colors;
 use crate::errors::get_error_class_name;
@@ -80,23 +81,23 @@ impl GraphData {
     let mut has_npm_specifier_in_graph = false;
 
     for (specifier, result) in graph.specifiers() {
-      if NpmPackageReference::from_specifier(&specifier).is_ok() {
+      if NpmPackageReference::from_specifier(specifier).is_ok() {
         has_npm_specifier_in_graph = true;
         continue;
       }
 
-      if !reload && self.modules.contains_key(&specifier) {
+      if !reload && self.modules.contains_key(specifier) {
         continue;
       }
 
-      if let Some(found) = graph.redirects.get(&specifier) {
+      if let Some(found) = graph.redirects.get(specifier) {
         let module_entry = ModuleEntry::Redirect(found.clone());
         self.modules.insert(specifier.clone(), module_entry);
         continue;
       }
       match result {
         Ok((_, _, media_type)) => {
-          let module = graph.get(&specifier).unwrap();
+          let module = graph.get(specifier).unwrap();
           let code = match &module.maybe_source {
             Some(source) => source.clone(),
             None => continue,
@@ -134,11 +135,11 @@ impl GraphData {
             checked_libs: Default::default(),
             maybe_types,
           };
-          self.modules.insert(specifier, module_entry);
+          self.modules.insert(specifier.clone(), module_entry);
         }
         Err(error) => {
-          let module_entry = ModuleEntry::Error(error);
-          self.modules.insert(specifier, module_entry);
+          let module_entry = ModuleEntry::Error(error.clone());
+          self.modules.insert(specifier.clone(), module_entry);
         }
       }
     }
@@ -475,10 +476,23 @@ pub fn graph_valid(
     .unwrap()
 }
 
-/// Calls `graph.lock()` and exits on errors.
-pub fn graph_lock_or_exit(graph: &ModuleGraph) {
-  if let Err(err) = graph.lock() {
-    log::error!("{} {}", colors::red("error:"), err);
-    std::process::exit(10);
+/// Checks the lockfile against the graph and and exits on errors.
+pub fn graph_lock_or_exit(graph: &ModuleGraph, lockfile: &mut Lockfile) {
+  for module in graph.modules() {
+    if let Some(source) = &module.maybe_source {
+      if !lockfile.check_or_insert_remote(module.specifier.as_str(), source) {
+        let err = format!(
+          concat!(
+            "The source code is invalid, as it does not match the expected hash in the lock file.\n",
+            "  Specifier: {}\n",
+            "  Lock file: {}",
+          ),
+          module.specifier,
+          lockfile.filename.display(),
+        );
+        log::error!("{} {}", colors::red("error:"), err);
+        std::process::exit(10);
+      }
+    }
   }
 }

cli/main.rs

@@ -61,7 +61,6 @@ use crate::util::display;
 use crate::util::file_watcher::ResolutionResult;
 
 use args::CliOptions;
-use args::Lockfile;
 use deno_ast::MediaType;
 use deno_core::anyhow::bail;
 use deno_core::error::generic_error;
@@ -315,7 +314,6 @@ async fn create_graph_and_maybe_check(
     Permissions::allow_all(),
     Permissions::allow_all(),
   );
-  let maybe_locker = Lockfile::as_maybe_locker(ps.lockfile.clone());
   let maybe_imports = ps.options.to_maybe_imports()?;
   let maybe_cli_resolver = CliResolver::maybe_new(
     ps.options.to_maybe_jsx_import_source_config(),
@@ -332,7 +330,6 @@ async fn create_graph_and_maybe_check(
         is_dynamic: false,
         imports: maybe_imports,
         resolver: maybe_graph_resolver,
-        locker: maybe_locker,
         module_analyzer: Some(&*analyzer),
         reporter: None,
       },
@@ -353,7 +350,9 @@ async fn create_graph_and_maybe_check(
   ps.npm_resolver
     .add_package_reqs(graph_data.npm_package_reqs().clone())
     .await?;
-  graph_lock_or_exit(&graph);
+  if let Some(lockfile) = &ps.lockfile {
+    graph_lock_or_exit(&graph, &mut lockfile.lock());
+  }
 
   if ps.options.type_check_mode() != TypeCheckMode::None {
     let ts_config_result =
@@ -433,7 +432,6 @@ async fn bundle_command(
 
       let mut paths_to_watch: Vec<PathBuf> = graph
         .specifiers()
-        .iter()
         .filter_map(|(_, r)| {
           r.as_ref().ok().and_then(|(s, _, _)| s.to_file_path().ok())
         })
@@ -533,9 +531,8 @@ fn error_for_any_npm_specifier(
 ) -> Result<(), AnyError> {
   let first_npm_specifier = graph
     .specifiers()
-    .values()
-    .filter_map(|r| match r {
-      Ok((specifier, kind, _)) if *kind == deno_graph::ModuleKind::External => {
+    .filter_map(|(_, r)| match r {
+      Ok((specifier, kind, _)) if kind == deno_graph::ModuleKind::External => {
         Some(specifier.clone())
       }
       _ => None,

cli/npm/resolution/specifier.rs

@@ -947,7 +947,6 @@ mod tests {
         is_dynamic: false,
         imports: None,
         resolver: None,
-        locker: None,
         module_analyzer: Some(&analyzer),
         reporter: None,
       },

cli/proc_state.rs

@@ -329,7 +329,6 @@ impl ProcState {
       root_permissions.clone(),
       dynamic_permissions.clone(),
     );
-    let maybe_locker = Lockfile::as_maybe_locker(self.lockfile.clone());
     let maybe_imports = self.options.to_maybe_imports()?;
     let maybe_resolver =
       self.maybe_resolver.as_ref().map(|r| r.as_graph_resolver());
@@ -383,16 +382,16 @@ impl ProcState {
         is_dynamic,
         imports: maybe_imports,
         resolver: maybe_resolver,
-        locker: maybe_locker,
         module_analyzer: Some(&*analyzer),
         reporter: maybe_file_watcher_reporter,
       },
     )
     .await;
 
-    // If there was a locker, validate the integrity of all the modules in the
-    // locker.
-    graph_lock_or_exit(&graph);
+    // If there is a lockfile, validate the integrity of all the modules.
+    if let Some(lockfile) = &self.lockfile {
+      graph_lock_or_exit(&graph, &mut lockfile.lock());
+    }
 
     // Determine any modules that have already been emitted this session and
     // should be skipped.
@@ -639,7 +638,6 @@ impl ProcState {
     roots: Vec<(ModuleSpecifier, ModuleKind)>,
     loader: &mut dyn Loader,
   ) -> Result<deno_graph::ModuleGraph, AnyError> {
-    let maybe_locker = Lockfile::as_maybe_locker(self.lockfile.clone());
     let maybe_imports = self.options.to_maybe_imports()?;
 
     let maybe_cli_resolver = CliResolver::maybe_new(
@@ -657,7 +655,6 @@ impl ProcState {
         is_dynamic: false,
         imports: maybe_imports,
         resolver: maybe_graph_resolver,
-        locker: maybe_locker,
         module_analyzer: Some(&*analyzer),
         reporter: None,
       },

cli/tools/doc.rs

@@ -49,7 +49,6 @@ pub async fn print_docs(
         is_dynamic: false,
         imports: None,
         resolver: None,
-        locker: None,
         module_analyzer: Some(&analyzer),
         reporter: None,
       },

cli/tools/info.rs

@@ -311,7 +311,7 @@ impl NpmInfo {
     }
 
     for (specifier, _) in graph.specifiers() {
-      if let Ok(reference) = NpmPackageReference::from_specifier(&specifier) {
+      if let Ok(reference) = NpmPackageReference::from_specifier(specifier) {
         info
           .specifiers
           .insert(specifier.clone(), reference.req.clone());
@@ -421,9 +421,8 @@ impl<'a> GraphDisplayContext<'a> {
           }
         }
         writeln!(writer, "{} {}", colors::bold("type:"), root.media_type)?;
-        let modules = self.graph.modules();
         let total_modules_size =
-          modules.iter().map(|m| m.size() as f64).sum::<f64>();
+          self.graph.modules().map(|m| m.size() as f64).sum::<f64>();
         let total_npm_package_size = self
           .npm_info
           .package_sizes
@@ -431,7 +430,8 @@ impl<'a> GraphDisplayContext<'a> {
           .map(|s| *s as f64)
           .sum::<f64>();
         let total_size = total_modules_size + total_npm_package_size;
-        let dep_count = modules.len() - 1 + self.npm_info.packages.len()
+        let dep_count = self.graph.modules().count() - 1
+          + self.npm_info.packages.len()
           - self.npm_info.resolved_reqs.len();
         writeln!(
           writer,
@@ -595,9 +595,6 @@ impl<'a> GraphDisplayContext<'a> {
   ) -> TreeNode {
     self.seen.insert(specifier.to_string());
     match err {
-      ModuleGraphError::InvalidSource(_, _) => {
-        self.build_error_msg(specifier, "(invalid source)")
-      }
       ModuleGraphError::InvalidTypeAssertion { .. } => {
         self.build_error_msg(specifier, "(invalid import assertion)")
       }

cli/tools/vendor/build.rs

@@ -3,18 +3,22 @@
 use std::fmt::Write as _;
 use std::path::Path;
 use std::path::PathBuf;
+use std::sync::Arc;
 
 use deno_ast::ModuleSpecifier;
 use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
+use deno_core::parking_lot::Mutex;
 use deno_graph::Module;
 use deno_graph::ModuleGraph;
 use deno_graph::ModuleKind;
 use import_map::ImportMap;
 use import_map::SpecifierMap;
 
+use crate::args::Lockfile;
 use crate::cache::ParsedSourceCache;
+use crate::graph_util::graph_lock_or_exit;
 
 use super::analyze::has_default_export;
 use super::import_map::build_import_map;
@@ -57,6 +61,7 @@ pub fn build(
   parsed_source_cache: &ParsedSourceCache,
   output_dir: &Path,
   original_import_map: Option<&ImportMap>,
+  maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
   environment: &impl VendorEnvironment,
 ) -> Result<usize, AnyError> {
   assert!(output_dir.is_absolute());
@@ -68,18 +73,20 @@ pub fn build(
   }
 
   // build the graph
-  graph.lock()?;
+  if let Some(lockfile) = maybe_lockfile {
+    graph_lock_or_exit(&graph, &mut lockfile.lock());
+  }
 
-  let graph_errors = graph.errors();
-  if !graph_errors.is_empty() {
-    for err in &graph_errors {
+  let mut graph_errors = graph.errors().peekable();
+  if graph_errors.peek().is_some() {
+    for err in graph_errors {
       log::error!("{}", err);
     }
     bail!("failed vendoring");
   }
 
   // figure out how to map remote modules to local
-  let all_modules = graph.modules();
+  let all_modules = graph.modules().collect::<Vec<_>>();
   let remote_modules = all_modules
     .iter()
     .filter(|m| is_remote_specifier(&m.specifier))

cli/tools/vendor/mod.rs

@@ -49,6 +49,7 @@ pub async fn vendor(
     &ps.parsed_source_cache,
     &output_dir,
     ps.maybe_import_map.as_deref(),
+    ps.lockfile.clone(),
     &build::RealVendorEnvironment,
   )?;
 

cli/tools/vendor/test.rs

@@ -234,6 +234,7 @@ impl VendorTestBuilder {
       &parsed_source_cache,
       &output_dir,
       self.original_import_map.as_ref(),
+      None,
       &self.environment,
     )?;
 
@@ -273,7 +274,6 @@ async fn build_test_graph(
       is_dynamic: false,
       imports: None,
       resolver: resolver.as_ref().map(|r| r.as_graph_resolver()),
-      locker: None,
       module_analyzer: Some(analyzer),
       reporter: None,
     },

cli/tsc/mod.rs

@@ -863,7 +863,6 @@ mod tests {
         is_dynamic: false,
         imports: None,
         resolver: None,
-        locker: None,
         module_analyzer: None,
         reporter: None,
       },
@@ -896,7 +895,6 @@ mod tests {
         is_dynamic: false,
         imports: None,
         resolver: None,
-        locker: None,
         module_analyzer: None,
         reporter: None,
       },