Move goto away from the linearizer (#1610)

* Move goto away from the linearizer

* Add regression test for 881

* Add tests, add ident lookup

* Review comments

* Better name

core/src/error.rs

@@ -1934,17 +1934,17 @@ impl IntoDiagnostics<FileId> for TypecheckError {
                 };
 
                 let note1 = if let TypeF::Record(rrows) = &expd.typ {
-                    match rrows.row_find_path(path.as_slice()) {
-                        Some(ty) => mk_expected_row_msg(&field, ty),
+                    match rrows.find_path(path.as_slice()) {
+                        Some(row) => mk_expected_row_msg(&field, row.typ),
                         None => mk_expected_msg(&expd),
                     }
                 } else {
                     mk_expected_msg(&expd)
                 };
 
                 let note2 = if let TypeF::Record(rrows) = &actual.typ {
-                    match rrows.row_find_path(path.as_slice()) {
-                        Some(ty) => mk_inferred_row_msg(&field, ty),
+                    match rrows.find_path(path.as_slice()) {
+                        Some(row) => mk_inferred_row_msg(&field, row.typ),
                         None => mk_inferred_msg(&actual),
                     }
                 } else {

core/src/typ.rs

@@ -852,21 +852,21 @@ impl RecordRows {
     /// - self: ` {a : {b : Number }}`
     /// - path: `["a", "b"]`
     /// - result: `Some(Number)`
-    pub fn row_find_path(&self, path: &[Ident]) -> Option<Type> {
+    pub fn find_path(&self, path: &[Ident]) -> Option<RecordRowF<&Type>> {
         if path.is_empty() {
             return None;
         }
 
         let next = self.iter().find_map(|item| match item {
-            RecordRowsIteratorItem::Row(row) if row.id.ident() == path[0] => Some(row.typ.clone()),
+            RecordRowsIteratorItem::Row(row) if row.id.ident() == path[0] => Some(row.clone()),
             _ => None,
         });
 
         if path.len() == 1 {
             next
         } else {
-            match next.map(|ty| ty.typ) {
-                Some(TypeF::Record(rrows)) => rrows.row_find_path(&path[1..]),
+            match next.map(|row| &row.typ.typ) {
+                Some(TypeF::Record(rrows)) => rrows.find_path(&path[1..]),
                 _ => None,
             }
         }

lsp/lsp-harness/src/lib.rs

@@ -4,7 +4,8 @@ mod output;
 pub use jsonrpc::Server;
 use log::error;
 use lsp_types::{
-    CompletionParams, DocumentFormattingParams, GotoDefinitionParams, HoverParams, Url,
+    CompletionParams, DocumentFormattingParams, GotoDefinitionParams, HoverParams, ReferenceParams,
+    Url,
 };
 pub use output::LspDebug;
 use serde::Deserialize;
@@ -30,6 +31,7 @@ pub struct TestFile {
 #[serde(tag = "type")]
 pub enum Request {
     GotoDefinition(GotoDefinitionParams),
+    References(ReferenceParams),
     Completion(CompletionParams),
     Formatting(DocumentFormattingParams),
     Hover(HoverParams),

lsp/lsp-harness/src/output.rs

@@ -48,6 +48,12 @@ impl<T: LspDebug, I: Iterator<Item = T> + Clone> LspDebug for Iter<I> {
     }
 }
 
+impl<T: LspDebug> LspDebug for Vec<T> {
+    fn debug(&self, w: impl Write) -> std::io::Result<()> {
+        Iter(self.iter()).debug(w)
+    }
+}
+
 impl LspDebug for lsp_types::Range {
     fn debug(&self, mut w: impl Write) -> std::io::Result<()> {
         write!(
@@ -116,12 +122,6 @@ impl LspDebug for lsp_types::TextEdit {
     }
 }
 
-impl LspDebug for Vec<lsp_types::TextEdit> {
-    fn debug(&self, w: impl Write) -> std::io::Result<()> {
-        Iter(self.iter()).debug(w)
-    }
-}
-
 impl LspDebug for lsp_types::Hover {
     fn debug(&self, mut w: impl Write) -> std::io::Result<()> {
         write!(w, "<{}>", self.range.debug_str())?;

lsp/nls/src/diagnostic.rs

@@ -3,6 +3,7 @@ use std::ops::Range;
 use codespan::{FileId, Files};
 use codespan_reporting::diagnostic::{self, Diagnostic};
 use lsp_types::{NumberOrString, Position};
+use nickel_lang_core::position::RawSpan;
 
 /// Convert [codespan_reporting::diagnostic::Diagnostic] into a list of another type
 /// Diagnostics tend to contain a list of labels pointing to errors in the code which
@@ -13,8 +14,16 @@ pub trait DiagnosticCompat: Sized {
 
 /// Determine the position of a [codespan_reporting::diagnostic::Label] by looking it up
 /// in the file cache
-pub trait LocationCompat {
+pub trait LocationCompat: Sized {
     fn from_codespan(file_id: &FileId, range: &Range<usize>, files: &Files<String>) -> Self;
+
+    fn from_span(span: &RawSpan, files: &Files<String>) -> Self {
+        Self::from_codespan(
+            &span.src_id,
+            &(span.start.to_usize()..span.end.to_usize()),
+            files,
+        )
+    }
 }
 
 impl LocationCompat for lsp_types::Range {
@@ -45,6 +54,15 @@ impl LocationCompat for lsp_types::Range {
     }
 }
 
+impl LocationCompat for lsp_types::Location {
+    fn from_codespan(file_id: &FileId, range: &Range<usize>, files: &Files<String>) -> Self {
+        lsp_types::Location {
+            uri: lsp_types::Url::from_file_path(files.name(*file_id)).unwrap(),
+            range: lsp_types::Range::from_codespan(file_id, range, files),
+        }
+    }
+}
+
 impl DiagnosticCompat for lsp_types::Diagnostic {
     fn from_codespan(diagnostic: Diagnostic<FileId>, files: &mut Files<String>) -> Vec<Self> {
         let severity = Some(match diagnostic.severity {

lsp/nls/src/field_walker.rs

@@ -30,7 +30,20 @@ impl FieldHaver {
                 .get(&id)
                 .map(|field| FieldContent::RecordField(field.clone())),
             FieldHaver::Dict(ty) => Some(FieldContent::Type(ty.clone())),
-            FieldHaver::RecordType(rows) => rows.row_find_path(&[id]).map(FieldContent::Type),
+            FieldHaver::RecordType(rows) => rows
+                .find_path(&[id])
+                .map(|row| FieldContent::Type(row.typ.clone())),
+        }
+    }
+
+    pub fn get_definition_pos(&self, id: Ident) -> Option<LocIdent> {
+        match self {
+            FieldHaver::RecordTerm(data) => data
+                .fields
+                .get_key_value(&id)
+                .map(|(id, _field)| (*id).into()),
+            FieldHaver::RecordType(rows) => rows.find_path(&[id]).map(|r| r.id.into()),
+            FieldHaver::Dict(_) => None,
         }
     }
 
@@ -197,7 +210,7 @@ impl<'a> FieldResolver<'a> {
     /// This a best-effort thing; it doesn't do full evaluation but it has some reasonable
     /// heuristics. For example, it knows that the fields defined on a merge of two records
     /// are the fields defined on either record.
-    fn resolve_term(&self, rt: &RichTerm) -> Vec<FieldHaver> {
+    pub fn resolve_term(&self, rt: &RichTerm) -> Vec<FieldHaver> {
         let term_fields = match rt.term.as_ref() {
             Term::Record(data) | Term::RecRecord(data, ..) => {
                 vec![FieldHaver::RecordTerm(data.clone())]

lsp/nls/src/linearization/mod.rs

@@ -79,6 +79,17 @@ impl LinRegistry {
         self.usage_lookups.get(&file)?.def(ident)
     }
 
+    pub fn get_usages(&self, ident: &LocIdent) -> impl Iterator<Item = &LocIdent> {
+        fn inner<'a>(
+            slf: &'a LinRegistry,
+            ident: &LocIdent,
+        ) -> Option<impl Iterator<Item = &'a LocIdent>> {
+            let file = ident.pos.as_opt_ref()?.src_id;
+            Some(slf.usage_lookups.get(&file)?.usages(ident))
+        }
+        inner(self, ident).into_iter().flatten()
+    }
+
     pub fn get_env(&self, rt: &RichTerm) -> Option<&crate::usage::Environment> {
         let file = rt.pos.as_opt_ref()?.src_id;
         self.usage_lookups.get(&file)?.env(rt)

lsp/nls/src/position.rs

@@ -3,10 +3,10 @@ use std::ops::Range;
 use codespan::ByteIndex;
 use nickel_lang_core::{
     position::TermPos,
-    term::{RichTerm, Traverse, TraverseControl},
+    term::{RichTerm, Term, Traverse, TraverseControl},
 };
 
-use crate::term::RichTermPtr;
+use crate::{identifier::LocIdent, term::RichTermPtr};
 
 /// Turn a collection of "nested" ranges into a collection of disjoint ranges.
 ///
@@ -102,30 +102,63 @@ fn make_disjoint<T: Clone>(mut all_ranges: Vec<(Range<u32>, T)>) -> Vec<(Range<u
 #[derive(Clone, Debug)]
 pub struct PositionLookup {
     // The intervals here are sorted and disjoint.
-    ranges: Vec<(Range<u32>, RichTermPtr)>,
+    term_ranges: Vec<(Range<u32>, RichTermPtr)>,
+    ident_ranges: Vec<(Range<u32>, LocIdent)>,
 }
 
 impl PositionLookup {
     /// Create a position lookup table for looking up subterms of `rt` based on their positions.
     pub fn new(rt: &RichTerm) -> Self {
-        let mut all_ranges = Vec::new();
+        let mut all_term_ranges = Vec::new();
+        let mut idents = Vec::new();
         let mut fun = |term: &RichTerm| {
             if let TermPos::Original(pos) = &term.pos {
-                all_ranges.push((
+                all_term_ranges.push((
                     Range {
                         start: pos.start.0,
                         end: pos.end.0,
                     },
                     RichTermPtr(term.clone()),
                 ));
             }
+
+            match term.as_ref() {
+                Term::Fun(id, _) | Term::Let(id, _, _, _) => idents.push(*id),
+                Term::FunPattern(id, pat, _) | Term::LetPattern(id, pat, _, _) => {
+                    let ids = pat.matches.iter().flat_map(|m| {
+                        m.to_flattened_bindings()
+                            .into_iter()
+                            .map(|(_path, id, _)| id)
+                    });
+                    idents.extend(ids.chain(*id).chain(pat.rest))
+                }
+                Term::Var(id) => idents.push(*id),
+                Term::Record(data) | Term::RecRecord(data, _, _) => {
+                    idents.extend(data.fields.keys().cloned());
+                }
+                Term::Match { cases, .. } => idents.extend(cases.keys().cloned()),
+                _ => {}
+            }
             TraverseControl::<()>::Continue
         };
 
         rt.traverse_ref(&mut fun);
 
+        let mut ident_ranges: Vec<_> = idents
+            .into_iter()
+            .filter_map(|id| {
+                id.pos
+                    .into_opt()
+                    .map(|span| (span.start.0..span.end.0, id.into()))
+            })
+            .collect();
+        // Ident ranges had better be disjoint, so we can just sort by the start position.
+        ident_ranges.sort_by_key(|(range, _id)| range.start);
+        ident_ranges.dedup();
+
         PositionLookup {
-            ranges: make_disjoint(all_ranges),
+            term_ranges: make_disjoint(all_term_ranges),
+            ident_ranges,
         }
     }
 
@@ -134,21 +167,29 @@ impl PositionLookup {
     /// Note that some positions (for example, positions belonging to top-level comments)
     /// may not be enclosed by any term.
     pub fn get(&self, index: ByteIndex) -> Option<&RichTerm> {
-        self.ranges
-            .binary_search_by(|(range, _term)| {
-                if range.end <= index.0 {
-                    std::cmp::Ordering::Less
-                } else if range.start > index.0 {
-                    std::cmp::Ordering::Greater
-                } else {
-                    std::cmp::Ordering::Equal
-                }
-            })
-            .ok()
-            .map(|idx| &self.ranges[idx].1 .0)
+        search(&self.term_ranges, index).map(|rt| &rt.0)
+    }
+
+    /// Returns the ident at the given position, if there is one.
+    pub fn get_ident(&self, index: ByteIndex) -> Option<LocIdent> {
+        search(&self.ident_ranges, index).cloned()
     }
 }
 
+fn search<T>(vec: &[(Range<u32>, T)], index: ByteIndex) -> Option<&T> {
+    vec.binary_search_by(|(range, _payload)| {
+        if range.end <= index.0 {
+            std::cmp::Ordering::Less
+        } else if range.start > index.0 {
+            std::cmp::Ordering::Greater
+        } else {
+            std::cmp::Ordering::Equal
+        }
+    })
+    .ok()
+    .map(|idx| &vec[idx].1)
+}
+
 #[cfg(test)]
 pub(crate) mod tests {
     use assert_matches::assert_matches;