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

improved unbound identifier detection #95

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ rnix = "0.10.2"
serde = "1.0.138"
serde_json = "1.0.82"
nixpkgs-fmt-rnix = "1.2.0"
maplit = "1"

[dev-dependencies]
stoppable_thread = "0.2.1"
maplit = "1"

[features]

Expand Down
6 changes: 4 additions & 2 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub struct Located<T: Error + Clone + Trace + Finalize> {
#[unsafe_ignore_trace]
pub range: TextRange,
/// The nature of the issue
pub kind: T
pub kind: T,
}

impl<T: Error + Clone + Trace + Finalize> std::error::Error for Located<T> {}
Expand Down Expand Up @@ -117,7 +117,9 @@ impl std::fmt::Display for ValueError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
ValueError::DivisionByZero => write!(f, "division by zero"),
ValueError::AttrAlreadyDefined(name) => write!(f, "attribute `{}` defined more than once", name),
ValueError::AttrAlreadyDefined(name) => {
write!(f, "attribute `{}` defined more than once", name)
}
ValueError::TypeError(msg) => write!(f, "{}", msg),
ValueError::UnboundIdentifier(name) => write!(f, "identifier {} is unbound", name),
}
Expand Down
170 changes: 169 additions & 1 deletion src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ use rnix::TextRange;
use std::borrow::Borrow;
use std::collections::HashMap;

/// A string part
///
/// in `"foo${bar}baz"`, parts are `Literal("foo")`, `Expression(bar)` and `Literal("baz")`
#[derive(Debug, Trace, Finalize)]
pub enum StringPartSource {
/// Literal string
Literal(String),
/// Interpolated expression
Expression(ExprResultBox),
}

// Expressions like BinOp have the only copy of their Expr children,
// so they use ExprResultBox. Expressions like Map, which may have
// contents copied in multiple places, need ExprResultGc.
Expand Down Expand Up @@ -36,6 +47,18 @@ pub enum ExprSource {
/// ```
definitions: Vec<ExprResultGc>,
},
LetIn {
aaronjanse marked this conversation as resolved.
Show resolved Hide resolved
/// We use a list because the user might define the same top-level
/// attribute in multiple places via path syntax. For example:
/// ```nix
/// {
/// xyz.foo = true;
/// xyz.bar = false;
/// }
/// ```
definitions: Vec<ExprResultGc>,
body: ExprResultBox,
},
/// See the AttrSet handling in Expr::parse for more details.
/// Note that this syntax is the exact opposite of Expr::Select.
KeyValuePair {
Expand All @@ -50,19 +73,76 @@ pub enum ExprSource {
from: ExprResultGc,
index: ExprResultBox,
},
/// `assert condition; body`
Assert {
/// the asserted condition
condition: ExprResultBox,
/// the body which is only evaluated if the assertion is true
body: ExprResultBox,
},
/// Dynamic attribute, such as the curly braces in `foo.${toString (1+1)}`
Dynamic {
inner: ExprResultBox,
},
/// `with inner; body`
With {
/// the expression used to provide bindings
inner: ExprResultGc,
/// the body evaluated with the new bindings
body: ExprResultBox,
},
Ident {
name: String,
},
Literal {
value: NixValue,
},
/// `if condition then body else else_body`
IfElse {
/// the condition evaluated
condition: ExprResultBox,
/// the body evaluated when the condition is true
body: ExprResultBox,
/// the body evaluated when the condition is false
else_body: ExprResultBox,
},
/// A string, possibly interpolated
String {
/// interpolated and literal parts of this string
parts: Vec<StringPartSource>,
},
/// `a.b or c`
OrDefault {
/// `a.b`, of type `Select`
index: ExprResultBox,
/// `c`
default: ExprResultBox,
},
Paren {
inner: ExprResultBox,
},
/// `{ arg1, ... } @ args` in a lambda definition `{ arg1, ... } @ args: body`
Pattern {
/// for `{ arg1, arg2 ? default }: body`, a map `"arg1" => None, "arg2" => default`
entries: HashMap<String, Option<ExprResultGc>>,
/// whether the patter is incomplete (contains `...`)
ellipsis: bool,
/// the identifier bound by `@`
at: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
at: Option<String>,
at: Option<ExprResultGc>,

This would let us put the identifier in a Scope, and the Expression struct would also give us more info about where the identifier is located.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it an ExprResultBox as there is no need for sharing as far as I can see

},
Lambda {
/// A `Pattern` or an `Identifier`
arg: ExprResultBox,
/// the body of the function
body: ExprResultBox,
},
/// Function application: `f x`
Apply {
/// the function `f` applied
function: ExprResultBox,
/// the argument `x`
arg: ExprResultBox,
},
BinOp {
op: BinOpKind,
left: ExprResultBox,
Expand All @@ -86,6 +166,9 @@ pub enum ExprSource {
UnaryNegate {
value: ExprResultBox,
},
List {
elements: Vec<ExprResultGc>,
},
}

/// Syntax node that has context and can be lazily evaluated.
Expand Down Expand Up @@ -119,7 +202,7 @@ impl Expr {
// from an .eval(), which is probably infinite recursion.
return Err(EvalError::Internal(InternalError::Unimplemented(
"infinite recursion".to_string(),
)))
)));
}
};
if let Some(ref value) = *value_borrow {
Expand All @@ -135,6 +218,7 @@ impl Expr {
fn eval_uncached(&self) -> Result<Gc<NixValue>, EvalError> {
match &self.source {
ExprSource::Paren { inner } => inner.as_ref()?.eval(),
ExprSource::LetIn { body, .. } => body.as_ref()?.eval(),
ExprSource::Literal { value } => Ok(Gc::new(value.clone())),
ExprSource::BoolAnd { left, right } => {
if left.as_ref()?.eval()?.as_bool()? {
Expand Down Expand Up @@ -236,6 +320,33 @@ impl Expr {
}
}))
}
ExprSource::IfElse { .. } => Err(EvalError::Internal(InternalError::Unimplemented(
"evaluating `if` is not implemented".to_string(),
))),
ExprSource::Assert { .. } => Err(EvalError::Internal(InternalError::Unimplemented(
"evaluating `assert` operator is not implemented".to_string(),
))),
ExprSource::OrDefault { .. } => Err(EvalError::Internal(InternalError::Unimplemented(
"evaluating `or` default operator is not implemented".to_string(),
))),
ExprSource::With { .. } => Err(EvalError::Internal(InternalError::Unimplemented(
"evaluating with expressions is not implemented".to_string(),
))),
ExprSource::String { .. } => Err(EvalError::Internal(InternalError::Unimplemented(
"evaluating strings is not implemented".to_string(),
))),
ExprSource::List { .. } => Err(EvalError::Internal(InternalError::Unimplemented(
"evaluating lists is not implemented".to_string(),
))),
ExprSource::Apply { .. } => Err(EvalError::Internal(InternalError::Unimplemented(
"evaluating function application is not implemented".to_string(),
))),
ExprSource::Lambda { .. } => Err(EvalError::Internal(InternalError::Unimplemented(
"evaluating function is not implemented".to_string(),
aaronjanse marked this conversation as resolved.
Show resolved Hide resolved
))),
ExprSource::Pattern { .. } => Err(EvalError::Internal(InternalError::Unimplemented(
"evaluating function argument pattern is not implemented".to_string(),
))),
ExprSource::AttrSet { .. } => Err(EvalError::Internal(InternalError::Unexpected(
"eval_uncached ExprSource::Map should be unreachable, ".to_string()
+ "since the Expr::value should be initialized at creation",
Expand Down Expand Up @@ -282,9 +393,51 @@ impl Expr {
ExprSource::BinOp { op: _, left, right } => vec![left, right],
ExprSource::BoolAnd { left, right } => vec![left, right],
ExprSource::BoolOr { left, right } => vec![left, right],
ExprSource::IfElse { condition, body, else_body } => vec![condition, body, else_body],
ExprSource::Assert { condition, body } => vec![condition, body],
ExprSource::Implication { left, right } => vec![left, right],
ExprSource::OrDefault { index, default } => vec![index, default],
ExprSource::UnaryInvert { value } => vec![value],
ExprSource::UnaryNegate { value } => vec![value],
ExprSource::Apply { function, arg } => vec![function, arg],
ExprSource::With { inner, body } => {
let mut res = vec![];
if let Ok(inner) = inner {
res.push(inner.as_ref())
}
if let Ok(body) = body {
res.push(body.as_ref())
}
return res
},
ExprSource::Pattern { entries, .. } => {
let mut res = vec![];
for entry in entries.values() {
if let Some(Ok(default)) = entry {
res.push(default.as_ref());
}
}
return res
},
ExprSource::Lambda { arg, body } => vec![arg, body],
ExprSource::List { elements } => {
let mut res = vec![];
for entry in elements.iter() {
if let Ok(default) = entry {
res.push(default.as_ref());
}
}
return res
}
ExprSource::String { parts } => {
let mut res = vec![];
for entry in parts.iter() {
if let StringPartSource::Expression(Ok(inner)) = entry {
res.push(inner.as_ref());
}
}
return res
}
ExprSource::AttrSet {
definitions,
} => {
Expand All @@ -300,6 +453,21 @@ impl Expr {
.map(|x| x.as_ref())
.collect();
}
ExprSource::LetIn {
definitions,
body
} => {
let mut out = vec![];
for def in definitions {
if let Ok(x) = def.as_ref() {
out.push(x.as_ref())
}
}
if let Ok(b) = body {
out.push(b.as_ref())
}
return out;
}
ExprSource::KeyValuePair { key, value } => {
let mut out = vec![];
if let Ok(x) = value {
Expand Down