mirror of
https://github.com/rust-lang/rust-clippy
synced 2025-02-17 22:48:48 +00:00
Add detect_same_item_push to its own module
This commit is contained in:
nahuakang
Yoshitomo Nakanishi
parent
453e6b97ac
commit
2c1f676bfe
2 changed files with 174 additions and 165 deletions
|
|
@ -7,6 +7,7 @@ mod for_single_element_loop;
|
|||
mod infinite_loop;
|
||||
mod manual_flatten;
|
||||
mod needless_collect;
|
||||
mod same_item_push;
|
||||
mod utils;
|
||||
|
||||
use crate::utils::sugg::Sugg;
|
||||
|
|
@ -14,18 +15,17 @@ use crate::utils::usage::mutated_variables;
|
|||
use crate::utils::{
|
||||
get_enclosing_block, get_parent_expr, get_trait_def_id, higher, implements_trait, is_in_panic_handler,
|
||||
is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item, last_path_segment, match_trait_method,
|
||||
path_to_local, path_to_local_id, paths, snippet, snippet_with_applicability, snippet_with_macro_callsite,
|
||||
span_lint, span_lint_and_help, span_lint_and_sugg, sugg,
|
||||
path_to_local, path_to_local_id, paths, snippet, snippet_with_applicability, span_lint, span_lint_and_help,
|
||||
span_lint_and_sugg, sugg,
|
||||
};
|
||||
use if_chain::if_chain;
|
||||
use rustc_ast::ast;
|
||||
use rustc_data_structures::fx::FxHashMap;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::def::{DefKind, Res};
|
||||
use rustc_hir::intravisit::{walk_block, walk_expr, walk_pat, walk_stmt, NestedVisitorMap, Visitor};
|
||||
use rustc_hir::{
|
||||
BinOpKind, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, HirId, InlineAsmOperand, LoopSource, MatchSource,
|
||||
Mutability, Node, Pat, PatKind, Stmt, StmtKind,
|
||||
BinOpKind, Block, BorrowKind, Expr, ExprKind, HirId, InlineAsmOperand, LoopSource, MatchSource, Mutability, Node,
|
||||
Pat, PatKind, Stmt, StmtKind,
|
||||
};
|
||||
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
||||
use rustc_middle::hir::map::Map;
|
||||
|
|
@ -866,7 +866,7 @@ fn check_for_loop<'tcx>(
|
|||
for_loop_over_map_kv::check_for_loop_over_map_kv(cx, pat, arg, body, expr);
|
||||
for_mut_range_bound::check_for_mut_range_bound(cx, arg, body);
|
||||
for_single_element_loop::check_for_single_element_loop(cx, pat, arg, body, expr);
|
||||
detect_same_item_push(cx, pat, arg, body, expr);
|
||||
same_item_push::detect_same_item_push(cx, pat, arg, body, expr);
|
||||
manual_flatten::check_manual_flatten(cx, pat, arg, body, span);
|
||||
}
|
||||
|
||||
|
|
@ -1307,165 +1307,6 @@ fn detect_manual_memcpy<'tcx>(
|
|||
false
|
||||
}
|
||||
|
||||
// Scans the body of the for loop and determines whether lint should be given
|
||||
struct SameItemPushVisitor<'a, 'tcx> {
|
||||
should_lint: bool,
|
||||
// this field holds the last vec push operation visited, which should be the only push seen
|
||||
vec_push: Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>,
|
||||
cx: &'a LateContext<'tcx>,
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> Visitor<'tcx> for SameItemPushVisitor<'a, 'tcx> {
|
||||
type Map = Map<'tcx>;
|
||||
|
||||
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
|
||||
match &expr.kind {
|
||||
// Non-determinism may occur ... don't give a lint
|
||||
ExprKind::Loop(..) | ExprKind::Match(..) => self.should_lint = false,
|
||||
ExprKind::Block(block, _) => self.visit_block(block),
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
|
||||
fn visit_block(&mut self, b: &'tcx Block<'_>) {
|
||||
for stmt in b.stmts.iter() {
|
||||
self.visit_stmt(stmt);
|
||||
}
|
||||
}
|
||||
|
||||
fn visit_stmt(&mut self, s: &'tcx Stmt<'_>) {
|
||||
let vec_push_option = get_vec_push(self.cx, s);
|
||||
if vec_push_option.is_none() {
|
||||
// Current statement is not a push so visit inside
|
||||
match &s.kind {
|
||||
StmtKind::Expr(expr) | StmtKind::Semi(expr) => self.visit_expr(&expr),
|
||||
_ => {},
|
||||
}
|
||||
} else {
|
||||
// Current statement is a push ...check whether another
|
||||
// push had been previously done
|
||||
if self.vec_push.is_none() {
|
||||
self.vec_push = vec_push_option;
|
||||
} else {
|
||||
// There are multiple pushes ... don't lint
|
||||
self.should_lint = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
|
||||
NestedVisitorMap::None
|
||||
}
|
||||
}
|
||||
|
||||
// Given some statement, determine if that statement is a push on a Vec. If it is, return
|
||||
// the Vec being pushed into and the item being pushed
|
||||
fn get_vec_push<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> {
|
||||
if_chain! {
|
||||
// Extract method being called
|
||||
if let StmtKind::Semi(semi_stmt) = &stmt.kind;
|
||||
if let ExprKind::MethodCall(path, _, args, _) = &semi_stmt.kind;
|
||||
// Figure out the parameters for the method call
|
||||
if let Some(self_expr) = args.get(0);
|
||||
if let Some(pushed_item) = args.get(1);
|
||||
// Check that the method being called is push() on a Vec
|
||||
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr), sym::vec_type);
|
||||
if path.ident.name.as_str() == "push";
|
||||
then {
|
||||
return Some((self_expr, pushed_item))
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Detects for loop pushing the same item into a Vec
|
||||
fn detect_same_item_push<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
pat: &'tcx Pat<'_>,
|
||||
_: &'tcx Expr<'_>,
|
||||
body: &'tcx Expr<'_>,
|
||||
_: &'tcx Expr<'_>,
|
||||
) {
|
||||
fn emit_lint(cx: &LateContext<'_>, vec: &Expr<'_>, pushed_item: &Expr<'_>) {
|
||||
let vec_str = snippet_with_macro_callsite(cx, vec.span, "");
|
||||
let item_str = snippet_with_macro_callsite(cx, pushed_item.span, "");
|
||||
|
||||
span_lint_and_help(
|
||||
cx,
|
||||
SAME_ITEM_PUSH,
|
||||
vec.span,
|
||||
"it looks like the same item is being pushed into this Vec",
|
||||
None,
|
||||
&format!(
|
||||
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
|
||||
item_str, vec_str, item_str
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
if !matches!(pat.kind, PatKind::Wild) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Determine whether it is safe to lint the body
|
||||
let mut same_item_push_visitor = SameItemPushVisitor {
|
||||
should_lint: true,
|
||||
vec_push: None,
|
||||
cx,
|
||||
};
|
||||
walk_expr(&mut same_item_push_visitor, body);
|
||||
if same_item_push_visitor.should_lint {
|
||||
if let Some((vec, pushed_item)) = same_item_push_visitor.vec_push {
|
||||
let vec_ty = cx.typeck_results().expr_ty(vec);
|
||||
let ty = vec_ty.walk().nth(1).unwrap().expect_ty();
|
||||
if cx
|
||||
.tcx
|
||||
.lang_items()
|
||||
.clone_trait()
|
||||
.map_or(false, |id| implements_trait(cx, ty, id, &[]))
|
||||
{
|
||||
// Make sure that the push does not involve possibly mutating values
|
||||
match pushed_item.kind {
|
||||
ExprKind::Path(ref qpath) => {
|
||||
match cx.qpath_res(qpath, pushed_item.hir_id) {
|
||||
// immutable bindings that are initialized with literal or constant
|
||||
Res::Local(hir_id) => {
|
||||
if_chain! {
|
||||
let node = cx.tcx.hir().get(hir_id);
|
||||
if let Node::Binding(pat) = node;
|
||||
if let PatKind::Binding(bind_ann, ..) = pat.kind;
|
||||
if !matches!(bind_ann, BindingAnnotation::RefMut | BindingAnnotation::Mutable);
|
||||
let parent_node = cx.tcx.hir().get_parent_node(hir_id);
|
||||
if let Some(Node::Local(parent_let_expr)) = cx.tcx.hir().find(parent_node);
|
||||
if let Some(init) = parent_let_expr.init;
|
||||
then {
|
||||
match init.kind {
|
||||
// immutable bindings that are initialized with literal
|
||||
ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item),
|
||||
// immutable bindings that are initialized with constant
|
||||
ExprKind::Path(ref path) => {
|
||||
if let Res::Def(DefKind::Const, ..) = cx.qpath_res(path, init.hir_id) {
|
||||
emit_lint(cx, vec, pushed_item);
|
||||
}
|
||||
}
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
// constant
|
||||
Res::Def(DefKind::Const, ..) => emit_lint(cx, vec, pushed_item),
|
||||
_ => {},
|
||||
}
|
||||
},
|
||||
ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item),
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn is_used_inside<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, container: &'tcx Expr<'_>) -> bool {
|
||||
let def_id = match path_to_local(expr) {
|
||||
Some(id) => id,
|
||||
|
|
|
|||
168
clippy_lints/src/loops/same_item_push.rs
Normal file
168
clippy_lints/src/loops/same_item_push.rs
Normal file
|
|
@ -0,0 +1,168 @@
|
|||
use crate::utils::{implements_trait, is_type_diagnostic_item, snippet_with_macro_callsite, span_lint_and_help};
|
||||
use if_chain::if_chain;
|
||||
use rustc_hir::def::{DefKind, Res};
|
||||
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
|
||||
use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, Node, Pat, PatKind, Stmt, StmtKind};
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_middle::hir::map::Map;
|
||||
use rustc_span::symbol::sym;
|
||||
use std::iter::Iterator;
|
||||
|
||||
/// Detects for loop pushing the same item into a Vec
|
||||
pub(super) fn detect_same_item_push<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
pat: &'tcx Pat<'_>,
|
||||
_: &'tcx Expr<'_>,
|
||||
body: &'tcx Expr<'_>,
|
||||
_: &'tcx Expr<'_>,
|
||||
) {
|
||||
fn emit_lint(cx: &LateContext<'_>, vec: &Expr<'_>, pushed_item: &Expr<'_>) {
|
||||
let vec_str = snippet_with_macro_callsite(cx, vec.span, "");
|
||||
let item_str = snippet_with_macro_callsite(cx, pushed_item.span, "");
|
||||
|
||||
span_lint_and_help(
|
||||
cx,
|
||||
super::SAME_ITEM_PUSH,
|
||||
vec.span,
|
||||
"it looks like the same item is being pushed into this Vec",
|
||||
None,
|
||||
&format!(
|
||||
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
|
||||
item_str, vec_str, item_str
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
if !matches!(pat.kind, PatKind::Wild) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Determine whether it is safe to lint the body
|
||||
let mut same_item_push_visitor = SameItemPushVisitor {
|
||||
should_lint: true,
|
||||
vec_push: None,
|
||||
cx,
|
||||
};
|
||||
walk_expr(&mut same_item_push_visitor, body);
|
||||
if same_item_push_visitor.should_lint {
|
||||
if let Some((vec, pushed_item)) = same_item_push_visitor.vec_push {
|
||||
let vec_ty = cx.typeck_results().expr_ty(vec);
|
||||
let ty = vec_ty.walk().nth(1).unwrap().expect_ty();
|
||||
if cx
|
||||
.tcx
|
||||
.lang_items()
|
||||
.clone_trait()
|
||||
.map_or(false, |id| implements_trait(cx, ty, id, &[]))
|
||||
{
|
||||
// Make sure that the push does not involve possibly mutating values
|
||||
match pushed_item.kind {
|
||||
ExprKind::Path(ref qpath) => {
|
||||
match cx.qpath_res(qpath, pushed_item.hir_id) {
|
||||
// immutable bindings that are initialized with literal or constant
|
||||
Res::Local(hir_id) => {
|
||||
if_chain! {
|
||||
let node = cx.tcx.hir().get(hir_id);
|
||||
if let Node::Binding(pat) = node;
|
||||
if let PatKind::Binding(bind_ann, ..) = pat.kind;
|
||||
if !matches!(bind_ann, BindingAnnotation::RefMut | BindingAnnotation::Mutable);
|
||||
let parent_node = cx.tcx.hir().get_parent_node(hir_id);
|
||||
if let Some(Node::Local(parent_let_expr)) = cx.tcx.hir().find(parent_node);
|
||||
if let Some(init) = parent_let_expr.init;
|
||||
then {
|
||||
match init.kind {
|
||||
// immutable bindings that are initialized with literal
|
||||
ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item),
|
||||
// immutable bindings that are initialized with constant
|
||||
ExprKind::Path(ref path) => {
|
||||
if let Res::Def(DefKind::Const, ..) = cx.qpath_res(path, init.hir_id) {
|
||||
emit_lint(cx, vec, pushed_item);
|
||||
}
|
||||
}
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
// constant
|
||||
Res::Def(DefKind::Const, ..) => emit_lint(cx, vec, pushed_item),
|
||||
_ => {},
|
||||
}
|
||||
},
|
||||
ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item),
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Scans the body of the for loop and determines whether lint should be given
|
||||
struct SameItemPushVisitor<'a, 'tcx> {
|
||||
should_lint: bool,
|
||||
// this field holds the last vec push operation visited, which should be the only push seen
|
||||
vec_push: Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>,
|
||||
cx: &'a LateContext<'tcx>,
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> Visitor<'tcx> for SameItemPushVisitor<'a, 'tcx> {
|
||||
type Map = Map<'tcx>;
|
||||
|
||||
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
|
||||
match &expr.kind {
|
||||
// Non-determinism may occur ... don't give a lint
|
||||
ExprKind::Loop(..) | ExprKind::Match(..) => self.should_lint = false,
|
||||
ExprKind::Block(block, _) => self.visit_block(block),
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
|
||||
fn visit_block(&mut self, b: &'tcx Block<'_>) {
|
||||
for stmt in b.stmts.iter() {
|
||||
self.visit_stmt(stmt);
|
||||
}
|
||||
}
|
||||
|
||||
fn visit_stmt(&mut self, s: &'tcx Stmt<'_>) {
|
||||
let vec_push_option = get_vec_push(self.cx, s);
|
||||
if vec_push_option.is_none() {
|
||||
// Current statement is not a push so visit inside
|
||||
match &s.kind {
|
||||
StmtKind::Expr(expr) | StmtKind::Semi(expr) => self.visit_expr(&expr),
|
||||
_ => {},
|
||||
}
|
||||
} else {
|
||||
// Current statement is a push ...check whether another
|
||||
// push had been previously done
|
||||
if self.vec_push.is_none() {
|
||||
self.vec_push = vec_push_option;
|
||||
} else {
|
||||
// There are multiple pushes ... don't lint
|
||||
self.should_lint = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
|
||||
NestedVisitorMap::None
|
||||
}
|
||||
}
|
||||
|
||||
// Given some statement, determine if that statement is a push on a Vec. If it is, return
|
||||
// the Vec being pushed into and the item being pushed
|
||||
fn get_vec_push<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> {
|
||||
if_chain! {
|
||||
// Extract method being called
|
||||
if let StmtKind::Semi(semi_stmt) = &stmt.kind;
|
||||
if let ExprKind::MethodCall(path, _, args, _) = &semi_stmt.kind;
|
||||
// Figure out the parameters for the method call
|
||||
if let Some(self_expr) = args.get(0);
|
||||
if let Some(pushed_item) = args.get(1);
|
||||
// Check that the method being called is push() on a Vec
|
||||
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr), sym::vec_type);
|
||||
if path.ident.name.as_str() == "push";
|
||||
then {
|
||||
return Some((self_expr, pushed_item))
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue