Repository: denoland/deno
Pull request: fix(lsp): ensure project version is incremented when config changes

I'm running into a node resolution bug in the lsp only and while tracking it down I noticed this one.

Fixed by moving the project version out of `Documents`.

Changed files:
filename: cli/lsp/diagnostics.rs
patch: @@ -1628,6 +1628,7 @@ mod tests {
       config.tree.inject_config_file(config_file).await;
     }
     StateSnapshot {
+      project_version: 0,
       documents,
       assets: Default::default(),
       cache_metadata: cache::CacheMetadata::new(Arc::new(

filename: cli/lsp/documents.rs
patch: @@ -851,7 +851,6 @@ pub struct Documents {
   redirect_resolver: Arc<RedirectResolver>,
   /// If --unstable-sloppy-imports is enabled.
   unstable_sloppy_imports: bool,
-  project_version: usize,
 }
 
 impl Documents {
@@ -878,7 +877,6 @@ impl Documents {
       has_injected_types_node_package: false,
       redirect_resolver: Arc::new(RedirectResolver::new(cache)),
       unstable_sloppy_imports: false,
-      project_version: 0,
     }
   }
 
@@ -890,14 +888,6 @@ impl Documents {
       .flat_map(|value| value.get_type().or_else(|| value.get_code()))
   }
 
-  pub fn project_version(&self) -> String {
-    self.project_version.to_string()
-  }
-
-  pub fn increment_project_version(&mut self) {
-    self.project_version += 1;
-  }
-
   /// "Open" a document from the perspective of the editor, meaning that
   /// requests for information from the document will come from the in-memory
   /// representation received from the language server client, versus reading
@@ -925,7 +915,6 @@ impl Documents {
     self.file_system_docs.set_dirty(true);
 
     self.open_docs.insert(specifier, document.clone());
-    self.increment_project_version();
     self.dirty = true;
     document
   }
@@ -957,7 +946,6 @@ impl Documents {
       self.get_npm_resolver(),
     )?;
     self.open_docs.insert(doc.specifier().clone(), doc.clone());
-    self.increment_project_version();
     Ok(doc)
   }
 
@@ -985,7 +973,6 @@ impl Documents {
         .docs
         .insert(specifier.clone(), document);
 
-      self.increment_project_version();
       self.dirty = true;
     }
     Ok(())

filename: cli/lsp/language_server.rs
patch: @@ -176,6 +176,7 @@ pub struct StateNpmSnapshot {
 /// Snapshot of the state used by TSC.
 #[derive(Clone, Debug)]
 pub struct StateSnapshot {
+  pub project_version: usize,
   pub assets: AssetsSnapshot,
   pub cache_metadata: cache::CacheMetadata,
   pub config: Arc<ConfigSnapshot>,
@@ -254,6 +255,7 @@ pub struct Inner {
   maybe_testing_server: Option<testing::TestServer>,
   /// Services used for dealing with npm related functionality.
   npm: LspNpmServices,
+  project_version: usize,
   /// A collection of measurements which instrument that performance of the LSP.
   performance: Arc<Performance>,
   /// A memoized version of fixable diagnostic codes retrieved from TypeScript.
@@ -537,6 +539,7 @@ impl Inner {
       initial_cwd: initial_cwd.clone(),
       jsr_search_api,
       maybe_global_cache_path: None,
+      project_version: 0,
       task_queue: Default::default(),
       maybe_testing_server: None,
       module_registries,
@@ -669,6 +672,7 @@ impl Inner {
         }
       });
     Arc::new(StateSnapshot {
+      project_version: self.project_version,
       assets: self.assets.snapshot(),
       cache_metadata: self.cache_metadata.clone(),
       config: self.config.snapshot(),
@@ -1190,15 +1194,7 @@ impl Inner {
     // a @types/node package and now's a good time to do that anyway
     self.refresh_npm_specifiers().await;
 
-    self
-      .ts_server
-      .project_changed(
-        self.snapshot(),
-        &[],
-        self.documents.project_version(),
-        true,
-      )
-      .await;
+    self.project_changed(&[], true).await;
   }
 
   fn shutdown(&self) -> LspResult<()> {
@@ -1233,15 +1229,8 @@ impl Inner {
       params.text_document.language_id.parse().unwrap(),
       params.text_document.text.into(),
     );
-    let version = self.documents.project_version();
     self
-      .ts_server
-      .project_changed(
-        self.snapshot(),
-        &[(document.specifier(), ChangeKind::Opened)],
-        version,
-        false,
-      )
+      .project_changed(&[(document.specifier(), ChangeKind::Opened)], false)
       .await;
 
     self.performance.measure(mark);
@@ -1260,13 +1249,9 @@ impl Inner {
     ) {
       Ok(document) => {
         if document.is_diagnosable() {
-          let version = self.documents.project_version();
           self
-            .ts_server
             .project_changed(
-              self.snapshot(),
               &[(document.specifier(), ChangeKind::Modified)],
-              version,
               false,
             )
             .await;
@@ -1320,15 +1305,8 @@ impl Inner {
     if let Err(err) = self.documents.close(&specifier) {
       error!("{:#}", err);
     }
-    let version = self.documents.project_version();
     self
-      .ts_server
-      .project_changed(
-        self.snapshot(),
-        &[(&specifier, ChangeKind::Closed)],
-        version,
-        false,
-      )
+      .project_changed(&[(&specifier, ChangeKind::Closed)], false)
       .await;
     self.performance.measure(mark);
   }
@@ -3015,6 +2993,18 @@ impl Inner {
     Ok(maybe_symbol_information)
   }
 
+  async fn project_changed(
+    &mut self,
+    modified_scripts: &[(&ModuleSpecifier, ChangeKind)],
+    config_changed: bool,
+  ) {
+    self.project_version += 1; // increment before getting the snapshot
+    self
+      .ts_server
+      .project_changed(self.snapshot(), modified_scripts, config_changed)
+      .await;
+  }
+
   fn send_diagnostics_update(&self) {
     let snapshot = DiagnosticServerUpdateMessage {
       snapshot: self.snapshot(),

filename: cli/lsp/tsc.rs
patch: @@ -306,7 +306,6 @@ impl TsServer {
     &self,
     snapshot: Arc<StateSnapshot>,
     modified_scripts: &[(&ModuleSpecifier, ChangeKind)],
-    new_project_version: String,
     config_changed: bool,
   ) {
     let modified_scripts = modified_scripts
@@ -315,7 +314,9 @@ impl TsServer {
       .collect::<Vec<_>>();
     let req = TscRequest {
       method: "$projectChanged",
-      args: json!([modified_scripts, new_project_version, config_changed,]),
+      args: json!(
+        [modified_scripts, snapshot.project_version, config_changed,]
+      ),
     };
     self
       .request::<()>(snapshot, req)
@@ -340,7 +341,7 @@ impl TsServer {
           .into_iter()
           .map(|s| self.specifier_map.denormalize(&s))
           .collect::<Vec<String>>(),
-        snapshot.documents.project_version()
+        snapshot.project_version,
       ]),
     };
     let raw_diagnostics = self.request_with_cancellation::<HashMap<String, Vec<crate::tsc::Diagnostic>>>(snapshot, req, token).await?;
@@ -4153,12 +4154,12 @@ fn op_ts_config(state: &mut OpState) -> serde_json::Value {
   r
 }
 
-#[op2]
-#[string]
-fn op_project_version(state: &mut OpState) -> String {
+#[op2(fast)]
+#[number]
+fn op_project_version(state: &mut OpState) -> usize {
   let state: &mut State = state.borrow_mut::<State>();
   let mark = state.performance.mark("tsc.op.op_project_version");
-  let r = state.state_snapshot.documents.project_version();
+  let r = state.state_snapshot.project_version;
   state.performance.measure(mark);
   r
 }
@@ -4253,6 +4254,7 @@ deno_core::extension!(deno_tsc,
   state = |state, options| {
     state.put(State::new(
       Arc::new(StateSnapshot {
+        project_version: 0,
         assets: Default::default(),
         cache_metadata: CacheMetadata::new(options.cache.clone()),
         config: Default::default(),
@@ -4723,6 +4725,7 @@ mod tests {
       )
       .await;
     StateSnapshot {
+      project_version: 0,
       documents,
       assets: Default::default(),
       cache_metadata: CacheMetadata::new(cache),
@@ -5178,18 +5181,15 @@ mod tests {
       )
       .unwrap();
     let snapshot = {
-      let mut documents = snapshot.documents.clone();
-      documents.increment_project_version();
       Arc::new(StateSnapshot {
-        documents,
+        project_version: snapshot.project_version + 1,
         ..snapshot.as_ref().clone()
       })
     };
     ts_server
       .project_changed(
         snapshot.clone(),
         &[(&specifier_dep, ChangeKind::Opened)],
-        snapshot.documents.project_version(),
         false,
       )
       .await;

filename: cli/tsc/99_main_compiler.js
patch: @@ -165,7 +165,7 @@ delete Object.prototype.__proto__;
   /** @type {ts.CompilerOptions | null} */
   let tsConfigCache = null;
 
-  /** @type {string | null} */
+  /** @type {number | null} */
   let projectVersionCache = null;
 
   const ChangeKind = {
@@ -1039,7 +1039,7 @@ delete Object.prototype.__proto__;
       case "$projectChanged": {
         /** @type {[string, number][]} */
         const changedScripts = args[0];
-        /** @type {string} */
+        /** @type {number} */
         const newProjectVersion = args[1];
         /** @type {boolean} */
         const configChanged = args[2];