diff --git a/src/xatu/BUILD b/src/xatu/BUILD index 4a72c08..ea83932 100644 --- a/src/xatu/BUILD +++ b/src/xatu/BUILD @@ -8,6 +8,7 @@ rust_library( "file_store.rs", "parser.rs", "rules.rs", + "rules/manual_inherit.rs", "rules/unquoted_uri.rs", "xatu.rs", ], diff --git a/src/xatu/ast.rs b/src/xatu/ast.rs index 1addbf4..73f6ce4 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), + AttrpathValue(AttrpathValue), LetIn(LetIn), Inherit(Inherit), } @@ -38,6 +39,12 @@ pub struct Apply { pub function: Expr, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AttrpathValue { + pub key: Ident, + pub value: Ident, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum Expr { Ident(Ident), diff --git a/src/xatu/parser.rs b/src/xatu/parser.rs index e92f605..dbc5e25 100644 --- a/src/xatu/parser.rs +++ b/src/xatu/parser.rs @@ -1,4 +1,4 @@ -use rnix::ast::HasEntry; +use rnix::ast::{Attr, HasEntry}; use rowan::ast::AstNode; use crate::{FileId, ast}; @@ -38,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_ATTRPATH_VALUE => convert_attrpath_value(node), rnix::SyntaxKind::NODE_LET_IN => convert_let_in(node), rnix::SyntaxKind::NODE_INHERIT => convert_inherit(node), _ => None, @@ -65,6 +66,40 @@ impl TryFrom for ast::Apply { } } +fn convert_attrpath_value(node: rnix::SyntaxNode) -> Option { + let attrpath_value = rnix::ast::AttrpathValue::cast(node)?; + let range = attrpath_value.syntax().text_range().into(); + let data = attrpath_value.try_into().ok()?; + + Some(ast::Node { + range, + data: ast::NodeData::AttrpathValue(data), + }) +} + +impl TryFrom for ast::AttrpathValue { + type Error = (); + + fn try_from(attrpath_value: rnix::ast::AttrpathValue) -> Result { + let mut attrs = attrpath_value.attrpath().ok_or(())?.attrs(); + let key = match attrs.next().ok_or(())? { + Attr::Ident(ident) => ident.into(), + _ => return Err(()), + }; + + if attrs.next().is_some() { + return Err(()); + } + + let value = match attrpath_value.value().ok_or(())? { + rnix::ast::Expr::Ident(ident) => ident.into(), + _ => return Err(()), + }; + + Ok(Self { key, value }) + } +} + impl TryFrom for ast::Expr { type Error = (); diff --git a/src/xatu/rules.rs b/src/xatu/rules.rs index 45af16e..598e8d7 100644 --- a/src/xatu/rules.rs +++ b/src/xatu/rules.rs @@ -1,7 +1,9 @@ use crate::FileId; use crate::ast; +use crate::rules::manual_inherit::ManualInherit; use crate::rules::unquoted_uri::UnquotedUri; +pub mod manual_inherit; pub mod unquoted_uri; #[derive(Debug)] @@ -27,7 +29,6 @@ impl std::fmt::Display for Diagnostic { } } -#[expect(dead_code, reason = "retained for planned node rules")] pub trait NodeRule { type Node; @@ -47,7 +48,11 @@ pub trait TokenRule { } pub fn run_rules(file: &ast::ParsedFile) -> Vec { - let node_diagnostics = file.nodes.iter().flat_map(|_| std::iter::empty()); + let node_diagnostics = file + .nodes + .iter() + .flat_map(|node| [run_node_rule::(node)]) + .flatten(); let token_diagnostics = file .tokens .iter() @@ -63,7 +68,6 @@ pub fn run_rules(file: &ast::ParsedFile) -> Vec { .collect() } -#[expect(dead_code, reason = "retained for planned node rules")] fn run_node_rule(node: &ast::Node) -> Option where R: NodeRule, diff --git a/src/xatu/rules/manual_inherit.rs b/src/xatu/rules/manual_inherit.rs new file mode 100644 index 0000000..af4d9a6 --- /dev/null +++ b/src/xatu/rules/manual_inherit.rs @@ -0,0 +1,24 @@ +use crate::{ast, rules::NodeRule}; + +pub struct ManualInherit; + +impl NodeRule for ManualInherit { + type Node = ast::AttrpathValue; + + const ID: &'static str = "manual_inherit"; + + fn select(data: &ast::NodeData) -> Option<&Self::Node> { + match data { + ast::NodeData::AttrpathValue(attrpath_value) => Some(attrpath_value), + _ => None, + } + } + + fn check(node: &Self::Node) -> Option<&'static str> { + if node.key == node.value { + Some("This assignment is better written with `inherit`") + } else { + None + } + } +} diff --git a/src/xatu/testdata/clean.nix b/src/xatu/testdata/clean.nix index 0963365..01c2d4b 100644 --- a/src/xatu/testdata/clean.nix +++ b/src/xatu/testdata/clean.nix @@ -4,4 +4,9 @@ path = ./. + "/foo"; url = "https://example.com"; inherit pkgs; + different = other; + nested.package = nested.package; + "quoted" = quoted; + ${dynamic} = dynamic; + from = pkgs.from; } diff --git a/src/xatu/testdata/lint_findings.nix b/src/xatu/testdata/lint_findings.nix index 1b922e8..e31135d 100644 --- a/src/xatu/testdata/lint_findings.nix +++ b/src/xatu/testdata/lint_findings.nix @@ -1,3 +1,4 @@ { + package = package; url = https://example.com; } diff --git a/src/xatu/xatu_test.rs b/src/xatu/xatu_test.rs index 57ce7f4..3a4e8b9 100644 --- a/src/xatu/xatu_test.rs +++ b/src/xatu/xatu_test.rs @@ -64,3 +64,16 @@ fn xatu_reports_unquoted_uri() -> googletest::Result<()> { ); Ok(()) } + +#[gtest] +fn xatu_reports_manual_inherit() -> googletest::Result<()> { + let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?; + + let manual_inherit_count = report + .diagnostics + .iter() + .filter(|diagnostic| diagnostic.finding.rule_id == "manual_inherit") + .count(); + expect_that!(manual_inherit_count, eq(1)); + Ok(()) +}