From 2d401a32e5cef36b44fd35b12c58489cd2595f72 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Sat, 13 Aug 2022 19:42:50 +0300
Subject: feat(tvix/eval): detect dynamic identifier names in `let`

Nix does not allow dynamic identifiers in let expressions (only in
attribute sets), but there are several different kinds of things it
considers static identifiers.

The functions introduced here put the path components of a let
expression into normalised (string) form, and surface an error about
dynamic keys if one is encountered.

Change-Id: Ia3ebd95c6f3ed3cd33b94e156930d2e9c39b6cbf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6189
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
---
 tvix/eval/src/compiler.rs | 49 +++++++++++++++++++++++++++++++++++++++++++----
 tvix/eval/src/errors.rs   |  3 +++
 2 files changed, 48 insertions(+), 4 deletions(-)

(limited to 'tvix')

diff --git a/tvix/eval/src/compiler.rs b/tvix/eval/src/compiler.rs
index cd98531c78..c0e895b7be 100644
--- a/tvix/eval/src/compiler.rs
+++ b/tvix/eval/src/compiler.rs
@@ -37,7 +37,7 @@ struct Local {
     // Definition name, which can be different kinds of tokens (plain
     // string or identifier). Nix does not allow dynamic names inside
     // of `let`-expressions.
-    name: rnix::SyntaxNode,
+    name: String,
 
     // Scope depth of this local.
     depth: usize,
@@ -415,7 +415,7 @@ impl Compiler {
     }
 
     // Compile list literals into equivalent bytecode. List
-    // construction is fairly simple, composing of pushing code for
+    // construction is fairly simple, consisting of pushing code for
     // each literal element and an instruction with the element count.
     //
     // The VM, after evaluating the code for each element, simply
@@ -655,7 +655,7 @@ impl Compiler {
         // can even refer to themselves).
         for entry in node.entries() {
             let key = entry.key().unwrap();
-            let path = key.path().collect::<Vec<_>>();
+            let mut path = normalise_ident_path(key.path())?;
 
             if path.len() != 1 {
                 todo!("nested bindings in let expressions :(")
@@ -664,7 +664,7 @@ impl Compiler {
             entries.push(entry.value().unwrap());
 
             self.locals.locals.push(Local {
-                name: key.node().clone(), // TODO(tazjin): Just an Rc?
+                name: path.pop().unwrap(),
                 depth: self.locals.scope_depth,
             });
         }
@@ -729,6 +729,47 @@ impl Compiler {
     }
 }
 
+/// Convert a single identifier path fragment to a string if possible,
+/// or raise an error about the node being dynamic.
+fn ident_fragment_to_string(node: rnix::SyntaxNode) -> EvalResult<String> {
+    match node.kind() {
+        rnix::SyntaxKind::NODE_IDENT => {
+            Ok(rnix::types::Ident::cast(node).unwrap().as_str().to_string())
+        }
+
+        rnix::SyntaxKind::NODE_STRING => {
+            let s = rnix::types::Str::cast(node).unwrap();
+            let mut parts = s.parts();
+
+            if parts.len() == 1 {
+                if let rnix::value::StrPart::Literal(lit) = parts.pop().unwrap() {
+                    return Ok(lit);
+                }
+            }
+
+            return Err(Error::DynamicKeyInLet(s.node().clone()));
+        }
+
+        // The dynamic node type is just a wrapper and we recurse to
+        // its inner node. C++ Nix does not care about the dynamic
+        // wrapper when determining whether the node itself is
+        // dynamic, it depends solely on the expression inside (i.e.
+        // `let ${"a"} = 1; in a` is valid)
+        rnix::SyntaxKind::NODE_DYNAMIC => {
+            ident_fragment_to_string(rnix::types::Dynamic::cast(node).unwrap().inner().unwrap())
+        }
+
+        _ => Err(Error::DynamicKeyInLet(node)),
+    }
+}
+
+// Normalises identifier fragments into a single string vector for
+// `let`-expressions; fails if fragments requiring dynamic computation
+// are encountered.
+fn normalise_ident_path<I: Iterator<Item = rnix::SyntaxNode>>(path: I) -> EvalResult<Vec<String>> {
+    path.map(ident_fragment_to_string).collect()
+}
+
 pub fn compile(ast: rnix::AST, location: Option<PathBuf>) -> EvalResult<CompilationResult> {
     let mut root_dir = match location {
         Some(dir) => Ok(dir),
diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs
index d8c3ce41cc..a0e3808225 100644
--- a/tvix/eval/src/errors.rs
+++ b/tvix/eval/src/errors.rs
@@ -26,6 +26,9 @@ pub enum Error {
 
     // Resolving a user-supplied path literal failed in some way.
     PathResolution(String),
+
+    // Dynamic keys are not allowed in let.
+    DynamicKeyInLet(rnix::SyntaxNode),
 }
 
 impl Display for Error {
-- 
cgit 1.4.1