Review comments

lsp/nls/src/files.rs

@@ -82,6 +82,10 @@ pub fn handle_save(server: &mut Server, params: DidChangeTextDocumentParams) ->
     let invalid = server.cache.get_rev_imports_transitive(file_id);
     for f in &invalid {
         server.lin_registry.map.remove(f);
+        server.lin_registry.position_lookups.remove(f);
+        server.lin_registry.usage_lookups.remove(f);
+        server.lin_registry.parent_lookups.remove(f);
+        server.lin_registry.type_lookups.remove(f);
     }
 
     // TODO: make this part more abstracted

lsp/nls/src/linearization/mod.rs

@@ -91,12 +91,17 @@ impl<'a> Iterator for ParentChainIter<'a> {
 pub struct LinRegistry {
     pub map: HashMap<FileId, Completed>,
     // TODO: these are supposed to eventually *replace* part of the linearization, at
-    // which point we'll rename `LinRegistry` (and probably just have one HashMap<FileId,
-    // everything>)
+    // which point we'll rename `LinRegistry` (and probably just have one
+    // HashMap<FileId, everything>)
+    //
+    // Most of these tables do one more lookup than necessary: they look up a
+    // file id and then they look up a term in an inner table. This is a little
+    // inefficient for lookups, but it makes it easy to invalidate a whole file
+    // in one go.
     pub position_lookups: HashMap<FileId, PositionLookup>,
     pub usage_lookups: HashMap<FileId, UsageLookup>,
     pub parent_lookups: HashMap<FileId, ParentLookup>,
-    pub type_lookups: HashMap<RichTermPtr, Type>,
+    pub type_lookups: HashMap<FileId, HashMap<RichTermPtr, Type>>,
 }
 
 impl LinRegistry {
@@ -116,8 +121,7 @@ impl LinRegistry {
             .insert(file_id, PositionLookup::new(term));
         self.usage_lookups.insert(file_id, UsageLookup::new(term));
         self.parent_lookups.insert(file_id, ParentLookup::new(term));
-
-        self.type_lookups.extend(type_lookups);
+        self.type_lookups.insert(file_id, type_lookups);
     }
 
     /// Look for the linearization corresponding to an item's id, and return the corresponding item
@@ -138,7 +142,8 @@ impl LinRegistry {
     }
 
     pub fn get_type(&self, rt: &RichTerm) -> Option<&Type> {
-        self.type_lookups.get(&RichTermPtr(rt.clone()))
+        let file = rt.pos.as_opt_ref()?.src_id;
+        self.type_lookups.get(&file)?.get(&RichTermPtr(rt.clone()))
     }
 
     pub fn get_parent_chain<'a>(&'a self, rt: &'a RichTerm) -> Option<ParentChainIter<'a>> {

lsp/nls/src/term.rs

@@ -6,19 +6,19 @@ use nickel_lang_core::{
     term::{RichTerm, SharedTerm, Term},
 };
 
-// A term that uses source position and a pointer to Term to implement Eq and Hash.
+// A term that uses a pointer to Term to implement Eq and Hash.
 #[derive(Clone, Debug)]
 pub struct RichTermPtr(pub RichTerm);
 
 impl PartialEq for RichTermPtr {
     fn eq(&self, other: &Self) -> bool {
-        self.0.pos == other.0.pos && SharedTerm::ptr_eq(&self.0.term, &other.0.term)
+        SharedTerm::ptr_eq(&self.0.term, &other.0.term)
     }
 }
 
 impl Hash for RichTermPtr {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
-        (self.0.pos, self.0.term.as_ref() as *const Term).hash(state)
+        (self.0.term.as_ref() as *const Term).hash(state)
     }
 }