feat(natu): lint direct listToAttrs maps #109

Merged
alexion merged 1 commits from feat/natu-gen-attrs-prime into main 2026-09-27 18:05:35 -04:00
8 changed files with 166 additions and 7 deletions
Showing only changes of commit 0e5731145e - Show all commits

View File

@@ -5,10 +5,8 @@
util, util,
}: }:
let let
definitions = lib.listToAttrs ( definitions = lib.genAttrs' (util.collectNixFiles root) (
map (path: lib.nameValuePair (util.removeNixSuffix path) (mkRegistry path)) ( path: lib.nameValuePair (util.removeNixSuffix path) (mkRegistry path)
util.collectNixFiles root
)
); );
mkRegistry = mkRegistry =

View File

@@ -10,6 +10,7 @@ rust_library(
"parser.rs", "parser.rs",
"rules.rs", "rules.rs",
"rules/let_order.rs", "rules/let_order.rs",
"rules/list_to_attrs_map.rs",
"rules/manual_inherit.rs", "rules/manual_inherit.rs",
"rules/manual_inherit_from.rs", "rules/manual_inherit_from.rs",
"rules/unquoted_uri.rs", "rules/unquoted_uri.rs",

View File

@@ -48,6 +48,7 @@ pub struct Ident {
pub enum Expr { pub enum Expr {
Ident(Ident), Ident(Ident),
Select(Select), Select(Select),
Apply(Apply),
Let { Let {
range: SourceRange, range: SourceRange,
bindings: Vec<Binding>, bindings: Vec<Binding>,
@@ -67,6 +68,12 @@ pub struct Select {
pub attr: Ident, pub attr: Ident,
} }
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Apply {
pub lambda: Box<Expr>,
pub argument: Box<Expr>,
}
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub struct Token { pub struct Token {
pub range: SourceRange, pub range: SourceRange,

View File

@@ -77,6 +77,63 @@ fn natu_preserves_suppressed_bindings() -> googletest::Result<()> {
Ok(()) 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] #[gtest]
fn natu_reports_unquoted_uri() -> googletest::Result<()> { fn natu_reports_unquoted_uri() -> googletest::Result<()> {
let report = natu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?; let report = natu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?;

View File

@@ -42,6 +42,13 @@ fn convert_syntax(syntax: rnix::SyntaxNode) -> (Vec<ast::Node>, Vec<ast::Token>)
fn convert_expr(expr: rnix::ast::Expr) -> ast::Expr { fn convert_expr(expr: rnix::ast::Expr) -> ast::Expr {
match expr { match expr {
rnix::ast::Expr::Ident(ident) => ast::Expr::Ident(ident.into()), 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 { rnix::ast::Expr::LetIn(let_in) => ast::Expr::Let {
range: let_in.syntax().text_range().into(), range: let_in.syntax().text_range().into(),
bindings: let_in 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<ast::Binding> { fn convert_binding_ast(binding: rnix::ast::AttrpathValue) -> Option<ast::Binding> {
let mut attrs = binding.attrpath()?.attrs(); let mut attrs = binding.attrpath()?.attrs();
let Attr::Ident(key) = attrs.next()? else { let Attr::Ident(key) = attrs.next()? else {
@@ -139,11 +159,21 @@ fn convert_node(node: rnix::SyntaxNode) -> Option<ast::Node> {
fn convert_binding(node: rnix::SyntaxNode) -> Option<ast::Node> { fn convert_binding(node: rnix::SyntaxNode) -> Option<ast::Node> {
let binding = rnix::ast::AttrpathValue::cast(node)?; let binding = rnix::ast::AttrpathValue::cast(node)?;
let range = binding.syntax().text_range().into(); 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 { Some(ast::Node {
range, range,
data: ast::NodeData::Binding(data), data: ast::NodeData::Binding(ast::Binding {
range,
key: key.into(),
value: convert_expr(binding.value()?),
}),
}) })
} }

View File

@@ -1,11 +1,13 @@
use crate::FileId; use crate::FileId;
use crate::ast; use crate::ast;
use crate::rules::list_to_attrs_map::ListToAttrsMap;
use crate::rules::manual_inherit::ManualInherit; use crate::rules::manual_inherit::ManualInherit;
use crate::rules::manual_inherit_from::ManualInheritFrom; use crate::rules::manual_inherit_from::ManualInheritFrom;
use crate::rules::unquoted_uri::UnquotedUri; use crate::rules::unquoted_uri::UnquotedUri;
use crate::scope_graph::ScopeGraph; use crate::scope_graph::ScopeGraph;
pub mod let_order; pub mod let_order;
pub mod list_to_attrs_map;
pub mod manual_inherit; pub mod manual_inherit;
pub mod manual_inherit_from; pub mod manual_inherit_from;
pub mod unquoted_uri; pub mod unquoted_uri;
@@ -65,6 +67,7 @@ pub fn run_rules(file: &ast::ParsedFile, graph: &ScopeGraph, source: &str) -> Ve
.iter() .iter()
.flat_map(|node| { .flat_map(|node| {
[ [
run_node_rule::<ListToAttrsMap>(node),
run_node_rule::<ManualInherit>(node), run_node_rule::<ManualInherit>(node),
run_node_rule::<ManualInheritFrom>(node), run_node_rule::<ManualInheritFrom>(node),
] ]

View File

@@ -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,
}
}

View File

@@ -42,11 +42,16 @@ impl ScopeGraph {
} }
self.build_expr(body, scope) 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 ast::Expr::Other(expressions) => expressions
.iter() .iter()
.flat_map(|expression| self.build_expr(expression, scope)) .flat_map(|expression| self.build_expr(expression, scope))
.collect(), .collect(),
ast::Expr::Select(_) => Vec::new(), ast::Expr::Select(select) => vec![self.add_reference(&select.receiver, scope)],
} }
} }