Skip to content

Commit

Permalink
Auto merge of #3674 - sinkuu:fmt_rustup, r=oli-obk
Browse files Browse the repository at this point in the history
Catch up with `format_args` change

Catches up with a change in rust-lang/rust#57537. (Since the optimization is optional, this clippy PR can be merged before the rustc PR.)

Happened to fix a bug in `expect_fun_call`, that is the lint ignores more than
one arguments to `format`.

```
warning: use of `expect` followed by a function call
 --> src/main.rs:2:17
  |
2 |     Some("foo").expect(format!("{} {}", 1, 2).as_ref());
  |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("{} {}", 1))`
  |
```
  • Loading branch information
bors committed Jan 19, 2019
2 parents 751d82e + 2ee713d commit e648adf
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 18 deletions.
11 changes: 8 additions & 3 deletions clippy_lints/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down
27 changes: 13 additions & 14 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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]));
}
}
}
Expand All @@ -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(
Expand Down Expand Up @@ -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(", ");

Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/expect_fun_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
8 changes: 7 additions & 1 deletion tests/ui/expect_fun_call.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit e648adf

Please sign in to comment.