Skip to content
This repository has been archived by the owner on Jan 25, 2024. It is now read-only.

Commit

Permalink
fix attrset merging for >= 3 instances (#96)
Browse files Browse the repository at this point in the history
  • Loading branch information
symphorien authored Sep 17, 2022
1 parent ff18e04 commit 6925256
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 12 deletions.
59 changes: 47 additions & 12 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,34 @@ impl Expr {

/// Used for merging sets during parsing. For example:
/// { a.b = 1; a.c = 2; } => { a = { b = 1; c = 2; }; }
///
/// Nix only allows merging several such KeyValuePairs when they correspond
/// to bare literals. Inserting a mere indirection through let or a function
/// prevents this from happening and throws an error instead:
/// ```text
/// nix-repl> :p { a = { b = 1; }; a.c = 2; }
/// { a = { b = 1; c = 2; }; }
///
/// nix-repl> :p { a = (x: x){ b = 1; }; a.c = 2; }
/// error: attribute 'a.c' already defined at (string):1:3
///
/// at «string»:1:25:
///
/// 1| { a = (x: x){ b = 1; }; a.c = 2; }
/// | ^
///
/// nix-repl> :p let y = { b = 1; }; in { a = y; a.c = 2; }
/// error: attribute 'a.c' already defined at (string):1:26
///
/// at «string»:1:33:
///
/// 1| let y = { b = 1; }; in { a = y; a.c = 2; }
/// | ^
///
/// ```
/// This function takes a and b, attempts to interpret them as literal
/// key value pairs or literal attrset values, and if it failed returns
/// such an error.
pub fn merge_set_literal(name: String, a: Gc<Expr>, b: Gc<Expr>) -> Result<Gc<Expr>, EvalError> {
// evaluate literal attr sets only, otherwise error
let eval_literal = |src: Gc<Expr>| {
Expand All @@ -391,18 +419,25 @@ pub fn merge_set_literal(name: String, a: Gc<Expr>, b: Gc<Expr>) -> Result<Gc<Ex
} else {
src
};
if let ExprSource::AttrSet { .. } = &src.source {
src.eval()?.as_map()
} else {
// We cannot merge a literal with a non-literal. This error is
// caused by incorrect expressions such as:
// ```
// repl> let x = { y = 1; }; in { a = x; a.z = 2; }
// error: attribute 'a.z' at (string):1:33 already defined at (string):1:26
// ```
// The above would be caught because `x` is an ExprSource::Ident (as
// opposed to being an ExprSource::AttrSet literal).
Err(EvalError::Value(ValueError::AttrAlreadyDefined(name.to_string())))
match &src.source {
ExprSource::AttrSet { .. } => src.eval()?.as_map(),
ExprSource::Literal {
value: NixValue::Map(m),
..
} => Ok(m.clone()),
_ => {
// We cannot merge a literal with a non-literal. This error is
// caused by incorrect expressions such as:
// ```
// repl> let x = { y = 1; }; in { a = x; a.z = 2; }
// error: attribute 'a.z' at (string):1:33 already defined at (string):1:26
// ```
// The above would be caught because `x` is an ExprSource::Ident (as
// opposed to being an ExprSource::AttrSet literal).
Err(EvalError::Value(ValueError::AttrAlreadyDefined(
name.to_string(),
)))
}
}
};

Expand Down
20 changes: 20 additions & 0 deletions src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use serde_json::json;
use std::time::Duration;
#[cfg(test)]
use stoppable_thread::*;
#[cfg(test)]
use crate::error::ValueError;

#[allow(dead_code)]
/// Evaluates a nix code snippet
Expand Down Expand Up @@ -294,13 +296,31 @@ fn attrs_merge() {
assert_eq!(eval(&format!("{}.a.c", code)).as_int().unwrap(), 2);
}

#[test]
fn attrs_merge_3() {
let code = "{ a.b = 1; a.c = 2; a.d = 3; }".to_string();
assert_eq!(eval(&format!("{}.a.b", code)).as_int().unwrap(), 1);
assert_eq!(eval(&format!("{}.a.c", code)).as_int().unwrap(), 2);
assert_eq!(eval(&format!("{}.a.d", code)).as_int().unwrap(), 3);
}

#[test]
fn attrs_merge_conflict() {
let ast = rnix::parse("{ a = { b = 1; c = 3; }; a.c = 2; }");
let root = ast.root().inner().unwrap();
let path = std::env::current_dir().unwrap();
let parse_result = Expr::parse(root, Gc::new(Scope::Root(path)));
assert!(matches!(parse_result, Err(EvalError::Value(ValueError::AttrAlreadyDefined(_)))));
}

#[test]
fn attrs_merge_conflict_2() {
let ast = rnix::parse("{ a = let y = { b = 1; }; in y; a.c = 2; }");
let root = ast.root().inner().unwrap();
let path = std::env::current_dir().unwrap();
let parse_result = Expr::parse(root, Gc::new(Scope::Root(path)));
assert!(parse_result.is_err());
// assert!(matches!(parse_result, Err(EvalError::Value(ValueError::AttrAlreadyDefined(_)))));
}

#[test]
Expand Down

0 comments on commit 6925256

Please sign in to comment.