fix(lock): autodiscovery of lockfile (#16498)

This commit adds autodiscovery of lockfile. 

This only happens if Deno discovers the configuration file (either 
"deno.json" or "deno.jsonc"). In such case Deno tries to load
"deno.lock"
file that sits next to the configuration file, or creates one for user
if
the lockfile doesn't exist yet.

As a consequence, "--lock" and "--lock-write" flags had been updated.
"--lock" no longer requires a value, if one is not provided, it defaults
to "./deno.lock" resolved from the current working directory.
"--lock-write"
description was updated to say that it forces to overwrite a lockfile.

Autodiscovery is currently not handled by the LSP.

15 files changed, 283 insertions, 60 deletions

diff --git a/cli/args/flags.rs b/cli/args/flags.rs
index 90899704e..d024f6576 100644
--- a/cli/args/flags.rs
+++ b/cli/args/flags.rs
@@ -458,7 +458,9 @@ impl Flags {
       Some(files.clone())
     } else if let Run(RunFlags { script }) = &self.subcommand {
       if let Ok(module_specifier) = deno_core::resolve_url_or_path(script) {
-        if module_specifier.scheme() == "file" {
+        if module_specifier.scheme() == "file"
+          || module_specifier.scheme() == "npm"
+        {
           if let Ok(p) = module_specifier.to_file_path() {
             Some(vec![p])
           } else {
@@ -2145,16 +2147,17 @@ fn lock_arg<'a>() -> Arg<'a> {
   Arg::new("lock")
     .long("lock")
     .value_name("FILE")
-    .help("Check the specified lock file")
+    .help("Check the specified lock file. If value is not provided, defaults to \"deno.lock\" in the current working directory.")
     .takes_value(true)
+    .min_values(0)
+    .max_values(1)
     .value_hint(ValueHint::FilePath)
 }
 
 fn lock_write_arg<'a>() -> Arg<'a> {
   Arg::new("lock-write")
     .long("lock-write")
-    .requires("lock")
-    .help("Write lock file (use with --lock)")
+    .help("Force overwriting the lock file.")
 }
 
 static CONFIG_HELP: Lazy<String> = Lazy::new(|| {
@@ -3098,7 +3101,11 @@ fn lock_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
 
 fn lock_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
   if matches.is_present("lock") {
-    let lockfile = matches.value_of("lock").unwrap();
+    let lockfile = if let Some(path) = matches.value_of("lock") {
+      path
+    } else {
+      "./deno.lock"
+    };
     flags.lock = Some(PathBuf::from(lockfile));
   }
 }
@@ -5335,6 +5342,57 @@ mod tests {
         ..Flags::default()
       }
     );
+
+    let r = flags_from_vec(svec![
+      "deno",
+      "run",
+      "--lock",
+      "--lock-write",
+      "script.ts"
+    ]);
+    assert_eq!(
+      r.unwrap(),
+      Flags {
+        subcommand: DenoSubcommand::Run(RunFlags {
+          script: "script.ts".to_string(),
+        }),
+        lock_write: true,
+        lock: Some(PathBuf::from("./deno.lock")),
+        ..Flags::default()
+      }
+    );
+
+    let r = flags_from_vec(svec![
+      "deno",
+      "run",
+      "--lock-write",
+      "--lock",
+      "lock.json",
+      "script.ts"
+    ]);
+    assert_eq!(
+      r.unwrap(),
+      Flags {
+        subcommand: DenoSubcommand::Run(RunFlags {
+          script: "script.ts".to_string(),
+        }),
+        lock_write: true,
+        lock: Some(PathBuf::from("lock.json")),
+        ..Flags::default()
+      }
+    );
+
+    let r = flags_from_vec(svec!["deno", "run", "--lock-write", "script.ts"]);
+    assert_eq!(
+      r.unwrap(),
+      Flags {
+        subcommand: DenoSubcommand::Run(RunFlags {
+          script: "script.ts".to_string(),
+        }),
+        lock_write: true,
+        ..Flags::default()
+      }
+    );
   }
 
   #[test]
diff --git a/cli/args/mod.rs b/cli/args/mod.rs
index e46313858..64755a494 100644
--- a/cli/args/mod.rs
+++ b/cli/args/mod.rs
@@ -24,6 +24,7 @@ use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
 use deno_core::normalize_path;
+use deno_core::parking_lot::Mutex;
 use deno_core::url::Url;
 use deno_runtime::colors;
 use deno_runtime::deno_tls::rustls::RootCertStore;
@@ -33,6 +34,7 @@ use std::collections::BTreeMap;
 use std::env;
 use std::net::SocketAddr;
 use std::path::PathBuf;
+use std::sync::Arc;
 
 use crate::args::config_file::JsxImportSourceConfig;
 use crate::deno_dir::DenoDir;
@@ -61,11 +63,16 @@ pub struct CliOptions {
   // application need not concern itself with, so keep these private
   flags: Flags,
   maybe_config_file: Option<ConfigFile>,
+  maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
   overrides: CliOptionOverrides,
 }
 
 impl CliOptions {
-  pub fn new(flags: Flags, maybe_config_file: Option<ConfigFile>) -> Self {
+  pub fn new(
+    flags: Flags,
+    maybe_config_file: Option<ConfigFile>,
+    maybe_lockfile: Option<Lockfile>,
+  ) -> Self {
     if let Some(insecure_allowlist) =
       flags.unsafely_ignore_certificate_errors.as_ref()
     {
@@ -80,8 +87,11 @@ impl CliOptions {
       eprintln!("{}", colors::yellow(msg));
     }
 
+    let maybe_lockfile = maybe_lockfile.map(|l| Arc::new(Mutex::new(l)));
+
     Self {
       maybe_config_file,
+      maybe_lockfile,
       flags,
       overrides: Default::default(),
     }
@@ -89,7 +99,9 @@ impl CliOptions {
 
   pub fn from_flags(flags: Flags) -> Result<Self, AnyError> {
     let maybe_config_file = ConfigFile::discover(&flags)?;
-    Ok(Self::new(flags, maybe_config_file))
+    let maybe_lock_file =
+      Lockfile::discover(&flags, maybe_config_file.as_ref())?;
+    Ok(Self::new(flags, maybe_config_file, maybe_lock_file))
   }
 
   pub fn maybe_config_file_specifier(&self) -> Option<ModuleSpecifier> {
@@ -210,13 +222,8 @@ impl CliOptions {
       .map(|host| InspectorServer::new(host, version::get_user_agent()))
   }
 
-  pub fn resolve_lock_file(&self) -> Result<Option<Lockfile>, AnyError> {
-    if let Some(filename) = &self.flags.lock {
-      let lockfile = Lockfile::new(filename.clone(), self.flags.lock_write)?;
-      Ok(Some(lockfile))
-    } else {
-      Ok(None)
-    }
+  pub fn maybe_lock_file(&self) -> Option<Arc<Mutex<Lockfile>>> {
+    self.maybe_lockfile.clone()
   }
 
   pub fn resolve_tasks_config(
diff --git a/cli/lockfile.rs b/cli/lockfile.rs
index 78f5f2ab8..b0cf68917 100644
--- a/cli/lockfile.rs
+++ b/cli/lockfile.rs
@@ -15,9 +15,11 @@ use std::path::PathBuf;
 use std::rc::Rc;
 use std::sync::Arc;
 
+use crate::args::ConfigFile;
 use crate::npm::NpmPackageReq;
 use crate::npm::NpmResolutionPackage;
 use crate::tools::fmt::format_json;
+use crate::Flags;
 
 #[derive(Debug)]
 pub struct LockfileError(String);
@@ -73,6 +75,16 @@ pub struct LockfileContent {
   pub npm: NpmContent,
 }
 
+impl LockfileContent {
+  fn empty() -> Self {
+    Self {
+      version: "2".to_string(),
+      remote: BTreeMap::new(),
+      npm: NpmContent::default(),
+    }
+  }
+}
+
 #[derive(Debug, Clone)]
 pub struct Lockfile {
   pub overwrite: bool,
@@ -82,36 +94,91 @@ pub struct Lockfile {
 }
 
 impl Lockfile {
+  pub fn discover(
+    flags: &Flags,
+    maybe_config_file: Option<&ConfigFile>,
+  ) -> Result<Option<Lockfile>, AnyError> {
+    let filename = match flags.lock {
+      Some(ref lock) => PathBuf::from(lock),
+      None => match maybe_config_file {
+        Some(config_file) => {
+          if config_file.specifier.scheme() == "file" {
+            let mut path = config_file.specifier.to_file_path().unwrap();
+            path.set_file_name("deno.lock");
+            path
+          } else {
+            return Ok(None);
+          }
+        }
+        None => return Ok(None),
+      },
+    };
+
+    let lockfile = Self::new(filename, flags.lock_write)?;
+    Ok(Some(lockfile))
+  }
+
   pub fn new(filename: PathBuf, overwrite: bool) -> Result<Lockfile, AnyError> {
     // Writing a lock file always uses the new format.
-    let content = if overwrite {
+    if overwrite {
+      return Ok(Lockfile {
+        overwrite,
+        has_content_changed: false,
+        content: LockfileContent::empty(),
+        filename,
+      });
+    }
+
+    let result = match std::fs::read_to_string(&filename) {
+      Ok(content) => Ok(content),
+      Err(e) => {
+        if e.kind() == std::io::ErrorKind::NotFound {
+          return Ok(Lockfile {
+            overwrite,
+            has_content_changed: false,
+            content: LockfileContent::empty(),
+            filename,
+          });
+        } else {
+          Err(e)
+        }
+      }
+    };
+
+    let s = result.with_context(|| {
+      format!("Unable to read lockfile: \"{}\"", filename.display())
+    })?;
+    let value: serde_json::Value =
+      serde_json::from_str(&s).with_context(|| {
+        format!(
+          "Unable to parse contents of the lockfile \"{}\"",
+          filename.display()
+        )
+      })?;
+    let version = value.get("version").and_then(|v| v.as_str());
+    let content = if version == Some("2") {
+      serde_json::from_value::<LockfileContent>(value).with_context(|| {
+        format!(
+          "Unable to parse contents of the lockfile \"{}\"",
+          filename.display()
+        )
+      })?
+    } else {
+      // If there's no version field, we assume that user is using the old
+      // version of the lockfile. We'll migrate it in-place into v2 and it
+      // will be writte in v2 if user uses `--lock-write` flag.
+      let remote: BTreeMap<String, String> = serde_json::from_value(value)
+        .with_context(|| {
+          format!(
+            "Unable to parse contents of the lockfile \"{}\"",
+            filename.display()
+          )
+        })?;
       LockfileContent {
         version: "2".to_string(),
-        remote: BTreeMap::new(),
+        remote,
         npm: NpmContent::default(),
       }
-    } else {
-      let s = std::fs::read_to_string(&filename).with_context(|| {
-        format!("Unable to read lockfile: {}", filename.display())
-      })?;
-      let value: serde_json::Value = serde_json::from_str(&s)
-        .context("Unable to parse contents of the lockfile")?;
-      let version = value.get("version").and_then(|v| v.as_str());
-      if version == Some("2") {
-        serde_json::from_value::<LockfileContent>(value)
-          .context("Unable to parse lockfile")?
-      } else {
-        // If there's no version field, we assume that user is using the old
-        // version of the lockfile. We'll migrate it in-place into v2 and it
-        // will be writte in v2 if user uses `--lock-write` flag.
-        let remote: BTreeMap<String, String> =
-          serde_json::from_value(value).context("Unable to parse lockfile")?;
-        LockfileContent {
-          version: "2".to_string(),
-          remote,
-          npm: NpmContent::default(),
-        }
-      }
     };
 
     Ok(Lockfile {
@@ -209,10 +276,15 @@ impl Lockfile {
         .unwrap_or(&package.dist.shasum);
       if &package_info.integrity != integrity {
         return Err(LockfileError(format!(
-          "Integrity check failed for npm package: \"{}\".
-  Cache has \"{}\" and lockfile has \"{}\".
-  Use \"--lock-write\" flag to update the lockfile.",
-          package.id, integrity, package_info.integrity
+          "Integrity check failed for npm package: \"{}\". Unable to verify that the package
+is the same as when the lockfile was generated.
+
+This could be caused by:
+  * the lock file may be corrupt
+  * the source itself may be corrupt
+
+Use \"--lock-write\" flag to regenerate the lockfile at \"{}\".",
+          package.id, self.filename.display()
         )));
       }
     } else {
@@ -338,9 +410,9 @@ mod tests {
   }
 
   #[test]
-  fn new_nonexistent_lockfile() {
+  fn create_lockfile_for_nonexistent_path() {
     let file_path = PathBuf::from("nonexistent_lock_file.json");
-    assert!(Lockfile::new(file_path, false).is_err());
+    assert!(Lockfile::new(file_path, false).is_ok());
   }
 
   #[test]
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index a3f516615..3a0906636 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -2916,6 +2916,8 @@ impl Inner {
         ..Default::default()
       },
       self.maybe_config_file.clone(),
+      // TODO(#16510): add support for lockfile
+      None,
     );
     cli_options.set_import_map_specifier(self.maybe_import_map_uri.clone());
 
diff --git a/cli/proc_state.rs b/cli/proc_state.rs
index 3ea4214ab..148f44923 100644
--- a/cli/proc_state.rs
+++ b/cli/proc_state.rs
@@ -168,9 +168,7 @@ impl ProcState {
       Some(progress_bar.clone()),
     )?;
 
-    let lockfile = cli_options
-      .resolve_lock_file()?
-      .map(|f| Arc::new(Mutex::new(f)));
+    let lockfile = cli_options.maybe_lock_file();
     let maybe_import_map_specifier =
       cli_options.resolve_import_map_specifier()?;
 
@@ -296,7 +294,6 @@ impl ProcState {
         npm_package_reqs.push(package_ref.req);
       }
     }
-
     let roots = roots
       .into_iter()
       .map(|s| (s, ModuleKind::Esm))
@@ -312,6 +309,13 @@ impl ProcState {
           self.options.type_check_mode() != TypeCheckMode::None,
           false,
         ) {
+          // TODO(bartlomieju): this is strange... ideally there should be only
+          // one codepath in `prepare_module_load` so we don't forget things
+          // like writing a lockfile. Figure a way to refactor this function.
+          if let Some(ref lockfile) = self.lockfile {
+            let g = lockfile.lock();
+            g.write()?;
+          }
           return result;
         }
       }
diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs
index a5b443171..72af72a76 100644
--- a/cli/tests/integration/npm_tests.rs
+++ b/cli/tests/integration/npm_tests.rs
@@ -998,6 +998,54 @@ fn lock_file_missing_top_level_package() {
   );
 }
 
+#[test]
+fn auto_discover_lock_file() {
+  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", "{}");
+
+  // write a lock file with borked integrity
+  let lock_file_content = r#"{
+    "version": "2",
+    "remote": {},
+    "npm": {
+      "specifiers": { "@denotest/bin": "@denotest/bin@1.0.0" },
+      "packages": {
+        "@denotest/bin@1.0.0": {
+          "integrity": "sha512-foobar",
+          "dependencies": {}
+        }
+      }
+    }
+  }"#;
+  temp_dir.write("deno.lock", lock_file_content);
+
+  let deno = util::deno_cmd_with_deno_dir(&deno_dir)
+    .current_dir(temp_dir.path())
+    .arg("run")
+    .arg("--unstable")
+    .arg("-A")
+    .arg("npm:@denotest/bin/cli-esm")
+    .arg("test")
+    .envs(env_vars())
+    .stdout(Stdio::piped())
+    .stderr(Stdio::piped())
+    .spawn()
+    .unwrap();
+  let output = deno.wait_with_output().unwrap();
+  assert!(!output.status.success());
+  assert_eq!(output.status.code(), Some(10));
+
+  let stderr = String::from_utf8(output.stderr).unwrap();
+  assert!(stderr.contains(
+    "Integrity check failed for npm package: \"@denotest/bin@1.0.0\""
+  ));
+}
+
 fn env_vars_no_sync_download() -> Vec<(String, String)> {
   vec![
     ("DENO_NODE_COMPAT_URL".to_string(), util::std_file_url()),
diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs
index 99e122c69..68d15bfd9 100644
--- a/cli/tests/integration/run_tests.rs
+++ b/cli/tests/integration/run_tests.rs
@@ -609,12 +609,6 @@ itest!(private_field_presence_no_check {
   output: "run/private_field_presence.ts.out",
 });
 
-itest!(lock_write_requires_lock {
-  args: "run --lock-write some_file.ts",
-  output: "run/lock_write_requires_lock.out",
-  exit_code: 1,
-});
-
 // TODO(bartlomieju): remove --unstable once Deno.spawn is stabilized
 itest!(lock_write_fetch {
   args:
@@ -3634,3 +3628,10 @@ fn websocket_server_idletimeout() {
 
   assert!(child.wait().unwrap().success());
 }
+
+itest!(auto_discover_lockfile {
+  args: "run run/auto_discover_lockfile/main.ts",
+  output: "run/auto_discover_lockfile/main.out",
+  http_server: true,
+  exit_code: 10,
+});
diff --git a/cli/tests/testdata/jsx/deno.lock b/cli/tests/testdata/jsx/deno.lock
new file mode 100644
index 000000000..64429f30a
--- /dev/null
+++ b/cli/tests/testdata/jsx/deno.lock
@@ -0,0 +1,7 @@
+{
+  "version": "2",
+  "remote": {
+    "http://localhost:4545/jsx/jsx-dev-runtime": "7cac3d940791b3c8e671b24f9678ca37d87d40487ed2b3720a2a40891aa6173d",
+    "http://localhost:4545/jsx/jsx-runtime": "7cac3d940791b3c8e671b24f9678ca37d87d40487ed2b3720a2a40891aa6173d"
+  }
+}
diff --git a/cli/tests/testdata/npm/lock_file/main.out b/cli/tests/testdata/npm/lock_file/main.out
index 4c034d03b..b8447bee6 100644
--- a/cli/tests/testdata/npm/lock_file/main.out
+++ b/cli/tests/testdata/npm/lock_file/main.out
@@ -1,4 +1,9 @@
 Download [WILDCARD]
-error: Integrity check failed for npm package: "@babel/parser@7.19.0".
-  Cache has "sha512-74bEXKX2h+8rrfQUfsBfuZZHzsEs6Eql4pqy/T4Nn6Y9wNPggQOqD6z6pn5Bl8ZfysKouFZT/UXEH94ummEeQw==" and lockfile has "sha512-foobar!".
-  Use "--lock-write" flag to update the lockfile.
+error: Integrity check failed for npm package: "@babel/parser@7.19.0". Unable to verify that the package
+is the same as when the lockfile was generated.
+
+This could be caused by:
+  * the lock file may be corrupt
+  * the source itself may be corrupt
+
+Use "--lock-write" flag to regenerate the lockfile at "[WILDCARD]lock.json".
diff --git a/cli/tests/testdata/run/auto_discover_lockfile/deno.json b/cli/tests/testdata/run/auto_discover_lockfile/deno.json
new file mode 100644
index 000000000..90faa728a
--- /dev/null
+++ b/cli/tests/testdata/run/auto_discover_lockfile/deno.json
@@ -0,0 +1,3 @@
+{
+  "tasks": {}
+}
diff --git a/cli/tests/testdata/run/auto_discover_lockfile/deno.lock b/cli/tests/testdata/run/auto_discover_lockfile/deno.lock
new file mode 100644
index 000000000..059f66789
--- /dev/null
+++ b/cli/tests/testdata/run/auto_discover_lockfile/deno.lock
@@ -0,0 +1,7 @@
+{
+  "version": "2",
+  "remote": {
+    "http://localhost:4545/subdir/mod2.ts": "cae1d3e9f3c38cd415ff52dff854be8f3d17d35f8d7b3d285e813fb0f6393a2f",
+    "http://localhost:4545/subdir/print_hello.ts": "foobar"
+  }
+}
diff --git a/cli/tests/testdata/run/auto_discover_lockfile/main.out b/cli/tests/testdata/run/auto_discover_lockfile/main.out
new file mode 100644
index 000000000..28f4724e9
--- /dev/null
+++ b/cli/tests/testdata/run/auto_discover_lockfile/main.out
@@ -0,0 +1,5 @@
+Download http://localhost:4545/subdir/mod2.ts
+Download http://localhost:4545/subdir/print_hello.ts
+error: The source code is invalid, as it does not match the expected hash in the lock file.
+  Specifier: http://localhost:4545/subdir/print_hello.ts
+  Lock file: [WILDCARD]auto_discover_lockfile[WILDCARD]deno.lock
diff --git a/cli/tests/testdata/run/auto_discover_lockfile/main.ts b/cli/tests/testdata/run/auto_discover_lockfile/main.ts
new file mode 100644
index 000000000..baa52775d
--- /dev/null
+++ b/cli/tests/testdata/run/auto_discover_lockfile/main.ts
@@ -0,0 +1 @@
+import "http://localhost:4545/subdir/mod2.ts";
diff --git a/cli/tests/testdata/run/config_types/deno.lock b/cli/tests/testdata/run/config_types/deno.lock
new file mode 100644
index 000000000..157bd98a2
--- /dev/null
+++ b/cli/tests/testdata/run/config_types/deno.lock
@@ -0,0 +1,6 @@
+{
+  "version": "2",
+  "remote": {
+    "http://localhost:4545/run/config_types/types.d.ts": "741c39165e37de0c12acc5c081841f4362487e3f17dc4cad7017b70af72c4605"
+  }
+}
diff --git a/cli/tests/testdata/run/lock_write_requires_lock.out b/cli/tests/testdata/run/lock_write_requires_lock.out
deleted file mode 100644
index 7cc5906f6..000000000
--- a/cli/tests/testdata/run/lock_write_requires_lock.out
+++ /dev/null
@@ -1,3 +0,0 @@
-error: The following required arguments were not provided:
-    --lock <FILE>
-[WILDCARD]
\ No newline at end of file