Repository: tweag/nickel
Pull request #1625 (2023-09-22 12:22:52)

Remove the double contract application introduced in #1194
Because of #1228 we syntactically forbid contracts from containing free type variables. This was implemented in #1271 and #1272. As a result, the double contract application in #1194 became dead code. This PR removes the `%dualize%` primop, the `dualize` field in `Label` and the double application logic when constructing recursive environments for records.


Patch:
diff --git a/core/src/eval/fixpoint.rs b/core/src/eval/fixpoint.rs
index 5789ca32..f84d27d5 100644
--- a/core/src/eval/fixpoint.rs
+++ b/core/src/eval/fixpoint.rs
@@ -1,6 +1,6 @@
 //! Compute the fixpoint of a recursive record.
 use super::{merge::RevertClosurize, *};
-use crate::{label::Label, position::TermPos};
+use crate::position::TermPos;
 
 // Update the environment of a term by extending it with a recursive environment. In the general
 // case, the term is expected to be a variable pointing to the element to be patched. Otherwise,
@@ -87,43 +87,7 @@ pub fn rec_env<'a, I: Iterator<Item = (&'a LocIdent, &'a Field)>, C: Cache>(
 
                 let with_ctr_applied = RuntimeContract::apply_all(
                     RichTerm::new(Term::Var(id_value), value.pos),
-                    field.pending_contracts.iter().cloned().flat_map(|ctr| {
-                        // This operation is the heart of our preliminary fix for
-                        // [#1161](https://github.com/tweag/nickel/issues/1161). Whenever we detect
-                        // the presence of free type variables in a contract, by witnessing a
-                        // nonempty type environment in the label, we need to not just apply the
-                        // original contract but also its dual. The rationale here is that a record
-                        // field that recursively depends on another of type `T`, say, should be
-                        // considered a function with domain `T`. Consequently, the same contract
-                        // that would be a applied to the argument of a function of type `T -> Dyn`
-                        // should be applied to the recursive reference.
-                        //
-                        // Thus, the recursive reference must satisfy the contract for `T` as well
-                        // as the "dual" contract `T.dualize()`; the latter is defined to be the
-                        // domain contract for a function of type `T -> Dyn`. This sublety only
-                        // matters if `T` contains free type variables because only then does
-                        // `T.dualize()` differ from `T` at all.
-                        //
-                        // We expect to implement a way of solving this dilemma without essentially
-                        // applying every contract twice. This will likely involve a rewriting of
-                        // contracts corresponding to free variables which is yet to be proved
-                        // sound.
-                        if ctr.label.type_environment.is_empty() {
-                            vec![ctr]
-                        } else {
-                            vec![
-                                ctr.clone(),
-                                RuntimeContract {
-                                    contract: ctr.contract,
-                                    label: Label {
-                                        polarity: ctr.label.polarity.flip(),
-                                        dualize: true,
-                                        ..ctr.label
-                                    },
-                                },
-                            ]
-                        }
-                    }),
+                    field.pending_contracts.iter().cloned(),
                     value.pos,
                 );
 
diff --git a/core/src/eval/operation.rs b/core/src/eval/operation.rs
index 6903180f..9c385760 100644
--- a/core/src/eval/operation.rs
+++ b/core/src/eval/operation.rs
@@ -1155,18 +1155,6 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
                     Err(mk_type_error!("label_push_diag", "Label"))
                 }}
             }
-            UnaryOp::Dualize() => {
-                match_sharedterm! {t, with {
-                    Term::Lbl(label) => {
-                        Ok(Closure::atomic_closure(RichTerm::new(
-                            Term::Bool(label.dualize),
-                            pos_op_inh,
-                        )))
-                    }
-                } else {
-                    Err(mk_type_error!("dualize", "Label"))
-                }}
-            }
         }
     }
 
diff --git a/core/src/label.rs b/core/src/label.rs
index f4ee61f6..12abab8b 100644
--- a/core/src/label.rs
+++ b/core/src/label.rs
@@ -303,10 +303,6 @@ pub struct Label {
     /// polymorphic contracts to decide which actions to take when encountering a `forall`.
     pub type_environment: HashMap<SealingKey, TypeVarData>,
 
-    /// Signal to a polymorphic contract that it should generate the dual contract. Part of the
-    /// preliminary fix for [#1161](https://github.com/tweag/nickel/issues/1161).
-    pub dualize: bool,
-
     /// The name of the record field to report in blame errors. This is set
     /// while first transforming a record as part of the pending contract generation.
     /// Contract applications outside of records will have this field set to `None`.
@@ -523,7 +519,6 @@ impl Default for Label {
             arg_pos: Default::default(),
             path: Default::default(),
             type_environment: Default::default(),
-            dualize: false,
             field_name: None,
         }
     }
diff --git a/core/src/parser/grammar.lalrpop b/core/src/parser/grammar.lalrpop
index cd0e3806..2a226ff0 100644
--- a/core/src/parser/grammar.lalrpop
+++ b/core/src/parser/grammar.lalrpop
@@ -667,7 +667,6 @@ UOp: UnaryOp = {
     "record_empty_with_tail" => UnaryOp::RecordEmptyWithTail(),
     "trace" => UnaryOp::Trace(),
     "label_push_diag" => UnaryOp::LabelPushDiag(),
-    "dualize" => UnaryOp::Dualize(),
 };
 
 MatchCase: MatchCase = {
@@ -1051,7 +1050,6 @@ extern {
         "trace" => Token::Normal(NormalToken::Trace),
         "insert_type_variable" => Token::Normal(NormalToken::InsertTypeVar),
         "lookup_type_variable" => Token::Normal(NormalToken::LookupTypeVar),
-        "dualize" => Token::Normal(NormalToken::Dualize),
 
         "has_field" => Token::Normal(NormalToken::HasField),
         "map" => Token::Normal(NormalToken::Map),
diff --git a/core/src/parser/lexer.rs b/core/src/parser/lexer.rs
index 2dbd2b06..9a3b9d1c 100644
--- a/core/src/parser/lexer.rs
+++ b/core/src/parser/lexer.rs
@@ -210,8 +210,6 @@ pub enum NormalToken<'input> {
     InsertTypeVar,
     #[token("%lookup_type_variable%")]
     LookupTypeVar,
-    #[token("%dualize%")]
-    Dualize,
 
     #[token("%seal%")]
     Seal,
diff --git a/core/src/term/mod.rs b/core/src/term/mod.rs
index 15cd8559..0cf5d2c5 100644
--- a/core/src/term/mod.rs
+++ b/core/src/term/mod.rs
@@ -1174,11 +1174,6 @@ pub enum UnaryOp {
     /// This primop shouldn't be used directly by user a priori, but is used internally during e.g.
     /// contract application.
     LabelPushDiag(),
-
-    /// Return the value of the `dualize` field in a label. Used by polymorphic contracts to check
-    /// if they are being invoked to generate a dual contract, as part of the preliminary fix for
-    /// [#1161](https://github.com/tweag/nickel/issues/1161).
-    Dualize(),
 }
 
 impl fmt::Display for UnaryOp {
@@ -1227,7 +1222,6 @@ impl fmt::Display for UnaryOp {
             RecordEmptyWithTail() => write!(f, "record_empty_with_tail"),
             Trace() => write!(f, "trace"),
             LabelPushDiag() => write!(f, "label_push_diag"),
-            Dualize() => write!(f, "dualize"),
         }
     }
 }
diff --git a/core/src/typecheck/operation.rs b/core/src/typecheck/operation.rs
index 736016cd..e4b586e0 100644
--- a/core/src/typecheck/operation.rs
+++ b/core/src/typecheck/operation.rs
@@ -207,9 +207,6 @@ pub fn get_uop_type(
         // Morally: Lbl -> Lbl
         // Actual: Dyn -> Dyn
         UnaryOp::LabelPushDiag() => (mk_uniftype::dynamic(), mk_uniftype::dynamic()),
-        // Morally: Lbl -> Bool
-        // Actual: Dyn -> Bool
-        UnaryOp::Dualize() => (mk_uniftype::dynamic(), mk_uniftype::bool()),
     })
 }
 
diff --git a/core/stdlib/internals.ncl b/core/stdlib/internals.ncl
index 9c5a8efc..514ba003 100644
--- a/core/stdlib/internals.ncl
+++ b/core/stdlib/internals.ncl
@@ -37,15 +37,8 @@
       # blaming polymorphic contracts.
       %seal% sealing_key (%chng_pol% label) value,
 
-  "$forall" =
-    let flip = match {
-      'Positive => 'Negative,
-      'Negative => 'Positive,
-    }
-    in
-    fun sealing_key polarity contract label value =>
-      let polarity = if %dualize% label then flip polarity else polarity in
-      contract (%insert_type_variable% sealing_key polarity label) value,
+  "$forall" = fun sealing_key polarity contract label value =>
+    contract (%insert_type_variable% sealing_key polarity label) value,
 
   "$enums" = fun case label value =>
     if %typeof% value == 'Enum then
diff --git a/lsp/lsp-harness/src/lib.rs b/lsp/lsp-harness/src/lib.rs
index e53f08ba..412314c2 100644
--- a/lsp/lsp-harness/src/lib.rs
+++ b/lsp/lsp-harness/src/lib.rs
@@ -4,8 +4,8 @@ mod output;
 pub use jsonrpc::Server;
 use log::error;
 use lsp_types::{
-    CompletionParams, DocumentFormattingParams, GotoDefinitionParams, HoverParams, ReferenceParams,
-    Url,
+    CompletionParams, DocumentFormattingParams, DocumentSymbolParams, GotoDefinitionParams,
+    HoverParams, ReferenceParams, Url,
 };
 pub use output::LspDebug;
 use serde::Deserialize;
@@ -35,6 +35,7 @@ pub enum Request {
     Completion(CompletionParams),
     Formatting(DocumentFormattingParams),
     Hover(HoverParams),
+    Symbols(DocumentSymbolParams),
 }
 
 #[derive(Deserialize, Debug, Default)]
diff --git a/lsp/lsp-harness/src/output.rs b/lsp/lsp-harness/src/output.rs
index bbd1fbc0..013cb358 100644
--- a/lsp/lsp-harness/src/output.rs
+++ b/lsp/lsp-harness/src/output.rs
@@ -4,7 +4,7 @@
 
 use std::io::Write;
 
-use lsp_types::GotoDefinitionResponse;
+use lsp_types::{DocumentSymbolResponse, GotoDefinitionResponse};
 
 pub trait LspDebug {
     fn debug(&self, w: impl Write) -> std::io::Result<()>;
@@ -157,3 +157,34 @@ impl LspDebug for lsp_types::MarkupContent {
         write!(w, "{}", self.value)
     }
 }
+
+impl LspDebug for lsp_types::SymbolInformation {
+    fn debug(&self, mut w: impl Write) -> std::io::Result<()> {
+        let name = &self.name;
+        let kind = self.kind;
+        write!(w, "{name} ({kind:?})@{}", self.location.debug_str())
+    }
+}
+
+impl LspDebug for lsp_types::DocumentSymbol {
+    fn debug(&self, mut w: impl Write) -> std::io::Result<()> {
+        let name = &self.name;
+        let kind = self.kind;
+        let detail = self.detail.clone().unwrap_or_default();
+        write!(
+            w,
+            "{name} ({kind:?} / {detail:?})@{} in {}",
+            self.selection_range.debug_str(),
+            self.range.debug_str()
+        )
+    }
+}
+
+impl LspDebug for DocumentSymbolResponse {
+    fn debug(&self, w: impl Write) -> std::io::Result<()> {
+        match self {
+            DocumentSymbolResponse::Flat(xs) => xs.debug(w),
+            DocumentSymbolResponse::Nested(xs) => xs.debug(w),
+        }
+    }
+}
diff --git a/lsp/nls/src/cache.rs b/lsp/nls/src/cache.rs
index 7176a091..be273c72 100644
--- a/lsp/nls/src/cache.rs
+++ b/lsp/nls/src/cache.rs
@@ -11,7 +11,7 @@ use nickel_lang_core::{
 };
 
 use crate::linearization::{building::Building, AnalysisHost, Environment, LinRegistry};
-use crate::linearization::{CombinedLinearizer, TypeCollector};
+use crate::linearization::{CollectedTypes, CombinedLinearizer, TypeCollector};
 
 pub trait CacheExt {
     fn typecheck_with_analysis(
@@ -78,7 +78,7 @@ impl CacheExt for Cache {
                 initial_ctxt.clone(),
                 self,
                 lin,
-                (building, HashMap::new()),
+                (building, CollectedTypes::default()),
             )
             .map_err(|err| vec![Error::TypecheckError(err)])?;
 
diff --git a/lsp/nls/src/linearization/mod.rs b/lsp/nls/src/linearization/mod.rs
index 6160e7a0..7a31f307 100644
--- a/lsp/nls/src/linearization/mod.rs
+++ b/lsp/nls/src/linearization/mod.rs
@@ -101,7 +101,7 @@ pub struct LinRegistry {
     pub position_lookups: HashMap<FileId, PositionLookup>,
     pub usage_lookups: HashMap<FileId, UsageLookup>,
     pub parent_lookups: HashMap<FileId, ParentLookup>,
-    pub type_lookups: HashMap<FileId, HashMap<RichTermPtr, Type>>,
+    pub type_lookups: HashMap<FileId, CollectedTypes<Type>>,
 }
 
 impl LinRegistry {
@@ -113,7 +113,7 @@ impl LinRegistry {
         &mut self,
         file_id: FileId,
         linearization: Completed,
-        type_lookups: HashMap<RichTermPtr, Type>,
+        type_lookups: CollectedTypes<Type>,
         term: &RichTerm,
     ) {
         self.map.insert(file_id, linearization);
@@ -154,7 +154,10 @@ impl LinRegistry {
 
     pub fn get_type(&self, rt: &RichTerm) -> Option<&Type> {
         let file = rt.pos.as_opt_ref()?.src_id;
-        self.type_lookups.get(&file)?.get(&RichTermPtr(rt.clone()))
+        self.type_lookups
+            .get(&file)?
+            .terms
+            .get(&RichTermPtr(rt.clone()))
     }
 
     pub fn get_parent_chain<'a>(&'a self, rt: &'a RichTerm) -> Option<ParentChainIter<'a>> {
@@ -788,9 +791,24 @@ pub struct TypeCollector {
     term_ids: Vec<RichTermPtr>,
 }
 
+#[derive(Clone, Debug)]
+pub struct CollectedTypes<Ty> {
+    pub terms: HashMap<RichTermPtr, Ty>,
+    pub idents: HashMap<LocIdent, Ty>,
+}
+
+impl<Ty> Default for CollectedTypes<Ty> {
+    fn default() -> Self {
+        Self {
+            terms: HashMap::new(),
+            idents: HashMap::new(),
+        }
+    }
+}
+
 impl Linearizer for TypeCollector {
-    type Building = HashMap<RichTermPtr, UnifType>;
-    type Completed = HashMap<RichTermPtr, Type>;
+    type Building = CollectedTypes<UnifType>;
+    type Completed = CollectedTypes<Type>;
     type CompletionExtra = Extra;
     type ItemId = usize;
 
@@ -804,7 +822,7 @@ impl Linearizer for TypeCollector {
 
     fn add_term(&mut self, lin: &mut Self::Building, rt: &RichTerm, ty: UnifType) -> Option<usize> {
         self.term_ids.push(RichTermPtr(rt.clone()));
-        lin.insert(RichTermPtr(rt.clone()), ty);
+        lin.terms.insert(RichTermPtr(rt.clone()), ty);
         Some(self.term_ids.len() - 1)
     }
 
@@ -827,16 +845,33 @@ impl Linearizer for TypeCollector {
             }
         };
 
-        lin.into_iter()
+        let terms = lin
+            .terms
+            .into_iter()
             .map(|(rt, uty)| (rt, transform_type(uty)))
-            .collect()
+            .collect();
+        let idents = lin
+            .idents
+            .into_iter()
+            .map(|(id, uty)| (id, transform_type(uty)))
+            .collect();
+        CollectedTypes { terms, idents }
     }
 
     fn retype(&mut self, lin: &mut Self::Building, item_id: Option<usize>, new_type: UnifType) {
         if let Some(id) = item_id {
-            lin.insert(self.term_ids[id].clone(), new_type);
+            lin.terms.insert(self.term_ids[id].clone(), new_type);
         }
     }
+
+    fn retype_ident(
+        &mut self,
+        lin: &mut Self::Building,
+        ident: &nickel_lang_core::identifier::LocIdent,
+        new_type: UnifType,
+    ) {
+        lin.idents.insert((*ident).into(), new_type);
+    }
 }
 
 pub struct CombinedLinearizer<T, U>(pub T, pub U);
diff --git a/lsp/nls/src/requests/symbols.rs b/lsp/nls/src/requests/symbols.rs
index 41a79395..7bbb5f59 100644
--- a/lsp/nls/src/requests/symbols.rs
+++ b/lsp/nls/src/requests/symbols.rs
@@ -1,15 +1,10 @@
-use crate::{
-    files::uri_to_path,
-    linearization::interface::TermKind,
-    term::RawSpanExt,
-    trace::{Enrich, Trace},
-};
 use lsp_server::{RequestId, Response, ResponseError};
 use lsp_types::{DocumentSymbol, DocumentSymbolParams, SymbolKind};
 use nickel_lang_core::cache::SourcePath;
-use serde_json::Value;
+use nickel_lang_core::typ::Type;
 
 use crate::server::Server;
+use crate::{files::uri_to_path, term::RawSpanExt};
 
 pub fn handle_document_symbols(
     params: DocumentSymbolParams,
@@ -17,47 +12,44 @@ pub fn handle_document_symbols(
     server: &mut Server,
 ) -> Result<(), ResponseError> {
     let path = uri_to_path(&params.text_document.uri)?;
-    let file_id = server.cache.id_of(&SourcePath::Path(path)).unwrap();
-
-    if let Some(completed) = server.lin_registry.map.get(&file_id) {
-        Trace::enrich(&id, completed);
-        let symbols = completed
-            .linearization
-            .iter()
-            .filter_map(|item| match &item.kind {
-                TermKind::Declaration { id: name, .. } => {
-                    // Although the position maybe shouldn't be `None`, opening the std library
-                    // source inside VSCode made the LSP panic on the previous `item.pos.unwrap()`.
-                    // Before investigating further, let's not make the VSCode extension panic in
-                    // the meantime.
-                    let (file_id, span) = item.pos.into_opt()?.to_range();
-
-                    let range =
-                        codespan_lsp::byte_span_to_range(server.cache.files(), file_id, span)
-                            .unwrap();
-
-                    // `deprecated` is a required field but causes a warning although we are not
-                    // using it
-                    #[allow(deprecated)]
-                    Some(DocumentSymbol {
-                        name: name.to_string(),
-                        detail: Some(format!("{}", item.ty)),
-                        kind: SymbolKind::Variable,
-                        tags: None,
-                        range,
-                        selection_range: range,
-                        children: None,
-                        deprecated: None,
-                    })
-                }
-                _ => None,
+    let file_id = server
+        .cache
+        .id_of(&SourcePath::Path(path))
+        .ok_or_else(|| crate::error::Error::FileNotFound(params.text_document.uri.clone()))?;
+
+    let usage_lookups = server.lin_registry.usage_lookups.get(&file_id).unwrap();
+
+    let type_lookups = server
+        .lin_registry
+        .type_lookups
+        .get(&file_id)
+        .ok_or_else(|| crate::error::Error::FileNotFound(params.text_document.uri.clone()))?;
+
+    let mut symbols = usage_lookups
+        .symbols()
+        .filter_map(|ident| {
+            let (file_id, span) = ident.pos.into_opt()?.to_range();
+            let range =
+                codespan_lsp::byte_span_to_range(server.cache.files(), file_id, span).ok()?;
+            let ty = type_lookups.idents.get(&ident);
+
+            #[allow(deprecated)] // because the `deprecated` field is... wait for it... deprecated.
+            Some(DocumentSymbol {
+                name: ident.ident.to_string(),
+                detail: ty.map(Type::to_string),
+                kind: SymbolKind::Variable,
+                tags: None,
+                range,
+                selection_range: range,
+                children: None,
+                deprecated: None,
             })
-            .collect::<Vec<_>>();
+        })
+        .collect::<Vec<_>>();
+
+    symbols.sort_by_key(|s| s.range.start);
 
-        server.reply(Response::new_ok(id, symbols));
-    } else {
-        server.reply(Response::new_ok(id, Value::Null));
-    }
+    server.reply(Response::new_ok(id, symbols));
 
     Ok(())
 }
diff --git a/lsp/nls/src/usage.rs b/lsp/nls/src/usage.rs
index 8819a289..3a918498 100644
--- a/lsp/nls/src/usage.rs
+++ b/lsp/nls/src/usage.rs
@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 
 use nickel_lang_core::{
     environment::Environment as GenericEnvironment,
@@ -87,6 +87,10 @@ pub struct UsageLookup {
     // environments) but its enough for us now.
     def_table: HashMap<RawSpan, Environment>,
     usage_table: HashMap<LocIdent, Vec<LocIdent>>,
+    // The list of all the symbols (and their locations) in the document.
+    //
+    // Currently, variables bound in `let` bindings and record fields count as symbols.
+    syms: HashSet<LocIdent>,
 }
 
 impl UsageLookup {
@@ -125,6 +129,15 @@ impl UsageLookup {
             .and_then(|span| self.def_table.get(span))
     }
 
+    /// Return the list of symbols in the document.
+    pub fn symbols(&self) -> impl Iterator<Item = LocIdent> + '_ {
+        self.syms.iter().cloned()
+    }
+
+    fn add_sym(&mut self, id: impl Into<LocIdent>) {
+        self.syms.insert(id.into());
+    }
+
     fn fill(&mut self, rt: &RichTerm, env: &Environment) {
         rt.traverse_ref(
             &mut |term: &RichTerm, env: &Environment| {
@@ -154,6 +167,7 @@ impl UsageLookup {
                     Term::Let(id, val, body, attrs) => {
                         let mut new_env = env.clone();
                         new_env.def(*id, Some(val.clone()), None);
+                        self.add_sym(*id);
 
                         self.fill(val, if attrs.rec { &new_env } else { env });
                         self.fill(body, &new_env);
@@ -164,6 +178,7 @@ impl UsageLookup {
                         let mut new_env = env.clone();
                         if let Some(id) = maybe_id {
                             new_env.def(*id, Some(val.clone()), None);
+                            self.add_sym(*id);
                         }
 
                         for m in &pat.matches {
@@ -173,7 +188,8 @@ impl UsageLookup {
                                     term: val.clone(),
                                     path,
                                 };
-                                new_env.def(id, Some(term), Some(field.metadata));
+                                new_env.def(id, Some(term.clone()), Some(field.metadata));
+                                self.add_sym(id);
                             }
                         }
                         TraverseControl::ContinueWithScope(new_env)
@@ -185,6 +201,7 @@ impl UsageLookup {
                         // all the fields in the environment and then recurse into their values.
                         for (id, field) in &data.fields {
                             new_env.def(*id, field.value.clone(), Some(field.metadata.clone()));
+                            self.add_sym(*id);
                         }
 
                         TraverseControl::ContinueWithScope(new_env)


Test patch:
diff --git a/lsp/nls/test2.ncl b/lsp/nls/test2.ncl
new file mode 100644
index 00000000..aea6f0da
--- /dev/null
+++ b/lsp/nls/test2.ncl
@@ -0,0 +1,2 @@
+let x : { _ : { foo : Number | default = 1 } } = {} in
+x.PATH.foo
diff --git a/lsp/nls/tests/inputs/symbols-basic.ncl b/lsp/nls/tests/inputs/symbols-basic.ncl
new file mode 100644
index 00000000..fba1a615
--- /dev/null
+++ b/lsp/nls/tests/inputs/symbols-basic.ncl
@@ -0,0 +1,15 @@
+### /syms.ncl
+let foo = 3 in
+let func = fun x => 1 in
+{
+  name = "value",
+  other_name = {
+    inner_name = let inner_binding = 3 in "value",
+  },
+  type_checked_block = {
+    inner_name = { name = "value" },
+  } : _,
+}
+### [[request]]
+### type = "Symbols"
+### textDocument.uri = "file:///syms.ncl"
diff --git a/lsp/nls/tests/main.rs b/lsp/nls/tests/main.rs
index 9e86aee2..3537b957 100644
--- a/lsp/nls/tests/main.rs
+++ b/lsp/nls/tests/main.rs
@@ -2,7 +2,8 @@ use std::collections::{hash_map::Entry, HashMap};
 
 use assert_cmd::cargo::CommandCargoExt;
 use lsp_types::request::{
-    Completion, Formatting, GotoDefinition, HoverRequest, References, Request as LspRequest,
+    Completion, DocumentSymbolRequest, Formatting, GotoDefinition, HoverRequest, References,
+    Request as LspRequest,
 };
 use nickel_lang_utils::project_root::project_root;
 use test_generator::test_resources;
@@ -40,6 +41,7 @@ impl TestHarness {
             Request::Formatting(f) => self.request::<Formatting>(f),
             Request::Hover(h) => self.request::<HoverRequest>(h),
             Request::References(r) => self.request::<References>(r),
+            Request::Symbols(s) => self.request::<DocumentSymbolRequest>(s),
         }
     }
 
diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__symbols-basic.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__symbols-basic.ncl.snap
new file mode 100644
index 00000000..6bb0416a
--- /dev/null
+++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__symbols-basic.ncl.snap
@@ -0,0 +1,6 @@
+---
+source: lsp/nls/tests/main.rs
+expression: output
+---
+[foo (Variable / "Number")@0:4-0:7 in 0:4-0:7, func (Variable / "Dyn")@1:4-1:8 in 1:4-1:8, name (Variable / "String")@3:2-3:6 in 3:2-3:6, other_name (Variable / "Dyn")@4:2-4:12 in 4:2-4:12, inner_name (Variable / "Dyn")@5:4-5:14 in 5:4-5:14, inner_binding (Variable / "Number")@5:21-5:34 in 5:21-5:34, type_checked_block (Variable / "Dyn")@7:2-7:20 in 7:2-7:20, inner_name (Variable / "{ name : String }")@8:4-8:14 in 8:4-8:14, name (Variable / "String")@8:19-8:23 in 8:19-8:23]
+