fix(els): file data inconsistencies

mtshiba · mtshiba · commit b0959b13e75f · 2023-03-31T19:49:49.000+09:00
diff --git a/crates/els/file_cache.rs b/crates/els/file_cache.rs
@@ -28,6 +28,7 @@ pub fn _get_code_from_uri(uri: &Url) -> ELSResult<String> {
 #[derive(Debug, Clone)]
 pub struct FileCacheEntry {
     pub code: String,
+    pub ver: i32,
     pub metadata: Metadata,
     pub token_stream: Option<TokenStream>,
 }
@@ -102,25 +103,11 @@ impl FileCache {
     pub fn get(&self, uri: &NormalizedUrl) -> ELSResult<&FileCacheEntry> {
         let Some(entry) = unsafe { self.files.as_ref() }.get(uri) else {
             let code = _get_code_from_uri(uri)?;
-            self.update(uri, code);
+            self.update(uri, code, None);
             let entry = unsafe { self.files.as_ref() }.get(uri).ok_or("not found")?;
             return Ok(entry);
         };
-        let last_modified = entry.metadata.modified().unwrap();
-        let current_modified = metadata(uri.to_file_path().unwrap())
-            .unwrap()
-            .modified()
-            .unwrap();
-        if last_modified != current_modified {
-            let code = _get_code_from_uri(uri)?;
-            self.update(uri, code);
-            unsafe { self.files.as_ref() }
-                .get(uri)
-                .ok_or("not found".into())
-        } else {
-            let entry = unsafe { self.files.as_ref() }.get(uri).ok_or("not found")?;
-            Ok(entry)
-        }
+        Ok(entry)
     }
 
     pub fn get_token_stream(&self, uri: &NormalizedUrl) -> Option<&TokenStream> {
@@ -163,13 +150,28 @@ impl FileCache {
         }
     }
 
-    pub(crate) fn update(&self, uri: &NormalizedUrl, code: String) {
+    pub(crate) fn update(&self, uri: &NormalizedUrl, code: String, ver: Option<i32>) {
+        let entry = unsafe { self.files.as_ref() }.get(uri);
+        if let Some(entry) = entry {
+            if ver.map_or(false, |ver| ver <= entry.ver) {
+                // crate::_log!("171: double update detected: {ver:?}, {}, code:\n{}", entry.ver, entry.code);
+                return;
+            }
+        }
         let metadata = metadata(uri.to_file_path().unwrap()).unwrap();
         let token_stream = Lexer::from_str(code.clone()).lex().ok();
+        let ver = ver.unwrap_or({
+            if let Some(entry) = entry {
+                entry.ver
+            } else {
+                1
+            }
+        });
         self.files.borrow_mut().insert(
             uri.clone(),
             FileCacheEntry {
                 code,
+                ver,
                 metadata,
                 token_stream,
             },
@@ -187,6 +189,7 @@ impl FileCache {
         code.replace_range(start..end, new_code);
         let token_stream = Lexer::from_str(code.clone()).lex().ok();
         entry.code = code;
+        // entry.ver += 1;
         entry.metadata = metadata;
         entry.token_stream = token_stream;
     }
@@ -196,6 +199,10 @@ impl FileCache {
         let Some(entry) = unsafe { self.files.as_mut() }.get_mut(&uri) else {
             return;
         };
+        if entry.ver >= params.text_document.version {
+            // crate::_log!("212: double update detected {}, {}, code:\n{}", entry.ver, params.text_document.version, entry.code);
+            return;
+        }
         let metadata = metadata(uri.to_file_path().unwrap()).unwrap();
         let mut code = entry.code.clone();
         for change in params.content_changes {
@@ -206,6 +213,7 @@ impl FileCache {
         }
         let token_stream = Lexer::from_str(code.clone()).lex().ok();
         entry.code = code;
+        entry.ver = params.text_document.version;
         entry.metadata = metadata;
         entry.token_stream = token_stream;
     }
diff --git a/crates/els/server.rs b/crates/els/server.rs
@@ -23,9 +23,9 @@ use erg_compiler::ty::HasType;
 
 use lsp_types::{
     ClientCapabilities, CodeActionKind, CodeActionOptions, CodeActionProviderCapability,
-    CodeLensOptions, CompletionOptions, DidChangeTextDocumentParams, ExecuteCommandOptions,
-    HoverProviderCapability, InitializeResult, OneOf, Position, SemanticTokenType,
-    SemanticTokensFullOptions, SemanticTokensLegend, SemanticTokensOptions,
+    CodeLensOptions, CompletionOptions, DidChangeTextDocumentParams, DidOpenTextDocumentParams,
+    ExecuteCommandOptions, HoverProviderCapability, InitializeResult, OneOf, Position,
+    SemanticTokenType, SemanticTokensFullOptions, SemanticTokensLegend, SemanticTokensOptions,
     SemanticTokensServerCapabilities, ServerCapabilities, SignatureHelpOptions,
     WorkDoneProgressOptions,
 };
@@ -104,9 +104,10 @@ impl From<&str> for OptionalFeatures {
     }
 }
 
+#[macro_export]
 macro_rules! _log {
     ($($arg:tt)*) => {
-        send_log(format!($($arg)*)).unwrap();
+        $crate::server::send_log(format!($($arg)*)).unwrap();
     };
 }
 
@@ -453,11 +454,12 @@ impl<Checker: BuildRunnable> Server<Checker> {
             "initialized" => send_log("successfully bound"),
             "exit" => self.exit(),
             "textDocument/didOpen" => {
-                let uri =
-                    NormalizedUrl::parse(msg["params"]["textDocument"]["uri"].as_str().unwrap())?;
+                let params = DidOpenTextDocumentParams::deserialize(msg["params"].clone())?;
+                let uri = NormalizedUrl::new(params.text_document.uri);
                 send_log(format!("{method}: {uri}"))?;
-                let code = msg["params"]["textDocument"]["text"].as_str().unwrap();
-                self.file_cache.update(&uri, code.to_string());
+                let code = params.text_document.text;
+                let ver = params.text_document.version;
+                self.file_cache.update(&uri, code.clone(), Some(ver));
                 self.check_file(uri, code)
             }
             "textDocument/didSave" => {
@@ -471,7 +473,6 @@ impl<Checker: BuildRunnable> Server<Checker> {
             "textDocument/didChange" => {
                 let params = DidChangeTextDocumentParams::deserialize(msg["params"].clone())?;
                 self.file_cache.incremental_update(params.clone());
-                send_log("file cache updated")?;
                 if self.opt_features.contains(&OptionalFeatures::CheckOnType) {
                     let uri = NormalizedUrl::new(params.text_document.uri);
                     self.quick_check_file(uri)?;
@@ -580,7 +581,7 @@ impl<Checker: BuildRunnable> Server<Checker> {
             let path = util::uri_to_path(uri);
             shared.mod_cache.remove(&path);
             shared.index.remove_path(&path);
-            shared.graph.initialize();
+            shared.graph.remove(&path);
         }
     }
 }
diff --git a/crates/erg_common/tsort.rs b/crates/erg_common/tsort.rs
@@ -22,7 +22,7 @@ impl std::error::Error for TopoSortError {}
 pub struct Node<T: Eq + Hash, U> {
     pub id: T,
     pub data: U,
-    depends_on: Set<T>,
+    pub depends_on: Set<T>,
 }
 
 impl<T: Eq + Hash, U> Node<T, U> {
diff --git a/crates/erg_compiler/module/graph.rs b/crates/erg_compiler/module/graph.rs
@@ -1,3 +1,4 @@
+use std::fmt;
 use std::path::{Path, PathBuf};
 
 use erg_common::shared::Shared;
@@ -7,6 +8,19 @@ use erg_common::{normalize_path, set};
 #[derive(Debug, Clone, Default)]
 pub struct ModuleGraph(Graph<PathBuf, ()>);
 
+impl fmt::Display for ModuleGraph {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for node in self.0.iter() {
+            writeln!(f, "{} depends on {{", node.id.display())?;
+            for dep in node.depends_on.iter() {
+                writeln!(f, "{}, ", dep.display())?;
+            }
+            writeln!(f, "}}, ")?;
+        }
+        Ok(())
+    }
+}
+
 impl IntoIterator for ModuleGraph {
     type Item = Node<PathBuf, ()>;
     type IntoIter = std::vec::IntoIter<Self::Item>;
@@ -62,6 +76,11 @@ impl ModuleGraph {
         Ok(())
     }
 
+    pub fn remove(&mut self, path: &Path) {
+        let path = normalize_path(path.to_path_buf());
+        self.0.retain(|n| n.id != path);
+    }
+
     pub fn initialize(&mut self) {
         self.0.clear();
     }
@@ -70,6 +89,12 @@ impl ModuleGraph {
 #[derive(Debug, Clone, Default)]
 pub struct SharedModuleGraph(Shared<ModuleGraph>);
 
+impl fmt::Display for SharedModuleGraph {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.0.borrow())
+    }
+}
+
 impl IntoIterator for SharedModuleGraph {
     type Item = Node<PathBuf, ()>;
     type IntoIter = std::vec::IntoIter<Self::Item>;
@@ -104,6 +129,10 @@ impl SharedModuleGraph {
         ref_graph.iter()
     }
 
+    pub fn remove(&self, path: &Path) {
+        self.0.borrow_mut().remove(path);
+    }
+
     #[allow(clippy::result_unit_err)]
     pub fn sort(&self) -> Result<(), TopoSortError> {
         self.0.borrow_mut().sort()

Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ impl std::error::Error for TopoSortError {}`
`22`	`22`	`pub struct Node<T: Eq + Hash, U> {`
`23`	`23`	`pub id: T,`
`24`	`24`	`pub data: U,`
`25`		`- depends_on: Set<T>,`
	`25`	`+ pub depends_on: Set<T>,`
`26`	`26`	`}`
`27`	`27`
`28`	`28`	`impl<T: Eq + Hash, U> Node<T, U> {`