Skip to content

Destructure args in methods #6913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions clippy_lints/src/methods/bind_instead_of_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ pub(crate) trait BindInsteadOfMap {
fn lint_closure_autofixable(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
args: &[hir::Expr<'_>],
recv: &hir::Expr<'_>,
closure_expr: &hir::Expr<'_>,
closure_args_span: Span,
) -> bool {
Expand All @@ -103,7 +103,7 @@ pub(crate) trait BindInsteadOfMap {
};

let closure_args_snip = snippet(cx, closure_args_span, "..");
let option_snip = snippet(cx, args[0].span, "..");
let option_snip = snippet(cx, recv.span, "..");
let note = format!("{}.{}({} {})", option_snip, Self::GOOD_METHOD_NAME, closure_args_snip, some_inner_snip);
span_lint_and_sugg(
cx,
Expand Down Expand Up @@ -158,17 +158,17 @@ pub(crate) trait BindInsteadOfMap {
}

/// Lint use of `_.and_then(|x| Some(y))` for `Option`s
fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) -> bool {
if !match_type(cx, cx.typeck_results().expr_ty(&args[0]), Self::TYPE_QPATH) {
fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) -> bool {
if !match_type(cx, cx.typeck_results().expr_ty(recv), Self::TYPE_QPATH) {
return false;
}

match args[1].kind {
match arg.kind {
hir::ExprKind::Closure(_, _, body_id, closure_args_span, _) => {
let closure_body = cx.tcx.hir().body(body_id);
let closure_expr = remove_blocks(&closure_body.value);

if Self::lint_closure_autofixable(cx, expr, args, closure_expr, closure_args_span) {
if Self::lint_closure_autofixable(cx, expr, recv, closure_expr, closure_args_span) {
true
} else {
Self::lint_closure(cx, expr, closure_expr)
Expand All @@ -182,7 +182,7 @@ pub(crate) trait BindInsteadOfMap {
expr.span,
Self::no_op_msg().as_ref(),
"use the expression directly",
snippet(cx, args[0].span, "..").into(),
snippet(cx, recv.span, "..").into(),
Applicability::MachineApplicable,
);
true
Expand Down
11 changes: 5 additions & 6 deletions clippy_lints/src/methods/bytes_nth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@ use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::is_type_diagnostic_item;
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_span::sym;

use super::BYTES_NTH;

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>]) {
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, recv: &'tcx Expr<'tcx>, n_arg: &'tcx Expr<'tcx>) {
if_chain! {
if let ExprKind::MethodCall(_, _, ref args, _) = expr.kind;
let ty = cx.typeck_results().expr_ty(&iter_args[0]).peel_refs();
let ty = cx.typeck_results().expr_ty(recv).peel_refs();
let caller_type = if is_type_diagnostic_item(cx, ty, sym::string_type) {
Some("String")
} else if ty.is_str() {
Expand All @@ -31,8 +30,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'
"try",
format!(
"{}.as_bytes().get({})",
snippet_with_applicability(cx, iter_args[0].span, "..", &mut applicability),
snippet_with_applicability(cx, args[1].span, "..", &mut applicability)
snippet_with_applicability(cx, recv.span, "..", &mut applicability),
snippet_with_applicability(cx, n_arg.span, "..", &mut applicability)
),
applicability,
);
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/methods/expect_used.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use rustc_span::sym;
use super::EXPECT_USED;

/// lint use of `expect()` for `Option`s and `Result`s
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, expect_args: &[hir::Expr<'_>]) {
let obj_ty = cx.typeck_results().expr_ty(&expect_args[0]).peel_refs();
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) {
let obj_ty = cx.typeck_results().expr_ty(recv).peel_refs();

let mess = if is_type_diagnostic_item(cx, obj_ty, sym::option_type) {
Some((EXPECT_USED, "an Option", "None"))
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/methods/filetype_is_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use rustc_span::source_map::Span;

use super::FILETYPE_IS_FILE;

pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
let ty = cx.typeck_results().expr_ty(&args[0]);
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) {
let ty = cx.typeck_results().expr_ty(recv);

if !match_type(cx, ty, &paths::FILE_TYPE) {
return;
Expand Down
50 changes: 29 additions & 21 deletions clippy_lints/src/methods/filter_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,17 @@ fn is_method<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, method_name: Sy
}
}

fn is_option_filter_map<'tcx>(
cx: &LateContext<'tcx>,
filter_arg: &'tcx hir::Expr<'_>,
map_arg: &'tcx hir::Expr<'_>,
) -> bool {
fn is_option_filter_map<'tcx>(cx: &LateContext<'tcx>, filter_arg: &hir::Expr<'_>, map_arg: &hir::Expr<'_>) -> bool {
is_method(cx, map_arg, sym::unwrap) && is_method(cx, filter_arg, sym!(is_some))
}

/// lint use of `filter().map()` for `Iterators`
fn lint_filter_some_map_unwrap<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
filter_recv: &'tcx hir::Expr<'_>,
filter_arg: &'tcx hir::Expr<'_>,
map_arg: &'tcx hir::Expr<'_>,
fn lint_filter_some_map_unwrap(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
filter_recv: &hir::Expr<'_>,
filter_arg: &hir::Expr<'_>,
map_arg: &hir::Expr<'_>,
target_span: Span,
methods_span: Span,
) {
Expand All @@ -86,14 +82,28 @@ fn lint_filter_some_map_unwrap<'tcx>(
}

/// lint use of `filter().map()` or `find().map()` for `Iterators`
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_find: bool, target_span: Span) {
#[allow(clippy::too_many_arguments)]
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &hir::Expr<'_>,
filter_recv: &hir::Expr<'_>,
filter_arg: &hir::Expr<'_>,
filter_span: Span,
map_recv: &hir::Expr<'_>,
map_arg: &hir::Expr<'_>,
map_span: Span,
is_find: bool,
) {
lint_filter_some_map_unwrap(
cx,
expr,
filter_recv,
filter_arg,
map_arg,
map_span,
filter_span.with_hi(expr.span.hi()),
);
if_chain! {
if let ExprKind::MethodCall(_, _, [map_recv, map_arg], map_span) = expr.kind;
if let ExprKind::MethodCall(_, _, [filter_recv, filter_arg], filter_span) = map_recv.kind;
then {
lint_filter_some_map_unwrap(cx, expr, filter_recv, filter_arg,
map_arg, target_span, filter_span.to(map_span));
if_chain! {
if is_trait_method(cx, map_recv, sym::Iterator);

// filter(|x| ...is_some())...
Expand Down Expand Up @@ -148,7 +158,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_
};
if SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg);
then {
let span = filter_span.to(map_span);
let span = filter_span.with_hi(expr.span.hi());
let (filter_name, lint) = if is_find {
("find", MANUAL_FIND_MAP)
} else {
Expand All @@ -160,7 +170,5 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_
snippet(cx, map_arg.span, ".."), to_opt);
span_lint_and_sugg(cx, lint, span, &msg, "try", sugg, Applicability::MachineApplicable);
}
}
}
}
}
15 changes: 4 additions & 11 deletions clippy_lints/src/methods/filter_map_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,8 @@ use rustc_span::{source_map::Span, sym};

use super::FILTER_MAP_IDENTITY;

pub(super) fn check(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
filter_map_args: &[hir::Expr<'_>],
filter_map_span: Span,
) {
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_map_arg: &hir::Expr<'_>, filter_map_span: Span) {
if is_trait_method(cx, expr, sym::Iterator) {
let arg_node = &filter_map_args[1].kind;

let apply_lint = |message: &str| {
span_lint_and_sugg(
cx,
Expand All @@ -30,8 +23,8 @@ pub(super) fn check(
};

if_chain! {
if let hir::ExprKind::Closure(_, _, body_id, _, _) = arg_node;
let body = cx.tcx.hir().body(*body_id);
if let hir::ExprKind::Closure(_, _, body_id, _, _) = filter_map_arg.kind;
let body = cx.tcx.hir().body(body_id);

if let hir::PatKind::Binding(_, binding_id, ..) = body.params[0].pat.kind;
if path_to_local_id(&body.value, binding_id);
Expand All @@ -41,7 +34,7 @@ pub(super) fn check(
}

if_chain! {
if let hir::ExprKind::Path(ref qpath) = arg_node;
if let hir::ExprKind::Path(ref qpath) = filter_map_arg.kind;

if match_qpath(qpath, &paths::STD_CONVERT_IDENTITY);

Expand Down
7 changes: 4 additions & 3 deletions clippy_lints/src/methods/filter_map_next.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ const FILTER_MAP_NEXT_MSRV: RustcVersion = RustcVersion::new(1, 30, 0);
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
filter_args: &'tcx [hir::Expr<'_>],
recv: &'tcx hir::Expr<'_>,
arg: &'tcx hir::Expr<'_>,
msrv: Option<&RustcVersion>,
) {
if is_trait_method(cx, expr, sym::Iterator) {
Expand All @@ -24,9 +25,9 @@ pub(super) fn check<'tcx>(

let msg = "called `filter_map(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
`.find_map(..)` instead";
let filter_snippet = snippet(cx, filter_args[1].span, "..");
let filter_snippet = snippet(cx, arg.span, "..");
if filter_snippet.lines().count() <= 1 {
let iter_snippet = snippet(cx, filter_args[0].span, "..");
let iter_snippet = snippet(cx, recv.span, "..");
span_lint_and_sugg(
cx,
FILTER_MAP_NEXT,
Expand Down
11 changes: 8 additions & 3 deletions clippy_lints/src/methods/filter_next.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,19 @@ use rustc_span::sym;
use super::FILTER_NEXT;

/// lint use of `filter().next()` for `Iterators`
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, filter_args: &'tcx [hir::Expr<'_>]) {
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
recv: &'tcx hir::Expr<'_>,
filter_arg: &'tcx hir::Expr<'_>,
) {
// lint if caller of `.filter().next()` is an Iterator
if is_trait_method(cx, expr, sym::Iterator) {
let msg = "called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
`.find(..)` instead";
let filter_snippet = snippet(cx, filter_args[1].span, "..");
let filter_snippet = snippet(cx, filter_arg.span, "..");
if filter_snippet.lines().count() <= 1 {
let iter_snippet = snippet(cx, filter_args[0].span, "..");
let iter_snippet = snippet(cx, recv.span, "..");
// add note if not multi-line
span_lint_and_sugg(
cx,
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/methods/flat_map_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ use super::FLAT_MAP_IDENTITY;
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
flat_map_args: &'tcx [hir::Expr<'_>],
flat_map_arg: &'tcx hir::Expr<'_>,
flat_map_span: Span,
) {
if is_trait_method(cx, expr, sym::Iterator) {
let arg_node = &flat_map_args[1].kind;
let arg_node = &flat_map_arg.kind;

let apply_lint = |message: &str| {
span_lint_and_sugg(
Expand Down
20 changes: 11 additions & 9 deletions clippy_lints/src/methods/get_unwrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,20 @@ use rustc_span::sym;

use super::GET_UNWRAP;

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, get_args: &'tcx [hir::Expr<'_>], is_mut: bool) {
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &hir::Expr<'_>,
recv: &'tcx hir::Expr<'tcx>,
get_arg: &'tcx hir::Expr<'_>,
is_mut: bool,
) {
// Note: we don't want to lint `get_mut().unwrap` for `HashMap` or `BTreeMap`,
// because they do not implement `IndexMut`
let mut applicability = Applicability::MachineApplicable;
let expr_ty = cx.typeck_results().expr_ty(&get_args[0]);
let get_args_str = if get_args.len() > 1 {
snippet_with_applicability(cx, get_args[1].span, "..", &mut applicability)
} else {
return; // not linting on a .get().unwrap() chain or variant
};
let expr_ty = cx.typeck_results().expr_ty(recv);
let get_args_str = snippet_with_applicability(cx, get_arg.span, "..", &mut applicability);
let mut needs_ref;
let caller_type = if derefs_to_slice(cx, &get_args[0], expr_ty).is_some() {
let caller_type = if derefs_to_slice(cx, recv, expr_ty).is_some() {
needs_ref = get_args_str.parse::<usize>().is_ok();
"slice"
} else if is_type_diagnostic_item(cx, expr_ty, sym::vec_type) {
Expand Down Expand Up @@ -77,7 +79,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, get_args
format!(
"{}{}[{}]",
borrow_str,
snippet_with_applicability(cx, get_args[0].span, "..", &mut applicability),
snippet_with_applicability(cx, recv.span, "..", &mut applicability),
get_args_str
),
applicability,
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/methods/iter_cloned_collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ use rustc_span::sym;

use super::ITER_CLONED_COLLECT;

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, iter_args: &'tcx [hir::Expr<'_>]) {
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, recv: &'tcx hir::Expr<'_>) {
if_chain! {
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::vec_type);
if let Some(slice) = derefs_to_slice(cx, &iter_args[0], cx.typeck_results().expr_ty(&iter_args[0]));
if let Some(slice) = derefs_to_slice(cx, recv, cx.typeck_results().expr_ty(recv));
if let Some(to_replace) = expr.span.trim_start(slice.span.source_callsite());

then {
Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/methods/iter_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use rustc_span::sym;

use super::ITER_COUNT;

pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>], iter_method: &str) {
let ty = cx.typeck_results().expr_ty(&iter_args[0]);
let caller_type = if derefs_to_slice(cx, &iter_args[0], ty).is_some() {
pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, recv: &'tcx Expr<'tcx>, iter_method: &str) {
let ty = cx.typeck_results().expr_ty(recv);
let caller_type = if derefs_to_slice(cx, recv, ty).is_some() {
"slice"
} else if is_type_diagnostic_item(cx, ty, sym::vec_type) {
"Vec"
Expand Down Expand Up @@ -42,7 +42,7 @@ pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'
"try",
format!(
"{}.len()",
snippet_with_applicability(cx, iter_args[0].span, "..", &mut applicability),
snippet_with_applicability(cx, recv.span, "..", &mut applicability),
),
applicability,
);
Expand Down
4 changes: 1 addition & 3 deletions clippy_lints/src/methods/iter_next_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ use rustc_span::symbol::sym;

use super::ITER_NEXT_SLICE;

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, iter_args: &'tcx [hir::Expr<'_>]) {
let caller_expr = &iter_args[0];

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, caller_expr: &'tcx hir::Expr<'_>) {
// Skip lint if the `iter().next()` expression is a for loop argument,
// since it is already covered by `&loops::ITER_NEXT_LOOP`
let mut parent_expr_opt = get_parent_expr(cx, expr);
Expand Down
14 changes: 7 additions & 7 deletions clippy_lints/src/methods/iter_nth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,20 @@ use super::ITER_NTH;
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &hir::Expr<'_>,
nth_and_iter_args: &[&'tcx [hir::Expr<'tcx>]],
iter_recv: &'tcx hir::Expr<'tcx>,
nth_recv: &hir::Expr<'_>,
nth_arg: &hir::Expr<'_>,
is_mut: bool,
) {
let iter_args = nth_and_iter_args[1];
let mut_str = if is_mut { "_mut" } else { "" };
let caller_type = if derefs_to_slice(cx, &iter_args[0], cx.typeck_results().expr_ty(&iter_args[0])).is_some() {
let caller_type = if derefs_to_slice(cx, iter_recv, cx.typeck_results().expr_ty(iter_recv)).is_some() {
"slice"
} else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&iter_args[0]), sym::vec_type) {
} else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::vec_type) {
"Vec"
} else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&iter_args[0]), sym::vecdeque_type) {
} else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::vecdeque_type) {
"VecDeque"
} else {
let nth_args = nth_and_iter_args[0];
iter_nth_zero::check(cx, expr, &nth_args);
iter_nth_zero::check(cx, expr, nth_recv, nth_arg);
return; // caller is not a type that we want to lint
};

Expand Down
Loading