refactor(xatu): narrow lint rule set

This commit is contained in:
2026-09-19 19:26:59 -04:00
parent 6e19169e11
commit 873ed546ff
18 changed files with 38 additions and 495 deletions

View File

@@ -103,8 +103,10 @@
- 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.
- The `fix` app passes its current directory to Deadnix.
Run `nix run .#fix` from the repository root so path exclusions match.
- Git pathspec searches started in a subdirectory default to that subtree.
Use `git -C <repository-root>` or top-anchored pathspecs for repository-wide history searches.
- 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.

1
Cargo.lock generated
View File

@@ -401,7 +401,6 @@ dependencies = [
"googletest",
"monorepo",
"rnix",
"rowan",
"status",
]

View File

@@ -17,7 +17,7 @@ Use the `Decision` line on each rule for `keep`, `discard`, `update`, or `need-m
### `bool_comparison`
Decision: keep
Decision: discard
Source: `statix` `W01`.
Status at collection time: enabled.
@@ -50,7 +50,7 @@ Direct predicates make conditions easier to scan.
### `empty_let_in`
Decision: keep
Decision: discard
Source: `statix` `W02`.
Status at collection time: enabled.
@@ -80,7 +80,7 @@ It usually remains after editing or debugging.
### `manual_inherit`
Decision: Keep
Decision: keep
Source: `statix` `W03`.
Status at collection time: enabled.
@@ -113,7 +113,7 @@ It removes duplicated names.
### `manual_inherit_from`
Decision:
Decision: keep
Source: `statix` `W04`.
Status at collection time: enabled.
@@ -142,7 +142,7 @@ It avoids repeated selector names.
### `legacy_let_syntax`
Decision: keep
Decision: discard
Source: `statix` `W05`.
Status at collection time: enabled.
@@ -180,7 +180,7 @@ Documented forms are clearer to tools and readers.
### `collapsible_let_in`
Decision: keep
Decision: discard
Source: `statix` `W06`.
Status at collection time: enabled.
@@ -220,7 +220,7 @@ A single binding group shows the local dependencies together.
### `eta_reduction`
Decision: keep
Decision: discard
Source: `statix` `W07`.
Status at collection time: enabled.
@@ -250,7 +250,7 @@ The direct function reference exposes the operation.
### `useless_parens`
Decision: keep
Decision: discard
Source: `statix` `W08`.
Status at collection time: enabled.
@@ -285,7 +285,7 @@ Required precedence parentheses remain valid.
### `empty_pattern`
Decision: keep
Decision: discard
Source: `statix` `W10`.
Status at collection time: enabled.
@@ -315,7 +315,7 @@ An empty variadic pattern looks like partial destructuring.
### `redundant_pattern_bind`
Decision: keep
Decision: discard
Source: `statix` `W11`.
Status at collection time: enabled.
@@ -375,7 +375,7 @@ They avoid ambiguity with punctuation after the URI.
### `empty_inherit`
Decision: keep
Decision: discard
Source: `statix` `W14`.
Status at collection time: enabled.
@@ -400,7 +400,7 @@ It usually remains after a refactor.
### `deprecated_to_path`
Decision: keep
Decision: discard
Source: `statix` `W17`.
Status at collection time: enabled.
@@ -431,7 +431,7 @@ Explicit path construction states whether the result is absolute or relative.
### `bool_simplification`
Decision: keep
Decision: discard
Source: `statix` `W18`.
Status at collection time: enabled.
@@ -459,7 +459,7 @@ The direct operator expresses inequality without nested boolean syntax.
### `useless_has_attr`
Decision: keep
Decision: discard
Source: `statix` `W19`.
Status at collection time: enabled.
@@ -488,7 +488,7 @@ It avoids repeating the attribute path.
### `repeated_keys`
Decision: update so that it doesn't conflict with the custom module convention.
Decision: discard
Source: `statix` `W20`.
Status at collection time: disabled in `statix.toml`.
@@ -529,7 +529,7 @@ The local config disables this rule, so it needs an explicit decision before pro
### `empty_list_concat`
Decision: keep
Decision: discard
Source: `statix` `W23`.
Status at collection time: enabled.
@@ -561,7 +561,7 @@ It usually remains after conditional list construction changed.
### Unused `let` bindings
Decision: keep
Decision: discard
Source: `deadnix` default check.
Status at collection time: active in `nix run .#fix`.
@@ -595,7 +595,7 @@ They also make refactors look unfinished.
### Unused `inherit` names
Decision: keep
Decision: discard
Source: `deadnix` default check.
Status at collection time: active in `nix run .#fix`.
@@ -628,7 +628,7 @@ An empty result also overlaps with `statix` `empty_inherit`.
### Unused lambda arguments
Decision: keep
Decision: discard
Source: `deadnix` default check.
Status at collection time: active in `nix run .#fix`.
@@ -658,7 +658,7 @@ A named unused argument implies missing logic.
### Unused lambda pattern names
Decision: keep
Decision: discard
Source: `deadnix` default check.
Status at collection time: active in `nix run .#fix`.
@@ -689,7 +689,7 @@ Unused names overstate the function's inputs.
### Unused lambda pattern aliases
Decision: keep
Decision: discard
Source: `deadnix` default check.
Status at collection time: active in `nix run .#fix`.
@@ -719,7 +719,7 @@ Keeping it unused makes the function contract look wider than it is.
### Ignored underscore bindings
Decision: keep
Decision: discard
Source: `deadnix` `--no-underscore` option.
Status at collection time: inactive; current fixer does not pass `--no-underscore`.
@@ -748,7 +748,7 @@ Skipping all underscore bindings can hide real dead code.
### Used underscore bindings
Decision: keep
Decision: discard
Source: `deadnix` `--warn-used-underscore` option.
Status at collection time: inactive; current fixer does not pass `--warn-used-underscore`.

View File

@@ -15,16 +15,10 @@ in
name = "fix";
runtimeInputs = [
formatter
pkgs.statix
pkgs.deadnix
];
text = ''
treefmt
statix fix \
--ignore 'configs/nixos/hosts/*/hardware-configuration.nix' \
--ignore 'src/xatu/testdata/**' \
--ignore third_party \
.
deadnix --edit \
--exclude \
configs/nixos/hosts/neogaia/hardware-configuration.nix \

View File

@@ -8,16 +8,12 @@ rust_library(
"file_store.rs",
"parser.rs",
"rules.rs",
"rules/deprecated_to_path.rs",
"rules/empty_inherit.rs",
"rules/empty_let_in.rs",
"rules/unquoted_uri.rs",
"xatu.rs",
],
deps = [
"//src/status:status",
"//third_party/crates:rnix",
"//third_party/crates/rowan-0.16.1:rowan",
],
)
@@ -34,8 +30,6 @@ rust_test(
"testdata/clean.nix",
"testdata/invalid.nix",
"testdata/lint_findings.nix",
"testdata/lint_interactions.nix",
"testdata/lint_variants.nix",
],
deps = [
":xatu_lib",

View File

@@ -14,7 +14,6 @@ path = "xatu_main.rs"
clap = { workspace = true, features = ["derive"] }
monorepo = { workspace = true }
rnix = { workspace = true }
rowan = { workspace = true }
status = { workspace = true }
[dev-dependencies]

View File

@@ -3,17 +3,10 @@ use crate::FileId;
#[derive(Debug)]
pub struct ParsedFile {
pub file_id: FileId,
pub nodes: Vec<Node>,
pub tokens: Vec<Token>,
pub errors: Vec<ParseError>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Node {
pub range: SourceRange,
pub data: NodeData,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct SourceRange {
pub start: usize,
@@ -26,45 +19,6 @@ impl std::fmt::Display for SourceRange {
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum NodeData {
Apply(Apply),
LetIn(LetIn),
Inherit(Inherit),
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Apply {
pub function: Expr,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Expr {
Ident(Ident),
Select(Box<Select>),
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Ident {
pub name: String,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Select {
pub receiver: Expr,
pub attrs: Vec<Ident>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct LetIn {
pub binding_count: usize,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Inherit {
pub attr_count: usize,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Token {
pub range: SourceRange,

View File

@@ -1,18 +1,12 @@
use rnix::ast::HasEntry;
use rowan::ast::AstNode;
use crate::{FileId, ast};
pub fn parse_file(file_id: FileId, source: &str) -> ast::ParsedFile {
let parse = rnix::Root::parse(source);
let source_len = source.len();
let (nodes, tokens) = convert_syntax(parse.syntax());
ast::ParsedFile {
file_id,
nodes,
tokens,
tokens: convert_tokens(parse.syntax()),
errors: parse
.errors()
.iter()
@@ -21,125 +15,14 @@ pub fn parse_file(file_id: FileId, source: &str) -> ast::ParsedFile {
}
}
fn convert_syntax(syntax: rnix::SyntaxNode) -> (Vec<ast::Node>, Vec<ast::Token>) {
let mut nodes = Vec::new();
let mut tokens = Vec::new();
for element in syntax.descendants_with_tokens() {
match element {
rnix::NodeOrToken::Node(node) => nodes.extend(convert_node(node)),
rnix::NodeOrToken::Token(token) => tokens.extend(convert_token(token)),
}
}
(nodes, tokens)
}
fn convert_node(node: rnix::SyntaxNode) -> Option<ast::Node> {
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,
}
}
fn convert_apply(node: rnix::SyntaxNode) -> Option<ast::Node> {
let apply = rnix::ast::Apply::cast(node)?;
let range = apply.syntax().text_range().into();
let data = apply.try_into().ok()?;
Some(ast::Node {
range,
data: ast::NodeData::Apply(data),
})
}
impl TryFrom<rnix::ast::Apply> for ast::Apply {
type Error = ();
fn try_from(apply: rnix::ast::Apply) -> Result<Self, Self::Error> {
Ok(Self {
function: apply.lambda().ok_or(())?.try_into()?,
fn convert_tokens(syntax: rnix::SyntaxNode) -> Vec<ast::Token> {
syntax
.descendants_with_tokens()
.filter_map(|element| match element {
rnix::NodeOrToken::Token(token) => convert_token(token),
_ => None,
})
}
}
impl TryFrom<rnix::ast::Expr> for ast::Expr {
type Error = ();
fn try_from(expr: rnix::ast::Expr) -> Result<Self, Self::Error> {
match expr {
rnix::ast::Expr::Ident(ident) => Ok(Self::Ident(ident.into())),
rnix::ast::Expr::Select(select) => Ok(Self::Select(Box::new(select.try_into()?))),
_ => Err(()),
}
}
}
impl From<rnix::ast::Ident> for ast::Ident {
fn from(ident: rnix::ast::Ident) -> Self {
Self {
name: ident.to_string(),
}
}
}
impl TryFrom<rnix::ast::Select> for ast::Select {
type Error = ();
fn try_from(select: rnix::ast::Select) -> Result<Self, Self::Error> {
Ok(Self {
receiver: select.expr().ok_or(())?.try_into()?,
attrs: select
.attrpath()
.ok_or(())?
.attrs()
.map(|attr| match attr {
rnix::ast::Attr::Ident(ident) => Ok(ident.into()),
_ => Err(()),
})
.collect::<Result<Vec<_>, _>>()?,
})
}
}
fn convert_let_in(node: rnix::SyntaxNode) -> Option<ast::Node> {
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<rnix::ast::LetIn> 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::<usize>(),
}
}
}
fn convert_inherit(node: rnix::SyntaxNode) -> Option<ast::Node> {
let inherit = rnix::ast::Inherit::cast(node)?;
Some(ast::Node {
range: inherit.syntax().text_range().into(),
data: ast::NodeData::Inherit(inherit.into()),
})
}
impl From<rnix::ast::Inherit> for ast::Inherit {
fn from(inherit: rnix::ast::Inherit) -> Self {
Self {
attr_count: inherit.attrs().count(),
}
}
.collect()
}
fn convert_token(token: rnix::SyntaxToken) -> Option<ast::Token> {

View File

@@ -1,13 +1,7 @@
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)]
@@ -33,15 +27,6 @@ impl std::fmt::Display for Diagnostic {
}
}
pub trait NodeRule {
type Node;
const ID: &'static str;
fn select(data: &ast::NodeData) -> Option<&Self::Node>;
fn check(node: &Self::Node) -> Option<&'static str>;
}
pub trait TokenRule {
type Token;
@@ -52,45 +37,15 @@ pub trait TokenRule {
}
pub fn run_rules(file: &ast::ParsedFile) -> Vec<Diagnostic> {
let node_diagnostics = file.nodes.iter().flat_map(|node| {
[
run_node_rule::<DeprecatedToPath>(node),
run_node_rule::<EmptyInherit>(node),
run_node_rule::<EmptyLetIn>(node),
]
.into_iter()
file.tokens
.iter()
.flat_map(|token| [run_token_rule::<UnquotedUri>(token)])
.flatten()
.map(|finding| Diagnostic {
file_id: file.file_id,
finding,
})
});
let token_diagnostics = file.tokens.iter().flat_map(|token| {
[run_token_rule::<UnquotedUri>(token)]
.into_iter()
.flatten()
.map(|finding| Diagnostic {
file_id: file.file_id,
finding,
})
});
node_diagnostics.chain(token_diagnostics).collect()
}
fn run_node_rule<R>(node: &ast::Node) -> Option<Finding>
where
R: NodeRule,
{
let typed_node = R::select(&node.data)?;
let message = R::check(typed_node)?;
Some(Finding {
rule_id: R::ID,
range: node.range,
message,
})
.collect()
}
fn run_token_rule<R>(token: &ast::Token) -> Option<Finding>

View File

@@ -1,41 +0,0 @@
use crate::{ast, rules::NodeRule};
pub struct DeprecatedToPath;
impl NodeRule for DeprecatedToPath {
type Node = ast::Apply;
const ID: &'static str = "deprecated_to_path";
fn select(data: &ast::NodeData) -> Option<&Self::Node> {
match data {
ast::NodeData::Apply(apply) => Some(apply),
_ => None,
}
}
fn check(node: &Self::Node) -> Option<&'static str> {
if calls_to_path(&node.function) {
Some("builtins.toPath is deprecated")
} else {
None
}
}
}
fn calls_to_path(function: &ast::Expr) -> bool {
// TODO: Once Xatu supports name resolution, report unqualified `toPath` only
// when it resolves to `builtins.toPath`.
match function {
ast::Expr::Ident(ident) => ident.name == "toPath",
ast::Expr::Select(select) => is_builtins_to_path(select),
}
}
fn is_builtins_to_path(select: &ast::Select) -> bool {
matches!(
&select.receiver,
ast::Expr::Ident(ident) if ident.name == "builtins"
) && select.attrs.len() == 1
&& select.attrs[0].name == "toPath"
}

View File

@@ -1,24 +0,0 @@
use crate::{ast, rules::NodeRule};
pub struct EmptyInherit;
impl NodeRule for EmptyInherit {
type Node = ast::Inherit;
const ID: &'static str = "empty_inherit";
fn select(data: &ast::NodeData) -> Option<&Self::Node> {
match data {
ast::NodeData::Inherit(inherit) => Some(inherit),
_ => None,
}
}
fn check(node: &Self::Node) -> Option<&'static str> {
if node.attr_count == 0 {
Some("inherit statement has no attributes")
} else {
None
}
}
}

View File

@@ -1,24 +0,0 @@
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
}
}
}

View File

@@ -1,11 +1,6 @@
{ pkgs }:
{
nonEmpty =
let
value = pkgs;
in
value;
path = ./. + "/foo";
url = "https://example.com";
inherit pkgs;

View File

@@ -1,8 +1,3 @@
{ src }:
{
inherit;
empty = let in src;
path = builtins.toPath src;
url = https://example.com;
}

View File

@@ -1,8 +0,0 @@
{ src }:
let
inherit;
in
{
value = src;
}

View File

@@ -1,10 +0,0 @@
{ src }:
{
commentedEmpty =
let
# This comment must not suppress the finding.
in
src;
path = toPath src;
}

View File

@@ -41,123 +41,6 @@ fn xatu_rejects_invalid_source() -> googletest::Result<()> {
Ok(())
}
#[gtest]
fn xatu_reports_deprecated_builtin_to_path() -> googletest::Result<()> {
let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?;
let findings = report
.diagnostics
.iter()
.map(|diagnostic| {
(
report.files.get(diagnostic.file_id).path.clone(),
diagnostic.finding.rule_id,
)
})
.collect::<Vec<_>>();
expect_that!(
findings,
contains(eq(&(
PathBuf::from("testdata/lint_findings.nix"),
"deprecated_to_path"
)))
);
Ok(())
}
#[gtest]
fn xatu_reports_deprecated_unqualified_to_path() -> googletest::Result<()> {
let report = xatu::future_sight(vec![fixture_source("testdata/lint_variants.nix")?]).or_fail()?;
let findings = report
.diagnostics
.iter()
.map(|diagnostic| {
(
report.files.get(diagnostic.file_id).path.clone(),
diagnostic.finding.rule_id,
)
})
.collect::<Vec<_>>();
expect_that!(
findings,
contains(eq(&(
PathBuf::from("testdata/lint_variants.nix"),
"deprecated_to_path"
)))
);
Ok(())
}
#[gtest]
fn xatu_reports_empty_inherit() -> googletest::Result<()> {
let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?;
let findings = report
.diagnostics
.iter()
.map(|diagnostic| {
(
report.files.get(diagnostic.file_id).path.clone(),
diagnostic.finding.rule_id,
)
})
.collect::<Vec<_>>();
expect_that!(
findings,
contains(eq(&(
PathBuf::from("testdata/lint_findings.nix"),
"empty_inherit"
)))
);
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::<Vec<_>>();
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::<Vec<_>>();
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()?;

View File

@@ -1,3 +0,0 @@
disabled = [
"repeated_keys",
]