feat(xatu): add manual inherit rules #85

Closed
alexion wants to merge 4 commits from feat/xatu-manual-inherit-rules into main
8 changed files with 168 additions and 93 deletions

View File

@@ -8,6 +8,8 @@ rust_library(
"file_store.rs", "file_store.rs",
"parser.rs", "parser.rs",
"rules.rs", "rules.rs",
"rules/manual_inherit.rs",
"rules/manual_inherit_from.rs",
"rules/unquoted_uri.rs", "rules/unquoted_uri.rs",
"xatu.rs", "xatu.rs",
], ],

View File

@@ -28,20 +28,26 @@ impl std::fmt::Display for SourceRange {
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub enum NodeData { pub enum NodeData {
Apply(Apply), Binding(Binding),
LetIn(LetIn),
Inherit(Inherit),
} }
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub struct Apply { pub struct Binding {
pub function: Expr, pub path: Vec<Attr>,
pub value: Expr,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Attr {
Ident(Ident),
Other,
} }
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub enum Expr { pub enum Expr {
Ident(Ident), Ident(Ident),
Select(Box<Select>), Select(Box<Select>),
Other,
} }
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
@@ -52,17 +58,8 @@ pub struct Ident {
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub struct Select { pub struct Select {
pub receiver: Expr, pub receiver: Expr,
pub attrs: Vec<Ident>, pub path: Vec<Attr>,
} pub has_default: bool,
#[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)] #[derive(Debug, Clone, PartialEq, Eq)]

View File

@@ -1,4 +1,3 @@
use rnix::ast::HasEntry;
use rowan::ast::AstNode; use rowan::ast::AstNode;
use crate::{FileId, ast}; use crate::{FileId, ast};
@@ -37,42 +36,51 @@ fn convert_syntax(syntax: rnix::SyntaxNode) -> (Vec<ast::Node>, Vec<ast::Token>)
fn convert_node(node: rnix::SyntaxNode) -> Option<ast::Node> { fn convert_node(node: rnix::SyntaxNode) -> Option<ast::Node> {
match node.kind() { match node.kind() {
rnix::SyntaxKind::NODE_APPLY => convert_apply(node), rnix::SyntaxKind::NODE_ATTRPATH_VALUE => convert_binding(node),
rnix::SyntaxKind::NODE_LET_IN => convert_let_in(node),
rnix::SyntaxKind::NODE_INHERIT => convert_inherit(node),
_ => None, _ => None,
} }
} }
fn convert_apply(node: rnix::SyntaxNode) -> Option<ast::Node> { fn convert_binding(node: rnix::SyntaxNode) -> Option<ast::Node> {
let apply = rnix::ast::Apply::cast(node)?; let binding = rnix::ast::AttrpathValue::cast(node)?;
let range = apply.syntax().text_range().into(); let range = binding.syntax().text_range().into();
let data = apply.try_into().ok()?; let data = binding.into();
Some(ast::Node { Some(ast::Node {
range, range,
data: ast::NodeData::Apply(data), data: ast::NodeData::Binding(data),
}) })
} }
impl TryFrom<rnix::ast::Apply> for ast::Apply { impl From<rnix::ast::AttrpathValue> for ast::Binding {
type Error = (); fn from(binding: rnix::ast::AttrpathValue) -> Self {
Self {
fn try_from(apply: rnix::ast::Apply) -> Result<Self, Self::Error> { path: binding
Ok(Self { .attrpath()
function: apply.lambda().ok_or(())?.try_into()?, .into_iter()
}) .flat_map(|path| path.attrs())
.map(Into::into)
.collect(),
value: binding.value().map(Into::into).unwrap_or(ast::Expr::Other),
}
} }
} }
impl TryFrom<rnix::ast::Expr> for ast::Expr { impl From<rnix::ast::Attr> for ast::Attr {
type Error = (); fn from(attr: rnix::ast::Attr) -> Self {
match attr {
rnix::ast::Attr::Ident(ident) => Self::Ident(ident.into()),
_ => Self::Other,
}
}
}
fn try_from(expr: rnix::ast::Expr) -> Result<Self, Self::Error> { impl From<rnix::ast::Expr> for ast::Expr {
fn from(expr: rnix::ast::Expr) -> Self {
match expr { match expr {
rnix::ast::Expr::Ident(ident) => Ok(Self::Ident(ident.into())), rnix::ast::Expr::Ident(ident) => Self::Ident(ident.into()),
rnix::ast::Expr::Select(select) => Ok(Self::Select(Box::new(select.try_into()?))), rnix::ast::Expr::Select(select) => Self::Select(Box::new(select.into())),
_ => Err(()), _ => Self::Other,
} }
} }
} }
@@ -85,59 +93,17 @@ impl From<rnix::ast::Ident> for ast::Ident {
} }
} }
impl TryFrom<rnix::ast::Select> for ast::Select { impl From<rnix::ast::Select> for ast::Select {
type Error = (); fn from(select: rnix::ast::Select) -> Self {
Self {
fn try_from(select: rnix::ast::Select) -> Result<Self, Self::Error> { receiver: select.expr().map(Into::into).unwrap_or(ast::Expr::Other),
Ok(Self { path: select
receiver: select.expr().ok_or(())?.try_into()?,
attrs: select
.attrpath() .attrpath()
.ok_or(())? .into_iter()
.attrs() .flat_map(|path| path.attrs())
.map(|attr| match attr { .map(Into::into)
rnix::ast::Attr::Ident(ident) => Ok(ident.into()), .collect(),
_ => Err(()), has_default: select.or_token().is_some(),
})
.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(),
} }
} }
} }

View File

@@ -1,7 +1,11 @@
use crate::FileId; use crate::FileId;
use crate::ast; use crate::ast;
use crate::rules::manual_inherit::ManualInherit;
use crate::rules::manual_inherit_from::ManualInheritFrom;
use crate::rules::unquoted_uri::UnquotedUri; use crate::rules::unquoted_uri::UnquotedUri;
pub mod manual_inherit;
pub mod manual_inherit_from;
pub mod unquoted_uri; pub mod unquoted_uri;
#[derive(Debug)] #[derive(Debug)]
@@ -27,7 +31,6 @@ impl std::fmt::Display for Diagnostic {
} }
} }
#[expect(dead_code, reason = "retained for planned node rules")]
pub trait NodeRule { pub trait NodeRule {
type Node; type Node;
@@ -47,7 +50,16 @@ pub trait TokenRule {
} }
pub fn run_rules(file: &ast::ParsedFile) -> Vec<Diagnostic> { pub fn run_rules(file: &ast::ParsedFile) -> Vec<Diagnostic> {
let node_diagnostics = file.nodes.iter().flat_map(|_| std::iter::empty()); let node_diagnostics = file
.nodes
.iter()
.flat_map(|node| {
[
run_node_rule::<ManualInherit>(node),
run_node_rule::<ManualInheritFrom>(node),
]
})
.flatten();
let token_diagnostics = file let token_diagnostics = file
.tokens .tokens
.iter() .iter()
@@ -63,7 +75,6 @@ pub fn run_rules(file: &ast::ParsedFile) -> Vec<Diagnostic> {
.collect() .collect()
} }
#[expect(dead_code, reason = "retained for planned node rules")]
fn run_node_rule<R>(node: &ast::Node) -> Option<Finding> fn run_node_rule<R>(node: &ast::Node) -> Option<Finding>
where where
R: NodeRule, R: NodeRule,

View File

@@ -0,0 +1,25 @@
use crate::{ast, rules::NodeRule};
pub struct ManualInherit;
impl NodeRule for ManualInherit {
type Node = ast::Binding;
const ID: &'static str = "manual_inherit";
fn select(data: &ast::NodeData) -> Option<&Self::Node> {
let ast::NodeData::Binding(binding) = data;
Some(binding)
}
fn check(binding: &Self::Node) -> Option<&'static str> {
let [ast::Attr::Ident(name)] = binding.path.as_slice() else {
return None;
};
let ast::Expr::Ident(value) = &binding.value else {
return None;
};
(name == value).then_some("Use `inherit` for this binding")
}
}

View File

@@ -0,0 +1,34 @@
use crate::{ast, rules::NodeRule};
pub struct ManualInheritFrom;
impl NodeRule for ManualInheritFrom {
type Node = ast::Binding;
const ID: &'static str = "manual_inherit_from";
fn select(data: &ast::NodeData) -> Option<&Self::Node> {
let ast::NodeData::Binding(binding) = data;
Some(binding)
}
fn check(binding: &Self::Node) -> Option<&'static str> {
let [ast::Attr::Ident(name)] = binding.path.as_slice() else {
return None;
};
let ast::Expr::Select(select) = &binding.value else {
return None;
};
let ast::Expr::Ident(_) = &select.receiver else {
return None;
};
let [ast::Attr::Ident(selected)] = select.path.as_slice() else {
return None;
};
if select.has_default {
return None;
}
(name == selected).then_some("Use `inherit (source)` for this binding")
}
}

View File

@@ -1,3 +1,5 @@
{ {
package = package;
sourcePackage = source.sourcePackage;
url = https://example.com; url = https://example.com;
} }

View File

@@ -41,6 +41,44 @@ fn xatu_rejects_invalid_source() -> googletest::Result<()> {
Ok(()) Ok(())
} }
#[gtest]
fn xatu_reports_manual_inherit() -> googletest::Result<()> {
let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?;
let findings = report
.diagnostics
.iter()
.map(|diagnostic| diagnostic.finding.rule_id)
.collect::<Vec<_>>();
expect_that!(findings, contains(eq(&"manual_inherit")));
Ok(())
}
#[gtest]
fn xatu_reports_manual_inherit_from() -> googletest::Result<()> {
let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?;
let findings = report
.diagnostics
.iter()
.map(|diagnostic| diagnostic.finding.rule_id)
.collect::<Vec<_>>();
expect_that!(findings, contains(eq(&"manual_inherit_from")));
Ok(())
}
#[gtest]
fn xatu_ignores_other_binding_forms() -> googletest::Result<()> {
let report = xatu::future_sight(vec![SourceFile {
path: PathBuf::from("other_binding_forms.nix"),
source: "{ nested = source.group.nested; target.path = path; defaulted = source.defaulted or null; renamed = source.original; }".into(),
}])
.or_fail()?;
expect_that!(report.is_clean(), is_true());
Ok(())
}
#[gtest] #[gtest]
fn xatu_reports_unquoted_uri() -> googletest::Result<()> { fn xatu_reports_unquoted_uri() -> googletest::Result<()> {
let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?; let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?;