also lint for anyhow::Error

Signed-off-by: Bugen Zhao <[email protected]>
risingwavelabs · Feb 20, 2024 · e3d158f · e3d158f
1 parent 7d4d66b
commit e3d158f
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 3 deletions.
diff --git a/lints/src/format_error.rs b/lints/src/format_error.rs
@@ -16,7 +16,7 @@ use clippy_utils::diagnostics::span_lint_and_help;
 use clippy_utils::macros::{
     find_format_arg_expr, find_format_args, is_format_macro, macro_backtrace,
 };
-use clippy_utils::ty::implements_trait;
+use clippy_utils::ty::{implements_trait, match_type};
 use clippy_utils::{
     is_in_cfg_test, is_in_test_function, is_trait_method, match_def_path, match_function_call,
 };
@@ -64,6 +64,7 @@ const TRACING_FIELD_DEBUG: [&str; 3] = ["tracing_core", "field", "debug"];
 const TRACING_FIELD_DISPLAY: [&str; 3] = ["tracing_core", "field", "display"];
 const TRACING_MACROS_EVENT: [&str; 3] = ["tracing", "macros", "event"];
 const ANYHOW_MACROS_ANYHOW: [&str; 3] = ["anyhow", "macros", "anyhow"];
+const ANYHOW_ERROR: [&str; 2] = ["anyhow", "Error"];
 
 impl<'tcx> LateLintPass<'tcx> for FormatError {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
@@ -176,7 +177,10 @@ fn check_arg(cx: &LateContext<'_>, arg_expr: &Expr<'_>, span: Span, help: &str)
 
     let ty = cx.typeck_results().expr_ty(arg_expr).peel_refs();
 
-    if implements_trait(cx, ty, error_trait_id, &[]) {
+    let is_error = || implements_trait(cx, ty, error_trait_id, &[]);
+    let is_anyhow = || match_type(cx, ty, &ANYHOW_ERROR);
+
+    if is_error() || is_anyhow() {
         if let Some(span) = core::iter::successors(Some(span), |s| s.parent_callsite())
             .find(|s| s.can_be_used_for_suggestions())
         {

diff --git a/lints/ui/format_error.rs b/lints/ui/format_error.rs
@@ -55,4 +55,22 @@ fn main() {
     let _ = anyhow!("{:?}", err);
     let _ = anyhow!("some error occurred: {}", err);
     let _ = anyhow!("some error occurred: {:?}", err);
+
+    // `anyhow::Error` does not implement `Error` trait, test the special path here.
+    let make_anyhow_err = || anyhow!("foobar");
+    let anyhow_err = make_anyhow_err();
+
+    let _ = format!("{}", anyhow_err);
+    let _ = format!("{}", &anyhow_err);
+    let _ = format!("{}", &&anyhow_err);
+    let _ = format!("{}", Box::new(&anyhow_err)); // TODO: fail to lint
+
+    tracing::field::display(&anyhow_err);
+    tracing::field::debug(make_anyhow_err());
+
+    let _ = anyhow_err.to_string();
+    let _ = (&&anyhow_err).to_string();
+
+    let _ = anyhow!("{}", anyhow_err);
+    let _ = anyhow!("some error occurred: {:?}", anyhow_err);
 }
diff --git a/lints/ui/format_error.stderr b/lints/ui/format_error.stderr
@@ -272,5 +272,77 @@ LL |     let _ = anyhow!("some error occurred: {:?}", err);
    |
    = help: consider using `anyhow::Error::context`, `anyhow::Context::(with_)context` to attach additional message to the error and make it an error source instead
 
-error: aborting due to 34 previous errors
+error: should not format error directly
+  --> $DIR/format_error.rs:63:27
+   |
+LL |     let _ = format!("{}", anyhow_err);
+   |                           ^^^^^^^^^^
+   |
+   = help: consider importing `thiserror_ext::AsReport` and using `.as_report()` instead
+
+error: should not format error directly
+  --> $DIR/format_error.rs:64:27
+   |
+LL |     let _ = format!("{}", &anyhow_err);
+   |                           ^^^^^^^^^^^
+   |
+   = help: consider importing `thiserror_ext::AsReport` and using `.as_report()` instead
+
+error: should not format error directly
+  --> $DIR/format_error.rs:65:27
+   |
+LL |     let _ = format!("{}", &&anyhow_err);
+   |                           ^^^^^^^^^^^^
+   |
+   = help: consider importing `thiserror_ext::AsReport` and using `.as_report()` instead
+
+error: should not format error directly
+  --> $DIR/format_error.rs:68:29
+   |
+LL |     tracing::field::display(&anyhow_err);
+   |                             ^^^^^^^^^^^
+   |
+   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field with `error = %<error>.as_report()` instead
+
+error: should not format error directly
+  --> $DIR/format_error.rs:69:27
+   |
+LL |     tracing::field::debug(make_anyhow_err());
+   |                           ^^^^^^^^^^^^^^^^^
+   |
+   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field with `error = %<error>.as_report()` instead
+
+error: should not format error directly
+  --> $DIR/format_error.rs:71:24
+   |
+LL |     let _ = anyhow_err.to_string();
+   |                        ^^^^^^^^^^^
+   |
+   = help: consider importing `thiserror_ext::AsReport` and using `.to_report_string()` instead
+
+error: should not format error directly
+  --> $DIR/format_error.rs:72:28
+   |
+LL |     let _ = (&&anyhow_err).to_string();
+   |                            ^^^^^^^^^^^
+   |
+   = help: consider importing `thiserror_ext::AsReport` and using `.to_report_string()` instead
+
+error: should not format error directly
+  --> $DIR/format_error.rs:74:27
+   |
+LL |     let _ = anyhow!("{}", anyhow_err);
+   |                           ^^^^^^^^^^
+   |
+   = help: consider directly wrapping the error with `anyhow::anyhow!(..)` instead of formatting it
+
+error: should not format error directly
+  --> $DIR/format_error.rs:75:50
+   |
+LL |     let _ = anyhow!("some error occurred: {:?}", anyhow_err);
+   |                                                  ^^^^^^^^^^
+   |
+   = help: consider using `anyhow::Error::context`, `anyhow::Context::(with_)context` to attach additional message to the error and make it an error source instead
+
+error: aborting due to 43 previous errors