refactor(tvix/eval): generalise error variant for dynamic keys r/4973

Change-Id: I08f40b4b53652a519e76d6e8344c7c3fe10a0689
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6767
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>

2 files changed, 17 insertions, 31 deletions

diff --git a/tvix/eval/src/compiler/bindings.rs b/tvix/eval/src/compiler/bindings.rs
index 55621aa0d8..3dd8514c02 100644
--- a/tvix/eval/src/compiler/bindings.rs
+++ b/tvix/eval/src/compiler/bindings.rs
@@ -56,9 +56,7 @@ impl Compiler<'_> {
                         let name = match self.expr_static_attr_str(&attr) {
                             Some(name) => name,
                             None => {
-                                // TODO(tazjin): error variant for dynamic
-                                // key in *inherit* (or generalise it)
-                                self.emit_error(&attr, ErrorKind::DynamicKeyInLet);
+                                self.emit_error(&attr, ErrorKind::DynamicKeyInScope("inherit"));
                                 continue;
                             }
                         };
@@ -95,9 +93,7 @@ impl Compiler<'_> {
                         let name = match self.expr_static_attr_str(&attr) {
                             Some(name) => name,
                             None => {
-                                // TODO(tazjin): error variant for dynamic
-                                // key in *inherit* (or generalise it)
-                                self.emit_error(&attr, ErrorKind::DynamicKeyInLet);
+                                self.emit_error(&attr, ErrorKind::DynamicKeyInScope("inherit"));
                                 continue;
                             }
                         };
@@ -303,9 +299,7 @@ impl Compiler<'_> {
                         let name = match self.expr_static_attr_str(&attr) {
                             Some(name) => name,
                             None => {
-                                // TODO(tazjin): error variant for dynamic
-                                // key in *inherit* (or generalise it)
-                                self.emit_error(&attr, ErrorKind::DynamicKeyInLet);
+                                self.emit_error(&attr, ErrorKind::DynamicKeyInScope("inherit"));
                                 continue;
                             }
                         };
@@ -342,9 +336,7 @@ impl Compiler<'_> {
                         let name = match self.expr_static_attr_str(&attr) {
                             Some(name) => name,
                             None => {
-                                // TODO(tazjin): error variant for dynamic
-                                // key in *inherit* (or generalise it)
-                                self.emit_error(&attr, ErrorKind::DynamicKeyInLet);
+                                self.emit_error(&attr, ErrorKind::DynamicKeyInScope("inherit"));
                                 continue;
                             }
                         };
@@ -627,8 +619,14 @@ impl Compiler<'_> {
     /// a string if possible, or raise an error about the node being
     /// dynamic.
     fn binding_name(&self, node: ast::Attr) -> EvalResult<String> {
-        self.expr_static_attr_str(&node)
-            .ok_or_else(|| self.dynamic_key_error(&node))
+        match self.expr_static_attr_str(&node) {
+            Some(s) => Ok(s),
+            None => Err(Error {
+                // this code path will go away soon, hence the TODO below
+                kind: ErrorKind::DynamicKeyInScope("TODO"),
+                span: self.span_for(&node),
+            }),
+        }
     }
 
     /// Normalises identifier fragments into a single string vector
@@ -641,18 +639,6 @@ impl Compiler<'_> {
         path.map(|node| self.binding_name(node)).collect()
     }
 
-    /// Construct the error returned when a dynamic attribute is found
-    /// in a `let`-expression.
-    fn dynamic_key_error<N>(&self, node: &N) -> Error
-    where
-        N: ToSpan,
-    {
-        Error {
-            kind: ErrorKind::DynamicKeyInLet,
-            span: self.span_for(node),
-        }
-    }
-
     /// Convert a non-dynamic string expression to a string if possible.
     fn expr_static_str(&self, node: &ast::Str) -> Option<String> {
         let mut parts = node.normalized_parts();
diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs
index 83496b59df..7ec43abb85 100644
--- a/tvix/eval/src/errors.rs
+++ b/tvix/eval/src/errors.rs
@@ -47,8 +47,8 @@ pub enum ErrorKind {
     /// Resolving a user-supplied path literal failed in some way.
     PathResolution(String),
 
-    /// Dynamic keys are not allowed in let.
-    DynamicKeyInLet,
+    /// Dynamic keys are not allowed in some scopes.
+    DynamicKeyInScope(&'static str),
 
     /// Unknown variable in statically known scope.
     UnknownStaticVariable,
@@ -175,8 +175,8 @@ impl Error {
 
             ErrorKind::PathResolution(err) => format!("could not resolve path: {}", err),
 
-            ErrorKind::DynamicKeyInLet => {
-                "dynamically evaluated keys are not allowed in let-bindings".to_string()
+            ErrorKind::DynamicKeyInScope(scope) => {
+                format!("dynamically evaluated keys are not allowed in {}", scope)
             }
 
             ErrorKind::UnknownStaticVariable => "variable not found".to_string(),
@@ -260,7 +260,7 @@ to a missing value in the attribute set(s) included via `with`."#,
             ErrorKind::TypeError { .. } => "E006",
             ErrorKind::Incomparable { .. } => "E007",
             ErrorKind::PathResolution(_) => "E008",
-            ErrorKind::DynamicKeyInLet => "E009",
+            ErrorKind::DynamicKeyInScope(_) => "E009",
             ErrorKind::UnknownStaticVariable => "E010",
             ErrorKind::UnknownDynamicVariable(_) => "E011",
             ErrorKind::VariableAlreadyDefined(_) => "E012",