diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index bd9a19b..0de20f4 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -28,15 +28,16 @@ jobs: uses: Swatinem/rust-cache@v2 - name: Check formatting - run: cargo fmt --all -- --check + run: make fmt-check - name: Clippy (deny warnings) - run: cargo clippy --all-targets -- -D warnings + run: make lint - name: Run tests - run: cargo test --all + run: make test - name: Security audit - uses: rustsec/audit-check@v2 - with: - token: ${{ secrets.GITHUB_TOKEN }} + run: make audit + + - name: Code metrics + run: make metrics diff --git a/.gitignore b/.gitignore index 96ef6c0..e27ef70 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ /target +/tools/hah-metrics/target Cargo.lock diff --git a/Makefile b/Makefile index 2b3d754..5fee1f9 100644 --- a/Makefile +++ b/Makefile @@ -1,44 +1,48 @@ -.PHONY: all setup fmt fmt-check lint test audit check doc-dependencies +.PHONY: all setup fmt fmt-check lint test audit coverage coverage-ci metrics check doc-dependencies + +# Configuration for quality gates +COVERAGE_MIN_THRESHOLD ?= 95 +METRIC_MAX_COMPLEXITY ?= 15 +METRIC_MAX_LENGTH ?= 60 all: check -## Install required Cargo tools (run once) +## Install required Cargo tools and pre-build the metrics analyser (run once) setup: + rustup component add llvm-tools-preview clippy rustfmt cargo install cargo-audit cargo install cargo-llvm-cov - rustup component add llvm-tools-preview + python3 --version + cargo build --manifest-path tools/hah-metrics/Cargo.toml --release -## Auto-format all code fmt: cargo fmt --all -## Check formatting without modifying files (used in CI) fmt-check: cargo fmt --all -- --check -## Run Clippy; treat all warnings as errors lint: cargo clippy --all-targets -- -D warnings -## Run all tests test: cargo test --all -## Run security audit against RustSec advisory database audit: cargo audit -## Generate HTML coverage report (opens in target/llvm-cov/html/) coverage: cargo llvm-cov --all-targets --workspace --html -## Fail the build if line coverage drops below 95 % coverage-ci: - cargo llvm-cov --all-targets --workspace --fail-under-lines 95 + cargo llvm-cov --all-targets --workspace --fail-under-lines $(COVERAGE_MIN_THRESHOLD) -## Full quality gate: format-check + lint + test + audit + coverage -check: fmt-check lint test audit coverage-ci +metrics: + cargo run --manifest-path tools/hah-metrics/Cargo.toml --release --quiet -- \ + --max-complexity $(METRIC_MAX_COMPLEXITY) \ + --max-length $(METRIC_MAX_LENGTH) -## Regenerate DEPENDENCIES.md from live cargo metadata doc-dependencies: python3 tools/gen_deps_doc.py > DEPENDENCIES.md + +check: fmt-check lint test audit coverage-ci metrics + diff --git a/crates/hah-checks/src/snap.rs b/crates/hah-checks/src/snap.rs index e5fdf40..7af4b65 100644 --- a/crates/hah-checks/src/snap.rs +++ b/crates/hah-checks/src/snap.rs @@ -1,10 +1,47 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use hah_core::{ check::{Check, Context}, model::{CheckResult, Finding, Remediation, Severity}, }; +// ── Finding builders ──────────────────────────────────────────────────────── + +fn disabled_revision_finding(name: &str, rev: &str) -> Finding { + Finding { + id: format!("snap-disabled-{name}-{rev}"), + title: format!("Snap '{name}' revision {rev} is disabled"), + description: format!( + "Disabled snap revisions still consume disk space. \ + Consider removing old revisions of '{name}'." + ), + severity: Severity::Info, + remediation: Some(Remediation { + description: format!("Remove disabled revision {rev} of {name}."), + commands: vec![format!("sudo snap remove {name} --revision={rev}")], + safe: false, + }), + } +} + +fn excess_revisions_finding(name: &str, count: u32, max_revisions: u64) -> Finding { + Finding { + id: format!("snap-too-many-revisions-{name}"), + title: format!("Snap '{name}' has {count} retained revisions (threshold: {max_revisions})"), + description: format!( + "Snap retains {count} revisions of '{name}', which wastes disk space." + ), + severity: Severity::Info, + remediation: Some(Remediation { + description: "Reduce the number of snap revisions retained.".into(), + commands: vec![format!( + "sudo snap set system refresh.retain={max_revisions}" + )], + safe: true, + }), + } +} + // ── SnapHealthCheck ────────────────────────────────────────────────────────── pub struct SnapHealthCheck; @@ -20,17 +57,13 @@ impl Check for SnapHealthCheck { fn run(&self, ctx: &Context) -> CheckResult { let max_revisions = ctx.config.threshold("snap_max_revisions", 2); - let out = match ctx.runner.run("snap", &["list", "--all"]) { Ok(o) => o, Err(_) => return CheckResult::default(), // snap not installed }; - let stdout = String::from_utf8_lossy(&out.stdout); - let mut snap_revisions: std::collections::HashMap = - std::collections::HashMap::new(); + let mut snap_revisions: HashMap = HashMap::new(); let mut result = CheckResult::default(); - for line in stdout.lines().skip(1) { let fields: Vec<&str> = line.split_whitespace().collect(); if fields.len() < 3 { @@ -38,48 +71,14 @@ impl Check for SnapHealthCheck { } let name = fields[0].to_string(); let rev = fields[2].to_string(); - // Notes column is last; "disabled" appears there for old revisions - let notes = fields.last().copied().unwrap_or(""); - - if notes.contains("disabled") { - result = result.with_finding(Finding { - id: format!("snap-disabled-{name}-{rev}"), - title: format!("Snap '{name}' revision {rev} is disabled"), - description: format!( - "Disabled snap revisions still consume disk space. \ - Consider removing old revisions of '{name}'." - ), - severity: Severity::Info, - remediation: Some(Remediation { - description: format!("Remove disabled revision {rev} of {name}."), - commands: vec![format!("sudo snap remove {name} --revision={rev}")], - safe: false, - }), - }); + if fields.last().copied().unwrap_or("").contains("disabled") { + result = result.with_finding(disabled_revision_finding(&name, &rev)); } - *snap_revisions.entry(name).or_default() += 1; } - for (name, count) in &snap_revisions { if *count > max_revisions as u32 { - result = result.with_finding(Finding { - id: format!("snap-too-many-revisions-{name}"), - title: format!( - "Snap '{name}' has {count} retained revisions (threshold: {max_revisions})" - ), - description: format!( - "Snap retains {count} revisions of '{name}', which wastes disk space." - ), - severity: Severity::Info, - remediation: Some(Remediation { - description: "Reduce the number of snap revisions retained.".into(), - commands: vec![format!( - "sudo snap set system refresh.retain={max_revisions}" - )], - safe: true, - }), - }); + result = result.with_finding(excess_revisions_finding(name, *count, max_revisions)); } } result diff --git a/crates/hah-dsl/src/pipeline.rs b/crates/hah-dsl/src/pipeline.rs index 594d26e..8f9adac 100644 --- a/crates/hah-dsl/src/pipeline.rs +++ b/crates/hah-dsl/src/pipeline.rs @@ -127,27 +127,7 @@ fn parse_filter(token: &str) -> Result { let name = token[..paren_pos].trim(); let raw_arg = token[paren_pos + 1..token.len() - 1].trim(); let arg = raw_arg.trim_matches('\'').trim_matches('"'); - return match name { - "skip" => arg - .parse::() - .map(Filter::Skip) - .map_err(|_| anyhow!("skip: expected an integer argument, got {arg:?}")), - "nth" => arg - .parse::() - .map(Filter::Nth) - .map_err(|_| anyhow!("nth: expected an integer argument, got {arg:?}")), - "field" => arg - .parse::() - .map(Filter::Field) - .map_err(|_| anyhow!("field: expected an integer argument, got {arg:?}")), - "prefix_strip" => Ok(Filter::PrefixStrip(arg.to_string())), - "starts_with" => Ok(Filter::StartsWith(arg.to_string())), - "contains" => Ok(Filter::Contains(arg.to_string())), - "reject_contains" => Ok(Filter::RejectContains(arg.to_string())), - "join" => Ok(Filter::Join(arg.to_string())), - "default" => Ok(Filter::Default(arg.to_string())), - other => Err(anyhow!("unknown filter with arguments: {other}")), - }; + return parse_filter_with_arg(name, arg); } match token { "trim" => Ok(Filter::Trim), @@ -163,6 +143,30 @@ fn parse_filter(token: &str) -> Result { } } +fn parse_filter_with_arg(name: &str, arg: &str) -> Result { + match name { + "skip" => arg + .parse::() + .map(Filter::Skip) + .map_err(|_| anyhow!("skip: expected an integer argument, got {arg:?}")), + "nth" => arg + .parse::() + .map(Filter::Nth) + .map_err(|_| anyhow!("nth: expected an integer argument, got {arg:?}")), + "field" => arg + .parse::() + .map(Filter::Field) + .map_err(|_| anyhow!("field: expected an integer argument, got {arg:?}")), + "prefix_strip" => Ok(Filter::PrefixStrip(arg.to_string())), + "starts_with" => Ok(Filter::StartsWith(arg.to_string())), + "contains" => Ok(Filter::Contains(arg.to_string())), + "reject_contains" => Ok(Filter::RejectContains(arg.to_string())), + "join" => Ok(Filter::Join(arg.to_string())), + "default" => Ok(Filter::Default(arg.to_string())), + other => Err(anyhow!("unknown filter with arguments: {other}")), + } +} + // ── Pipeline ───────────────────────────────────────────────────────────────── /// A parsed transformation pipeline. @@ -223,198 +227,281 @@ pub fn parse_pipeline(expr: &str) -> Result { Ok(Pipeline { source, filters }) } -// ── Filter application ──────────────────────────────────────────────────────── +// ── Individual filter implementations ──────────────────────────────────────── + +fn filter_trim(value: RuleValue) -> Result { + match value { + RuleValue::Str(s) => Ok(RuleValue::Str(s.trim().to_string())), + other => Ok(other), + } +} + +fn filter_lines(value: RuleValue) -> Result { + match value { + RuleValue::Str(s) => Ok(RuleValue::List( + s.lines().map(|l| RuleValue::Str(l.to_string())).collect(), + )), + other => Ok(RuleValue::List(vec![other])), + } +} + +fn filter_non_empty(value: RuleValue) -> Result { + match value { + RuleValue::List(v) => Ok(RuleValue::List( + v.into_iter() + .filter(|x| match x { + RuleValue::Str(s) => !s.is_empty(), + RuleValue::Null => false, + _ => true, + }) + .collect(), + )), + RuleValue::Str(s) if s.is_empty() => Ok(RuleValue::Null), + other => Ok(other), + } +} + +fn filter_skip(value: RuleValue, n: usize) -> Result { + match value { + RuleValue::List(v) => Ok(RuleValue::List(v.into_iter().skip(n).collect())), + other => Err(anyhow!("skip: expected a list, got {:?}", other.display())), + } +} + +fn filter_first(value: RuleValue) -> Result { + match value { + RuleValue::List(mut v) => Ok(if v.is_empty() { + RuleValue::Null + } else { + v.remove(0) + }), + other => Ok(other), + } +} + +fn filter_nth(value: RuleValue, n: usize) -> Result { + match value { + RuleValue::List(v) => Ok(v.into_iter().nth(n).unwrap_or(RuleValue::Null)), + other => Err(anyhow!("nth: expected a list, got {:?}", other.display())), + } +} + +fn filter_field(value: RuleValue, n: usize) -> Result { + match value { + RuleValue::Str(s) => Ok(s + .split_whitespace() + .nth(n) + .map_or(RuleValue::Null, |f| RuleValue::Str(f.to_string()))), + other => Err(anyhow!( + "field: expected a string, got {:?}", + other.display() + )), + } +} + +fn filter_number(value: RuleValue) -> Result { + match value { + RuleValue::Int(n) => Ok(RuleValue::Int(n)), + RuleValue::Str(s) => s + .trim() + .parse::() + .map(RuleValue::Int) + .map_err(|_| anyhow!("number: cannot parse {:?} as an integer", s)), + other => Err(anyhow!( + "number: expected a string or int, got {:?}", + other.display() + )), + } +} + +fn filter_count(value: &RuleValue) -> RuleValue { + let n: i64 = match value { + RuleValue::List(v) => v.len() as i64, + RuleValue::Str(s) => i64::from(!s.is_empty()), + RuleValue::Null => 0, + _ => 1, + }; + RuleValue::Int(n) +} + +fn filter_prefix_strip(value: RuleValue, prefix: &str) -> Result { + match value { + RuleValue::Str(s) => Ok(RuleValue::Str( + s.strip_prefix(prefix).unwrap_or(&s).to_string(), + )), + RuleValue::List(v) => Ok(RuleValue::List( + v.into_iter() + .map(|item| match item { + RuleValue::Str(s) => { + RuleValue::Str(s.strip_prefix(prefix).unwrap_or(&s).to_string()) + } + other => other, + }) + .collect(), + )), + other => Err(anyhow!( + "prefix_strip: expected a string or list, got {:?}", + other.display() + )), + } +} + +fn filter_starts_with(value: RuleValue, prefix: &str) -> Result { + match value { + RuleValue::List(v) => Ok(RuleValue::List( + v.into_iter() + .filter(|item| match item { + RuleValue::Str(s) => s.starts_with(prefix), + _ => false, + }) + .collect(), + )), + RuleValue::Str(s) => Ok(if s.starts_with(prefix) { + RuleValue::Str(s) + } else { + RuleValue::Null + }), + other => Err(anyhow!( + "starts_with: expected a list or string, got {:?}", + other.display() + )), + } +} + +fn filter_contains(value: &RuleValue, substring: &str) -> Result { + match value { + RuleValue::List(v) => Ok(RuleValue::Bool(v.iter().any(|item| match item { + RuleValue::Str(s) => s.contains(substring), + _ => false, + }))), + RuleValue::Str(s) => Ok(RuleValue::Bool(s.contains(substring))), + _ => Ok(RuleValue::Bool(false)), + } +} + +fn filter_reject_contains(value: RuleValue, substring: &str) -> Result { + match value { + RuleValue::List(v) => Ok(RuleValue::List( + v.into_iter() + .filter(|item| match item { + RuleValue::Str(s) => !s.contains(substring), + _ => true, + }) + .collect(), + )), + other => Err(anyhow!( + "reject_contains: expected a list, got {:?}", + other.display() + )), + } +} + +fn filter_sort(value: RuleValue) -> Result { + match value { + RuleValue::List(mut v) => { + v.sort_by_key(RuleValue::display); + Ok(RuleValue::List(v)) + } + other => Ok(other), + } +} + +fn filter_unique(value: RuleValue) -> Result { + match value { + RuleValue::List(v) => { + let mut seen = HashSet::new(); + Ok(RuleValue::List( + v.into_iter().filter(|x| seen.insert(x.display())).collect(), + )) + } + other => Ok(other), + } +} + +fn filter_join(value: RuleValue, sep: &str) -> Result { + match value { + RuleValue::List(v) => Ok(RuleValue::Str( + v.iter() + .map(RuleValue::display) + .collect::>() + .join(sep), + )), + RuleValue::Str(s) => Ok(RuleValue::Str(s)), + other => Err(anyhow!("join: expected a list, got {:?}", other.display())), + } +} + +fn filter_bytes_to_mb(value: RuleValue) -> Result { + match value { + RuleValue::Int(n) => Ok(RuleValue::Int(n / 1_048_576)), + RuleValue::Str(s) => s + .trim() + .parse::() + .map(|n| RuleValue::Int(n / 1_048_576)) + .map_err(|_| anyhow!("bytes_to_mb: cannot parse {:?} as an integer", s)), + other => Err(anyhow!( + "bytes_to_mb: expected int or string, got {:?}", + other.display() + )), + } +} + +fn filter_default(value: RuleValue, default_val: &str) -> RuleValue { + let use_default = + matches!(value, RuleValue::Null) || matches!(&value, RuleValue::Str(s) if s.is_empty()); + if use_default { + RuleValue::Str(default_val.to_string()) + } else { + value + } +} + +// ── Filter dispatch ─────────────────────────────────────────────────────────── + +fn apply_scalar_filter(value: RuleValue, filter: &Filter) -> Result { + match filter { + Filter::Trim => filter_trim(value), + Filter::Lines => filter_lines(value), + Filter::NonEmpty => filter_non_empty(value), + Filter::First => filter_first(value), + Filter::Sort => filter_sort(value), + Filter::Unique => filter_unique(value), + Filter::Count => Ok(filter_count(&value)), + Filter::Skip(n) => filter_skip(value, *n), + Filter::Nth(n) => filter_nth(value, *n), + Filter::Field(n) => filter_field(value, *n), + Filter::Number => filter_number(value), + _ => unreachable!(), + } +} + +fn apply_string_filter(value: RuleValue, filter: &Filter) -> Result { + match filter { + Filter::PrefixStrip(p) => filter_prefix_strip(value, p), + Filter::StartsWith(p) => filter_starts_with(value, p), + Filter::Contains(s) => filter_contains(&value, s), + Filter::RejectContains(s) => filter_reject_contains(value, s), + Filter::Join(sep) => filter_join(value, sep), + Filter::BytesToMb => filter_bytes_to_mb(value), + Filter::Default(d) => Ok(filter_default(value, d)), + _ => unreachable!(), + } +} fn apply_filter(value: RuleValue, filter: &Filter) -> Result { match filter { - Filter::Trim => match value { - RuleValue::Str(s) => Ok(RuleValue::Str(s.trim().to_string())), - other => Ok(other), - }, - - Filter::Lines => match value { - RuleValue::Str(s) => Ok(RuleValue::List( - s.lines().map(|l| RuleValue::Str(l.to_string())).collect(), - )), - other => Ok(RuleValue::List(vec![other])), - }, - - Filter::NonEmpty => match value { - RuleValue::List(v) => Ok(RuleValue::List( - v.into_iter() - .filter(|x| match x { - RuleValue::Str(s) => !s.is_empty(), - RuleValue::Null => false, - _ => true, - }) - .collect(), - )), - RuleValue::Str(s) if s.is_empty() => Ok(RuleValue::Null), - other => Ok(other), - }, - - Filter::Skip(n) => match value { - RuleValue::List(v) => Ok(RuleValue::List(v.into_iter().skip(*n).collect())), - other => Err(anyhow!("skip: expected a list, got {:?}", other.display())), - }, - - Filter::First => match value { - RuleValue::List(mut v) => Ok(if v.is_empty() { - RuleValue::Null - } else { - v.remove(0) - }), - other => Ok(other), - }, - - Filter::Nth(n) => match value { - RuleValue::List(v) => Ok(v.into_iter().nth(*n).unwrap_or(RuleValue::Null)), - other => Err(anyhow!("nth: expected a list, got {:?}", other.display())), - }, - - Filter::Field(n) => match value { - RuleValue::Str(s) => Ok(s - .split_whitespace() - .nth(*n) - .map_or(RuleValue::Null, |f| RuleValue::Str(f.to_string()))), - other => Err(anyhow!( - "field: expected a string, got {:?}", - other.display() - )), - }, - - Filter::Number => match value { - RuleValue::Int(n) => Ok(RuleValue::Int(n)), - RuleValue::Str(s) => s - .trim() - .parse::() - .map(RuleValue::Int) - .map_err(|_| anyhow!("number: cannot parse {:?} as an integer", s)), - other => Err(anyhow!( - "number: expected a string or int, got {:?}", - other.display() - )), - }, - - Filter::PrefixStrip(prefix) => match value { - RuleValue::Str(s) => Ok(RuleValue::Str( - s.strip_prefix(prefix.as_str()).unwrap_or(&s).to_string(), - )), - RuleValue::List(v) => Ok(RuleValue::List( - v.into_iter() - .map(|item| match item { - RuleValue::Str(s) => RuleValue::Str( - s.strip_prefix(prefix.as_str()).unwrap_or(&s).to_string(), - ), - other => other, - }) - .collect(), - )), - other => Err(anyhow!( - "prefix_strip: expected a string or list, got {:?}", - other.display() - )), - }, - - Filter::StartsWith(prefix) => match value { - RuleValue::List(v) => Ok(RuleValue::List( - v.into_iter() - .filter(|item| match item { - RuleValue::Str(s) => s.starts_with(prefix.as_str()), - _ => false, - }) - .collect(), - )), - RuleValue::Str(s) => Ok(if s.starts_with(prefix.as_str()) { - RuleValue::Str(s) - } else { - RuleValue::Null - }), - other => Err(anyhow!( - "starts_with: expected a list or string, got {:?}", - other.display() - )), - }, - - Filter::Contains(substring) => match &value { - RuleValue::List(v) => Ok(RuleValue::Bool(v.iter().any(|item| match item { - RuleValue::Str(s) => s.contains(substring.as_str()), - _ => false, - }))), - RuleValue::Str(s) => Ok(RuleValue::Bool(s.contains(substring.as_str()))), - _ => Ok(RuleValue::Bool(false)), - }, - - Filter::RejectContains(substring) => match value { - RuleValue::List(v) => Ok(RuleValue::List( - v.into_iter() - .filter(|item| match item { - RuleValue::Str(s) => !s.contains(substring.as_str()), - _ => true, - }) - .collect(), - )), - other => Err(anyhow!( - "reject_contains: expected a list, got {:?}", - other.display() - )), - }, - - Filter::Count => Ok(RuleValue::Int(match &value { - RuleValue::List(v) => v.len() as i64, - RuleValue::Str(s) if s.is_empty() => 0, - RuleValue::Str(_) => 1, - RuleValue::Null => 0, - _ => 1, - })), - - Filter::Sort => match value { - RuleValue::List(mut v) => { - v.sort_by_key(RuleValue::display); - Ok(RuleValue::List(v)) - } - other => Ok(other), - }, - - Filter::Unique => match value { - RuleValue::List(v) => { - let mut seen = HashSet::new(); - Ok(RuleValue::List( - v.into_iter().filter(|x| seen.insert(x.display())).collect(), - )) - } - other => Ok(other), - }, - - Filter::Join(sep) => match value { - RuleValue::List(v) => Ok(RuleValue::Str( - v.iter() - .map(RuleValue::display) - .collect::>() - .join(sep), - )), - RuleValue::Str(s) => Ok(RuleValue::Str(s)), - other => Err(anyhow!("join: expected a list, got {:?}", other.display())), - }, - - Filter::BytesToMb => match value { - RuleValue::Int(n) => Ok(RuleValue::Int(n / 1_048_576)), - RuleValue::Str(s) => s - .trim() - .parse::() - .map(|n| RuleValue::Int(n / 1_048_576)) - .map_err(|_| anyhow!("bytes_to_mb: cannot parse {:?} as an integer", s)), - other => Err(anyhow!( - "bytes_to_mb: expected int or string, got {:?}", - other.display() - )), - }, - - Filter::Default(default_val) => Ok(match value { - RuleValue::Null => RuleValue::Str(default_val.clone()), - RuleValue::Str(s) if s.is_empty() => RuleValue::Str(default_val.clone()), - other => other, - }), + Filter::Trim + | Filter::Lines + | Filter::NonEmpty + | Filter::First + | Filter::Sort + | Filter::Unique + | Filter::Count + | Filter::Skip(_) + | Filter::Nth(_) + | Filter::Field(_) + | Filter::Number => apply_scalar_filter(value, filter), + _ => apply_string_filter(value, filter), } } diff --git a/crates/hah-dsl/src/rule.rs b/crates/hah-dsl/src/rule.rs index 6aae62a..1d4f8e3 100644 --- a/crates/hah-dsl/src/rule.rs +++ b/crates/hah-dsl/src/rule.rs @@ -329,18 +329,7 @@ impl Check for RuleBasedCheck { } // ── 2. Seed value map with context ──────────────────────────────────── - let mut values: ValueMap = HashMap::new(); - for (key, &val) in &ctx.config.thresholds { - values.insert(format!("config.{key}"), RuleValue::Int(val as i64)); - } - values.insert( - "distro.family".into(), - RuleValue::Str(if ctx.distro.is_debian_family() { - "debian".into() - } else { - "unknown".into() - }), - ); + let mut values = self.seed_context_values(ctx); // ── 3. Run triggers ─────────────────────────────────────────────────── for trigger in &self.rule.triggers { @@ -385,6 +374,26 @@ impl Check for RuleBasedCheck { } } +// ── Context seeding ─────────────────────────────────────────────────────────── + +impl RuleBasedCheck { + fn seed_context_values(&self, ctx: &Context) -> ValueMap { + let mut values: ValueMap = HashMap::new(); + for (key, &val) in &ctx.config.thresholds { + values.insert(format!("config.{key}"), RuleValue::Int(val as i64)); + } + values.insert( + "distro.family".into(), + RuleValue::Str(if ctx.distro.is_debian_family() { + "debian".into() + } else { + "unknown".into() + }), + ); + values + } +} + // ── Guard evaluation ────────────────────────────────────────────────────────── impl RuleBasedCheck { @@ -527,6 +536,40 @@ fn numeric_compare(lhs: i64, op: &CompareOp, rhs: i64) -> bool { } } +fn eval_numeric_threshold( + value: &str, + operator: &CompareOp, + threshold: &str, + values: &ValueMap, +) -> Result { + let lhs = eval_expr(value, values)?; + let rhs = eval_expr(threshold, values)?; + match (lhs.as_int(), rhs.as_int()) { + (Some(l), Some(r)) => Ok(numeric_compare(l, operator, r)), + _ => Err(anyhow!( + "numeric_threshold: both sides must be numeric (got {:?} and {:?})", + lhs.display(), + rhs.display() + )), + } +} + +fn eval_equals(value: &str, expected: &ExpectedValue, values: &ValueMap) -> Result { + let actual = eval_expr(value, values)?; + Ok(match expected { + ExpectedValue::Bool(b) => actual.as_bool() == Some(*b), + ExpectedValue::Int(n) => actual.as_int() == Some(*n), + ExpectedValue::Str(s) => actual.as_str() == Some(s.as_str()), + }) +} + +fn eval_regex_match(value: &str, pattern: &str, values: &ValueMap) -> Result { + let re = regex::Regex::new(pattern) + .map_err(|e| anyhow!("invalid regex pattern {pattern:?}: {e}"))?; + let v = eval_expr(value, values)?; + Ok(re.is_match(v.as_str().unwrap_or(""))) +} + impl RuleBasedCheck { fn eval_condition(&self, condition: &RuleCondition, values: &ValueMap) -> Result { match condition { @@ -535,42 +578,16 @@ impl RuleBasedCheck { operator, threshold, .. - } => { - let lhs = eval_expr(value, values)?; - let rhs = eval_expr(threshold, values)?; - match (lhs.as_int(), rhs.as_int()) { - (Some(l), Some(r)) => Ok(numeric_compare(l, operator, r)), - _ => Err(anyhow!( - "numeric_threshold: both sides must be numeric (got {:?} and {:?})", - lhs.display(), - rhs.display() - )), - } - } + } => eval_numeric_threshold(value, operator, threshold, values), RuleCondition::Equals { value, expected, .. - } => { - let actual = eval_expr(value, values)?; - let matches = match expected { - ExpectedValue::Bool(b) => actual.as_bool() == Some(*b), - ExpectedValue::Int(n) => actual.as_int() == Some(*n), - ExpectedValue::Str(s) => actual.as_str() == Some(s.as_str()), - }; - Ok(matches) - } + } => eval_equals(value, expected, values), - RuleCondition::NonEmpty { value, .. } => { - let v = eval_expr(value, values)?; - Ok(v.is_truthy()) - } + RuleCondition::NonEmpty { value, .. } => Ok(eval_expr(value, values)?.is_truthy()), RuleCondition::RegexMatch { value, pattern, .. } => { - let re = regex::Regex::new(pattern) - .map_err(|e| anyhow!("invalid regex pattern {pattern:?}: {e}"))?; - let v = eval_expr(value, values)?; - let s = v.as_str().unwrap_or(""); - Ok(re.is_match(s)) + eval_regex_match(value, pattern, values) } RuleCondition::All { conditions, .. } => conditions diff --git a/tools/hah-metrics/Cargo.toml b/tools/hah-metrics/Cargo.toml new file mode 100644 index 0000000..6aa554f --- /dev/null +++ b/tools/hah-metrics/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "hah-metrics" +version = "0.1.0" +edition = "2021" + +[workspace] + +[[bin]] +name = "hah-metrics" +path = "src/main.rs" + +[dependencies] +# AST-accurate Rust parsing +syn = { version = "2", features = ["full", "visit"] } +# span-locations enables Span::start()/end() for line/column info outside proc-macros +proc-macro2 = { version = "1", features = ["span-locations"] } +# CLI argument parsing +clap = { version = "4", features = ["derive"] } +# Recursive directory walking +walkdir = "2" diff --git a/tools/hah-metrics/src/main.rs b/tools/hah-metrics/src/main.rs new file mode 100644 index 0000000..c98002a --- /dev/null +++ b/tools/hah-metrics/src/main.rs @@ -0,0 +1,302 @@ +use clap::Parser; +use std::path::Path; +use syn::{ + visit::{self, Visit}, + Arm, Attribute, BinOp, Block, ExprBinary, ExprForLoop, ExprIf, ExprTry, ExprWhile, + Ident, ImplItemFn, ItemFn, ItemMod, Signature, +}; +use walkdir::WalkDir; + +type Result = std::result::Result>; + +// --------------------------------------------------------------------------- +// CLI +// --------------------------------------------------------------------------- + +#[derive(Parser)] +#[command( + name = "hah-metrics", + about = "AST-accurate Rust code metrics analyser" +)] +struct Args { + /// Maximum allowed cyclomatic complexity per function + #[arg(long, default_value_t = 15)] + max_complexity: usize, + + /// Maximum allowed function length in lines + #[arg(long, default_value_t = 60)] + max_length: usize, + + /// Root directory to analyse (defaults to "crates") + #[arg(default_value = "crates")] + dir: String, +} + +// --------------------------------------------------------------------------- +// Data +// --------------------------------------------------------------------------- + +struct FnMetrics { + name: String, + file: String, + start_line: usize, + length: usize, + complexity: usize, + is_test: bool, +} + +// --------------------------------------------------------------------------- +// Complexity visitor +// +// Cyclomatic complexity = 1 + decision points. +// Decision points counted: +// if / else-if (+1 each) while (+1) for (+1) +// match arms (+1 each) && / || (+1) ? try-operator (+1) +// +// Nested *named* function definitions are NOT counted as part of the +// enclosing function (stop recursion via visit_item_fn / visit_impl_item_fn). +// Closures are treated as inline expressions and DO count. +// --------------------------------------------------------------------------- + +#[derive(Default)] +struct ComplexityVisitor { + count: usize, +} + +impl<'ast> Visit<'ast> for ComplexityVisitor { + fn visit_expr_if(&mut self, node: &'ast ExprIf) { + self.count += 1; + visit::visit_expr_if(self, node); + } + + fn visit_expr_while(&mut self, node: &'ast ExprWhile) { + self.count += 1; + visit::visit_expr_while(self, node); + } + + fn visit_expr_for_loop(&mut self, node: &'ast ExprForLoop) { + self.count += 1; + visit::visit_expr_for_loop(self, node); + } + + fn visit_arm(&mut self, node: &'ast Arm) { + self.count += 1; + visit::visit_arm(self, node); + } + + fn visit_expr_binary(&mut self, node: &'ast ExprBinary) { + if matches!(node.op, BinOp::And(_) | BinOp::Or(_)) { + self.count += 1; + } + visit::visit_expr_binary(self, node); + } + + fn visit_expr_try(&mut self, node: &'ast ExprTry) { + self.count += 1; + visit::visit_expr_try(self, node); + } + + // Stop recursion into nested named functions so their complexity is + // attributed to themselves, not the outer function. + fn visit_item_fn(&mut self, _node: &'ast ItemFn) {} + fn visit_impl_item_fn(&mut self, _node: &'ast ImplItemFn) {} +} + +fn compute_complexity(block: &Block) -> usize { + let mut v = ComplexityVisitor { count: 1 }; + v.visit_block(block); + v.count +} + +// --------------------------------------------------------------------------- +// File visitor — collects FnMetrics for every function in a file +// --------------------------------------------------------------------------- + +struct FileVisitor { + metrics: Vec, + in_test: bool, + filepath: String, +} + +fn has_cfg_test(attrs: &[Attribute]) -> bool { + attrs.iter().any(|attr| { + attr.path().is_ident("cfg") + && attr + .parse_args::() + .map(|id| id == "test") + .unwrap_or(false) + }) +} + +fn has_test_attr(attrs: &[Attribute]) -> bool { + attrs.iter().any(|attr| attr.path().is_ident("test")) +} + +impl FileVisitor { + fn record(&mut self, sig: &Signature, block: &Block, attrs: &[Attribute]) { + let is_test = self.in_test || has_test_attr(attrs); + let start_line = sig.fn_token.span.start().line; + let end_line = block.brace_token.span.close().end().line; + let length = end_line.saturating_sub(start_line).saturating_add(1); + self.metrics.push(FnMetrics { + name: sig.ident.to_string(), + file: self.filepath.clone(), + start_line, + length, + complexity: compute_complexity(block), + is_test, + }); + } +} + +impl<'ast> Visit<'ast> for FileVisitor { + fn visit_item_mod(&mut self, node: &'ast ItemMod) { + let prev = self.in_test; + if has_cfg_test(&node.attrs) { + self.in_test = true; + } + visit::visit_item_mod(self, node); + self.in_test = prev; + } + + fn visit_item_fn(&mut self, node: &'ast ItemFn) { + self.record(&node.sig, &node.block, &node.attrs); + // Recurse so nested functions inside the body are also collected. + visit::visit_item_fn(self, node); + } + + fn visit_impl_item_fn(&mut self, node: &'ast ImplItemFn) { + self.record(&node.sig, &node.block, &node.attrs); + visit::visit_impl_item_fn(self, node); + } +} + +// --------------------------------------------------------------------------- +// Per-file analysis +// --------------------------------------------------------------------------- + +fn analyze_file(path: &Path) -> Result<(Vec, usize)> { + let content = std::fs::read_to_string(path)?; + let total_loc = content.lines().count(); + let syntax = syn::parse_file(&content)?; + + let path_str = path.to_string_lossy(); + // Treat whole-file test files as entirely test code. + let is_test_file = path_str.contains("/tests/") + || path_str.ends_with("integration.rs") + || path_str.ends_with("tests.rs"); + + let mut visitor = FileVisitor { + metrics: Vec::new(), + in_test: is_test_file, + filepath: path_str.to_string(), + }; + visitor.visit_file(&syntax); + + Ok((visitor.metrics, total_loc)) +} + +// --------------------------------------------------------------------------- +// Entry point +// --------------------------------------------------------------------------- + +fn main() { + let args = Args::parse(); + + let mut all_metrics: Vec = Vec::new(); + let mut total_loc: usize = 0; + let mut parse_errors: usize = 0; + + for entry in WalkDir::new(&args.dir) + .into_iter() + .filter_map(|e| e.ok()) + .filter(|e| e.path().extension().is_some_and(|ext| ext == "rs")) + { + match analyze_file(entry.path()) { + Ok((metrics, loc)) => { + total_loc += loc; + all_metrics.extend(metrics); + } + Err(e) => { + eprintln!("Warning: could not parse {}: {}", entry.path().display(), e); + parse_errors += 1; + } + } + } + + // Approximate test LoC as the sum of lengths of all test-marked functions. + let test_loc: usize = all_metrics + .iter() + .filter(|m| m.is_test) + .map(|m| m.length) + .sum(); + let source_loc = total_loc.saturating_sub(test_loc); + + let source: Vec<&FnMetrics> = all_metrics.iter().filter(|m| !m.is_test).collect(); + + // --- Summary ----------------------------------------------------------- + println!("Code Metrics Report"); + println!("==================="); + println!("Total LoC: {total_loc}"); + println!("Source LoC: {source_loc}"); + println!("Test LoC: {test_loc} (sum of test-function lengths)"); + if source_loc > 0 { + println!( + "Ratio (Test/Source): {:.2}", + test_loc as f64 / source_loc as f64 + ); + } + println!(); + + let complexities: Vec = source.iter().map(|m| m.complexity).collect(); + if !complexities.is_empty() { + println!("Function Complexity ({} source functions):", complexities.len()); + println!(" Min: {}", complexities.iter().min().unwrap()); + println!(" Max: {}", complexities.iter().max().unwrap()); + println!( + " Avg: {:.2}", + complexities.iter().sum::() as f64 / complexities.len() as f64 + ); + } + + let lengths: Vec = source.iter().map(|m| m.length).collect(); + if !lengths.is_empty() { + println!("Function Length (lines):"); + println!(" Min: {}", lengths.iter().min().unwrap()); + println!(" Max: {}", lengths.iter().max().unwrap()); + println!( + " Avg: {:.2}", + lengths.iter().sum::() as f64 / lengths.len() as f64 + ); + } + + // --- Violations -------------------------------------------------------- + let mut violations: Vec = Vec::new(); + + for m in &source { + if m.complexity > args.max_complexity { + violations.push(format!( + "HIGH COMPLEXITY: {}:{} - fn `{}` complexity {} (limit {})", + m.file, m.start_line, m.name, m.complexity, args.max_complexity + )); + } + if m.length > args.max_length { + violations.push(format!( + "LONG FUNCTION: {}:{} - fn `{}` {} lines (limit {})", + m.file, m.start_line, m.name, m.length, args.max_length + )); + } + } + + if !violations.is_empty() { + println!("\nThreshold Violations:"); + for v in &violations { + println!(" {v}"); + } + std::process::exit(1); + } + + if parse_errors > 0 { + eprintln!("Note: {parse_errors} file(s) had parse errors"); + } +}