Skip to content

Commit 7c86a9c

Browse files
bors[bot]Michael Wright
bors[bot]
and
Michael Wright
committed
Merge #3387
3387: Replace big if/else expression with match r=flip1995 a=mikerite Co-authored-by: Michael Wright <[email protected]>
2 parents ae137d5 + 0a41dfd commit 7c86a9c

File tree

2 files changed

+60
-57
lines changed

2 files changed

+60
-57
lines changed

‎clippy_lints/src/methods/mod.rs

+36-56
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,13 @@ use crate::rustc::{declare_tool_lint, lint_array};
1616
use crate::rustc_errors::Applicability;
1717
use crate::syntax::ast;
1818
use crate::syntax::source_map::{BytePos, Span};
19+
use crate::syntax::symbol::LocalInternedString;
1920
use crate::utils::paths;
2021
use crate::utils::sugg;
2122
use crate::utils::{
2223
get_arg_name, get_trait_def_id, implements_trait, in_macro, is_copy, is_expn_of, is_self, is_self_ty,
2324
iter_input_pats, last_path_segment, match_def_path, match_path, match_qpath, match_trait_method, match_type,
24-
match_var, method_chain_args, remove_blocks, return_ty, same_tys, single_segment_path, snippet, snippet_with_macro_callsite, span_lint,
25+
match_var, method_calls, method_chain_args, remove_blocks, return_ty, same_tys, single_segment_path, snippet, snippet_with_macro_callsite, span_lint,
2526
span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq,
2627
};
2728
use if_chain::if_chain;
@@ -790,63 +791,42 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
790791
return;
791792
}
792793

794+
let (method_names, arg_lists) = method_calls(expr, 2);
795+
let method_names: Vec<LocalInternedString> = method_names.iter().map(|s| s.as_str()).collect();
796+
let method_names: Vec<&str> = method_names.iter().map(|s| s.as_ref()).collect();
797+
798+
match method_names.as_slice() {
799+
["unwrap", "get"] => lint_get_unwrap(cx, expr, arg_lists[1], false),
800+
["unwrap", "get_mut"] => lint_get_unwrap(cx, expr, arg_lists[1], true),
801+
["unwrap", ..] => lint_unwrap(cx, expr, arg_lists[0]),
802+
["expect", "ok"] => lint_ok_expect(cx, expr, arg_lists[1]),
803+
["unwrap_or", "map"] => lint_map_unwrap_or(cx, expr, arg_lists[1], arg_lists[0]),
804+
["unwrap_or_else", "map"] => lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]),
805+
["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]),
806+
["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
807+
["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]),
808+
["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
809+
["flat_map", "filter"] => lint_filter_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
810+
["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
811+
["flatten", "map"] => lint_map_flatten(cx, expr, arg_lists[1]),
812+
["is_some", "find"] => lint_search_is_some(cx, expr, "find", arg_lists[1], arg_lists[0]),
813+
["is_some", "position"] => lint_search_is_some(cx, expr, "position", arg_lists[1], arg_lists[0]),
814+
["is_some", "rposition"] => lint_search_is_some(cx, expr, "rposition", arg_lists[1], arg_lists[0]),
815+
["extend", ..] => lint_extend(cx, expr, arg_lists[0]),
816+
["as_ptr", "unwrap"] => lint_cstring_as_ptr(cx, expr, &arg_lists[1][0], &arg_lists[0][0]),
817+
["nth", "iter"] => lint_iter_nth(cx, expr, arg_lists[1], false),
818+
["nth", "iter_mut"] => lint_iter_nth(cx, expr, arg_lists[1], true),
819+
["next", "skip"] => lint_iter_skip_next(cx, expr),
820+
["collect", "cloned"] => lint_iter_cloned_collect(cx, expr, arg_lists[1]),
821+
["as_ref", ..] => lint_asref(cx, expr, "as_ref", arg_lists[0]),
822+
["as_mut", ..] => lint_asref(cx, expr, "as_mut", arg_lists[0]),
823+
["fold", ..] => lint_unnecessary_fold(cx, expr, arg_lists[0]),
824+
["filter_map", ..] => unnecessary_filter_map::lint(cx, expr, arg_lists[0]),
825+
_ => {}
826+
}
827+
793828
match expr.node {
794829
hir::ExprKind::MethodCall(ref method_call, ref method_span, ref args) => {
795-
// Chain calls
796-
// GET_UNWRAP needs to be checked before general `UNWRAP` lints
797-
if let Some(arglists) = method_chain_args(expr, &["get", "unwrap"]) {
798-
lint_get_unwrap(cx, expr, arglists[0], false);
799-
} else if let Some(arglists) = method_chain_args(expr, &["get_mut", "unwrap"]) {
800-
lint_get_unwrap(cx, expr, arglists[0], true);
801-
} else if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
802-
lint_unwrap(cx, expr, arglists[0]);
803-
} else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) {
804-
lint_ok_expect(cx, expr, arglists[0]);
805-
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) {
806-
lint_map_unwrap_or(cx, expr, arglists[0], arglists[1]);
807-
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) {
808-
lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]);
809-
} else if let Some(arglists) = method_chain_args(expr, &["map_or"]) {
810-
lint_map_or_none(cx, expr, arglists[0]);
811-
} else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) {
812-
lint_filter_next(cx, expr, arglists[0]);
813-
} else if let Some(arglists) = method_chain_args(expr, &["filter", "map"]) {
814-
lint_filter_map(cx, expr, arglists[0], arglists[1]);
815-
} else if let Some(arglists) = method_chain_args(expr, &["filter_map", "map"]) {
816-
lint_filter_map_map(cx, expr, arglists[0], arglists[1]);
817-
} else if let Some(arglists) = method_chain_args(expr, &["filter", "flat_map"]) {
818-
lint_filter_flat_map(cx, expr, arglists[0], arglists[1]);
819-
} else if let Some(arglists) = method_chain_args(expr, &["filter_map", "flat_map"]) {
820-
lint_filter_map_flat_map(cx, expr, arglists[0], arglists[1]);
821-
} else if let Some(arglists) = method_chain_args(expr, &["map", "flatten"]) {
822-
lint_map_flatten(cx, expr, arglists[0]);
823-
} else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) {
824-
lint_search_is_some(cx, expr, "find", arglists[0], arglists[1]);
825-
} else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) {
826-
lint_search_is_some(cx, expr, "position", arglists[0], arglists[1]);
827-
} else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) {
828-
lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
829-
} else if let Some(arglists) = method_chain_args(expr, &["extend"]) {
830-
lint_extend(cx, expr, arglists[0]);
831-
} else if let Some(arglists) = method_chain_args(expr, &["unwrap", "as_ptr"]) {
832-
lint_cstring_as_ptr(cx, expr, &arglists[0][0], &arglists[1][0]);
833-
} else if let Some(arglists) = method_chain_args(expr, &["iter", "nth"]) {
834-
lint_iter_nth(cx, expr, arglists[0], false);
835-
} else if let Some(arglists) = method_chain_args(expr, &["iter_mut", "nth"]) {
836-
lint_iter_nth(cx, expr, arglists[0], true);
837-
} else if method_chain_args(expr, &["skip", "next"]).is_some() {
838-
lint_iter_skip_next(cx, expr);
839-
} else if let Some(arglists) = method_chain_args(expr, &["cloned", "collect"]) {
840-
lint_iter_cloned_collect(cx, expr, arglists[0]);
841-
} else if let Some(arglists) = method_chain_args(expr, &["as_ref"]) {
842-
lint_asref(cx, expr, "as_ref", arglists[0]);
843-
} else if let Some(arglists) = method_chain_args(expr, &["as_mut"]) {
844-
lint_asref(cx, expr, "as_mut", arglists[0]);
845-
} else if let Some(arglists) = method_chain_args(expr, &["fold"]) {
846-
lint_unnecessary_fold(cx, expr, arglists[0]);
847-
} else if let Some(arglists) = method_chain_args(expr, &["filter_map"]) {
848-
unnecessary_filter_map::lint(cx, expr, arglists[0]);
849-
}
850830

851831
lint_or_fun_call(cx, expr, *method_span, &method_call.ident.as_str(), args);
852832
lint_expect_fun_call(cx, expr, *method_span, &method_call.ident.as_str(), args);

‎clippy_lints/src/utils/mod.rs

+24-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use crate::syntax::ast::{self, LitKind};
3131
use crate::syntax::attr;
3232
use crate::syntax::source_map::{Span, DUMMY_SP};
3333
use crate::syntax::errors::DiagnosticBuilder;
34-
use crate::syntax::symbol::keywords;
34+
use crate::syntax::symbol::{keywords, Symbol};
3535

3636
pub mod camel_case;
3737

@@ -274,6 +274,29 @@ pub fn resolve_node(cx: &LateContext<'_, '_>, qpath: &QPath, id: HirId) -> def::
274274
cx.tables.qpath_def(qpath, id)
275275
}
276276

277+
/// Return the method names and argument list of nested method call expressions that make up
278+
/// `expr`.
279+
pub fn method_calls<'a>(expr: &'a Expr, max_depth: usize) -> (Vec<Symbol>, Vec<&'a [Expr]>) {
280+
let mut method_names = Vec::with_capacity(max_depth);
281+
let mut arg_lists = Vec::with_capacity(max_depth);
282+
283+
let mut current = expr;
284+
for _ in 0..max_depth {
285+
if let ExprKind::MethodCall(path, _, args) = &current.node {
286+
if args.iter().any(|e| in_macro(e.span)) {
287+
break;
288+
}
289+
method_names.push(path.ident.name);
290+
arg_lists.push(&**args);
291+
current = &args[0];
292+
} else {
293+
break;
294+
}
295+
}
296+
297+
(method_names, arg_lists)
298+
}
299+
277300
/// Match an `Expr` against a chain of methods, and return the matched `Expr`s.
278301
///
279302
/// For example, if `expr` represents the `.baz()` in `foo.bar().baz()`,

0 commit comments

Comments
 (0)