From c44eafdcd7a20bfe1af761d07a7ad8aaf5254d91 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Fri, 5 Feb 2021 16:27:23 -0600 Subject: [PATCH 1/2] Use id instead of name --- clippy_lints/src/vec_init_then_push.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/vec_init_then_push.rs b/clippy_lints/src/vec_init_then_push.rs index e632a7e57ee..4768cb4830b 100644 --- a/clippy_lints/src/vec_init_then_push.rs +++ b/clippy_lints/src/vec_init_then_push.rs @@ -1,12 +1,14 @@ -use crate::utils::{is_type_diagnostic_item, match_def_path, paths, snippet, span_lint_and_sugg}; +use crate::utils::{ + is_type_diagnostic_item, match_def_path, path_to_local, path_to_local_id, paths, snippet, span_lint_and_sugg, +}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, Local, PatKind, QPath, Stmt, StmtKind}; +use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, Local, PatKind, QPath, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::{symbol::sym, Span, Symbol}; +use rustc_span::{symbol::sym, Span}; use std::convert::TryInto; declare_clippy_lint! { @@ -45,8 +47,8 @@ enum VecInitKind { WithCapacity(u64), } struct VecPushSearcher { + local_id: HirId, init: VecInitKind, - name: Symbol, lhs_is_local: bool, lhs_span: Span, err_span: Span, @@ -86,12 +88,12 @@ fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) { if_chain! { if !in_external_macro(cx.sess(), local.span); if let Some(init) = local.init; - if let PatKind::Binding(BindingAnnotation::Mutable, _, ident, None) = local.pat.kind; + if let PatKind::Binding(BindingAnnotation::Mutable, id, _, None) = local.pat.kind; if let Some(init_kind) = get_vec_init_kind(cx, init); then { self.searcher = Some(VecPushSearcher { + local_id: id, init: init_kind, - name: ident.name, lhs_is_local: true, lhs_span: local.ty.map_or(local.pat.span, |t| local.pat.span.to(t.span)), err_span: local.span, @@ -106,13 +108,12 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if_chain! { if !in_external_macro(cx.sess(), expr.span); if let ExprKind::Assign(left, right, _) = expr.kind; - if let ExprKind::Path(QPath::Resolved(_, path)) = left.kind; - if let Some(name) = path.segments.get(0); + if let Some(id) = path_to_local(left); if let Some(init_kind) = get_vec_init_kind(cx, right); then { self.searcher = Some(VecPushSearcher { + local_id: id, init: init_kind, - name: name.ident.name, lhs_is_local: false, lhs_span: left.span, err_span: expr.span, @@ -128,10 +129,8 @@ fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { if_chain! { if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = stmt.kind; if let ExprKind::MethodCall(path, _, [self_arg, _], _) = expr.kind; + if path_to_local_id(self_arg, searcher.local_id); if path.ident.name.as_str() == "push"; - if let ExprKind::Path(QPath::Resolved(_, self_path)) = self_arg.kind; - if let [self_name] = self_path.segments; - if self_name.ident.name == searcher.name; then { self.searcher = Some(VecPushSearcher { found: searcher.found + 1, From a42be8589a0b380da18a78da4c81205ff8086243 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Fri, 5 Feb 2021 16:28:13 -0600 Subject: [PATCH 2/2] Fix vec_init_then_push FP --- clippy_lints/src/vec_init_then_push.rs | 5 ++++- tests/ui/vec_init_then_push.rs | 25 +++++++++++++++++++++++++ tests/ui/vec_init_then_push.stderr | 2 +- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/vec_init_then_push.rs b/clippy_lints/src/vec_init_then_push.rs index 4768cb4830b..8d111f98add 100644 --- a/clippy_lints/src/vec_init_then_push.rs +++ b/clippy_lints/src/vec_init_then_push.rs @@ -83,8 +83,11 @@ fn display_err(&self, cx: &LateContext<'_>) { } impl LateLintPass<'_> for VecInitThenPush { - fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) { + fn check_block(&mut self, _: &LateContext<'tcx>, _: &'tcx Block<'tcx>) { self.searcher = None; + } + + fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) { if_chain! { if !in_external_macro(cx.sess(), local.span); if let Some(init) = local.init; diff --git a/tests/ui/vec_init_then_push.rs b/tests/ui/vec_init_then_push.rs index 642ce504009..5099aad83bc 100644 --- a/tests/ui/vec_init_then_push.rs +++ b/tests/ui/vec_init_then_push.rs @@ -12,10 +12,35 @@ fn main() { cap_err.push(0); cap_err.push(1); cap_err.push(2); + if true { + // don't include this one + cap_err.push(3); + } let mut cap_ok = Vec::with_capacity(10); cap_ok.push(0); new_err = Vec::new(); new_err.push(0); + + let mut vec = Vec::new(); + // control flow at block final expression + if true { + // no lint + vec.push(1); + } +} + +pub fn no_lint() -> Vec { + let mut p = Some(1); + let mut vec = Vec::new(); + loop { + match p { + None => return vec, + Some(i) => { + vec.push(i); + p = None; + }, + } + } } diff --git a/tests/ui/vec_init_then_push.stderr b/tests/ui/vec_init_then_push.stderr index 819ed47d099..9ec3e10e624 100644 --- a/tests/ui/vec_init_then_push.stderr +++ b/tests/ui/vec_init_then_push.stderr @@ -24,7 +24,7 @@ LL | | cap_err.push(2); | |____________________^ help: consider using the `vec![]` macro: `let mut cap_err = vec![..];` error: calls to `push` immediately after creation - --> $DIR/vec_init_then_push.rs:19:5 + --> $DIR/vec_init_then_push.rs:23:5 | LL | / new_err = Vec::new(); LL | | new_err.push(0);