diff --git a/Cargo.lock b/Cargo.lock index a950d58..01385d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -108,6 +108,18 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" +[[package]] +name = "countme" +version = "3.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7704b5fdd17b18ae31c4c1da5a2e0305a2bf17b5249300a9ee9ed7b72114c636" + +[[package]] +name = "hashbrown" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" + [[package]] name = "heck" version = "0.5.0" @@ -164,6 +176,33 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "rnix" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c163bd17372eecdf10d351c34584b7de7c1a33be4e92a32f3fb3f5a7fe3f579b" +dependencies = [ + "rowan", +] + +[[package]] +name = "rowan" +version = "0.16.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "417a3a9f582e349834051b8a10c8d71ca88da4211e4093528e36b9845f6b5f21" +dependencies = [ + "countme", + "hashbrown", + "rustc-hash", + "text-size", +] + +[[package]] +name = "rustc-hash" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" + [[package]] name = "serde" version = "1.0.229" @@ -228,6 +267,12 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "text-size" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f18aa187839b2bdb1ad2fa35ead8c4c2976b64e4363c386d45ac0f7ee85c9233" + [[package]] name = "unicode-ident" version = "1.0.26" @@ -261,6 +306,7 @@ version = "0.1.0" dependencies = [ "clap", "monorepo", + "rnix", "status", ] diff --git a/Cargo.toml b/Cargo.toml index 64d4935..8d046b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ resolver = "3" [workspace.dependencies] clap = "4" monorepo = { path = "src/monorepo" } +rnix = "=0.14.0" serde = "1.0" -status = { path = "src/status" } serde_json = "1.0" +status = { path = "src/status" } diff --git a/src/xatu/BUILD b/src/xatu/BUILD index ac03412..502fff2 100644 --- a/src/xatu/BUILD +++ b/src/xatu/BUILD @@ -1,10 +1,13 @@ -load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library") +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library", "rust_test") rust_library( name = "xatu_lib", crate_name = "xatu", srcs = ["xatu.rs"], - deps = ["//src/status:status"], + deps = [ + "//src/status:status", + "//third_party/crates:rnix", + ], ) rust_binary( @@ -18,3 +21,9 @@ rust_binary( "//third_party/crates:clap", ], ) + +rust_test( + name = "xatu_test", + srcs = ["xatu_test.rs"], + deps = [":xatu_lib"], +) diff --git a/src/xatu/Cargo.toml b/src/xatu/Cargo.toml index 10d4d0b..2f87e0b 100644 --- a/src/xatu/Cargo.toml +++ b/src/xatu/Cargo.toml @@ -13,4 +13,5 @@ path = "xatu_main.rs" [dependencies] clap = { workspace = true, features = ["derive"] } monorepo = { workspace = true } +rnix = { workspace = true } status = { workspace = true } diff --git a/src/xatu/xatu.rs b/src/xatu/xatu.rs index 353efa8..8be3adf 100644 --- a/src/xatu/xatu.rs +++ b/src/xatu/xatu.rs @@ -1,14 +1,318 @@ -use std::path::PathBuf; +use std::ops::Range; +use std::path::{Path, PathBuf}; -pub fn run(files: Vec) -> status::Result<()> { - for file in files { - println!("{}", file.path.display()); - } - Ok(()) +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct FileId(usize); + +#[derive(Debug, Default)] +pub struct FileStore { + files: Vec, } -#[derive(Debug)] +impl FileStore { + pub fn add(&mut self, file: SourceFile) -> FileId { + let id = FileId(self.files.len()); + self.files.push(file); + id + } + + pub fn get(&self, id: FileId) -> &SourceFile { + &self.files[id.0] + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Severity { + Error, + Warning, +} + +impl Severity { + fn as_str(self) -> &'static str { + match self { + Self::Error => "error", + Self::Warning => "warning", + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct RuleMetadata { + pub id: &'static str, + pub severity: Severity, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Finding { + pub file_id: FileId, + pub range: Range, + pub rule_id: &'static str, + pub message: String, +} + +#[derive(Debug, Clone)] pub struct SourceFile { pub path: PathBuf, pub source: String, } + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NodeKind { + Inherit, +} + +impl NodeKind { + fn from_rnix(kind: rnix::SyntaxKind) -> Option { + match kind { + rnix::SyntaxKind::NODE_INHERIT => Some(Self::Inherit), + _ => None, + } + } +} + +#[derive(Debug, Clone)] +pub struct SyntaxNode { + raw: rnix::SyntaxNode, +} + +impl SyntaxNode { + fn new(raw: rnix::SyntaxNode) -> Self { + Self { raw } + } + + pub fn kind(&self) -> Option { + NodeKind::from_rnix(self.raw.kind()) + } + + pub fn text_range(&self) -> Range { + let range = self.raw.text_range(); + usize::from(range.start())..usize::from(range.end()) + } + + pub fn as_inherit(&self) -> Option { + if self.kind() == Some(NodeKind::Inherit) { + Some(InheritNode { + raw: self.raw.clone(), + }) + } else { + None + } + } +} + +#[derive(Debug, Clone)] +pub struct InheritNode { + raw: rnix::SyntaxNode, +} + +impl InheritNode { + pub fn is_empty(&self) -> bool { + !self.raw.children().any(|child| { + matches!( + child.kind(), + rnix::SyntaxKind::NODE_IDENT + | rnix::SyntaxKind::NODE_DYNAMIC + | rnix::SyntaxKind::NODE_STRING + ) + }) + } + + pub fn text_range(&self) -> Range { + let range = self.raw.text_range(); + usize::from(range.start())..usize::from(range.end()) + } +} + +trait NodeRule: Sync { + fn metadata(&self) -> RuleMetadata; + fn node_kinds(&self) -> &'static [NodeKind]; + fn check(&self, file_id: FileId, node: &SyntaxNode, findings: &mut Vec); +} + +struct EmptyInherit; + +impl EmptyInherit { + const METADATA: RuleMetadata = RuleMetadata { + id: "empty_inherit", + severity: Severity::Warning, + }; +} + +impl NodeRule for EmptyInherit { + fn metadata(&self) -> RuleMetadata { + Self::METADATA + } + + fn node_kinds(&self) -> &'static [NodeKind] { + &[NodeKind::Inherit] + } + + fn check(&self, file_id: FileId, node: &SyntaxNode, findings: &mut Vec) { + let Some(inherit) = node.as_inherit() else { + return; + }; + + if inherit.is_empty() { + findings.push(Finding { + file_id, + range: inherit.text_range(), + rule_id: Self::METADATA.id, + message: "empty inherit statement".to_owned(), + }); + } + } +} + +const PARSE_ERROR: RuleMetadata = RuleMetadata { + id: "parse_error", + severity: Severity::Error, +}; + +static NODE_RULES: &[&dyn NodeRule] = &[&EmptyInherit]; + +#[derive(Debug)] +pub struct Report { + pub files: FileStore, + pub findings: Vec, +} + +pub fn analyze(files: Vec) -> Report { + let mut store = FileStore::default(); + let mut findings = Vec::new(); + + for file in files { + let file_id = store.add(file); + analyze_file(file_id, &store, &mut findings); + } + + Report { + files: store, + findings, + } +} + +fn analyze_file(file_id: FileId, store: &FileStore, findings: &mut Vec) { + let file = store.get(file_id); + let parse = rnix::Root::parse(&file.source); + + for error in parse.errors() { + findings.push(Finding { + file_id, + range: parse_error_range(error, file.source.len()), + rule_id: PARSE_ERROR.id, + message: error.to_string(), + }); + } + + for raw_node in parse.syntax().descendants() { + let node = SyntaxNode::new(raw_node); + let Some(kind) = node.kind() else { + continue; + }; + + for rule in NODE_RULES { + if rule.node_kinds().contains(&kind) { + rule.check(file_id, &node, findings); + } + } + } +} + +fn parse_error_range(error: &rnix::ParseError, source_len: usize) -> Range { + match error { + rnix::ParseError::Unexpected(range) + | rnix::ParseError::UnexpectedExtra(range) + | rnix::ParseError::UnexpectedWanted(_, range, _) + | rnix::ParseError::UnexpectedDoubleBind(range) + | rnix::ParseError::DuplicatedArgs(range, _) => { + usize::from(range.start())..usize::from(range.end()) + } + rnix::ParseError::UnexpectedEOF + | rnix::ParseError::UnexpectedEOFWanted(_) + | rnix::ParseError::RecursionLimitExceeded => source_len..source_len, + _ => source_len..source_len, + } +} + +fn rule_severity(rule_id: &str) -> Severity { + if rule_id == PARSE_ERROR.id { + return PARSE_ERROR.severity; + } + + NODE_RULES + .iter() + .map(|rule| rule.metadata()) + .find(|metadata| metadata.id == rule_id) + .map(|metadata| metadata.severity) + .unwrap_or(Severity::Error) +} + +pub fn format_findings(report: &Report) -> Vec { + report + .findings + .iter() + .map(|finding| format_finding(&report.files, finding)) + .collect() +} + +fn format_finding(files: &FileStore, finding: &Finding) -> String { + let file = files.get(finding.file_id); + let position = line_column(&file.source, finding.range.start); + let severity = rule_severity(finding.rule_id); + + format!( + "{}:{}:{}: {} {} {}", + display_path(&file.path), + position.line, + position.column, + severity.as_str(), + finding.rule_id, + finding.message + ) +} + +fn display_path(path: &Path) -> String { + path.display().to_string() +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct Position { + line: usize, + column: usize, +} + +fn line_column(source: &str, offset: usize) -> Position { + let bounded_offset = offset.min(source.len()); + let mut line = 1; + let mut line_start = 0; + + for (index, byte) in source.bytes().enumerate() { + if index >= bounded_offset { + break; + } + + if byte == b'\n' { + line += 1; + line_start = index + 1; + } + } + + Position { + line, + column: bounded_offset.saturating_sub(line_start) + 1, + } +} + +pub fn exit_code_for_findings(findings: &[Finding]) -> i32 { + if findings.is_empty() { 0 } else { 1 } +} + +pub fn run(files: Vec) -> status::Result { + let report = analyze(files); + + for line in format_findings(&report) { + println!("{line}"); + } + + Ok(exit_code_for_findings(&report.findings)) +} diff --git a/src/xatu/xatu_main.rs b/src/xatu/xatu_main.rs index 24d519e..a32f647 100644 --- a/src/xatu/xatu_main.rs +++ b/src/xatu/xatu_main.rs @@ -37,12 +37,18 @@ fn collect_source_files(path: &Path) -> status::Result> { let mut files = Vec::new(); - for entry in std::fs::read_dir(path) + let mut entries: Vec = std::fs::read_dir(path) .with_context(|| format!("failed to read directory {}", path.display()))? - { - let entry = - entry.with_context(|| format!("failed to read entry in {}", path.display()))?; - files.extend(collect_source_files(&entry.path())?); + .map(|entry| { + entry + .map(|entry| entry.path()) + .with_context(|| format!("failed to read entry in {}", path.display())) + }) + .collect::>>()?; + entries.sort(); + + for entry in entries { + files.extend(collect_source_files(&entry)?); } return Ok(files); @@ -65,7 +71,7 @@ struct Args { paths: Vec, } -fn main() -> status::Result<()> { +fn try_main() -> status::Result { let args = Args::parse(); let paths: Vec = args @@ -84,3 +90,13 @@ fn main() -> status::Result<()> { xatu::run(files) } + +fn main() { + match try_main() { + Ok(code) => std::process::exit(code), + Err(error) => { + eprintln!("{error}"); + std::process::exit(2); + } + } +} diff --git a/src/xatu/xatu_test.rs b/src/xatu/xatu_test.rs new file mode 100644 index 0000000..5ee982d --- /dev/null +++ b/src/xatu/xatu_test.rs @@ -0,0 +1,67 @@ +use std::path::PathBuf; +use xatu::{SourceFile, analyze, exit_code_for_findings, format_findings}; + +fn source(source: &str) -> SourceFile { + SourceFile { + path: PathBuf::from("sample.nix"), + source: source.to_owned(), + } +} + +#[test] +fn parse_errors_become_findings() { + let report = analyze(vec![source("{")]); + + assert!(report + .findings + .iter() + .any(|finding| finding.rule_id == "parse_error")); +} + +#[test] +fn empty_inherit_reports_finding() { + let report = analyze(vec![source("{ inherit; }")]); + + assert_eq!(report.findings.len(), 1); + assert_eq!(report.findings[0].rule_id, "empty_inherit"); +} + +#[test] +fn inherit_from_without_attrs_reports_finding() { + let report = analyze(vec![source("{ inherit (pkgs); }")]); + + assert_eq!(report.findings.len(), 1); + assert_eq!(report.findings[0].rule_id, "empty_inherit"); +} + +#[test] +fn non_empty_inherit_does_not_report_finding() { + let report = analyze(vec![source("{ inherit foo; inherit (pkgs) bar; }")]); + + assert!(report.findings.is_empty()); +} + +#[test] +fn findings_use_compact_one_line_format() { + let report = analyze(vec![source("{\n inherit;\n}")]); + let lines = format_findings(&report); + + assert_eq!( + lines, + vec!["sample.nix:2:3: warning empty_inherit empty inherit statement"] + ); +} + +#[test] +fn exit_code_zero_means_no_findings() { + let report = analyze(vec![source("{ inherit foo; }")]); + + assert_eq!(exit_code_for_findings(&report.findings), 0); +} + +#[test] +fn exit_code_one_means_findings_exist() { + let report = analyze(vec![source("{ inherit; }")]); + + assert_eq!(exit_code_for_findings(&report.findings), 1); +} diff --git a/third_party/crates/crates.bzl b/third_party/crates/crates.bzl index 9991d05..69d9612 100644 --- a/third_party/crates/crates.bzl +++ b/third_party/crates/crates.bzl @@ -340,6 +340,7 @@ _NORMAL_DEPENDENCIES = { "src/xatu": { _COMMON_CONDITION: { "clap": Label("//third_party/crates/clap-4.6.7:clap"), + "rnix": Label("//third_party/crates/rnix-0.14.0:rnix"), }, }, }