Skip to content

Commit

Permalink
feat(lints): lint on formatting Report in anyhow construction
Browse files Browse the repository at this point in the history
Signed-off-by: Bugen Zhao <[email protected]>
  • Loading branch information
BugenZhao committed Mar 5, 2024
1 parent 71504d7 commit 3398990
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 2 deletions.
23 changes: 23 additions & 0 deletions lints/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions lints/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ dylint_testing = "2.6.0"

# UI test dependencies
anyhow = "1"
thiserror-ext = "0.1"
tracing = "0.1"

[package.metadata.rust-analyzer]
Expand Down
19 changes: 18 additions & 1 deletion lints/src/format_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ 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"];
const THISERROR_EXT_REPORT_REPORT: [&str; 3] = ["thiserror_ext", "report", "Report"];

impl<'tcx> LateLintPass<'tcx> for FormatError {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
Expand Down Expand Up @@ -147,6 +148,7 @@ fn check_fmt_arg_in_anyhow_error(cx: &LateContext<'_>, arg_expr: &Expr<'_>) {
(
"consider directly wrapping the error with `anyhow::anyhow!(..)` instead of formatting it",
"consider removing the redundant wrapping of `anyhow::anyhow!(..)`",
"consider directly wrapping the error with `anyhow::anyhow!(..)` instead of formatting its report",
),
);
}
Expand All @@ -159,6 +161,8 @@ fn check_fmt_arg_in_anyhow_context(cx: &LateContext<'_>, arg_expr: &Expr<'_>) {
"consider using `anyhow::Context::(with_)context` to \
attach additional message to the error and make it an error source instead",
"consider using `.context(..)` to \
attach additional message to the error and make it an error source instead",
"consider using `anyhow::Context::(with_)context` to \
attach additional message to the error and make it an error source instead",
),
);
Expand Down Expand Up @@ -188,6 +192,12 @@ fn check_arg(cx: &LateContext<'_>, arg_expr: &Expr<'_>, span: Span, help: impl H
help.normal_help()
} else if match_type(cx, ty, &ANYHOW_ERROR) {
help.anyhow_help()
} else if match_type(cx, ty, &THISERROR_EXT_REPORT_REPORT) {
if let Some(help) = help.report_help() {
help
} else {
return;
}
} else {
return;
};
Expand All @@ -212,6 +222,9 @@ trait Help {
fn anyhow_help(&self) -> &str {
self.normal_help()
}
fn report_help(&self) -> Option<&str> {
None
}
}

impl Help for &str {
Expand All @@ -220,14 +233,18 @@ impl Help for &str {
}
}

impl Help for (&str, &str) {
impl Help for (&str, &str, &str) {
fn normal_help(&self) -> &str {
self.0
}

fn anyhow_help(&self) -> &str {
self.1
}

fn report_help(&self) -> Option<&str> {
Some(self.2)
}
}

#[test]
Expand Down
7 changes: 7 additions & 0 deletions lints/ui/format_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,11 @@ fn main() {

let _ = anyhow!("{}", anyhow_err);
let _ = anyhow!("some error occurred: {:?}", anyhow_err);

use thiserror_ext::AsReport;

let _ = anyhow!("{}", err.as_report());
let _ = anyhow!("some error occurred: {}", err.as_report());
let _ = anyhow!("{:?}", anyhow_err.as_report());
let _ = anyhow!("some error occurred: {:?}", anyhow_err.as_report());
}
34 changes: 33 additions & 1 deletion lints/ui/format_error.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -344,5 +344,37 @@ LL | let _ = anyhow!("some error occurred: {:?}", anyhow_err);
|
= help: consider using `.context(..)` to attach additional message to the error and make it an error source instead

error: aborting due to 43 previous errors
error: should not format error directly
--> $DIR/format_error.rs:79:27
|
LL | let _ = anyhow!("{}", err.as_report());
| ^^^^^^^^^^^^^^^
|
= help: consider directly wrapping the error with `anyhow::anyhow!(..)` instead of formatting its report

error: should not format error directly
--> $DIR/format_error.rs:80:48
|
LL | let _ = anyhow!("some error occurred: {}", err.as_report());
| ^^^^^^^^^^^^^^^
|
= help: consider using `anyhow::Context::(with_)context` to attach additional message to the error and make it an error source instead

error: should not format error directly
--> $DIR/format_error.rs:81:29
|
LL | let _ = anyhow!("{:?}", anyhow_err.as_report());
| ^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider directly wrapping the error with `anyhow::anyhow!(..)` instead of formatting its report

error: should not format error directly
--> $DIR/format_error.rs:82:50
|
LL | let _ = anyhow!("some error occurred: {:?}", anyhow_err.as_report());
| ^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider using `anyhow::Context::(with_)context` to attach additional message to the error and make it an error source instead

error: aborting due to 47 previous errors

0 comments on commit 3398990

Please sign in to comment.