Fix manual_map: don't lint when partially moved values are used.
Fix `manual_map`: don't lint when `return`, `break`, and `continue` are used.
This commit is contained in:
Jason Newcomb
parent
6343446b89
commit
ef87e58993
5 changed files with 168 additions and 22 deletions
|
|
@ -2,13 +2,17 @@ use crate::{
|
|||
map_unit_fn::OPTION_MAP_UNIT_FN,
|
||||
matches::MATCH_AS_REF,
|
||||
utils::{
|
||||
is_allowed, is_type_diagnostic_item, match_def_path, match_var, paths, peel_hir_expr_refs,
|
||||
peel_mid_ty_refs_is_mutable, snippet_with_applicability, span_lint_and_sugg,
|
||||
can_partially_move_ty, is_allowed, is_type_diagnostic_item, match_def_path, match_var, paths,
|
||||
peel_hir_expr_refs, peel_mid_ty_refs_is_mutable, snippet_with_applicability, span_lint_and_sugg,
|
||||
},
|
||||
};
|
||||
use rustc_ast::util::parser::PREC_POSTFIX;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, Mutability, Pat, PatKind, QPath};
|
||||
use rustc_hir::{
|
||||
def::Res,
|
||||
intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor},
|
||||
Arm, BindingAnnotation, Block, Expr, ExprKind, Mutability, Pat, PatKind, Path, QPath,
|
||||
};
|
||||
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
||||
use rustc_middle::lint::in_external_macro;
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
|
|
@ -99,6 +103,10 @@ impl LateLintPass<'_> for ManualMap {
|
|||
return;
|
||||
}
|
||||
|
||||
if !can_move_expr_to_closure(cx, some_expr) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Determine which binding mode to use.
|
||||
let explicit_ref = some_pat.contains_explicit_ref_binding();
|
||||
let binding_ref = explicit_ref.or_else(|| (ty_ref_count != pat_ref_count).then(|| ty_mutability));
|
||||
|
|
@ -171,6 +179,46 @@ impl LateLintPass<'_> for ManualMap {
|
|||
}
|
||||
}
|
||||
|
||||
// Checks if the expression can be moved into a closure as is.
|
||||
fn can_move_expr_to_closure(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
|
||||
struct V<'cx, 'tcx> {
|
||||
cx: &'cx LateContext<'tcx>,
|
||||
make_closure: bool,
|
||||
}
|
||||
impl Visitor<'tcx> for V<'_, 'tcx> {
|
||||
type Map = ErasedMap<'tcx>;
|
||||
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
|
||||
NestedVisitorMap::None
|
||||
}
|
||||
|
||||
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
|
||||
match e.kind {
|
||||
ExprKind::Break(..) | ExprKind::Continue(_) | ExprKind::Ret(_) => {
|
||||
self.make_closure = false;
|
||||
},
|
||||
// Accessing a field of a local value can only be done if the type isn't
|
||||
// partially moved.
|
||||
ExprKind::Field(base_expr, _)
|
||||
if matches!(
|
||||
base_expr.kind,
|
||||
ExprKind::Path(QPath::Resolved(_, Path { res: Res::Local(_), .. }))
|
||||
) && can_partially_move_ty(self.cx, self.cx.typeck_results().expr_ty(base_expr)) =>
|
||||
{
|
||||
// TODO: check if the local has been partially moved. Assume it has for now.
|
||||
self.make_closure = false;
|
||||
return;
|
||||
}
|
||||
_ => (),
|
||||
};
|
||||
walk_expr(self, e);
|
||||
}
|
||||
}
|
||||
|
||||
let mut v = V { cx, make_closure: true };
|
||||
v.visit_expr(expr);
|
||||
v.make_closure
|
||||
}
|
||||
|
||||
// Checks whether the expression could be passed as a function, or whether a closure is needed.
|
||||
// Returns the function to be passed to `map` if it exists.
|
||||
fn can_pass_as_func(cx: &LateContext<'tcx>, binding: Ident, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
|
||||
|
|
|
|||
|
|
@ -456,6 +456,18 @@ pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
|
|||
}
|
||||
}
|
||||
|
||||
/// Checks whether a type can be partially moved.
|
||||
pub fn can_partially_move_ty(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
|
||||
if has_drop(cx, ty) || is_copy(cx, ty) {
|
||||
return false;
|
||||
}
|
||||
match ty.kind() {
|
||||
ty::Param(_) => false,
|
||||
ty::Adt(def, subs) => def.all_fields().any(|f| !is_copy(cx, f.ty(cx.tcx, subs))),
|
||||
_ => true,
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the method names and argument list of nested method call expressions that make up
|
||||
/// `expr`. method/span lists are sorted with the most recent call first.
|
||||
pub fn method_calls<'tcx>(
|
||||
|
|
|
|||
|
|
@ -1,7 +1,13 @@
|
|||
// run-rustfix
|
||||
|
||||
#![warn(clippy::manual_map)]
|
||||
#![allow(clippy::no_effect, clippy::map_identity, clippy::unit_arg, clippy::match_ref_pats)]
|
||||
#![allow(
|
||||
clippy::no_effect,
|
||||
clippy::map_identity,
|
||||
clippy::unit_arg,
|
||||
clippy::match_ref_pats,
|
||||
dead_code
|
||||
)]
|
||||
|
||||
fn main() {
|
||||
Some(0).map(|_| 2);
|
||||
|
|
@ -67,4 +73,41 @@ fn main() {
|
|||
Some(Some((x, 1))) => Some(x),
|
||||
_ => None,
|
||||
};
|
||||
|
||||
// #6795
|
||||
fn f1() -> Result<(), ()> {
|
||||
let _ = match Some(Ok(())) {
|
||||
Some(x) => Some(x?),
|
||||
None => None,
|
||||
};
|
||||
Ok(())
|
||||
}
|
||||
|
||||
for &x in Some(Some(true)).iter() {
|
||||
let _ = match x {
|
||||
Some(x) => Some(if x { continue } else { x }),
|
||||
None => None,
|
||||
};
|
||||
}
|
||||
|
||||
// #6797
|
||||
let x1 = (Some(String::new()), 0);
|
||||
let x2 = x1.0;
|
||||
match x2 {
|
||||
Some(x) => Some((x, x1.1)),
|
||||
None => None,
|
||||
};
|
||||
|
||||
struct S1 {
|
||||
x: Option<String>,
|
||||
y: u32,
|
||||
}
|
||||
impl S1 {
|
||||
fn f(self) -> Option<(String, u32)> {
|
||||
match self.x {
|
||||
Some(x) => Some((x, self.y)),
|
||||
None => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,7 +1,13 @@
|
|||
// run-rustfix
|
||||
|
||||
#![warn(clippy::manual_map)]
|
||||
#![allow(clippy::no_effect, clippy::map_identity, clippy::unit_arg, clippy::match_ref_pats)]
|
||||
#![allow(
|
||||
clippy::no_effect,
|
||||
clippy::map_identity,
|
||||
clippy::unit_arg,
|
||||
clippy::match_ref_pats,
|
||||
dead_code
|
||||
)]
|
||||
|
||||
fn main() {
|
||||
match Some(0) {
|
||||
|
|
@ -119,4 +125,41 @@ fn main() {
|
|||
Some(Some((x, 1))) => Some(x),
|
||||
_ => None,
|
||||
};
|
||||
|
||||
// #6795
|
||||
fn f1() -> Result<(), ()> {
|
||||
let _ = match Some(Ok(())) {
|
||||
Some(x) => Some(x?),
|
||||
None => None,
|
||||
};
|
||||
Ok(())
|
||||
}
|
||||
|
||||
for &x in Some(Some(true)).iter() {
|
||||
let _ = match x {
|
||||
Some(x) => Some(if x { continue } else { x }),
|
||||
None => None,
|
||||
};
|
||||
}
|
||||
|
||||
// #6797
|
||||
let x1 = (Some(String::new()), 0);
|
||||
let x2 = x1.0;
|
||||
match x2 {
|
||||
Some(x) => Some((x, x1.1)),
|
||||
None => None,
|
||||
};
|
||||
|
||||
struct S1 {
|
||||
x: Option<String>,
|
||||
y: u32,
|
||||
}
|
||||
impl S1 {
|
||||
fn f(self) -> Option<(String, u32)> {
|
||||
match self.x {
|
||||
Some(x) => Some((x, self.y)),
|
||||
None => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:7:5
|
||||
--> $DIR/manual_map_option.rs:13:5
|
||||
|
|
||||
LL | / match Some(0) {
|
||||
LL | | Some(_) => Some(2),
|
||||
|
|
@ -10,7 +10,7 @@ LL | | };
|
|||
= note: `-D clippy::manual-map` implied by `-D warnings`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:12:5
|
||||
--> $DIR/manual_map_option.rs:18:5
|
||||
|
|
||||
LL | / match Some(0) {
|
||||
LL | | Some(x) => Some(x + 1),
|
||||
|
|
@ -19,7 +19,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some(0).map(|x| x + 1)`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:17:5
|
||||
--> $DIR/manual_map_option.rs:23:5
|
||||
|
|
||||
LL | / match Some("") {
|
||||
LL | | Some(x) => Some(x.is_empty()),
|
||||
|
|
@ -28,7 +28,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some("").map(|x| x.is_empty())`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:22:5
|
||||
--> $DIR/manual_map_option.rs:28:5
|
||||
|
|
||||
LL | / if let Some(x) = Some(0) {
|
||||
LL | | Some(!x)
|
||||
|
|
@ -38,7 +38,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some(0).map(|x| !x)`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:29:5
|
||||
--> $DIR/manual_map_option.rs:35:5
|
||||
|
|
||||
LL | / match Some(0) {
|
||||
LL | | Some(x) => { Some(std::convert::identity(x)) }
|
||||
|
|
@ -47,7 +47,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some(0).map(std::convert::identity)`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:34:5
|
||||
--> $DIR/manual_map_option.rs:40:5
|
||||
|
|
||||
LL | / match Some(&String::new()) {
|
||||
LL | | Some(x) => Some(str::len(x)),
|
||||
|
|
@ -56,7 +56,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some(&String::new()).map(|x| str::len(x))`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:44:5
|
||||
--> $DIR/manual_map_option.rs:50:5
|
||||
|
|
||||
LL | / match &Some([0, 1]) {
|
||||
LL | | Some(x) => Some(x[0]),
|
||||
|
|
@ -65,7 +65,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some([0, 1]).as_ref().map(|x| x[0])`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:49:5
|
||||
--> $DIR/manual_map_option.rs:55:5
|
||||
|
|
||||
LL | / match &Some(0) {
|
||||
LL | | &Some(x) => Some(x * 2),
|
||||
|
|
@ -74,7 +74,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some(0).map(|x| x * 2)`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:54:5
|
||||
--> $DIR/manual_map_option.rs:60:5
|
||||
|
|
||||
LL | / match Some(String::new()) {
|
||||
LL | | Some(ref x) => Some(x.is_empty()),
|
||||
|
|
@ -83,7 +83,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some(String::new()).as_ref().map(|x| x.is_empty())`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:59:5
|
||||
--> $DIR/manual_map_option.rs:65:5
|
||||
|
|
||||
LL | / match &&Some(String::new()) {
|
||||
LL | | Some(x) => Some(x.len()),
|
||||
|
|
@ -92,7 +92,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some(String::new()).as_ref().map(|x| x.len())`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:64:5
|
||||
--> $DIR/manual_map_option.rs:70:5
|
||||
|
|
||||
LL | / match &&Some(0) {
|
||||
LL | | &&Some(x) => Some(x + x),
|
||||
|
|
@ -101,7 +101,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some(0).map(|x| x + x)`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:77:9
|
||||
--> $DIR/manual_map_option.rs:83:9
|
||||
|
|
||||
LL | / match &mut Some(String::new()) {
|
||||
LL | | Some(x) => Some(x.push_str("")),
|
||||
|
|
@ -110,7 +110,7 @@ LL | | };
|
|||
| |_________^ help: try this: `Some(String::new()).as_mut().map(|x| x.push_str(""))`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:83:5
|
||||
--> $DIR/manual_map_option.rs:89:5
|
||||
|
|
||||
LL | / match &mut Some(String::new()) {
|
||||
LL | | Some(ref x) => Some(x.len()),
|
||||
|
|
@ -119,7 +119,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some(String::new()).as_ref().map(|x| x.len())`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:88:5
|
||||
--> $DIR/manual_map_option.rs:94:5
|
||||
|
|
||||
LL | / match &mut &Some(String::new()) {
|
||||
LL | | Some(x) => Some(x.is_empty()),
|
||||
|
|
@ -128,7 +128,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some(String::new()).as_ref().map(|x| x.is_empty())`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:93:5
|
||||
--> $DIR/manual_map_option.rs:99:5
|
||||
|
|
||||
LL | / match Some((0, 1, 2)) {
|
||||
LL | | Some((x, y, z)) => Some(x + y + z),
|
||||
|
|
@ -137,7 +137,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some((0, 1, 2)).map(|(x, y, z)| x + y + z)`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:98:5
|
||||
--> $DIR/manual_map_option.rs:104:5
|
||||
|
|
||||
LL | / match Some([1, 2, 3]) {
|
||||
LL | | Some([first, ..]) => Some(first),
|
||||
|
|
@ -146,7 +146,7 @@ LL | | };
|
|||
| |_____^ help: try this: `Some([1, 2, 3]).map(|[first, ..]| first)`
|
||||
|
||||
error: manual implementation of `Option::map`
|
||||
--> $DIR/manual_map_option.rs:103:5
|
||||
--> $DIR/manual_map_option.rs:109:5
|
||||
|
|
||||
LL | / match &Some((String::new(), "test")) {
|
||||
LL | | Some((x, y)) => Some((y, x)),
|
||||
|
|
|
|||
Loading…
Reference in a new issue