Skip to content

Commit

Permalink
Destructure args in methods module
Browse files Browse the repository at this point in the history
  • Loading branch information
camsteffen committed Mar 31, 2021
1 parent 775ef47 commit 2108387
Show file tree
Hide file tree
Showing 37 changed files with 412 additions and 375 deletions.
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

0 comments on commit 2108387

Please sign in to comment.