Repository: tweag/nickel
Pull request #1982 (2024-07-01 16:52:03)

Add record/split_pair and record/disjoint_merge
This is preliminary work for the rewriting of native contracts using the new immediate/delayed representation introduced in #1975, although this PR is fairly well-separated (and this is why this is a small separate PR).

In order to efficiently implement the contracts for static record types, which needs to know which fields to lock in the sealed tail, we need a way to split and recombine records which is efficient and preserves field metadata. The current implementation can do without because the immediate part and the delayed part are crammed into one function, and the current implementation is also probably quite inefficient (computing 2 record-as-sets differences in pure Nickel).

This PR adds two primops:

- `record/split_pair` corresponds exactly to what the Rust code `eval::merge::split::split` is doing: separate a pair of records into 4 parts, the fields that are in one of the operands but not the other, plus the fields that are common (but which is split in two because we have to choose if we take the value and field metadata's from the left operand or from the right operand).
- `record/disjoint_merge` is a fast merge operation when both operands are known to be disjoints. It's closer to a union than a merge, although both coincide on such a special case, in that it doesn't interact with metadata or recursive definitions (although it does preserve metadata). This operation just puts everything in one record.

Those primops might be also useful to efficiently implement other operations (record differences, record projection, etc.) in the stdlibin the future.


Patch:
diff --git a/core/src/eval/operation.rs b/core/src/eval/operation.rs
index 84d8c46b..2ca3b7c7 100644
--- a/core/src/eval/operation.rs
+++ b/core/src/eval/operation.rs
@@ -8,7 +8,7 @@
 //! receive evaluated operands and implement the actual semantics of operators.
 use super::{
     cache::lazy::Thunk,
-    merge::{self, MergeMode},
+    merge::{self, split, MergeMode},
     stack::StrAccData,
     subst, Cache, Closure, Environment, ImportResolver, VirtualMachine,
 };
@@ -30,7 +30,7 @@ use crate::{
     term::{
         array::{Array, ArrayAttrs, OutOfBoundError},
         make as mk_term,
-        record::{self, Field, FieldMetadata, RecordData},
+        record::*,
         string::NickelString,
         *,
     },
@@ -2580,6 +2580,148 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
                     pos_op_inh,
                 )))
             }
+            BinaryOp::RecordSplitPair => {
+                let t1 = t1.into_owned();
+                let t2 = t2.into_owned();
+
+                let Term::Record(record1) = t1 else {
+                    return Err(mk_type_error!(
+                        "record/full_difference",
+                        "Record",
+                        1,
+                        t1.into(),
+                        pos1
+                    ));
+                };
+
+                let Term::Record(record2) = t2 else {
+                    return Err(mk_type_error!(
+                        "record/full_difference",
+                        "Record",
+                        2,
+                        t2.into(),
+                        pos2
+                    ));
+                };
+
+                let split::SplitResult {
+                    left,
+                    center,
+                    right,
+                } = split::split(record1.fields, record2.fields);
+
+                let left_only = Term::Record(RecordData {
+                    fields: left,
+                    sealed_tail: record1.sealed_tail,
+                    attrs: record1.attrs,
+                });
+
+                let right_only = Term::Record(RecordData {
+                    fields: right,
+                    sealed_tail: record2.sealed_tail,
+                    attrs: record2.attrs,
+                });
+
+                let (center1, center2): (IndexMap<LocIdent, Field>, IndexMap<LocIdent, Field>) =
+                    center
+                        .into_iter()
+                        .map(|(id, (left, right))| ((id, left), (id, right)))
+                        .unzip();
+
+                let left_center = Term::Record(RecordData {
+                    fields: center1,
+                    sealed_tail: None,
+                    attrs: RecordAttrs::default().closurized(),
+                });
+
+                let right_center = Term::Record(RecordData {
+                    fields: center2,
+                    sealed_tail: None,
+                    attrs: RecordAttrs::default().closurized(),
+                });
+
+                Ok(Closure::atomic_closure(RichTerm::new(
+                    Term::Record(RecordData {
+                        fields: IndexMap::from([
+                            (
+                                LocIdent::from("left_only"),
+                                Field::from(RichTerm::from(left_only)),
+                            ),
+                            (
+                                LocIdent::from("left_center"),
+                                Field::from(RichTerm::from(left_center)),
+                            ),
+                            (
+                                LocIdent::from("right_center"),
+                                Field::from(RichTerm::from(right_center)),
+                            ),
+                            (
+                                LocIdent::from("right_only"),
+                                Field::from(RichTerm::from(right_only)),
+                            ),
+                        ]),
+                        attrs: RecordAttrs::default().closurized(),
+                        sealed_tail: None,
+                    }),
+                    pos_op_inh,
+                )))
+            }
+            BinaryOp::RecordDisjointMerge => {
+                let t1 = t1.into_owned();
+                let t2 = t2.into_owned();
+
+                let Term::Record(mut record1) = t1 else {
+                    return Err(mk_type_error!(
+                        "record/disjoint_merge",
+                        "Record",
+                        1,
+                        t1.into(),
+                        pos1
+                    ));
+                };
+
+                let Term::Record(record2) = t2 else {
+                    return Err(mk_type_error!(
+                        "record/disjoint_merge",
+                        "Record",
+                        2,
+                        t2.into(),
+                        pos2
+                    ));
+                };
+
+                // As for merge, we refuse to combine two records if one of them has a sealed tail
+                if let Some(record::SealedTail { label, .. }) =
+                    record1.sealed_tail.or(record2.sealed_tail)
+                {
+                    return Err(EvalError::IllegalPolymorphicTailAccess {
+                        action: IllegalPolymorphicTailAction::Merge,
+                        evaluated_arg: label.get_evaluated_arg(&self.cache),
+                        label,
+                        call_stack: std::mem::take(&mut self.call_stack),
+                    });
+                }
+
+                // Note that because of record closurization, we assume here that the record data
+                // of each record are already closurized, so we don't really care about
+                // environments. Should that invariant change, we might get into trouble (trouble
+                // meaning undue `UnboundIdentifier` errors).
+                debug_assert!(record1.attrs.closurized && record2.attrs.closurized);
+                record1.fields.extend(record2.fields);
+                record1.attrs.open = record1.attrs.open || record2.attrs.open;
+
+                Ok(Closure::atomic_closure(RichTerm::new(
+                    Term::Record(RecordData {
+                        fields: record1.fields,
+                        attrs: record1.attrs,
+                        // We need to set the tail to `None` explicitly to appease the borrow
+                        // checker which objects that `sealed_tail` has been moved in the if
+                        // let above.
+                        sealed_tail: None,
+                    }),
+                    pos_op_inh,
+                )))
+            }
         }
     }
 
diff --git a/core/src/parser/grammar.lalrpop b/core/src/parser/grammar.lalrpop
index a937d5fd..c6d391df 100644
--- a/core/src/parser/grammar.lalrpop
+++ b/core/src/parser/grammar.lalrpop
@@ -1330,6 +1330,8 @@ BOpPre: BinaryOp = {
     },
     "record/remove" => BinaryOp::RecordRemove(RecordOpKind::IgnoreEmptyOpt),
     "record/remove_with_opts" => BinaryOp::RecordRemove(RecordOpKind::ConsiderAllFields),
+    "record/split_pair" => BinaryOp::RecordSplitPair,
+    "record/disjoint_merge" => BinaryOp::RecordDisjointMerge,
     "label/with_message" => BinaryOp::LabelWithMessage,
     "label/with_notes" => BinaryOp::LabelWithNotes,
     "label/append_note" => BinaryOp::LabelAppendNote,
@@ -1555,6 +1557,8 @@ extern {
         "record/has_field_with_opts" => Token::Normal(NormalToken::RecordHasFieldWithOpts),
         "record/field_is_defined" => Token::Normal(NormalToken::RecordFieldIsDefined),
         "record/field_is_defined_with_opts" => Token::Normal(NormalToken::RecordFieldIsDefinedWithOpts),
+        "record/split_pair" => Token::Normal(NormalToken::RecordSplitPair),
+        "record/disjoint_merge" => Token::Normal(NormalToken::RecordDisjointMerge),
         "array/map" => Token::Normal(NormalToken::ArrayMap),
         "array/generate" => Token::Normal(NormalToken::ArrayGen),
         "array/at" => Token::Normal(NormalToken::ArrayAt),
diff --git a/core/src/parser/lexer.rs b/core/src/parser/lexer.rs
index b0bbae2e..e8d7c8d1 100644
--- a/core/src/parser/lexer.rs
+++ b/core/src/parser/lexer.rs
@@ -261,9 +261,9 @@ pub enum NormalToken<'input> {
     RecordFields,
     #[token("%record/fields_with_opts%")]
     RecordFieldsWithOpts,
-
     #[token("%record/values%")]
     RecordValues,
+
     #[token("%pow%")]
     Pow,
     #[token("%trace%")]
@@ -287,6 +287,10 @@ pub enum NormalToken<'input> {
     RecordFieldIsDefined,
     #[token("%record/field_is_defined_with_opts%")]
     RecordFieldIsDefinedWithOpts,
+    #[token("%record/split_pair%")]
+    RecordSplitPair,
+    #[token("%record/disjoint_merge%")]
+    RecordDisjointMerge,
 
     #[token("merge")]
     Merge,
diff --git a/core/src/term/mod.rs b/core/src/term/mod.rs
index 505c9c6b..dfdfa723 100644
--- a/core/src/term/mod.rs
+++ b/core/src/term/mod.rs
@@ -1733,6 +1733,30 @@ pub enum BinaryOp {
     /// Test if the field of a record exists and has a definition.
     RecordFieldIsDefined(RecordOpKind),
 
+    /// Take a pair of records and split them into four separate records:
+    ///
+    /// - `left_only`: fields of the left argument but not in the right
+    /// - `left_center`: fields of the left argument that happens to also be in the right (but the
+    /// value and the metadata are taken from the left)
+    /// - `right_center`: fields of the right argument that happens to also be in the left (but the
+    /// value and the metadata are taken from the right)
+    /// - `right_only`: fields of the right argument but not in the left
+    ///
+    /// As opposed to an equivalent user-defined implementation, this primop has better performance
+    /// and is able to preserve field metadata.
+    ///
+    /// If `left` (resp. `right`) is open or has a sealed tail, then `left_only` (resp.
+    /// `right_only`) will inherit the same properties. `left_center` (resp. `right_center`) are
+    /// always closed and without a sealed tail.
+    RecordSplitPair,
+
+    /// Take a pair of disjoint records (i.e. records with no common field) and combine them into
+    /// one. It's a form of merging, but based on the assumption that the records are disjoint and
+    /// thus non-conflicting, it's simpler and more efficient than a general merge.
+    ///
+    /// As for merge, this raises a blame error if one of the arguments has a sealed tail.
+    RecordDisjointMerge,
+
     /// Concatenate two arrays.
     ArrayConcat,
 
@@ -1835,6 +1859,8 @@ impl fmt::Display for BinaryOp {
             RecordFieldIsDefined(RecordOpKind::ConsiderAllFields) => {
                 write!(f, "record/field_is_defined_with_opts")
             }
+            Self::RecordSplitPair => write!(f, "record/full_difference"),
+            Self::RecordDisjointMerge => write!(f, "record/disjoint_merge"),
             ArrayConcat => write!(f, "array/concat"),
             ArrayAt => write!(f, "array/at"),
             Merge(_) => write!(f, "merge"),
diff --git a/core/src/typecheck/operation.rs b/core/src/typecheck/operation.rs
index e481863d..2f8946b3 100644
--- a/core/src/typecheck/operation.rs
+++ b/core/src/typecheck/operation.rs
@@ -500,6 +500,33 @@ pub fn get_bop_type(
             mk_uniftype::dynamic(),
             TypeVarData::unif_type(),
         ),
+        // {_ : a} -> {_ : a}
+        // -> {
+        //  left_only: {_ : a},
+        //  right_only: {_ : a},
+        //  left_center: {_ : a},
+        //  right_center: {_ : a},
+        // }
+        BinaryOp::RecordSplitPair => {
+            let elt = state.table.fresh_type_uvar(var_level);
+            let dict = mk_uniftype::dict(elt.clone());
+
+            let split_result = mk_uty_record!(
+                ("left_only", dict.clone()),
+                ("right_only", dict.clone()),
+                ("left_center", dict.clone()),
+                ("right_center", dict.clone())
+            );
+
+            (dict.clone(), dict, split_result)
+        }
+        // {_ : a} -> {_ : a} -> {_ : a}
+        BinaryOp::RecordDisjointMerge => {
+            let elt = state.table.fresh_type_uvar(var_level);
+            let dict = mk_uniftype::dict(elt.clone());
+
+            (dict.clone(), dict.clone(), dict)
+        }
     })
 }
 
diff --git a/lsp/lsp-harness/src/jsonrpc.rs b/lsp/lsp-harness/src/jsonrpc.rs
index a2a68d06..bc11942d 100644
--- a/lsp/lsp-harness/src/jsonrpc.rs
+++ b/lsp/lsp-harness/src/jsonrpc.rs
@@ -90,11 +90,11 @@ pub struct Response {
 }
 
 impl Server {
-    /// Launch a language server by running the given command.
-    ///
-    /// The command's stdin and stdout will be overridden to "piped" (because
-    /// that's what LSes do).
-    pub fn new(mut cmd: std::process::Command) -> Result<Server> {
+    /// Similar to `new`, but allows passing custom stuff
+    pub fn new_with_options(
+        mut cmd: std::process::Command,
+        initialization_options: Option<serde_json::Value>,
+    ) -> Result<Server> {
         let lsp = cmd.stdin(Stdio::piped()).stdout(Stdio::piped()).spawn()?;
 
         let mut lsp = Server {
@@ -104,11 +104,19 @@ impl Server {
             id: 0,
         };
 
-        lsp.initialize()?;
+        lsp.initialize(initialization_options)?;
 
         Ok(lsp)
     }
 
+    /// Launch a language server by running the given command.
+    ///
+    /// The command's stdin and stdout will be overridden to "piped" (because
+    /// that's what LSes do).
+    pub fn new(cmd: std::process::Command) -> Result<Server> {
+        Server::new_with_options(cmd, None)
+    }
+
     /// Make the language server aware of a file.
     pub fn send_file(&mut self, uri: Url, contents: &str) -> Result<()> {
         self.send_notification::<DidOpenTextDocument>(DidOpenTextDocumentParams {
@@ -151,7 +159,7 @@ impl Server {
         self.send_notification::<Exit>(())
     }
 
-    fn initialize(&mut self) -> Result<()> {
+    fn initialize(&mut self, initialization_options: Option<serde_json::Value>) -> Result<()> {
         // `root_path` is deprecated, but we need ot initialize the struct
         // somehow. There is no `Default` implementation for `InitilizeParams`
         // in versions of `lsp-types` compatible with `codespan-lsp`
@@ -160,7 +168,7 @@ impl Server {
             process_id: None,
             root_path: None,
             root_uri: None,
-            initialization_options: None,
+            initialization_options,
             capabilities: ClientCapabilities::default(),
             trace: None,
             workspace_folders: None,
diff --git a/lsp/lsp-harness/src/lib.rs b/lsp/lsp-harness/src/lib.rs
index 954d21df..5c99a57f 100644
--- a/lsp/lsp-harness/src/lib.rs
+++ b/lsp/lsp-harness/src/lib.rs
@@ -126,14 +126,17 @@ impl Default for TestHarness {
 }
 
 impl TestHarness {
-    pub fn new() -> Self {
+    pub fn new_with_options(initialization_options: Option<serde_json::Value>) -> Self {
         let cmd = std::process::Command::cargo_bin("nls").unwrap();
-        let srv = Server::new(cmd).unwrap();
+        let srv = Server::new_with_options(cmd, initialization_options).unwrap();
         Self {
             srv,
             out: Vec::new(),
         }
     }
+    pub fn new() -> Self {
+        Self::new_with_options(None)
+    }
 
     pub fn request<T: LspRequest>(&mut self, params: T::Params)
     where
diff --git a/lsp/nls/src/background.rs b/lsp/nls/src/background.rs
index 3beacacd..1a7b10a3 100644
--- a/lsp/nls/src/background.rs
+++ b/lsp/nls/src/background.rs
@@ -16,14 +16,12 @@ use nickel_lang_core::{
 use serde::{Deserialize, Serialize};
 
 use crate::{
-    cache::CacheExt as _, diagnostic::SerializableDiagnostic, files::uri_to_path, world::World,
+    cache::CacheExt as _, config, diagnostic::SerializableDiagnostic, files::uri_to_path,
+    world::World,
 };
 
-const EVAL_TIMEOUT: Duration = Duration::from_secs(1);
-const RECURSION_LIMIT: usize = 128;
-// The duration during which a file causing the evaluator to timeout will be blacklisted from further
-// evaluations
-const BLACKLIST_DURATION: Duration = Duration::from_secs(30);
+// Environment variable used to pass the recursion limit value to the child worker
+const RECURSION_LIMIT_ENV_VAR_NAME: &str = "NICKEL_NLS_RECURSION_LIMIT";
 
 #[derive(Debug, Serialize, Deserialize)]
 enum Command {
@@ -106,7 +104,8 @@ pub fn worker_main() -> anyhow::Result<()> {
             // We've already checked that parsing and typechecking are successful, so we
             // don't expect further errors.
             let rt = vm.prepare_eval(file_id).unwrap();
-            let errors = vm.eval_permissive(rt, RECURSION_LIMIT);
+            let recursion_limit = std::env::var(RECURSION_LIMIT_ENV_VAR_NAME)?.parse::<usize>()?;
+            let errors = vm.eval_permissive(rt, recursion_limit);
             diagnostics.extend(
                 errors
                     .into_iter()
@@ -142,12 +141,18 @@ struct SupervisorState {
     eval_stack: Vec<Url>,
 
     // If evaluating a file causes the worker to time out or crash, we blacklist that file
-    // and refuse to evaluate it for `BLACKLIST_DURATION`
+    // and refuse to evaluate it for `self.config.blacklist_duration`
     banned_files: HashMap<Url, Instant>,
+
+    config: config::LspEvalConfig,
 }
 
 impl SupervisorState {
-    fn new(cmd_rx: Receiver<Command>, response_tx: Sender<Diagnostics>) -> anyhow::Result<Self> {
+    fn new(
+        cmd_rx: Receiver<Command>,
+        response_tx: Sender<Diagnostics>,
+        config: config::LspEvalConfig,
+    ) -> anyhow::Result<Self> {
         Ok(Self {
             cmd_rx,
             response_tx,
@@ -155,6 +160,7 @@ impl SupervisorState {
             deps: HashMap::new(),
             banned_files: HashMap::new(),
             eval_stack: Vec::new(),
+            config,
         })
     }
 
@@ -182,6 +188,10 @@ impl SupervisorState {
     fn eval(&self, uri: &Url) -> anyhow::Result<Diagnostics> {
         let path = std::env::current_exe()?;
         let mut child = std::process::Command::new(path)
+            .env(
+                RECURSION_LIMIT_ENV_VAR_NAME,
+                self.config.eval_limits.recursion_limit.to_string(),
+            )
             .arg("--background-eval")
             .stdout(std::process::Stdio::piped())
             .stdin(std::process::Stdio::piped())
@@ -214,7 +224,10 @@ impl SupervisorState {
         };
         bincode::serialize_into(&mut tx, &eval)?;
 
-        let result = run_with_timeout(move || bincode::deserialize_from(rx), EVAL_TIMEOUT);
+        let result = run_with_timeout(
+            move || bincode::deserialize_from(rx),
+            self.config.eval_limits.timeout,
+        );
 
         Ok(result??)
     }
@@ -230,7 +243,8 @@ impl SupervisorState {
             }
             Command::EvalFile { uri } => {
                 match self.banned_files.get(&uri) {
-                    Some(blacklist_time) if blacklist_time.elapsed() < BLACKLIST_DURATION => {}
+                    Some(blacklist_time)
+                        if blacklist_time.elapsed() < self.config.blacklist_duration => {}
                     _ => {
                         // If we re-request an evaluation, remove the old one. (This is quadratic in the
                         // size of the eval stack, but it only contains unique entries so we don't expect it
@@ -285,10 +299,10 @@ impl SupervisorState {
 }
 
 impl BackgroundJobs {
-    pub fn new() -> Self {
+    pub fn new(config: config::LspEvalConfig) -> Self {
         let (cmd_tx, cmd_rx) = crossbeam::channel::unbounded();
         let (diag_tx, diag_rx) = crossbeam::channel::unbounded();
-        match SupervisorState::new(cmd_rx, diag_tx) {
+        match SupervisorState::new(cmd_rx, diag_tx, config) {
             Ok(mut sup) => {
                 std::thread::spawn(move || {
                     sup.run();
diff --git a/lsp/nls/src/config.rs b/lsp/nls/src/config.rs
new file mode 100644
index 00000000..47ae77f0
--- /dev/null
+++ b/lsp/nls/src/config.rs
@@ -0,0 +1,52 @@
+//! Configuration for the Nickel Language Server
+use serde::{Deserialize, Serialize};
+
+use std::time::Duration;
+
+/**
+Limits to appy to the LSP background evaluator.
+If an evaluation reaches one of these limits, it will be canceled and the offending file will be
+temporarily blacklisted.
+*/
+#[derive(Debug, Deserialize, Serialize)]
+#[serde(default)]
+pub struct LspEvalLimits {
+    /// Time out at which to cancel the background evaluation
+    pub timeout: Duration,
+    /// The maximum recursion level to allow in the background evaluator
+    pub recursion_limit: usize,
+}
+impl Default for LspEvalLimits {
+    fn default() -> Self {
+        LspEvalLimits {
+            timeout: Duration::from_secs(1),
+            recursion_limit: 128,
+        }
+    }
+}
+
+/// The configuration of the LSP evaluator
+#[derive(Debug, Deserialize, Serialize)]
+#[serde(default)]
+pub struct LspEvalConfig {
+    pub eval_limits: LspEvalLimits,
+    /// The duration during which a file that broke the background evaluator will be blacklisted
+    /// from it
+    pub blacklist_duration: Duration,
+}
+
+impl Default for LspEvalConfig {
+    fn default() -> Self {
+        LspEvalConfig {
+            eval_limits: Default::default(),
+            blacklist_duration: Duration::from_secs(30),
+        }
+    }
+}
+
+#[derive(Debug, Deserialize, Serialize, Default)]
+#[serde(default)]
+pub struct LspConfig {
+    /// Configuration for the background evaluator in the LSP
+    pub eval_config: LspEvalConfig,
+}
diff --git a/lsp/nls/src/main.rs b/lsp/nls/src/main.rs
index 4908d283..39380d03 100644
--- a/lsp/nls/src/main.rs
+++ b/lsp/nls/src/main.rs
@@ -12,6 +12,7 @@ mod background;
 mod cache;
 mod codespan_lsp;
 mod command;
+mod config;
 mod diagnostic;
 mod error;
 mod field_walker;
@@ -29,7 +30,7 @@ mod usage;
 mod utils;
 mod world;
 
-use crate::trace::Trace;
+use crate::{config::LspConfig, trace::Trace};
 
 #[derive(clap::Parser, Debug)]
 /// The language server of the Nickel language.
@@ -83,9 +84,17 @@ fn main() -> Result<()> {
 
     let capabilities = Server::capabilities();
 
-    connection.initialize(serde_json::to_value(capabilities)?)?;
+    let initialize_params = connection.initialize(serde_json::to_value(capabilities)?)?;
 
-    let _server = Server::new(connection).run();
+    debug!("Raw InitializeParams: {:?}", initialize_params);
+    let config = match initialize_params.get("initializationOptions") {
+        Some(opts) => serde_json::from_value::<LspConfig>(opts.clone())?,
+        None => LspConfig::default(),
+    };
+
+    debug!("Parsed InitializeParams: {:?}", config);
+
+    let _server = Server::new(connection, config).run();
 
     Ok(())
 }
diff --git a/lsp/nls/src/server.rs b/lsp/nls/src/server.rs
index 0ef22975..7bf9ae98 100644
--- a/lsp/nls/src/server.rs
+++ b/lsp/nls/src/server.rs
@@ -19,6 +19,7 @@ use crate::{
     actions,
     background::BackgroundJobs,
     command,
+    config::LspConfig,
     requests::{completion, formatting, goto, hover, rename, symbols},
     trace::Trace,
     world::World,
@@ -73,11 +74,11 @@ impl Server {
         }
     }
 
-    pub fn new(connection: Connection) -> Server {
+    pub fn new(connection: Connection, config: LspConfig) -> Server {
         Server {
             connection,
             world: World::default(),
-            background_jobs: BackgroundJobs::new(),
+            background_jobs: BackgroundJobs::new(config.eval_config),
         }
     }
 


Test patch:
diff --git a/lsp/nls/tests/main.rs b/lsp/nls/tests/main.rs
index 442245f7..173610e4 100644
--- a/lsp/nls/tests/main.rs
+++ b/lsp/nls/tests/main.rs
@@ -1,4 +1,6 @@
 use nickel_lang_utils::project_root::project_root;
+use pretty_assertions::assert_eq;
+use serde_json::json;
 use test_generator::test_resources;
 
 use lsp_harness::{TestFixture, TestHarness};
@@ -111,3 +113,32 @@ fn reload_broken_imports() {
         }
     }
 }
+
+#[test]
+fn apply_client_options() {
+    let _ = env_logger::try_init();
+    let lsp_options = json!({
+        "eval_config": {
+            "eval_limits": {
+                "recursion_limit": 1
+            }
+        }
+    });
+    let mut harness = TestHarness::new_with_options(Some(lsp_options));
+    let url = |s: &str| lsp_types::Url::from_file_path(s).unwrap();
+    harness.send_file(
+        url("/test.ncl"),
+        "{ C = fun n => if n == 0 then String else C (n - 1), res = 2 | C 5 }",
+    );
+
+    // Typecheck diagnostics. Empty because there's nothing to error on
+    let diags = harness.wait_for_diagnostics();
+    assert!(diags.diagnostics.is_empty());
+
+    // Evaluator diagnostics.
+    // These shouldn't be empty (because `C 5 == String` so `2 | C 5` is a contract
+    // violation), but they are because `recursion_limit` is too low for the evaluator to be able
+    // to compute that.
+    let diags = harness.wait_for_diagnostics();
+    assert!(diags.diagnostics.is_empty());
+}