From 6e19169e111beca73be8f4670f989900a8e07887 Mon Sep 17 00:00:00 2001 From: alexion Date: Sat, 19 Sep 2026 18:25:25 -0400 Subject: [PATCH] feat(xatu): report empty let expressions --- AGENTS.md | 4 +++ src/xatu/BUILD | 2 ++ src/xatu/ast.rs | 6 ++++ src/xatu/parser.rs | 23 +++++++++++++ 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 +++++++++++++++++++++++++ 11 files changed, 126 insertions(+) create mode 100644 src/xatu/rules/empty_let_in.rs create mode 100644 src/xatu/testdata/lint_interactions.nix diff --git a/AGENTS.md b/AGENTS.md index 993638c..5661726 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,11 +96,15 @@ - `rules_rust` `rust_test` targets cannot set both `crate` and `srcs`. Use one input style per test target. - Rust tests in this monorepo live in separate `*_test.rs` files, not inline `#[cfg(test)]` modules. +- `rnix::ast::HasEntry::entries()` includes assignments and `inherit` statements. + Use `attrpath_values()` when code needs only assignment entries. - Ariadne Rust tests are declared in Bazel `BUILD` files; do not add `[[test]]` entries to `src/ariadne/Cargo.toml` unless explicitly requested. - In this monorepo, "dotfiles" refers to the dotfiles/NixOS configuration under `src/dotfiles/` and `configs/nixos/`. - The flake does not currently define `formatter.x86_64-linux`. Do not use `nix fmt` as a validation command until a formatter is added. +- The `fix` app passes its current directory to Statix and Deadnix. + Run `nix run .#fix` from the repository root so path exclusions match. - Ariadne extraction must preserve the game-root season prefix for archived source. Different game installs can contain compiled scripts with the same internal path. - Clew owns Ren'Py extraction, preparation, and compiled-script AST export. Ariadne owns analysis over Clew-produced data. 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..1addbf4 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,11 @@ pub struct Select { pub attrs: Vec, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LetIn { + pub binding_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..e92f605 100644 --- a/src/xatu/parser.rs +++ b/src/xatu/parser.rs @@ -1,3 +1,4 @@ +use rnix::ast::HasEntry; use rowan::ast::AstNode; use crate::{FileId, ast}; @@ -37,6 +38,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 +104,27 @@ 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 { + Self { + binding_count: let_in.attrpath_values().count() + + let_in + .inherits() + .map(|inherit| inherit.attrs().count()) + .sum::(), + } + } +} + 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..fb0c1e9 --- /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.binding_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..bc3fe3c 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 xatu_reports_empty_let_with_empty_inherit() -> 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_let_in", "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()?; -- 2.47.3