-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
allowing [
map_flatten] to split long suggestions
add new function `span_lint_and_sugg_` for edges in `clippy_utils::diagnostics`
- Loading branch information
Showing
9 changed files
with
395 additions
and
124 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,83 +1,73 @@ | ||
| use clippy_utils::diagnostics::span_lint_and_sugg; | ||
| use clippy_utils::diagnostics::span_lint_and_sugg_for_edges; | ||
| use clippy_utils::is_trait_method; | ||
| use clippy_utils::source::snippet; | ||
| use clippy_utils::source::snippet_with_applicability; | ||
| use clippy_utils::ty::is_type_diagnostic_item; | ||
| use rustc_errors::Applicability; | ||
| use rustc_hir as hir; | ||
| use rustc_hir::Expr; | ||
| use rustc_lint::LateContext; | ||
| use rustc_middle::ty; | ||
| use rustc_span::symbol::sym; | ||
| use rustc_span::{symbol::sym, Span}; | ||
|
|
||
| use super::MAP_FLATTEN; | ||
|
|
||
| /// lint use of `map().flatten()` for `Iterators` and 'Options' | ||
| pub(super) fn check<'tcx>( | ||
| cx: &LateContext<'tcx>, | ||
| expr: &'tcx hir::Expr<'_>, | ||
| recv: &'tcx hir::Expr<'_>, | ||
| map_arg: &'tcx hir::Expr<'_>, | ||
| ) { | ||
| // lint if caller of `.map().flatten()` is an Iterator | ||
| if is_trait_method(cx, expr, sym::Iterator) { | ||
| let map_closure_ty = cx.typeck_results().expr_ty(map_arg); | ||
| let is_map_to_option = match map_closure_ty.kind() { | ||
| ty::Closure(_, _) | ty::FnDef(_, _) | ty::FnPtr(_) => { | ||
| let map_closure_sig = match map_closure_ty.kind() { | ||
| ty::Closure(_, substs) => substs.as_closure().sig(), | ||
| _ => map_closure_ty.fn_sig(cx.tcx), | ||
| }; | ||
| let map_closure_return_ty = cx.tcx.erase_late_bound_regions(map_closure_sig.output()); | ||
| is_type_diagnostic_item(cx, map_closure_return_ty, sym::Option) | ||
| }, | ||
| _ => false, | ||
| }; | ||
|
|
||
| let method_to_use = if is_map_to_option { | ||
| // `(...).map(...)` has type `impl Iterator<Item=Option<...>> | ||
| "filter_map" | ||
| } else { | ||
| // `(...).map(...)` has type `impl Iterator<Item=impl Iterator<...>> | ||
| "flat_map" | ||
| }; | ||
| let func_snippet = snippet(cx, map_arg.span, ".."); | ||
| let hint = format!(".{0}({1})", method_to_use, func_snippet); | ||
| span_lint_and_sugg( | ||
| pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, map_arg: &Expr<'_>, map_span: Span) { | ||
| if let Some((caller_ty_name, method_to_use)) = try_get_caller_ty_name_and_method_name(cx, expr, recv, map_arg) { | ||
| let mut applicability = Applicability::MachineApplicable; | ||
| let help_msgs = [ | ||
| &format!("try replacing `map` with `{}`", method_to_use), | ||
| "and remove the `.flatten()`", | ||
| ]; | ||
| let closure_snippet = snippet_with_applicability(cx, map_arg.span, "..", &mut applicability); | ||
| span_lint_and_sugg_for_edges( | ||
| cx, | ||
| MAP_FLATTEN, | ||
| expr.span.with_lo(recv.span.hi()), | ||
| "called `map(..).flatten()` on an `Iterator`", | ||
| &format!("try using `{}` instead", method_to_use), | ||
| hint, | ||
| Applicability::MachineApplicable, | ||
| expr.span.with_lo(map_span.lo()), | ||
| &format!("called `map(..).flatten()` on `{}`", caller_ty_name), | ||
| &help_msgs, | ||
| format!("{}({})", method_to_use, closure_snippet), | ||
| applicability, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // lint if caller of `.map().flatten()` is an Option or Result | ||
| let caller_type = match cx.typeck_results().expr_ty(recv).kind() { | ||
| ty::Adt(adt, _) => { | ||
| fn try_get_caller_ty_name_and_method_name( | ||
| cx: &LateContext<'_>, | ||
| expr: &Expr<'_>, | ||
| caller_expr: &Expr<'_>, | ||
| map_arg: &Expr<'_>, | ||
| ) -> Option<(&'static str, &'static str)> { | ||
| if is_trait_method(cx, expr, sym::Iterator) { | ||
| if is_map_to_option(cx, map_arg) { | ||
| // `(...).map(...)` has type `impl Iterator<Item=Option<...>> | ||
| Some(("Iterator", "filter_map")) | ||
| } else { | ||
| // `(...).map(...)` has type `impl Iterator<Item=impl Iterator<...>> | ||
| Some(("Iterator", "flat_map")) | ||
| } | ||
| } else { | ||
| if let ty::Adt(adt, _) = cx.typeck_results().expr_ty(caller_expr).kind() { | ||
| if cx.tcx.is_diagnostic_item(sym::Option, adt.did()) { | ||
| "Option" | ||
| return Some(("Option", "and_then")); | ||
| } else if cx.tcx.is_diagnostic_item(sym::Result, adt.did()) { | ||
| "Result" | ||
| } else { | ||
| return; | ||
| return Some(("Result", "and_then")); | ||
| } | ||
| }, | ||
| _ => { | ||
| return; | ||
| }, | ||
| }; | ||
| } | ||
| None | ||
| } | ||
| } | ||
|
|
||
| let func_snippet = snippet(cx, map_arg.span, ".."); | ||
| let hint = format!(".and_then({})", func_snippet); | ||
| let lint_info = format!("called `map(..).flatten()` on an `{}`", caller_type); | ||
| span_lint_and_sugg( | ||
| cx, | ||
| MAP_FLATTEN, | ||
| expr.span.with_lo(recv.span.hi()), | ||
| &lint_info, | ||
| "try using `and_then` instead", | ||
| hint, | ||
| Applicability::MachineApplicable, | ||
| ); | ||
| fn is_map_to_option(cx: &LateContext<'_>, map_arg: &Expr<'_>) -> bool { | ||
| let map_closure_ty = cx.typeck_results().expr_ty(map_arg); | ||
| match map_closure_ty.kind() { | ||
| ty::Closure(_, _) | ty::FnDef(_, _) | ty::FnPtr(_) => { | ||
| let map_closure_sig = match map_closure_ty.kind() { | ||
| ty::Closure(_, substs) => substs.as_closure().sig(), | ||
| _ => map_closure_ty.fn_sig(cx.tcx), | ||
| }; | ||
| let map_closure_return_ty = cx.tcx.erase_late_bound_regions(map_closure_sig.output()); | ||
| is_type_diagnostic_item(cx, map_closure_return_ty, sym::Option) | ||
| }, | ||
| _ => false, | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,31 +1,55 @@ | ||
| // run-rustfix | ||
|
|
||
| #![warn(clippy::all, clippy::pedantic)] | ||
| #![allow(clippy::let_underscore_drop)] | ||
| #![allow(clippy::missing_docs_in_private_items)] | ||
| #![allow(clippy::map_identity)] | ||
| #![allow(clippy::redundant_closure)] | ||
| #![allow(clippy::unnecessary_wraps)] | ||
| #![warn(clippy::map_flatten)] | ||
| #![feature(result_flattening)] | ||
|
|
||
| fn main() { | ||
| // mapping to Option on Iterator | ||
| fn option_id(x: i8) -> Option<i8> { | ||
| Some(x) | ||
| } | ||
| let option_id_ref: fn(i8) -> Option<i8> = option_id; | ||
| let option_id_closure = |x| Some(x); | ||
| let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect(); | ||
| let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect(); | ||
| let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect(); | ||
| let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect(); | ||
| // issue #8506, multi-line | ||
| #[rustfmt::skip] | ||
| fn long_span() { | ||
| let _: Option<i32> = Some(1) | ||
| .map(|x| { | ||
| if x <= 5 { | ||
| Some(x) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| .flatten(); | ||
|
|
||
| // mapping to Iterator on Iterator | ||
| let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); | ||
| let _: Result<i32, i32> = Ok(1) | ||
| .map(|x| { | ||
| if x == 1 { | ||
| Ok(x) | ||
| } else { | ||
| Err(0) | ||
| } | ||
| }) | ||
| .flatten(); | ||
|
|
||
| // mapping to Option on Option | ||
| let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); | ||
| let result: Result<i32, i32> = Ok(2); | ||
| fn do_something() { } | ||
| let _: Result<i32, i32> = result | ||
| .map(|res| { | ||
| if res > 0 { | ||
| do_something(); | ||
| Ok(res) | ||
| } else { | ||
| Err(0) | ||
| } | ||
| }) | ||
| .flatten(); | ||
|
|
||
| let _: Vec<_> = vec![5_i8; 6] | ||
| .into_iter() | ||
| .map(|some_value| { | ||
| if some_value > 3 { | ||
| Some(some_value) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| .flatten() | ||
| .collect(); | ||
| } | ||
|
|
||
| // mapping to Result on Result | ||
| let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten(); | ||
| fn main() { | ||
| long_span(); | ||
| } |
Oops, something went wrong.