Add a recursion limit to background evaluation (#1878)

* Add a recursion limit to eval_permissive

* Add a test

* Review comments

core/src/eval/mod.rs

@@ -943,12 +943,19 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
     ///   fields and array elements, we keep evaluating subsequent elements even if one
     ///   fails.
     /// - We only return the accumulated errors; we don't return the eval'ed term.
-    pub fn eval_permissive(&mut self, rt: RichTerm) -> Vec<EvalError> {
+    /// - We support a recursion limit, to limit the number of times we recurse into
+    ///   arrays or records.
+    pub fn eval_permissive(&mut self, rt: RichTerm, recursion_limit: usize) -> Vec<EvalError> {
         fn inner<R: ImportResolver, C: Cache>(
             slf: &mut VirtualMachine<R, C>,
             acc: &mut Vec<EvalError>,
             rt: RichTerm,
+            recursion_limit: usize,
         ) {
+            if recursion_limit == 0 {
+                return;
+            }
+
             let pos = rt.pos;
             match slf.eval(rt) {
                 Err(e) => {
@@ -966,7 +973,7 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
                                 attrs.pending_contracts.iter().cloned(),
                                 t.pos,
                             );
-                            inner(slf, acc, value_with_ctr);
+                            inner(slf, acc, value_with_ctr, recursion_limit.saturating_sub(1));
                         }
                     }
                     Term::Record(data) => {
@@ -977,7 +984,7 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
                                     field.pending_contracts.iter().cloned(),
                                     v.pos,
                                 );
-                                inner(slf, acc, value_with_ctr);
+                                inner(slf, acc, value_with_ctr, recursion_limit.saturating_sub(1));
                             } else {
                                 acc.push(EvalError::MissingFieldDef {
                                     id: *id,
@@ -993,7 +1000,7 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
             }
         }
         let mut ret = Vec::new();
-        inner(self, &mut ret, rt);
+        inner(self, &mut ret, rt, recursion_limit);
         ret
     }
 }

lsp/nls/src/background.rs

@@ -19,6 +19,7 @@ use crate::{
 };
 
 const EVAL_TIMEOUT: Duration = Duration::from_secs(1);
+const RECURSION_LIMIT: usize = 128;
 
 #[derive(Debug, Serialize, Deserialize)]
 enum Command {
@@ -97,7 +98,7 @@ 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);
+            let errors = vm.eval_permissive(rt, RECURSION_LIMIT);
             diagnostics.extend(
                 errors
                     .into_iter()

lsp/nls/tests/inputs/diagnostics-recursion.ncl

@@ -0,0 +1,7 @@
+### /diagnostics-recursion.ncl
+let rec foo = { bar = foo } in
+[
+  foo,
+  foo.bar.bar.bar.bar.bar.baz
+]
+### diagnostic = ["file:///diagnostics-recursion.ncl"]

lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__diagnostics-recursion.ncl.snap

@@ -0,0 +1,8 @@
+---
+source: lsp/nls/tests/main.rs
+expression: output
+---
+(file:///diagnostics-recursion.ncl, 0:14-0:27: this record lacks the field `baz`)
+(file:///diagnostics-recursion.ncl, 3:2-3:29: missing field `baz`
+Did you mean `bar`?)
+(file:///diagnostics-recursion.ncl, 3:2-3:29: this requires the field `baz` to exist)