fix(lsp): set lockfile_skip_write unconditionally (#31191)

Author: nayeemrmn

cli/lsp/language_server.rs

@@ -379,7 +379,6 @@ impl LanguageServer {
     specifiers: Vec<ModuleSpecifier>,
     referrer: ModuleSpecifier,
     force_global_cache: bool,
-    lockfile_skip_write: bool,
   ) -> LspResult<Option<Value>> {
     async fn create_graph_for_caching(
       factory: CliFactory,
@@ -419,15 +418,6 @@ impl LanguageServer {
           allow_unknown_jsr_exports: false,
         },
       )?;
-
-      // Update the lockfile on the file system with anything new
-      // found after caching
-      if let Ok(Some(lockfile)) = factory.maybe_lockfile().await
-        && let Err(err) = &lockfile.write_if_changed()
-      {
-        lsp_warn!("{:#}", err);
-      }
-
       Ok(())
     }
 
@@ -442,7 +432,6 @@ impl LanguageServer {
       specifiers,
       referrer,
       force_global_cache,
-      lockfile_skip_write,
     );
 
     match prepare_cache_result {
@@ -462,12 +451,7 @@ impl LanguageServer {
 
         // now get the lock back to update with the new information
         *self.did_change_batch_queue.borrow_mut() = None;
-        self
-          .inner
-          .write()
-          .await
-          .post_cache(!lockfile_skip_write)
-          .await;
+        self.inner.write().await.post_cache().await;
         self.performance.measure(mark);
       }
       Err(err) => {
@@ -1278,9 +1262,7 @@ impl Inner {
             };
             specifier
           };
-          if let Err(err) =
-            ls.cache(vec![specifier], referrer, false, true).await
-          {
+          if let Err(err) = ls.cache(vec![specifier], referrer, false).await {
             lsp_warn!("{:#}", err);
           }
         });
@@ -1477,7 +1459,7 @@ impl Inner {
     self.task_queue.queue_task(Box::new(|ls: LanguageServer| {
       spawn(async move {
         if let Err(err) = ls
-          .cache(vec![], module.specifier.as_ref().clone(), false, true)
+          .cache(vec![], module.specifier.as_ref().clone(), false)
           .await
         {
           lsp_warn!(
@@ -4175,7 +4157,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
         serde_json::from_value(json!(params.arguments))
           .map_err(|err| LspError::invalid_params(err.to_string()))?;
       self
-        .cache(specifiers, referrer, options.force_global_cache, false)
+        .cache(specifiers, referrer, options.force_global_cache)
         .await
     } else if params.command == "deno.reloadImportRegistries" {
       *self.did_change_batch_queue.borrow_mut() = None;
@@ -4784,7 +4766,6 @@ impl Inner {
     specifiers: Vec<ModuleSpecifier>,
     referrer: ModuleSpecifier,
     force_global_cache: bool,
-    lockfile_skip_write: bool,
   ) -> Result<PrepareCacheResult, AnyError> {
     let config_data = self.config.tree.data_for_specifier(&referrer);
     let scope = config_data.map(|d| d.scope.clone());
@@ -4816,7 +4797,7 @@ impl Inner {
     let mut cli_factory = CliFactory::from_flags(Arc::new(Flags {
       internal: InternalFlags {
         cache_path: Some(self.cache.deno_dir().root.clone()),
-        lockfile_skip_write,
+        lockfile_skip_write: true,
         ..Default::default()
       },
       ca_stores: workspace_settings.certificate_stores.clone(),
@@ -4868,22 +4849,15 @@ impl Inner {
   }
 
   #[cfg_attr(feature = "lsp-tracing", tracing::instrument(skip_all))]
-  async fn post_cache(&mut self, did_write_lockfile: bool) {
-    if did_write_lockfile {
-      // Most of the refresh steps will happen in `did_change_watched_files()`,
-      // since the lockfile was written.
-      self.resolver.did_cache();
-      self.refresh_dep_info();
-    } else {
-      self.refresh_config_tree().await;
-      self.update_cache();
-      self.refresh_resolver().await;
-      self.refresh_compiler_options_resolver();
-      self.refresh_linter_resolver();
-      self.refresh_documents_config();
-      self.resolver.did_cache();
-      self.refresh_dep_info();
-    }
+  async fn post_cache(&mut self) {
+    self.refresh_config_tree().await;
+    self.update_cache();
+    self.refresh_resolver().await;
+    self.refresh_compiler_options_resolver();
+    self.refresh_linter_resolver();
+    self.refresh_documents_config();
+    self.resolver.did_cache();
+    self.refresh_dep_info();
     self.project_changed(vec![], ProjectScopesChange::Config);
     self.ts_server.cleanup_semantic_cache(self.snapshot()).await;
     self.send_diagnostics_update();

tests/integration/lsp_tests.rs

@@ -15579,14 +15579,9 @@ fn lsp_node_modules_dir() {
 
   assert!(!temp_dir.path().join("node_modules").exists());
 
-  // a lockfile will be created here because someone did an explicit cache
-  let lockfile_path = temp_dir.path().join("deno.lock");
-  assert!(lockfile_path.exists());
-  lockfile_path.remove_file();
-
   temp_dir.write(
     temp_dir.path().join("deno.json"),
-    "{ \"nodeModulesDir\": \"auto\", \"lock\": false }\n",
+    "{ \"nodeModulesDir\": \"auto\" }\n",
   );
   let refresh_config = |client: &mut LspClient| {
     client.change_configuration(json!({ "deno": {
@@ -15616,7 +15611,6 @@ fn lsp_node_modules_dir() {
 
   assert!(temp_dir.path().join("node_modules/chalk").exists());
   assert!(temp_dir.path().join("node_modules/@types/node").exists());
-  assert!(!lockfile_path.exists()); // was disabled
 
   // now add a lockfile and cache
   temp_dir.write(
@@ -15627,8 +15621,6 @@ fn lsp_node_modules_dir() {
   let diagnostics = cache(&mut client);
   assert_eq!(diagnostics.all().len(), 0, "{:#?}", diagnostics);
 
-  assert!(lockfile_path.exists());
-
   // the declaration should be found in the node_modules directory
   let res = client.write_request(
     "textDocument/references",