From 4a08dc131715e31c1117ff92c560046227f4181c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Wed, 30 Sep 2026 14:32:04 -0300 Subject: [PATCH 1/4] refactor(core): drive metrics from a Metric registry The seven metric names were listed by hand in config validation, limit resolution, violation emission, and baseline lookup, so a missed entry compiled fine and silently dropped the metric. A Metric enum now holds the list, names, and applicability, and exhaustive accessors make a new variant fail to compile until every path handles it. Output and config are unchanged. --- crates/core/src/baseline.rs | 43 +++++----- crates/core/src/config.rs | 69 +++++++++++----- crates/core/src/lib.rs | 2 + crates/core/src/metric.rs | 156 ++++++++++++++++++++++++++++++++++++ crates/core/src/scan.rs | 52 ++++++------ 5 files changed, 254 insertions(+), 68 deletions(-) create mode 100644 crates/core/src/metric.rs diff --git a/crates/core/src/baseline.rs b/crates/core/src/baseline.rs index 1e99ae1..e1c36f5 100644 --- a/crates/core/src/baseline.rs +++ b/crates/core/src/baseline.rs @@ -5,7 +5,7 @@ use std::{cmp::Reverse, collections::BTreeMap}; use serde::Serialize; -use crate::FunctionMetrics; +use crate::{FunctionMetrics, Metric}; const ANONYMOUS: &str = ""; @@ -64,30 +64,33 @@ pub struct MetricValues { } impl MetricValues { - pub fn get(&self, metric: &str) -> Option { - Some(match metric { - "complexity" => self.complexity, - "cognitive" => self.cognitive, - "depth" => self.depth, - "lines" => self.lines, - "params" => self.params, - "bool_ops" => self.bool_ops, - "widget_depth" => self.widget_depth, - _ => return None, - }) + pub fn get(&self, name: &str) -> Option { + Metric::parse(name).map(|metric| self.value(metric)) + } + + pub fn value(&self, metric: Metric) -> usize { + match metric { + Metric::Complexity => self.complexity, + Metric::Cognitive => self.cognitive, + Metric::Depth => self.depth, + Metric::Lines => self.lines, + Metric::Params => self.params, + Metric::BoolOps => self.bool_ops, + Metric::WidgetDepth => self.widget_depth, + } } } impl From<&FunctionMetrics> for MetricValues { fn from(unit: &FunctionMetrics) -> Self { Self { - complexity: unit.complexity, - cognitive: unit.cognitive, - depth: unit.depth, - lines: unit.lines, - params: unit.params, - bool_ops: unit.bool_ops, - widget_depth: unit.widget_depth, + complexity: unit.value(Metric::Complexity), + cognitive: unit.value(Metric::Cognitive), + depth: unit.value(Metric::Depth), + lines: unit.value(Metric::Lines), + params: unit.value(Metric::Params), + bool_ops: unit.value(Metric::BoolOps), + widget_depth: unit.value(Metric::WidgetDepth), } } } @@ -131,7 +134,7 @@ pub fn status( value: usize, ) -> Status { let base_value = match (pairing, base) { - (Pairing::Paired(_), Some(unit)) => MetricValues::from(unit).get(metric), + (Pairing::Paired(_), Some(unit)) => Metric::parse(metric).map(|metric| unit.value(metric)), (Pairing::Unmatched, _) => return Status::Unmatched, _ => None, }; diff --git a/crates/core/src/config.rs b/crates/core/src/config.rs index b09bcc0..f817999 100644 --- a/crates/core/src/config.rs +++ b/crates/core/src/config.rs @@ -9,16 +9,9 @@ use globset::{Glob, GlobSet, GlobSetBuilder}; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; +use crate::Metric; + const DEFAULTS: &str = include_str!("../../../config.default.json"); -const LIMIT_KEYS: &[&str] = &[ - "complexity", - "cognitive", - "depth", - "lines", - "params", - "bool_ops", - "widget_depth", -]; /// A `None` limit is `null` in config: the metric is measured but never fails. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] @@ -33,6 +26,32 @@ pub struct Limits { pub widget_depth: Option, } +impl Limits { + pub fn get(&self, metric: Metric) -> Option { + match metric { + Metric::Complexity => self.complexity, + Metric::Cognitive => self.cognitive, + Metric::Depth => self.depth, + Metric::Lines => self.lines, + Metric::Params => self.params, + Metric::BoolOps => self.bool_ops, + Metric::WidgetDepth => self.widget_depth, + } + } + + pub fn get_mut(&mut self, metric: Metric) -> &mut Option { + match metric { + Metric::Complexity => &mut self.complexity, + Metric::Cognitive => &mut self.cognitive, + Metric::Depth => &mut self.depth, + Metric::Lines => &mut self.lines, + Metric::Params => &mut self.params, + Metric::BoolOps => &mut self.bool_ops, + Metric::WidgetDepth => &mut self.widget_depth, + } + } +} + #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] pub struct TestsConfig { @@ -96,6 +115,20 @@ pub struct LimitOverrides { pub widget_depth: Option>, } +impl LimitOverrides { + pub fn get(&self, metric: Metric) -> Option> { + match metric { + Metric::Complexity => self.complexity, + Metric::Cognitive => self.cognitive, + Metric::Depth => self.depth, + Metric::Lines => self.lines, + Metric::Params => self.params, + Metric::BoolOps => self.bool_ops, + Metric::WidgetDepth => self.widget_depth, + } + } +} + fn explicit<'de, D>(deserializer: D) -> Result>, D::Error> where D: serde::Deserializer<'de>, @@ -199,7 +232,7 @@ fn validate_keys(value: &Value, path: &Path) -> Result<()> { "", path, )?; - nested_keys(object, "limits", LIMIT_KEYS, path)?; + nested_keys(object, "limits", &Metric::ALL.map(Metric::name), path)?; nested_keys(object, "tests", &["patterns", "exempt"], path)?; validate_test_exempt(object, path)?; nested_keys(object, "hook", &["max_blocks"], path)?; @@ -249,7 +282,7 @@ fn validate_language_keys(root: &Map, path: &Path) -> Result<()> .as_object() .ok_or_else(|| anyhow::anyhow!("languages.{name} must be an object"))?; allowed(object, &["limits"], &format!("languages.{name}"), path)?; - nested_keys(object, "limits", LIMIT_KEYS, path)?; + nested_keys(object, "limits", &Metric::ALL.map(Metric::name), path)?; } Ok(()) } @@ -295,15 +328,13 @@ impl Config { else { return self.limits.clone(); }; - Limits { - complexity: overrides.complexity.unwrap_or(self.limits.complexity), - cognitive: overrides.cognitive.unwrap_or(self.limits.cognitive), - depth: overrides.depth.unwrap_or(self.limits.depth), - lines: overrides.lines.unwrap_or(self.limits.lines), - params: overrides.params.unwrap_or(self.limits.params), - bool_ops: overrides.bool_ops.unwrap_or(self.limits.bool_ops), - widget_depth: overrides.widget_depth.unwrap_or(self.limits.widget_depth), + let mut limits = self.limits.clone(); + for metric in Metric::ALL { + if let Some(limit) = overrides.get(metric) { + *limits.get_mut(metric) = limit; + } } + limits } pub fn matcher(patterns: &[String]) -> Result { diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index 28022fd..fce1544 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -5,6 +5,7 @@ mod cognitive; pub mod config; pub mod diff; pub mod language; +mod metric; pub mod scan; pub use baseline::{MetricValues, Pairing, Status, pair_units}; @@ -14,4 +15,5 @@ pub use diff::{ parse_diff_hunks, }; pub use language::{FunctionMetrics, Language, coverage_unknowns, grammar_inventory, parse_source}; +pub use metric::Metric; pub use scan::{Baseline, ScanOptions, ScanResult, Unverified, Violation, scan}; diff --git a/crates/core/src/metric.rs b/crates/core/src/metric.rs new file mode 100644 index 0000000..070aed9 --- /dev/null +++ b/crates/core/src/metric.rs @@ -0,0 +1,156 @@ +//! The one list of metrics. Accessors elsewhere match on `Metric` without a +//! wildcard, so a new variant fails to compile until every path handles it. + +use crate::FunctionMetrics; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Metric { + Complexity, + Cognitive, + Depth, + Lines, + Params, + BoolOps, + WidgetDepth, +} + +impl Metric { + pub const ALL: [Self; 7] = [ + Self::Complexity, + Self::Cognitive, + Self::Depth, + Self::Lines, + Self::Params, + Self::BoolOps, + Self::WidgetDepth, + ]; + + /// The config key and JSON name. + pub fn name(self) -> &'static str { + match self { + Self::Complexity => "complexity", + Self::Cognitive => "cognitive", + Self::Depth => "depth", + Self::Lines => "lines", + Self::Params => "params", + Self::BoolOps => "bool_ops", + Self::WidgetDepth => "widget_depth", + } + } + + pub fn parse(name: &str) -> Option { + Self::ALL.into_iter().find(|metric| metric.name() == name) + } + + /// Whether this metric's limit can fail the unit at all: `lines` and + /// `params` skip Svelte template units, and `widget_depth` only covers + /// Dart `build` methods. + pub fn applies_to(self, unit: &FunctionMetrics) -> bool { + match self { + Self::Lines | Self::Params => !unit.template, + Self::WidgetDepth => unit.widget, + Self::Complexity | Self::Cognitive | Self::Depth | Self::BoolOps => true, + } + } +} + +impl FunctionMetrics { + pub fn value(&self, metric: Metric) -> usize { + match metric { + Metric::Complexity => self.complexity, + Metric::Cognitive => self.cognitive, + Metric::Depth => self.depth, + Metric::Lines => self.lines, + Metric::Params => self.params, + Metric::BoolOps => self.bool_ops, + Metric::WidgetDepth => self.widget_depth, + } + } +} + +#[cfg(test)] +mod tests { + use std::fs; + + use super::*; + use crate::{MetricValues, load_config}; + + fn unit(template: bool, widget: bool) -> FunctionMetrics { + FunctionMetrics { + function: "f".to_owned(), + line: 1, + end_line: 1, + complexity: 1, + cognitive: 2, + depth: 3, + lines: 4, + params: 5, + bool_ops: 6, + widget_depth: 7, + span: (0, 1), + template, + widget, + } + } + + #[test] + fn every_metric_round_trips_through_config_limits_and_values() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.json"); + let values = MetricValues::from(&unit(false, false)); + let json = serde_json::to_value(&values).unwrap(); + for (index, metric) in Metric::ALL.into_iter().enumerate() { + let name = metric.name(); + assert_eq!(Metric::parse(name), Some(metric)); + let global = 20 + index; + let language = 40 + index; + fs::write( + &path, + format!( + r#"{{"limits":{{"{name}":{global}}},"languages":{{"rust":{{"limits":{{"{name}":{language}}}}}}}}}"# + ), + ) + .unwrap(); + let config = load_config(dir.path(), Some(&path)).unwrap().config; + assert_eq!(config.limits.get(metric), Some(global)); + assert_eq!(config.limits_for("go").get(metric), Some(global)); + assert_eq!(config.limits_for("rust").get(metric), Some(language)); + assert_eq!(json[name], index + 1); + assert_eq!(values.get(name), Some(index + 1)); + } + assert_eq!(Metric::parse("missing"), None); + } + + #[test] + fn applicability_follows_template_and_widget_units() { + let ordinary = unit(false, false); + let template = unit(true, false); + let widget = unit(false, true); + for metric in Metric::ALL { + let lines_or_params = matches!(metric, Metric::Lines | Metric::Params); + let widget_depth = metric == Metric::WidgetDepth; + assert_eq!(metric.applies_to(&ordinary), !widget_depth); + assert_eq!( + metric.applies_to(&template), + !lines_or_params && !widget_depth + ); + assert!(metric.applies_to(&widget)); + } + } + + #[test] + fn names_match_the_default_config_limits() { + let defaults: serde_json::Value = + serde_json::from_str(include_str!("../../../config.default.json")).unwrap(); + let mut keys = defaults["limits"] + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect::>(); + let mut names = Metric::ALL.map(Metric::name).to_vec(); + keys.sort_unstable(); + names.sort_unstable(); + assert_eq!(names, keys); + } +} diff --git a/crates/core/src/scan.rs b/crates/core/src/scan.rs index e7f4fc2..f111459 100644 --- a/crates/core/src/scan.rs +++ b/crates/core/src/scan.rs @@ -9,7 +9,8 @@ use ignore::WalkBuilder; use serde::Serialize; use crate::{ - BaseBlob, ChangedFiles, Config, FunctionMetrics, Language, Limits, LineRange, base_blob, + BaseBlob, ChangedFiles, Config, FunctionMetrics, Language, Limits, LineRange, Metric, + base_blob, baseline::{self, MetricValues, Pairing, Status}, diff::repository_root, load_config, parse_source, @@ -79,6 +80,18 @@ struct FileContext<'a> { test_file: bool, } +impl FileContext<'_> { + fn test_exempt(&self, metric: Metric) -> bool { + self.test_file + && self + .config + .tests + .exempt + .iter() + .any(|item| item == metric.name()) + } +} + pub fn scan(options: &ScanOptions<'_>) -> Result { let roots = scan_roots(options); let match_base = match_base(options, &roots)?; @@ -369,28 +382,17 @@ fn base_units( fn effective_limits(file: &FileContext<'_>, function: &FunctionMetrics) -> Limits { let mut limits = file.config.limits_for(file.language.name()); - for (metric, limit) in [ - ("complexity", &mut limits.complexity), - ("cognitive", &mut limits.cognitive), - ("depth", &mut limits.depth), - ("lines", &mut limits.lines), - ("params", &mut limits.params), - ("bool_ops", &mut limits.bool_ops), - ("widget_depth", &mut limits.widget_depth), - ] { + for metric in Metric::ALL { if !applies(file, function, metric) { - *limit = None; + *limits.get_mut(metric) = None; } } limits } /// Whether a metric's limit can fail this function. -fn applies(file: &FileContext<'_>, function: &FunctionMetrics, metric: &str) -> bool { - let test_exempt = file.test_file && file.config.tests.exempt.iter().any(|item| item == metric); - let template_exempt = function.template && matches!(metric, "lines" | "params"); - let widget_exempt = !function.widget && metric == "widget_depth"; - !(test_exempt || template_exempt || widget_exempt) +fn applies(file: &FileContext<'_>, function: &FunctionMetrics, metric: Metric) -> bool { + !file.test_exempt(metric) && metric.applies_to(function) } fn changed_spans<'a>(path: &Path, changed: Option<&'a ChangedFiles>) -> Option<&'a [LineRange]> { @@ -409,27 +411,19 @@ fn touches(function: &FunctionMetrics, ranges: &[LineRange]) -> bool { fn add_violations(file: &FileContext<'_>, function: &FunctionMetrics, result: &mut ScanResult) { let limits = file.config.limits_for(file.language.name()); - let metrics = [ - ("complexity", function.complexity, limits.complexity), - ("cognitive", function.cognitive, limits.cognitive), - ("depth", function.depth, limits.depth), - ("lines", function.lines, limits.lines), - ("params", function.params, limits.params), - ("bool_ops", function.bool_ops, limits.bool_ops), - ("widget_depth", function.widget_depth, limits.widget_depth), - ]; - for (metric, value, limit) in metrics { - let Some(limit) = limit.filter(|limit| value > *limit) else { + for metric in Metric::ALL { + let value = function.value(metric); + let Some(limit) = limits.get(metric).filter(|limit| value > *limit) else { continue; }; - if file.test_file && file.config.tests.exempt.iter().any(|item| item == metric) { + if file.test_exempt(metric) { continue; } result.violations.push(Violation { file: file.display.to_path_buf(), line: function.line, function: function.function.clone(), - metric: metric.to_owned(), + metric: metric.name().to_owned(), value, limit, baseline: None, From 27b6a9553cfbd57134cf2f95bb369494e3f9ea03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Wed, 30 Sep 2026 14:34:19 -0300 Subject: [PATCH 2/4] test(core): tie Metric::ALL to the metric structs ALL is the one list an exhaustive match cannot check. The test compares it with the serialized keys of Limits, MetricValues, and FunctionMetrics, so a metric field added without an ALL entry fails instead of being skipped. --- crates/core/src/metric.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/core/src/metric.rs b/crates/core/src/metric.rs index 070aed9..e5e2a5f 100644 --- a/crates/core/src/metric.rs +++ b/crates/core/src/metric.rs @@ -138,6 +138,34 @@ mod tests { } } + /// `ALL` is the one list a match cannot check, so tie it to the structs a + /// new metric has to be added to. + #[test] + fn all_covers_every_metric_field() { + fn keys(value: serde_json::Value) -> Vec { + let mut keys = value + .as_object() + .unwrap() + .keys() + .filter(|key| !matches!(key.as_str(), "function" | "line" | "end_line")) + .cloned() + .collect::>(); + keys.sort_unstable(); + keys + } + let unit = unit(false, false); + let limits: crate::Config = + serde_json::from_str(include_str!("../../../config.default.json")).unwrap(); + let mut names = Metric::ALL.map(|metric| metric.name().to_owned()).to_vec(); + names.sort_unstable(); + assert_eq!(keys(serde_json::to_value(&limits.limits).unwrap()), names); + assert_eq!( + keys(serde_json::to_value(MetricValues::from(&unit)).unwrap()), + names + ); + assert_eq!(keys(serde_json::to_value(&unit).unwrap()), names); + } + #[test] fn names_match_the_default_config_limits() { let defaults: serde_json::Value = From e14f76dbaf868b0359b77efebaebb02a3a198bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Wed, 30 Sep 2026 14:39:45 -0300 Subject: [PATCH 3/4] fix(core): emit violations through the registry's applicability add_violations read raw language limits and applied only the test-file exemption, so a restricted metric with a nonzero value on an excluded unit would fail while --base reported its limit as null. It now uses effective_limits, the same path as --base. baseline::status takes a Metric, and the golden projection destructures FunctionMetrics so a new metric field cannot go unchecked. --- crates/core/src/baseline.rs | 17 +++++++----- crates/core/src/scan.rs | 55 +++++++++++++++++++++++++++++++++---- crates/core/tests/golden.rs | 35 +++++++++++++++++------ 3 files changed, 86 insertions(+), 21 deletions(-) diff --git a/crates/core/src/baseline.rs b/crates/core/src/baseline.rs index e1c36f5..04d9ca2 100644 --- a/crates/core/src/baseline.rs +++ b/crates/core/src/baseline.rs @@ -130,11 +130,11 @@ pub fn pair_units(current: &[FunctionMetrics], base: &[FunctionMetrics]) -> Vec< pub fn status( base: Option<&FunctionMetrics>, pairing: Pairing, - metric: &str, + metric: Metric, value: usize, ) -> Status { let base_value = match (pairing, base) { - (Pairing::Paired(_), Some(unit)) => Metric::parse(metric).map(|metric| unit.value(metric)), + (Pairing::Paired(_), Some(unit)) => Some(unit.value(metric)), (Pairing::Unmatched, _) => return Status::Unmatched, _ => None, }; @@ -293,20 +293,23 @@ mod tests { let base = unit("f", (0, 10), 16); let paired = Pairing::Paired(0); assert_eq!( - status(Some(&base), paired, "cognitive", 18), + status(Some(&base), paired, Metric::Cognitive, 18), Status::Worsened ); assert_eq!( - status(Some(&base), paired, "cognitive", 16), + status(Some(&base), paired, Metric::Cognitive, 16), Status::Unchanged ); assert_eq!( - status(Some(&base), paired, "cognitive", 15), + status(Some(&base), paired, Metric::Cognitive, 15), Status::Improved ); - assert_eq!(status(None, Pairing::New, "cognitive", 18), Status::New); assert_eq!( - status(None, Pairing::Unmatched, "lines", 3), + status(None, Pairing::New, Metric::Cognitive, 18), + Status::New + ); + assert_eq!( + status(None, Pairing::Unmatched, Metric::Lines, 3), Status::Unmatched ); assert_eq!(Status::parse("worsened"), Some(Status::Worsened)); diff --git a/crates/core/src/scan.rs b/crates/core/src/scan.rs index f111459..4936a47 100644 --- a/crates/core/src/scan.rs +++ b/crates/core/src/scan.rs @@ -337,7 +337,7 @@ fn compare_with_base( status: baseline::status( base_unit, pairing[index], - &violation.metric, + Metric::parse(&violation.metric).expect("violations carry metric names"), violation.value, ), base_metrics: base_unit.map(MetricValues::from), @@ -410,15 +410,12 @@ fn touches(function: &FunctionMetrics, ranges: &[LineRange]) -> bool { } fn add_violations(file: &FileContext<'_>, function: &FunctionMetrics, result: &mut ScanResult) { - let limits = file.config.limits_for(file.language.name()); + let limits = effective_limits(file, function); for metric in Metric::ALL { let value = function.value(metric); let Some(limit) = limits.get(metric).filter(|limit| value > *limit) else { continue; }; - if file.test_exempt(metric) { - continue; - } result.violations.push(Violation { file: file.display.to_path_buf(), line: function.line, @@ -500,3 +497,51 @@ fn read_reason(error: &std::io::Error) -> String { format!("cannot read: {error}") } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn violations_skip_metrics_that_do_not_apply_to_the_unit() { + let config: Config = + serde_json::from_str(include_str!("../../../config.default.json")).unwrap(); + let file = FileContext { + display: Path::new("page.svelte"), + matched: Path::new("page.svelte"), + language: Language::Svelte, + config: &config, + test_file: false, + }; + let unit = FunctionMetrics { + function: "