diff --git a/src/dotfiles/registry.nix b/src/dotfiles/registry.nix index c8ce56e..c1ac884 100644 --- a/src/dotfiles/registry.nix +++ b/src/dotfiles/registry.nix @@ -5,10 +5,8 @@ util, }: let - definitions = lib.listToAttrs ( - map (path: lib.nameValuePair (util.removeNixSuffix path) (mkRegistry path)) ( - util.collectNixFiles root - ) + definitions = lib.genAttrs' (util.collectNixFiles root) ( + path: lib.nameValuePair (util.removeNixSuffix path) (mkRegistry path) ); mkRegistry = diff --git a/src/natu/BUILD b/src/natu/BUILD index 40c8de6..bf6efcf 100644 --- a/src/natu/BUILD +++ b/src/natu/BUILD @@ -10,6 +10,7 @@ rust_library( "parser.rs", "rules.rs", "rules/let_order.rs", + "rules/list_to_attrs_map.rs", "rules/manual_inherit.rs", "rules/manual_inherit_from.rs", "rules/unquoted_uri.rs", diff --git a/src/natu/ast.rs b/src/natu/ast.rs index 61038d9..b4fe8fa 100644 --- a/src/natu/ast.rs +++ b/src/natu/ast.rs @@ -48,6 +48,7 @@ pub struct Ident { pub enum Expr { Ident(Ident), Select(Select), + Apply(Apply), Let { range: SourceRange, bindings: Vec, @@ -67,6 +68,12 @@ pub struct Select { pub attr: Ident, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Apply { + pub lambda: Box, + pub argument: Box, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct Token { pub range: SourceRange, diff --git a/src/natu/natu_test.rs b/src/natu/natu_test.rs index 7584791..90078bf 100644 --- a/src/natu/natu_test.rs +++ b/src/natu/natu_test.rs @@ -77,6 +77,63 @@ fn natu_preserves_suppressed_bindings() -> googletest::Result<()> { Ok(()) } +#[gtest] +fn natu_reports_select_dependency_order() -> googletest::Result<()> { + let report = natu::future_sight(vec![SourceFile { + path: PathBuf::from("testdata/select_dependencies.nix"), + source: r#" +let + dependency = { value = 1; }; + entry = dependency.value; +in +entry +"# + .into(), + }]) + .or_fail()?; + + let count = report + .diagnostics + .iter() + .filter(|diagnostic| diagnostic.finding.rule_id == "let_order") + .count(); + expect_that!(count, eq(1)); + Ok(()) +} + +#[gtest] +fn natu_reports_list_to_attrs_after_map() -> googletest::Result<()> { + let report = natu::future_sight(vec![SourceFile { + path: PathBuf::from("testdata/list_to_attrs.nix"), + source: r#" +let + generated = lib.listToAttrs ( + map (item: lib.nameValuePair item item) items + ); + generated_pairs = lib.listToAttrs ( + concatLists ( + mapAttrsToList (name: _: lib.nameValuePair name name) attrs + ) + ); + precomputed = lib.listToAttrs pairs; +in +{ + inherit generated generated_pairs precomputed; +} +"# + .into(), + }]) + .or_fail()?; + + let count = report + .diagnostics + .iter() + .filter(|diagnostic| diagnostic.finding.rule_id == "list_to_attrs_map") + .count(); + expect_that!(count, eq(1)); + Ok(()) +} + #[gtest] fn natu_reports_unquoted_uri() -> googletest::Result<()> { let report = natu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?; diff --git a/src/natu/parser.rs b/src/natu/parser.rs index 9ece9db..e81de4a 100644 --- a/src/natu/parser.rs +++ b/src/natu/parser.rs @@ -42,6 +42,13 @@ fn convert_syntax(syntax: rnix::SyntaxNode) -> (Vec, Vec) fn convert_expr(expr: rnix::ast::Expr) -> ast::Expr { match expr { rnix::ast::Expr::Ident(ident) => ast::Expr::Ident(ident.into()), + rnix::ast::Expr::Select(select) => convert_select_expr(select), + rnix::ast::Expr::Apply(apply) => ast::Expr::Apply(ast::Apply { + lambda: Box::new(convert_expr(apply.lambda().expect("applications have a lambda"))), + argument: Box::new(convert_expr( + apply.argument().expect("applications have an argument"), + )), + }), rnix::ast::Expr::LetIn(let_in) => ast::Expr::Let { range: let_in.syntax().text_range().into(), bindings: let_in @@ -67,6 +74,19 @@ fn convert_expr(expr: rnix::ast::Expr) -> ast::Expr { } } +fn convert_select_expr(select: rnix::ast::Select) -> ast::Expr { + select.clone().try_into().map(ast::Expr::Select).unwrap_or_else(|_| { + ast::Expr::Other( + select + .syntax() + .children() + .filter_map(rnix::ast::Expr::cast) + .map(convert_expr) + .collect(), + ) + }) +} + fn convert_binding_ast(binding: rnix::ast::AttrpathValue) -> Option { let mut attrs = binding.attrpath()?.attrs(); let Attr::Ident(key) = attrs.next()? else { @@ -139,11 +159,21 @@ fn convert_node(node: rnix::SyntaxNode) -> Option { fn convert_binding(node: rnix::SyntaxNode) -> Option { let binding = rnix::ast::AttrpathValue::cast(node)?; let range = binding.syntax().text_range().into(); - let data = binding.try_into().ok()?; + let mut attrs = binding.attrpath()?.attrs(); + let Attr::Ident(key) = attrs.next()? else { + return None; + }; + if attrs.next().is_some() { + return None; + } Some(ast::Node { range, - data: ast::NodeData::Binding(data), + data: ast::NodeData::Binding(ast::Binding { + range, + key: key.into(), + value: convert_expr(binding.value()?), + }), }) } diff --git a/src/natu/rules.rs b/src/natu/rules.rs index b295ecd..4ab650f 100644 --- a/src/natu/rules.rs +++ b/src/natu/rules.rs @@ -1,11 +1,13 @@ use crate::FileId; use crate::ast; +use crate::rules::list_to_attrs_map::ListToAttrsMap; use crate::rules::manual_inherit::ManualInherit; use crate::rules::manual_inherit_from::ManualInheritFrom; use crate::rules::unquoted_uri::UnquotedUri; use crate::scope_graph::ScopeGraph; pub mod let_order; +pub mod list_to_attrs_map; pub mod manual_inherit; pub mod manual_inherit_from; pub mod unquoted_uri; @@ -65,6 +67,7 @@ pub fn run_rules(file: &ast::ParsedFile, graph: &ScopeGraph, source: &str) -> Ve .iter() .flat_map(|node| { [ + run_node_rule::(node), run_node_rule::(node), run_node_rule::(node), ] diff --git a/src/natu/rules/list_to_attrs_map.rs b/src/natu/rules/list_to_attrs_map.rs new file mode 100644 index 0000000..b45ba07 --- /dev/null +++ b/src/natu/rules/list_to_attrs_map.rs @@ -0,0 +1,58 @@ +use crate::{ast, rules::NodeRule}; + +pub struct ListToAttrsMap; + +impl NodeRule for ListToAttrsMap { + type Node = ast::Binding; + + const ID: &'static str = "list_to_attrs_map"; + + fn select(data: &ast::NodeData) -> Option<&Self::Node> { + let ast::NodeData::Binding(binding) = data; + + Some(binding) + } + + fn check(node: &Self::Node) -> Option<&'static str> { + matches_list_to_attrs_map(&node.value) + .then_some("Use `lib.genAttrs'` instead of `lib.listToAttrs` after `map`") + } +} + +fn matches_list_to_attrs_map(expr: &ast::Expr) -> bool { + let ast::Expr::Apply(application) = unwrap_expr(expr) else { + return false; + }; + + is_lib_list_to_attrs(&application.lambda) && is_map_application(&application.argument) +} + +fn unwrap_expr(expr: &ast::Expr) -> &ast::Expr { + match expr { + ast::Expr::Other(expressions) if expressions.len() == 1 => unwrap_expr(&expressions[0]), + _ => expr, + } +} + +fn is_lib_list_to_attrs(expr: &ast::Expr) -> bool { + matches!( + unwrap_expr(expr), + ast::Expr::Select(select) if select.receiver == "lib" && select.attr.name == "listToAttrs" + ) +} + +fn is_map_application(expr: &ast::Expr) -> bool { + let ast::Expr::Apply(application) = unwrap_expr(expr) else { + return false; + }; + + is_map(&application.lambda) +} + +fn is_map(expr: &ast::Expr) -> bool { + match unwrap_expr(expr) { + ast::Expr::Ident(ident) => ident.name == "map", + ast::Expr::Apply(application) => is_map(&application.lambda), + _ => false, + } +} diff --git a/src/natu/scope_graph.rs b/src/natu/scope_graph.rs index e509f6d..2dd2139 100644 --- a/src/natu/scope_graph.rs +++ b/src/natu/scope_graph.rs @@ -42,11 +42,16 @@ impl ScopeGraph { } self.build_expr(body, scope) } + ast::Expr::Apply(application) => [ + self.build_expr(&application.lambda, scope), + self.build_expr(&application.argument, scope), + ] + .concat(), ast::Expr::Other(expressions) => expressions .iter() .flat_map(|expression| self.build_expr(expression, scope)) .collect(), - ast::Expr::Select(_) => Vec::new(), + ast::Expr::Select(select) => vec![self.add_reference(&select.receiver, scope)], } }