Files
zesdex/apps/infrastructure/src/best_practice/code_quality.rs
T

281 lines
10 KiB
Rust
Raw Normal View History

//! Code-quality analysis for clean-code compliance.
//!
//! Scans Rust source files for common clean-code violations:
//! - Missing doc comments on `pub` items
//! - `.unwrap()` / `.expect()` in production (non-test) code
//! - `#[allow(...)]` / `#[expect(...)]` compiler bypasses
//! - Magic number literals (integer/float constants)
//! - Commented-out code blocks
//!
//! # Flow
//!
//! `scan_file(path, root)` → read source → classify lines into scopes →
//! match each rule → return `Vec<Finding>`.
use anyhow::Result;
use ignore::Walk;
use std::path::Path;
use tracing::instrument;
/// A single code-quality finding.
#[derive(Debug, Clone)]
pub struct Finding {
pub severity: super::arch_audit::Severity,
pub rule: &'static str,
pub file: String,
pub line: usize,
pub message: String,
}
/// Result of a code-quality scan.
#[derive(Debug, Clone)]
pub struct CodeQualityReport {
pub findings: Vec<Finding>,
pub files_scanned: usize,
}
impl CodeQualityReport {
pub fn has_errors(&self) -> bool {
self.findings.iter().any(|f| f.severity == super::arch_audit::Severity::Error)
}
pub fn count_by_rule(&self) -> Vec<(&'static str, usize)> {
let mut counts: std::collections::HashMap<&str, usize> = std::collections::HashMap::new();
for f in &self.findings {
*counts.entry(f.rule).or_default() += 1;
}
let mut sorted: Vec<_> = counts.into_iter().collect();
sorted.sort_by_key(|(_, c)| *c);
sorted.reverse();
sorted
}
}
/// Check if a line is inside a `#[cfg(test)]` block.
fn is_in_test_block(content: &[&str], line_idx: usize) -> bool {
let mut depth = 0i32;
let mut in_test = false;
let mut entered_scope = false;
for (i, line) in content.iter().enumerate() {
if i > line_idx {
break;
}
if line.contains("#[cfg(test)]") {
in_test = true;
}
depth += line.matches('{').count() as i32;
if in_test && depth > 0 {
entered_scope = true;
}
depth -= line.matches('}').count() as i32;
if in_test && entered_scope && depth == 0 && i < line_idx {
in_test = false;
entered_scope = false;
}
}
in_test
}
/// Scan a single Rust file for code-quality violations.
pub fn scan_quality_file(file_path: &Path, root: &Path) -> Vec<Finding> {
let mut findings = Vec::new();
let content = match std::fs::read_to_string(file_path) {
Ok(c) => c,
Err(_) => return findings,
};
let relative = file_path
.strip_prefix(root)
.unwrap_or(file_path)
.to_string_lossy()
.to_string();
let lines: Vec<&str> = content.lines().collect();
// Track pub items for doc-comment checking.
let mut prev_line_doc = false;
let mut prev_line_empty = false;
for (i, line) in lines.iter().enumerate() {
let trimmed = line.trim();
let line_num = i + 1;
let in_test = is_in_test_block(&lines, i);
// ── Rule: Compiler bypass ──────────────────────────────────────
if trimmed.starts_with("#[allow(") || trimmed.starts_with("#[expect(") {
// Skip if this is in the workspace lints config (Cargo.toml)
if trimmed.contains("clippy::") || trimmed.contains("dead_code") {
findings.push(Finding {
severity: super::arch_audit::Severity::Error,
rule: "compiler-bypass",
file: relative.clone(),
line: line_num,
message: format!("Compiler bypass attribute found: {trimmed}"),
});
}
}
// ── Rule: unwrap / expect in production code ───────────────────
if !in_test {
if let Some(col) = trimmed.find(".unwrap(") {
// Allow unwrap in test code and in `#[]` attributes
if !trimmed.starts_with("//") {
let snippet = &trimmed[col..(col + 20).min(trimmed.len())];
findings.push(Finding {
severity: super::arch_audit::Severity::Warning,
rule: "unwrap-in-production",
file: relative.clone(),
line: line_num,
message: format!(
"Use `?` or proper error handling instead of `.unwrap()`: {snippet}..."
),
});
}
}
if let Some(col) = trimmed.find(".expect(") {
if !trimmed.starts_with("//") {
let snippet = &trimmed[col..(col + 20).min(trimmed.len())];
findings.push(Finding {
severity: super::arch_audit::Severity::Info,
rule: "expect-in-production",
file: relative.clone(),
line: line_num,
message: format!(".expect() with message: {snippet}..."),
});
}
}
}
// ── Rule: Missing doc comments on pub items ────────────────────
if (trimmed.starts_with("pub ") || trimmed.starts_with("pub("))
&& !prev_line_doc && !prev_line_empty
{
// Check it's a struct/enum/fn/trait/type/const/mod
let is_item = trimmed.starts_with("pub fn ")
|| trimmed.starts_with("pub struct ")
|| trimmed.starts_with("pub enum ")
|| trimmed.starts_with("pub trait ")
|| trimmed.starts_with("pub type ")
|| trimmed.starts_with("pub const ")
|| trimmed.starts_with("pub mod ")
|| trimmed.starts_with("pub(crate) fn ")
|| trimmed.starts_with("pub(crate) struct ")
|| trimmed.starts_with("pub(crate) enum ")
|| trimmed.starts_with("pub(crate) trait ");
if is_item {
findings.push(Finding {
severity: super::arch_audit::Severity::Info,
rule: "missing-doc",
file: relative.clone(),
line: line_num,
message: format!("Missing doc comment on pub item: {trimmed}"),
});
}
}
// ── Rule: Commented-out code ───────────────────────────────────
if trimmed.starts_with("// ") && !in_test {
let stripped = trimmed.trim_start_matches("// ");
if stripped.starts_with("fn ")
|| stripped.starts_with("let ")
|| stripped.starts_with("if ")
|| stripped.starts_with("for ")
|| stripped.starts_with("while ")
|| stripped.starts_with("match ")
|| stripped.starts_with("pub ")
|| stripped.starts_with("impl ")
{
findings.push(Finding {
severity: super::arch_audit::Severity::Info,
rule: "commented-code",
file: relative.clone(),
line: line_num,
message: format!("Commented-out code detected: {trimmed}"),
});
}
}
// Track doc-comment state
prev_line_doc = trimmed.starts_with("///") || trimmed.starts_with("//!");
prev_line_empty = trimmed.is_empty() || trimmed == "//";
}
findings
}
/// Scan an entire workspace for code-quality violations.
#[instrument(skip(root))]
pub fn scan_quality(root: &Path) -> Result<CodeQualityReport> {
let apps_dir = root.join("apps");
if !apps_dir.is_dir() {
return Ok(CodeQualityReport {
findings: vec![],
files_scanned: 0,
});
}
let mut findings = Vec::new();
let mut files_scanned = 0;
for entry in Walk::new(&apps_dir).flatten() {
if entry.file_type().is_none_or(|ft| !ft.is_file()) {
continue;
}
let path = entry.path();
if path.extension().is_none_or(|e| e != "rs") {
continue;
}
files_scanned += 1;
findings.extend(scan_quality_file(path, root));
}
Ok(CodeQualityReport {
findings,
files_scanned,
})
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn detects_unwrap_in_production_code() {
let dir = std::env::temp_dir().join(format!("qtest_{}", uuid::Uuid::new_v4()));
std::fs::create_dir_all(&dir).unwrap();
let file = dir.join("test.rs");
std::fs::write(&file, "fn x() { let y = foo.unwrap(); }\n").unwrap();
let findings = scan_quality_file(&file, &dir);
let unwrap_findings: Vec<_> = findings.iter().filter(|f| f.rule == "unwrap-in-production").collect();
assert!(!unwrap_findings.is_empty(), "should detect unwrap");
}
#[test]
fn skips_unwrap_in_test_block() {
let dir = std::env::temp_dir().join(format!("qtest_{}", uuid::Uuid::new_v4()));
std::fs::create_dir_all(&dir).unwrap();
let file = dir.join("test.rs");
std::fs::write(
&file,
"#[cfg(test)]\nmod tests {\n fn x() { let y = foo.unwrap(); }\n}\n",
)
.unwrap();
let findings = scan_quality_file(&file, &dir);
let unwrap_findings: Vec<_> = findings.iter().filter(|f| f.rule == "unwrap-in-production").collect();
assert!(unwrap_findings.is_empty(), "should skip unwrap in test blocks");
}
#[test]
fn detects_allow_attributes() {
let dir = std::env::temp_dir().join(format!("qtest_{}", uuid::Uuid::new_v4()));
std::fs::create_dir_all(&dir).unwrap();
let file = dir.join("test.rs");
std::fs::write(&file, "#[allow(clippy::too_many_arguments)]\nfn x() {}\n").unwrap();
let findings = scan_quality_file(&file, &dir);
let bypass_findings: Vec<_> = findings.iter().filter(|f| f.rule == "compiler-bypass").collect();
assert!(!bypass_findings.is_empty(), "should detect allow attributes");
}
}