use crate::core::{DebtItem, DebtType, Priority};
use crate::debt::suppression::SuppressionContext;
use std::path::Path;
use syn::spanned::Spanned;
use syn::visit::Visit;
use syn::{Expr, ExprIf, ExprLet, ExprMatch, ExprMethodCall, File, ItemFn, Pat, Stmt};
pub struct ErrorSwallowingDetector<'a> {
items: Vec<DebtItem>,
current_file: &'a Path,
suppression: Option<&'a SuppressionContext>,
in_test_function: bool,
}
impl<'a> ErrorSwallowingDetector<'a> {
pub fn new(file_path: &'a Path, suppression: Option<&'a SuppressionContext>) -> Self {
Self {
items: Vec::new(),
current_file: file_path,
suppression,
in_test_function: false,
}
}
pub fn detect(mut self, file: &File) -> Vec<DebtItem> {
self.visit_file(file);
self.items
}
fn get_line_number(&self, span: proc_macro2::Span) -> usize {
span.start().line
}
fn add_debt_item(&mut self, line: usize, pattern: ErrorSwallowingPattern, context: &str) {
let debt_type = DebtType::ErrorSwallowing {
pattern: pattern.to_string(),
context: Some(context.to_string()),
};
if let Some(checker) = self.suppression {
if checker.is_suppressed(line, &debt_type) {
return;
}
}
let priority = self.determine_priority(&pattern);
let message = format!("{}: {}", pattern.description(), pattern.remediation());
self.items.push(DebtItem {
id: format!("error-swallow-{}-{}", self.current_file.display(), line),
debt_type,
priority,
file: self.current_file.to_path_buf(),
line,
column: None,
message,
context: Some(context.to_string()),
});
}
fn determine_priority(&self, pattern: &ErrorSwallowingPattern) -> Priority {
if self.in_test_function {
return Priority::Low;
}
match pattern {
ErrorSwallowingPattern::IfLetOkNoElse | ErrorSwallowingPattern::IfLetOkEmptyElse => {
Priority::Medium
}
ErrorSwallowingPattern::LetUnderscoreResult => Priority::High,
ErrorSwallowingPattern::OkMethodDiscard => Priority::Medium,
ErrorSwallowingPattern::MatchIgnoredErr => Priority::Medium,
ErrorSwallowingPattern::UnwrapOrNoLog
| ErrorSwallowingPattern::UnwrapOrDefaultNoLog => Priority::Low,
}
}
fn check_if_let_ok(&mut self, expr_if: &ExprIf) {
if !Self::is_if_let_ok_pattern(&expr_if.cond) {
return;
}
let line = self.get_line_number(expr_if.if_token.span);
if let Some((pattern, description)) = Self::classify_error_handling(&expr_if.else_branch) {
self.add_debt_item(line, pattern, description);
}
}
fn is_if_let_ok_pattern(cond: &Expr) -> bool {
match cond {
Expr::Let(ExprLet { pat, .. }) => Self::is_ok_pattern(pat),
_ => false,
}
}
fn is_ok_pattern(pat: &Pat) -> bool {
match pat {
Pat::TupleStruct(pat_tuple) => pat_tuple
.path
.get_ident()
.is_some_and(|ident| ident == "Ok"),
_ => false,
}
}
fn classify_error_handling(
else_branch: &Option<(syn::token::Else, Box<Expr>)>,
) -> Option<(ErrorSwallowingPattern, &'static str)> {
match else_branch {
Some((_, expr)) if is_empty_block(expr) => Some((
ErrorSwallowingPattern::IfLetOkEmptyElse,
"Empty else branch for Result handling",
)),
None => Some((
ErrorSwallowingPattern::IfLetOkNoElse,
"No error handling for Result",
)),
_ => None, }
}
fn check_let_underscore(&mut self, stmt: &Stmt) {
if let Stmt::Local(local) = stmt {
if let Pat::Wild(wild) = &local.pat {
if let Some(init) = &local.init {
if is_result_type(&init.expr) {
let line = self.get_line_number(wild.underscore_token.span);
self.add_debt_item(
line,
ErrorSwallowingPattern::LetUnderscoreResult,
"Result discarded with let _ pattern",
);
}
}
}
}
}
fn check_ok_method(&mut self, method_call: &ExprMethodCall) {
let ident = &method_call.method;
if ident == "ok" && method_call.args.is_empty() {
let line = self.get_line_number(method_call.method.span());
self.add_debt_item(
line,
ErrorSwallowingPattern::OkMethodDiscard,
"Error information discarded with .ok()",
);
}
}
fn check_unwrap_or_methods(&mut self, method_call: &ExprMethodCall) {
let ident = &method_call.method;
let method_name = ident.to_string();
if method_name == "unwrap_or" || method_name == "unwrap_or_default" {
let line = self.get_line_number(method_call.method.span());
let pattern = if method_name == "unwrap_or" {
ErrorSwallowingPattern::UnwrapOrNoLog
} else {
ErrorSwallowingPattern::UnwrapOrDefaultNoLog
};
self.add_debt_item(
line,
pattern,
&format!("Error swallowed by {} without logging", method_name),
);
}
}
fn check_match_expr(&mut self, expr_match: &ExprMatch) {
for arm in &expr_match.arms {
if let Pat::TupleStruct(pat_tuple) = &arm.pat {
if let Some(path) = pat_tuple.path.get_ident() {
if path == "Err" {
if is_empty_expr(&arm.body) {
let line = self.get_line_number(pat_tuple.path.span());
self.add_debt_item(
line,
ErrorSwallowingPattern::MatchIgnoredErr,
"Error variant ignored in match expression",
);
}
}
}
}
}
}
}
impl<'a> Visit<'_> for ErrorSwallowingDetector<'a> {
fn visit_item_fn(&mut self, node: &ItemFn) {
let was_in_test = self.in_test_function;
self.in_test_function = node
.attrs
.iter()
.any(|attr| attr.path().get_ident().map(|i| i.to_string()).as_deref() == Some("test"));
syn::visit::visit_item_fn(self, node);
self.in_test_function = was_in_test;
}
fn visit_expr_if(&mut self, node: &ExprIf) {
self.check_if_let_ok(node);
syn::visit::visit_expr_if(self, node);
}
fn visit_stmt(&mut self, node: &Stmt) {
self.check_let_underscore(node);
syn::visit::visit_stmt(self, node);
}
fn visit_expr_method_call(&mut self, node: &ExprMethodCall) {
self.check_ok_method(node);
self.check_unwrap_or_methods(node);
syn::visit::visit_expr_method_call(self, node);
}
fn visit_expr_match(&mut self, node: &ExprMatch) {
self.check_match_expr(node);
syn::visit::visit_expr_match(self, node);
}
}
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum ErrorSwallowingPattern {
IfLetOkNoElse,
IfLetOkEmptyElse,
LetUnderscoreResult,
OkMethodDiscard,
MatchIgnoredErr,
UnwrapOrNoLog,
UnwrapOrDefaultNoLog,
}
impl ErrorSwallowingPattern {
fn description(&self) -> &'static str {
match self {
Self::IfLetOkNoElse => "if let Ok(...) without else branch",
Self::IfLetOkEmptyElse => "if let Ok(...) with empty else branch",
Self::LetUnderscoreResult => "let _ = discarding Result",
Self::OkMethodDiscard => ".ok() discarding error information",
Self::MatchIgnoredErr => "match with ignored Err variant",
Self::UnwrapOrNoLog => "unwrap_or without error logging",
Self::UnwrapOrDefaultNoLog => "unwrap_or_default without error logging",
}
}
fn remediation(&self) -> &'static str {
match self {
Self::IfLetOkNoElse | Self::IfLetOkEmptyElse => {
"Use ? operator or handle error case explicitly"
}
Self::LetUnderscoreResult => "Add error handling or logging before discarding",
Self::OkMethodDiscard => "Use map_err to log before converting to Option",
Self::MatchIgnoredErr => "Handle or log the error in the Err arm",
Self::UnwrapOrNoLog | Self::UnwrapOrDefaultNoLog => {
"Use unwrap_or_else with error logging"
}
}
}
}
impl std::fmt::Display for ErrorSwallowingPattern {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.description())
}
}
fn is_empty_block(expr: &Expr) -> bool {
match expr {
Expr::Block(block) => block.block.stmts.is_empty(),
_ => false,
}
}
fn is_empty_expr(expr: &Expr) -> bool {
match expr {
Expr::Block(block) => block.block.stmts.is_empty(),
Expr::Tuple(tuple) => tuple.elems.is_empty(),
_ => false,
}
}
fn is_result_type(expr: &Expr) -> bool {
match expr {
Expr::Call(_call) => {
true }
Expr::MethodCall(_method) => {
true }
Expr::Try(_) => true,
_ => false,
}
}
pub fn detect_error_swallowing(
file: &File,
file_path: &Path,
suppression: Option<&SuppressionContext>,
) -> Vec<DebtItem> {
let detector = ErrorSwallowingDetector::new(file_path, suppression);
detector.detect(file)
}
pub fn detect_error_swallowing_in_function(block: &syn::Block) -> (u32, Vec<String>) {
let mut detector = FunctionErrorSwallowingDetector::new();
detector.visit_block(block);
(detector.count, detector.patterns)
}
struct FunctionErrorSwallowingDetector {
count: u32,
patterns: Vec<String>,
}
impl FunctionErrorSwallowingDetector {
fn new() -> Self {
Self {
count: 0,
patterns: Vec::new(),
}
}
fn add_pattern(&mut self, pattern: ErrorSwallowingPattern) {
self.count += 1;
let desc = pattern.description().to_string();
if !self.patterns.contains(&desc) {
self.patterns.push(desc);
}
}
fn check_if_let_ok(&mut self, expr_if: &ExprIf) {
if !ErrorSwallowingDetector::is_if_let_ok_pattern(&expr_if.cond) {
return;
}
if let Some((pattern, _)) =
ErrorSwallowingDetector::classify_error_handling(&expr_if.else_branch)
{
self.add_pattern(pattern);
}
}
fn check_let_underscore(&mut self, stmt: &Stmt) {
if let Stmt::Local(local) = stmt {
if let Pat::Wild(_) = &local.pat {
if let Some(init) = &local.init {
if is_result_type(&init.expr) {
self.add_pattern(ErrorSwallowingPattern::LetUnderscoreResult);
}
}
}
}
}
fn check_ok_method(&mut self, method_call: &ExprMethodCall) {
let ident = &method_call.method;
if ident == "ok" && method_call.args.is_empty() {
self.add_pattern(ErrorSwallowingPattern::OkMethodDiscard);
}
}
fn check_unwrap_or_methods(&mut self, method_call: &ExprMethodCall) {
let ident = &method_call.method;
let method_name = ident.to_string();
if method_name == "unwrap_or" {
self.add_pattern(ErrorSwallowingPattern::UnwrapOrNoLog);
} else if method_name == "unwrap_or_default" {
self.add_pattern(ErrorSwallowingPattern::UnwrapOrDefaultNoLog);
}
}
fn check_match_expr(&mut self, expr_match: &ExprMatch) {
for arm in &expr_match.arms {
if let Pat::TupleStruct(pat_tuple) = &arm.pat {
if let Some(path) = pat_tuple.path.get_ident() {
if path == "Err" && is_empty_expr(&arm.body) {
self.add_pattern(ErrorSwallowingPattern::MatchIgnoredErr);
}
}
}
}
}
}
impl Visit<'_> for FunctionErrorSwallowingDetector {
fn visit_expr_if(&mut self, node: &ExprIf) {
self.check_if_let_ok(node);
syn::visit::visit_expr_if(self, node);
}
fn visit_stmt(&mut self, node: &Stmt) {
self.check_let_underscore(node);
syn::visit::visit_stmt(self, node);
}
fn visit_expr_method_call(&mut self, node: &ExprMethodCall) {
self.check_ok_method(node);
self.check_unwrap_or_methods(node);
syn::visit::visit_expr_method_call(self, node);
}
fn visit_expr_match(&mut self, node: &ExprMatch) {
self.check_match_expr(node);
syn::visit::visit_expr_match(self, node);
}
}
#[cfg(test)]
mod tests {
use super::*;
use syn::{parse_quote, parse_str, Pat};
#[test]
fn test_detect_error_swallowing_in_function() {
let block: syn::Block = parse_quote! {{
if let Ok(value) = some_function() {
println!("{}", value);
}
let _ = another_result();
}};
let (count, patterns) = detect_error_swallowing_in_function(&block);
assert_eq!(count, 2, "Should detect 2 error swallowing patterns");
assert!(
patterns.iter().any(|p| p.contains("if let Ok")),
"Should detect if let Ok pattern"
);
assert!(
patterns.iter().any(|p| p.contains("let _")),
"Should detect let _ pattern"
);
}
#[test]
fn test_detect_error_swallowing_in_function_no_issues() {
let block: syn::Block = parse_quote! {{
let value = some_function()?;
println!("{}", value);
}};
let (count, patterns) = detect_error_swallowing_in_function(&block);
assert_eq!(count, 0, "Should detect 0 error swallowing patterns");
assert!(patterns.is_empty(), "Should have no patterns");
}
#[test]
fn test_detect_error_swallowing_in_function_multiple_same_type() {
let block: syn::Block = parse_quote! {{
if let Ok(a) = func_a() { use_a(a); }
if let Ok(b) = func_b() { use_b(b); }
if let Ok(c) = func_c() { use_c(c); }
}};
let (count, patterns) = detect_error_swallowing_in_function(&block);
assert_eq!(count, 3, "Should count all 3 occurrences");
assert_eq!(patterns.len(), 1, "Should have only 1 unique pattern type");
}
#[test]
fn test_if_let_ok_no_else() {
let code = r#"
fn example() {
if let Ok(value) = some_function() {
println!("{}", value);
}
}
"#;
let file = parse_str::<File>(code).expect("Failed to parse test code");
let items = detect_error_swallowing(&file, Path::new("test.rs"), None);
assert_eq!(items.len(), 1);
assert!(matches!(
items[0].debt_type,
DebtType::ErrorSwallowing { .. }
));
assert!(items[0].message.contains("if let Ok"));
assert_eq!(items[0].line, 3, "Expected detection at line 3");
}
#[test]
fn test_let_underscore_result() {
let code = r#"
fn example() {
let _ = function_returning_result();
}
"#;
let file = parse_str::<File>(code).expect("Failed to parse test code");
let items = detect_error_swallowing(&file, Path::new("test.rs"), None);
assert!(!items.is_empty());
assert!(matches!(
items[0].debt_type,
DebtType::ErrorSwallowing { .. }
));
}
#[test]
fn test_ok_method_discard() {
let code = r#"
fn example() {
some_result.ok();
}
"#;
let file = parse_str::<File>(code).expect("Failed to parse test code");
let items = detect_error_swallowing(&file, Path::new("test.rs"), None);
assert!(!items.is_empty());
assert!(matches!(
items[0].debt_type,
DebtType::ErrorSwallowing { .. }
));
assert!(items[0].message.contains(".ok()"));
}
#[test]
fn test_match_ignored_err() {
let code = r#"
fn example() {
match some_result {
Ok(v) => println!("{}", v),
Err(_) => {},
}
}
"#;
let file = parse_str::<File>(code).expect("Failed to parse test code");
let items = detect_error_swallowing(&file, Path::new("test.rs"), None);
assert!(!items.is_empty());
assert!(matches!(
items[0].debt_type,
DebtType::ErrorSwallowing { .. }
));
assert!(items[0].message.contains("match"));
}
#[test]
fn test_unwrap_or_no_log() {
let code = r#"
fn example() {
let value = some_result.unwrap_or(0);
}
"#;
let file = parse_str::<File>(code).expect("Failed to parse test code");
let items = detect_error_swallowing(&file, Path::new("test.rs"), None);
assert!(!items.is_empty());
assert!(matches!(
items[0].debt_type,
DebtType::ErrorSwallowing { .. }
));
assert!(items[0].message.contains("unwrap_or"));
}
#[test]
fn test_is_if_let_ok_pattern() {
let code = "if let Ok(value) = some_function() { }";
let expr: syn::Expr = parse_str(code).expect("Failed to parse");
if let syn::Expr::If(expr_if) = expr {
assert!(
ErrorSwallowingDetector::is_if_let_ok_pattern(&expr_if.cond),
"Should recognize if let Ok pattern"
);
}
let code = "if true { }";
let expr: syn::Expr = parse_str(code).expect("Failed to parse");
if let syn::Expr::If(expr_if) = expr {
assert!(
!ErrorSwallowingDetector::is_if_let_ok_pattern(&expr_if.cond),
"Should not recognize regular if as Ok pattern"
);
}
let code = "if let Err(e) = some_function() { }";
let expr: syn::Expr = parse_str(code).expect("Failed to parse");
if let syn::Expr::If(expr_if) = expr {
assert!(
!ErrorSwallowingDetector::is_if_let_ok_pattern(&expr_if.cond),
"Should not recognize if let Err as Ok pattern"
);
}
}
#[test]
fn test_is_ok_pattern() {
let pat: Pat = parse_quote!(Ok(value));
assert!(
ErrorSwallowingDetector::is_ok_pattern(&pat),
"Should recognize Ok pattern"
);
let pat: Pat = parse_quote!(Err(e));
assert!(
!ErrorSwallowingDetector::is_ok_pattern(&pat),
"Should not recognize Err as Ok pattern"
);
let pat: Pat = parse_quote!(Some(x));
assert!(
!ErrorSwallowingDetector::is_ok_pattern(&pat),
"Should not recognize Some as Ok pattern"
);
let pat: Pat = parse_quote!(_);
assert!(
!ErrorSwallowingDetector::is_ok_pattern(&pat),
"Should not recognize wildcard as Ok pattern"
);
}
#[test]
fn test_classify_error_handling() {
use syn::{parse_quote, Expr};
let else_branch = None;
let result = ErrorSwallowingDetector::classify_error_handling(&else_branch);
assert!(result.is_some());
let (pattern, desc) = result.unwrap();
assert_eq!(pattern, ErrorSwallowingPattern::IfLetOkNoElse);
assert_eq!(desc, "No error handling for Result");
let empty_block: Expr = parse_quote! { {} };
let else_branch = Some((syn::token::Else::default(), Box::new(empty_block)));
let result = ErrorSwallowingDetector::classify_error_handling(&else_branch);
assert!(result.is_some());
let (pattern, desc) = result.unwrap();
assert_eq!(pattern, ErrorSwallowingPattern::IfLetOkEmptyElse);
assert_eq!(desc, "Empty else branch for Result handling");
let non_empty_block: Expr = parse_quote! { { println!("error"); } };
let else_branch = Some((syn::token::Else::default(), Box::new(non_empty_block)));
let result = ErrorSwallowingDetector::classify_error_handling(&else_branch);
assert!(
result.is_none(),
"Should not flag debt for proper error handling"
);
}
#[test]
fn test_if_let_ok_with_empty_else() {
let code = r#"
fn example() {
if let Ok(value) = some_function() {
println!("{}", value);
} else {
// Empty else block
}
}
"#;
let file = parse_str::<File>(code).expect("Failed to parse test code");
let items = detect_error_swallowing(&file, Path::new("test.rs"), None);
assert_eq!(items.len(), 1);
assert!(matches!(
items[0].debt_type,
DebtType::ErrorSwallowing { .. }
));
assert!(items[0].message.contains("empty else branch"));
}
#[test]
fn test_if_let_ok_with_proper_handling() {
let code = r#"
fn example() {
if let Ok(value) = some_function() {
println!("{}", value);
} else {
eprintln!("Error occurred");
}
}
"#;
let file = parse_str::<File>(code).expect("Failed to parse test code");
let items = detect_error_swallowing(&file, Path::new("test.rs"), None);
assert_eq!(
items.len(),
0,
"Should not detect debt for proper error handling"
);
}
#[test]
fn test_lower_priority_in_tests() {
let code = r#"
#[test]
fn test_example() {
if let Ok(value) = some_function() {
assert_eq!(value, 42);
}
}
"#;
let file = parse_str::<File>(code).expect("Failed to parse test code");
let items = detect_error_swallowing(&file, Path::new("test.rs"), None);
assert_eq!(items.len(), 1);
assert_eq!(items[0].priority, Priority::Low);
}
#[test]
fn test_check_match_expr_comprehensive() {
let code = r#"
fn example() {
match complex_operation() {
Ok(data) => process_data(data),
Err(_) => (), // Empty error handling
}
match another_operation() {
Ok(result) => use_result(result),
Err(e) => log::error!("Failed: {}", e), // Proper error handling
}
}
"#;
let file = parse_str::<File>(code).expect("Failed to parse test code");
let items = detect_error_swallowing(&file, Path::new("test.rs"), None);
assert_eq!(items.len(), 1);
assert!(items[0].message.contains("match"));
assert!(items[0].message.contains("ignored"));
}
#[test]
fn test_unwrap_or_default_detection() {
let code = r#"
fn example() {
let value1 = risky_operation().unwrap_or(42);
let value2 = another_operation().unwrap_or_default();
let value3 = safe_operation().unwrap_or_else(|e| {
log::warn!("Using default due to error: {}", e);
0
});
}
"#;
let file = parse_str::<File>(code).expect("Failed to parse test code");
let items = detect_error_swallowing(&file, Path::new("test.rs"), None);
assert_eq!(items.len(), 2);
assert!(items.iter().any(|item| item.message.contains("unwrap_or")));
assert!(items
.iter()
.any(|item| item.message.contains("unwrap_or_default")));
}
#[test]
fn test_is_result_type_heuristics() {
let function_call: syn::Expr = syn::parse_quote!(some_function());
assert!(is_result_type(&function_call));
let method_call: syn::Expr = syn::parse_quote!(obj.method());
assert!(is_result_type(&method_call));
let try_expr: syn::Expr = syn::parse_quote!(operation()?);
assert!(is_result_type(&try_expr));
let literal: syn::Expr = syn::parse_quote!(42);
assert!(!is_result_type(&literal));
}
#[test]
fn test_pattern_descriptions_and_remediations() {
use ErrorSwallowingPattern::*;
assert_eq!(
IfLetOkNoElse.description(),
"if let Ok(...) without else branch"
);
assert_eq!(
IfLetOkEmptyElse.description(),
"if let Ok(...) with empty else branch"
);
assert_eq!(
LetUnderscoreResult.description(),
"let _ = discarding Result"
);
assert_eq!(
OkMethodDiscard.description(),
".ok() discarding error information"
);
assert_eq!(
MatchIgnoredErr.description(),
"match with ignored Err variant"
);
assert_eq!(
UnwrapOrNoLog.description(),
"unwrap_or without error logging"
);
assert_eq!(
UnwrapOrDefaultNoLog.description(),
"unwrap_or_default without error logging"
);
assert!(IfLetOkNoElse.remediation().contains("? operator"));
assert!(LetUnderscoreResult.remediation().contains("error handling"));
assert!(OkMethodDiscard.remediation().contains("map_err"));
assert!(MatchIgnoredErr.remediation().contains("Handle or log"));
assert!(UnwrapOrNoLog.remediation().contains("unwrap_or_else"));
}
#[test]
fn test_priority_determination() {
let detector = ErrorSwallowingDetector::new(Path::new("test.rs"), None);
assert_eq!(
detector.determine_priority(&ErrorSwallowingPattern::LetUnderscoreResult),
Priority::High
);
assert_eq!(
detector.determine_priority(&ErrorSwallowingPattern::IfLetOkNoElse),
Priority::Medium
);
assert_eq!(
detector.determine_priority(&ErrorSwallowingPattern::UnwrapOrNoLog),
Priority::Low
);
}
#[test]
fn test_nested_error_swallowing() {
let code = r#"
fn complex_example() {
if condition {
if let Ok(inner) = nested_operation() {
// Process inner
let _ = another_operation();
}
}
match outer_operation() {
Ok(data) => {
if let Ok(processed) = process(data) {
// Use processed
}
},
Err(_) => {},
}
}
"#;
let file = parse_str::<File>(code).expect("Failed to parse test code");
let items = detect_error_swallowing(&file, Path::new("test.rs"), None);
assert!(items.len() >= 3);
assert!(items.iter().any(|item| item.message.contains("if let Ok")));
assert!(items.iter().any(|item| item.message.contains("let _")));
assert!(items.iter().any(|item| item.message.contains("match")));
}
}