From 2ee713dc7bcc8e9bb7223b49451ec6ac9685f4c6 Mon Sep 17 00:00:00 2001
From: Shotaro Yamada <sinkuu@sinkuu.xyz>
Date: Sat, 19 Jan 2019 21:13:06 +0900
Subject: [PATCH] Catch up with `format_args` change
Catches up with a change in rust-lang/rust#57537
Happened to fix a bug in `expect_fun_call`, that is the lint ignores more than
one arguments to `format`.
---
clippy_lints/src/format.rs | 11 ++++++++---
clippy_lints/src/methods/mod.rs | 27 +++++++++++++--------------
clippy_lints/src/utils/paths.rs | 1 +
tests/ui/expect_fun_call.rs | 2 ++
tests/ui/expect_fun_call.stderr | 8 +++++++-
5 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs
index 57c21bee722..90e19af15d0 100644
--- a/clippy_lints/src/format.rs
+++ b/clippy_lints/src/format.rs
@@ -53,12 +53,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
ExprKind::Call(ref fun, ref args) => {
if_chain! {
if let ExprKind::Path(ref qpath) = fun.node;
- if args.len() == 3;
if let Some(fun_def_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id));
- if match_def_path(cx.tcx, fun_def_id, &paths::FMT_ARGUMENTS_NEWV1FORMATTED);
+ let new_v1 = match_def_path(cx.tcx, fun_def_id, &paths::FMT_ARGUMENTS_NEWV1);
+ let new_v1_fmt = match_def_path(
+ cx.tcx,
+ fun_def_id,
+ &paths::FMT_ARGUMENTS_NEWV1FORMATTED
+ );
+ if new_v1 || new_v1_fmt;
if check_single_piece(&args[0]);
if let Some(format_arg) = get_single_string_arg(cx, &args[1]);
- if check_unformatted(&args[2]);
+ if new_v1 || check_unformatted(&args[2]);
if let ExprKind::AddrOf(_, ref format_arg) = format_arg.node;
then {
let (message, sugg) = if_chain! {
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 41011e8f66a..14fdd6e6225 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -1148,7 +1148,7 @@ fn lint_or_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Spa
/// Checks for the `EXPECT_FUN_CALL` lint.
fn lint_expect_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Span, name: &str, args: &[hir::Expr]) {
- fn extract_format_args(arg: &hir::Expr) -> Option<&hir::HirVec<hir::Expr>> {
+ fn extract_format_args(arg: &hir::Expr) -> Option<(&hir::Expr, &hir::Expr)> {
let arg = match &arg.node {
hir::ExprKind::AddrOf(_, expr) => expr,
hir::ExprKind::MethodCall(method_name, _, args)
@@ -1161,8 +1161,8 @@ fn lint_expect_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span:
if let hir::ExprKind::Call(ref inner_fun, ref inner_args) = arg.node {
if is_expn_of(inner_fun.span, "format").is_some() && inner_args.len() == 1 {
- if let hir::ExprKind::Call(_, ref format_args) = inner_args[0].node {
- return Some(format_args);
+ if let hir::ExprKind::Call(_, format_args) = &inner_args[0].node {
+ return Some((&format_args[0], &format_args[1]));
}
}
}
@@ -1174,17 +1174,19 @@ fn lint_expect_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span:
cx: &LateContext<'_, '_>,
a: &hir::Expr,
applicability: &mut Applicability,
- ) -> String {
+ ) -> Vec<String> {
if let hir::ExprKind::AddrOf(_, ref format_arg) = a.node {
if let hir::ExprKind::Match(ref format_arg_expr, _, _) = format_arg.node {
if let hir::ExprKind::Tup(ref format_arg_expr_tup) = format_arg_expr.node {
- return snippet_with_applicability(cx, format_arg_expr_tup[0].span, "..", applicability)
- .into_owned();
+ return format_arg_expr_tup
+ .iter()
+ .map(|a| snippet_with_applicability(cx, a.span, "..", applicability).into_owned())
+ .collect();
}
}
};
- snippet(cx, a.span, "..").into_owned()
+ unreachable!()
}
fn check_general_case(
@@ -1233,14 +1235,11 @@ fn lint_expect_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span:
};
let span_replace_word = method_span.with_hi(span.hi());
- if let Some(format_args) = extract_format_args(arg) {
+ if let Some((fmt_spec, fmt_args)) = extract_format_args(arg) {
let mut applicability = Applicability::MachineApplicable;
- let args_len = format_args.len();
- let args: Vec<String> = format_args
- .into_iter()
- .take(args_len - 1)
- .map(|a| generate_format_arg_snippet(cx, a, &mut applicability))
- .collect();
+ let mut args = vec![snippet(cx, fmt_spec.span, "..").into_owned()];
+
+ args.extend(generate_format_arg_snippet(cx, fmt_args, &mut applicability));
let sugg = args.join(", ");
diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs
index a74e457a9fd..d2dc2812575 100644
--- a/clippy_lints/src/utils/paths.rs
+++ b/clippy_lints/src/utils/paths.rs
@@ -26,6 +26,7 @@ pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleE
pub const DROP: [&str; 3] = ["core", "mem", "drop"];
pub const DURATION: [&str; 3] = ["core", "time", "Duration"];
pub const EARLY_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "EarlyContext"];
+pub const FMT_ARGUMENTS_NEWV1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"];
pub const FMT_ARGUMENTS_NEWV1FORMATTED: [&str; 4] = ["core", "fmt", "Arguments", "new_v1_formatted"];
pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"];
pub const FROM_TRAIT: [&str; 3] = ["core", "convert", "From"];
diff --git a/tests/ui/expect_fun_call.rs b/tests/ui/expect_fun_call.rs
index 0f930f6a8a2..7f0ca0fe809 100644
--- a/tests/ui/expect_fun_call.rs
+++ b/tests/ui/expect_fun_call.rs
@@ -57,6 +57,8 @@ fn expect_fun_call() {
Some("foo").expect({ &format!("error") });
Some("foo").expect(format!("error").as_ref());
+
+ Some("foo").expect(format!("{} {}", 1, 2).as_ref());
}
fn main() {}
diff --git a/tests/ui/expect_fun_call.stderr b/tests/ui/expect_fun_call.stderr
index 09844e29911..a60bd7e4ed3 100644
--- a/tests/ui/expect_fun_call.stderr
+++ b/tests/ui/expect_fun_call.stderr
@@ -36,5 +36,11 @@ error: use of `expect` followed by a function call
LL | Some("foo").expect(format!("error").as_ref());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("error"))`
-error: aborting due to 6 previous errors
+error: use of `expect` followed by a function call
+ --> $DIR/expect_fun_call.rs:61:17
+ |
+LL | Some("foo").expect(format!("{} {}", 1, 2).as_ref());
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("{} {}", 1, 2))`
+
+error: aborting due to 7 previous errors