From 6486dda6ec0850b149f865176c5f15076b7e3a6c Mon Sep 17 00:00:00 2001 From: alexion Date: Sat, 19 Sep 2026 18:13:11 -0400 Subject: [PATCH] feat(xatu): report empty let expressions --- src/xatu/BUILD | 2 ++ src/xatu/ast.rs | 7 ++++ src/xatu/parser.rs | 21 ++++++++++++ src/xatu/rules.rs | 3 ++ src/xatu/rules/empty_let_in.rs | 24 +++++++++++++ src/xatu/testdata/clean.nix | 5 +++ src/xatu/testdata/lint_findings.nix | 1 + src/xatu/testdata/lint_interactions.nix | 8 +++++ src/xatu/testdata/lint_variants.nix | 5 +++ src/xatu/xatu_test.rs | 45 +++++++++++++++++++++++++ 10 files changed, 121 insertions(+) create mode 100644 src/xatu/rules/empty_let_in.rs create mode 100644 src/xatu/testdata/lint_interactions.nix diff --git a/src/xatu/BUILD b/src/xatu/BUILD index a10f057..d88e2a7 100644 --- a/src/xatu/BUILD +++ b/src/xatu/BUILD @@ -10,6 +10,7 @@ rust_library( "rules.rs", "rules/deprecated_to_path.rs", "rules/empty_inherit.rs", + "rules/empty_let_in.rs", "rules/unquoted_uri.rs", "xatu.rs", ], @@ -33,6 +34,7 @@ rust_test( "testdata/clean.nix", "testdata/invalid.nix", "testdata/lint_findings.nix", + "testdata/lint_interactions.nix", "testdata/lint_variants.nix", ], deps = [ diff --git a/src/xatu/ast.rs b/src/xatu/ast.rs index 220f61b..a1e9959 100644 --- a/src/xatu/ast.rs +++ b/src/xatu/ast.rs @@ -29,6 +29,7 @@ impl std::fmt::Display for SourceRange { #[derive(Debug, Clone, PartialEq, Eq)] pub enum NodeData { Apply(Apply), + LetIn(LetIn), Inherit(Inherit), } @@ -54,6 +55,12 @@ pub struct Select { pub attrs: Vec, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LetIn { + pub entry_count: usize, + pub inherit_count: usize, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct Inherit { pub attr_count: usize, diff --git a/src/xatu/parser.rs b/src/xatu/parser.rs index 97a7ccb..4d43133 100644 --- a/src/xatu/parser.rs +++ b/src/xatu/parser.rs @@ -37,6 +37,7 @@ fn convert_syntax(syntax: rnix::SyntaxNode) -> (Vec, Vec) fn convert_node(node: rnix::SyntaxNode) -> Option { match node.kind() { rnix::SyntaxKind::NODE_APPLY => convert_apply(node), + rnix::SyntaxKind::NODE_LET_IN => convert_let_in(node), rnix::SyntaxKind::NODE_INHERIT => convert_inherit(node), _ => None, } @@ -102,6 +103,26 @@ impl TryFrom for ast::Select { } } +fn convert_let_in(node: rnix::SyntaxNode) -> Option { + let let_in = rnix::ast::LetIn::cast(node)?; + + Some(ast::Node { + range: let_in.syntax().text_range().into(), + data: ast::NodeData::LetIn(let_in.into()), + }) +} + +impl From for ast::LetIn { + fn from(let_in: rnix::ast::LetIn) -> Self { + use rnix::ast::HasEntry; + + Self { + entry_count: let_in.entries().count(), + inherit_count: let_in.inherits().count(), + } + } +} + fn convert_inherit(node: rnix::SyntaxNode) -> Option { let inherit = rnix::ast::Inherit::cast(node)?; diff --git a/src/xatu/rules.rs b/src/xatu/rules.rs index 02aef3b..8def783 100644 --- a/src/xatu/rules.rs +++ b/src/xatu/rules.rs @@ -2,10 +2,12 @@ use crate::FileId; use crate::ast; use crate::rules::deprecated_to_path::DeprecatedToPath; use crate::rules::empty_inherit::EmptyInherit; +use crate::rules::empty_let_in::EmptyLetIn; use crate::rules::unquoted_uri::UnquotedUri; pub mod deprecated_to_path; pub mod empty_inherit; +pub mod empty_let_in; pub mod unquoted_uri; #[derive(Debug)] @@ -54,6 +56,7 @@ pub fn run_rules(file: &ast::ParsedFile) -> Vec { [ run_node_rule::(node), run_node_rule::(node), + run_node_rule::(node), ] .into_iter() .flatten() diff --git a/src/xatu/rules/empty_let_in.rs b/src/xatu/rules/empty_let_in.rs new file mode 100644 index 0000000..e4577f9 --- /dev/null +++ b/src/xatu/rules/empty_let_in.rs @@ -0,0 +1,24 @@ +use crate::{ast, rules::NodeRule}; + +pub struct EmptyLetIn; + +impl NodeRule for EmptyLetIn { + type Node = ast::LetIn; + + const ID: &'static str = "empty_let_in"; + + fn select(data: &ast::NodeData) -> Option<&Self::Node> { + match data { + ast::NodeData::LetIn(let_in) => Some(let_in), + _ => None, + } + } + + fn check(node: &Self::Node) -> Option<&'static str> { + if node.entry_count == 0 && node.inherit_count == 0 { + Some("let expression has no bindings") + } else { + None + } + } +} diff --git a/src/xatu/testdata/clean.nix b/src/xatu/testdata/clean.nix index 0963365..5ef32ac 100644 --- a/src/xatu/testdata/clean.nix +++ b/src/xatu/testdata/clean.nix @@ -1,6 +1,11 @@ { pkgs }: { + nonEmpty = + let + value = pkgs; + in + value; path = ./. + "/foo"; url = "https://example.com"; inherit pkgs; diff --git a/src/xatu/testdata/lint_findings.nix b/src/xatu/testdata/lint_findings.nix index bd543bb..3eb8cbb 100644 --- a/src/xatu/testdata/lint_findings.nix +++ b/src/xatu/testdata/lint_findings.nix @@ -2,6 +2,7 @@ { inherit; + empty = let in src; path = builtins.toPath src; url = https://example.com; } diff --git a/src/xatu/testdata/lint_interactions.nix b/src/xatu/testdata/lint_interactions.nix new file mode 100644 index 0000000..89a5948 --- /dev/null +++ b/src/xatu/testdata/lint_interactions.nix @@ -0,0 +1,8 @@ +{ src }: + +let + inherit; +in +{ + value = src; +} diff --git a/src/xatu/testdata/lint_variants.nix b/src/xatu/testdata/lint_variants.nix index 4ff8b0d..34ac99c 100644 --- a/src/xatu/testdata/lint_variants.nix +++ b/src/xatu/testdata/lint_variants.nix @@ -1,5 +1,10 @@ { src }: { + commentedEmpty = + let + # This comment must not suppress the finding. + in + src; path = toPath src; } diff --git a/src/xatu/xatu_test.rs b/src/xatu/xatu_test.rs index 09b4d27..b5b559f 100644 --- a/src/xatu/xatu_test.rs +++ b/src/xatu/xatu_test.rs @@ -113,6 +113,51 @@ fn xatu_reports_empty_inherit() -> googletest::Result<()> { Ok(()) } +#[gtest] +fn xatu_reports_empty_let_in() -> googletest::Result<()> { + let source = fixture_source("testdata/lint_findings.nix")?; + let range_start = source.source.find("let in src").or_fail()?; + let range_end = range_start + "let in src".len(); + let report = xatu::future_sight(vec![source]).or_fail()?; + + let finding = report + .diagnostics + .iter() + .map(|diagnostic| &diagnostic.finding) + .find(|finding| finding.rule_id == "empty_let_in") + .or_fail()?; + let expected_range = format!("{range_start}..{range_end}"); + expect_that!(finding.range.to_string(), eq(&expected_range)); + Ok(()) +} + +#[gtest] +fn xatu_reports_commented_empty_let_in() -> googletest::Result<()> { + let report = xatu::future_sight(vec![fixture_source("testdata/lint_variants.nix")?]).or_fail()?; + + let findings = report + .diagnostics + .iter() + .map(|diagnostic| diagnostic.finding.rule_id) + .collect::>(); + expect_that!(findings, contains(eq(&"empty_let_in"))); + Ok(()) +} + +#[gtest] +fn empty_inherit_prevents_empty_let_in_finding() -> googletest::Result<()> { + let report = + xatu::future_sight(vec![fixture_source("testdata/lint_interactions.nix")?]).or_fail()?; + + let findings = report + .diagnostics + .iter() + .map(|diagnostic| diagnostic.finding.rule_id) + .collect::>(); + expect_that!(findings, eq(&vec!["empty_inherit"])); + Ok(()) +} + #[gtest] fn xatu_reports_unquoted_uri() -> googletest::Result<()> { let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?;