Move unnecessary_filter_map to a submodule

This commit is contained in:
Michael Wright 2018-09-29 14:12:40 +02:00
parent db5c63b77a
commit efdc739dfc
2 changed files with 148 additions and 140 deletions

View file

@ -1,6 +1,5 @@
use crate::rustc::hir; use crate::rustc::hir;
use crate::rustc::hir::def::Def; use crate::rustc::hir::def::Def;
use crate::rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use crate::rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass}; use crate::rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass};
use crate::rustc::ty::{self, Ty}; use crate::rustc::ty::{self, Ty};
use crate::rustc::{declare_tool_lint, lint_array}; use crate::rustc::{declare_tool_lint, lint_array};
@ -9,7 +8,6 @@ use crate::syntax::ast;
use crate::syntax::source_map::{BytePos, Span}; use crate::syntax::source_map::{BytePos, Span};
use crate::utils::paths; use crate::utils::paths;
use crate::utils::sugg; use crate::utils::sugg;
use crate::utils::usage::mutated_variables;
use crate::utils::{ use crate::utils::{
get_arg_name, get_trait_def_id, implements_trait, in_macro, is_copy, is_expn_of, is_self, is_self_ty, get_arg_name, get_trait_def_id, implements_trait, in_macro, is_copy, is_expn_of, is_self, is_self_ty,
iter_input_pats, last_path_segment, match_def_path, match_path, match_qpath, match_trait_method, match_type, iter_input_pats, last_path_segment, match_def_path, match_path, match_qpath, match_trait_method, match_type,
@ -22,6 +20,8 @@ use std::borrow::Cow;
use std::fmt; use std::fmt;
use std::iter; use std::iter;
mod unnecessary_filter_map;
#[derive(Clone)] #[derive(Clone)]
pub struct Pass; pub struct Pass;
@ -1424,144 +1424,6 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args:
}; };
} }
mod unnecessary_filter_map {
use super::*;
pub(super) fn lint(cx: &LateContext<'_, '_>, expr: &hir::Expr, args: &[hir::Expr]) {
if !match_trait_method(cx, expr, &paths::ITERATOR) {
return;
}
if let hir::ExprKind::Closure(_, _, body_id, ..) = args[1].node {
let body = cx.tcx.hir.body(body_id);
let arg_id = body.arguments[0].pat.id;
let mutates_arg = match mutated_variables(&body.value, cx) {
Some(used_mutably) => used_mutably.contains(&arg_id),
None => true,
};
let (mut found_mapping, mut found_filtering) = check_expression(&cx, arg_id, &body.value);
let mut return_visitor = ReturnVisitor::new(&cx, arg_id);
return_visitor.visit_expr(&body.value);
found_mapping |= return_visitor.found_mapping;
found_filtering |= return_visitor.found_filtering;
if !found_filtering {
span_lint(
cx,
UNNECESSARY_FILTER_MAP,
expr.span,
"this `.filter_map` can be written more simply using `.map`",
);
return;
}
if !found_mapping && !mutates_arg {
span_lint(
cx,
UNNECESSARY_FILTER_MAP,
expr.span,
"this `.filter_map` can be written more simply using `.filter`",
);
return;
}
}
}
// returns (found_mapping, found_filtering)
fn check_expression<'a, 'tcx: 'a>(cx: &'a LateContext<'a, 'tcx>, arg_id: ast::NodeId, expr: &'tcx hir::Expr) -> (bool, bool) {
match &expr.node {
hir::ExprKind::Call(ref func, ref args) => {
if_chain! {
if let hir::ExprKind::Path(ref path) = func.node;
then {
if match_qpath(path, &paths::OPTION_SOME) {
if_chain! {
if let hir::ExprKind::Path(path) = &args[0].node;
if let Def::Local(ref local) = cx.tables.qpath_def(path, args[0].hir_id);
then {
if arg_id == *local {
return (false, false)
}
}
}
return (true, false);
} else {
// We don't know. It might do anything.
return (true, true);
}
}
}
(true, true)
},
hir::ExprKind::Block(ref block, _) => {
if let Some(expr) = &block.expr {
check_expression(cx, arg_id, &expr)
} else {
(false, false)
}
},
// There must be an else_arm or there will be a type error
hir::ExprKind::If(_, ref if_arm, Some(ref else_arm)) => {
let if_check = check_expression(cx, arg_id, if_arm);
let else_check = check_expression(cx, arg_id, else_arm);
(if_check.0 | else_check.0, if_check.1 | else_check.1)
},
hir::ExprKind::Match(_, ref arms, _) => {
let mut found_mapping = false;
let mut found_filtering = false;
for arm in arms {
let (m, f) = check_expression(cx, arg_id, &arm.body);
found_mapping |= m;
found_filtering |= f;
}
(found_mapping, found_filtering)
},
hir::ExprKind::Path(path) if match_qpath(path, &paths::OPTION_NONE) => (false, true),
_ => (true, true)
}
}
struct ReturnVisitor<'a, 'tcx: 'a> {
cx: &'a LateContext<'a, 'tcx>,
arg_id: ast::NodeId,
// Found a non-None return that isn't Some(input)
found_mapping: bool,
// Found a return that isn't Some
found_filtering: bool,
}
impl<'a, 'tcx: 'a> ReturnVisitor<'a, 'tcx> {
fn new(cx: &'a LateContext<'a, 'tcx>, arg_id: ast::NodeId) -> ReturnVisitor<'a, 'tcx> {
ReturnVisitor {
cx,
arg_id,
found_mapping: false,
found_filtering: false,
}
}
}
impl<'a, 'tcx> Visitor<'tcx> for ReturnVisitor<'a, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
if let hir::ExprKind::Ret(Some(expr)) = &expr.node {
let (found_mapping, found_filtering) = check_expression(self.cx, self.arg_id, expr);
self.found_mapping |= found_mapping;
self.found_filtering |= found_filtering;
} else {
walk_expr(self, expr);
}
}
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
NestedVisitorMap::None
}
}
}
fn lint_iter_nth(cx: &LateContext<'_, '_>, expr: &hir::Expr, iter_args: &[hir::Expr], is_mut: bool) { fn lint_iter_nth(cx: &LateContext<'_, '_>, expr: &hir::Expr, iter_args: &[hir::Expr], is_mut: bool) {
let mut_str = if is_mut { "_mut" } else { "" }; let mut_str = if is_mut { "_mut" } else { "" };
let caller_type = if derefs_to_slice(cx, &iter_args[0], cx.tables.expr_ty(&iter_args[0])).is_some() { let caller_type = if derefs_to_slice(cx, &iter_args[0], cx.tables.expr_ty(&iter_args[0])).is_some() {

View file

@ -0,0 +1,146 @@
use crate::rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use crate::rustc::lint::LateContext;
use crate::rustc::hir;
use crate::rustc::hir::def::Def;
use crate::syntax::ast;
use crate::utils::{match_qpath, match_trait_method, span_lint};
use crate::utils::paths;
use crate::utils::usage::mutated_variables;
use if_chain::if_chain;
use super::UNNECESSARY_FILTER_MAP;
pub(super) fn lint(cx: &LateContext<'_, '_>, expr: &hir::Expr, args: &[hir::Expr]) {
if !match_trait_method(cx, expr, &paths::ITERATOR) {
return;
}
if let hir::ExprKind::Closure(_, _, body_id, ..) = args[1].node {
let body = cx.tcx.hir.body(body_id);
let arg_id = body.arguments[0].pat.id;
let mutates_arg = match mutated_variables(&body.value, cx) {
Some(used_mutably) => used_mutably.contains(&arg_id),
None => true,
};
let (mut found_mapping, mut found_filtering) = check_expression(&cx, arg_id, &body.value);
let mut return_visitor = ReturnVisitor::new(&cx, arg_id);
return_visitor.visit_expr(&body.value);
found_mapping |= return_visitor.found_mapping;
found_filtering |= return_visitor.found_filtering;
if !found_filtering {
span_lint(
cx,
UNNECESSARY_FILTER_MAP,
expr.span,
"this `.filter_map` can be written more simply using `.map`",
);
return;
}
if !found_mapping && !mutates_arg {
span_lint(
cx,
UNNECESSARY_FILTER_MAP,
expr.span,
"this `.filter_map` can be written more simply using `.filter`",
);
return;
}
}
}
// returns (found_mapping, found_filtering)
fn check_expression<'a, 'tcx: 'a>(cx: &'a LateContext<'a, 'tcx>, arg_id: ast::NodeId, expr: &'tcx hir::Expr) -> (bool, bool) {
match &expr.node {
hir::ExprKind::Call(ref func, ref args) => {
if_chain! {
if let hir::ExprKind::Path(ref path) = func.node;
then {
if match_qpath(path, &paths::OPTION_SOME) {
if_chain! {
if let hir::ExprKind::Path(path) = &args[0].node;
if let Def::Local(ref local) = cx.tables.qpath_def(path, args[0].hir_id);
then {
if arg_id == *local {
return (false, false)
}
}
}
return (true, false);
} else {
// We don't know. It might do anything.
return (true, true);
}
}
}
(true, true)
},
hir::ExprKind::Block(ref block, _) => {
if let Some(expr) = &block.expr {
check_expression(cx, arg_id, &expr)
} else {
(false, false)
}
},
// There must be an else_arm or there will be a type error
hir::ExprKind::If(_, ref if_arm, Some(ref else_arm)) => {
let if_check = check_expression(cx, arg_id, if_arm);
let else_check = check_expression(cx, arg_id, else_arm);
(if_check.0 | else_check.0, if_check.1 | else_check.1)
},
hir::ExprKind::Match(_, ref arms, _) => {
let mut found_mapping = false;
let mut found_filtering = false;
for arm in arms {
let (m, f) = check_expression(cx, arg_id, &arm.body);
found_mapping |= m;
found_filtering |= f;
}
(found_mapping, found_filtering)
},
hir::ExprKind::Path(path) if match_qpath(path, &paths::OPTION_NONE) => (false, true),
_ => (true, true)
}
}
struct ReturnVisitor<'a, 'tcx: 'a> {
cx: &'a LateContext<'a, 'tcx>,
arg_id: ast::NodeId,
// Found a non-None return that isn't Some(input)
found_mapping: bool,
// Found a return that isn't Some
found_filtering: bool,
}
impl<'a, 'tcx: 'a> ReturnVisitor<'a, 'tcx> {
fn new(cx: &'a LateContext<'a, 'tcx>, arg_id: ast::NodeId) -> ReturnVisitor<'a, 'tcx> {
ReturnVisitor {
cx,
arg_id,
found_mapping: false,
found_filtering: false,
}
}
}
impl<'a, 'tcx> Visitor<'tcx> for ReturnVisitor<'a, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
if let hir::ExprKind::Ret(Some(expr)) = &expr.node {
let (found_mapping, found_filtering) = check_expression(self.cx, self.arg_id, expr);
self.found_mapping |= found_mapping;
self.found_filtering |= found_filtering;
} else {
walk_expr(self, expr);
}
}
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
NestedVisitorMap::None
}
}