diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index dfc79ac36..34ea014dd 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -293,10 +293,11 @@ fn lint_same_then_else<'tcx>( /// The return tuple is structured as follows: /// 1. The amount of equal statements from the start /// 2. The amount of equal statements from the end -/// 3. An indication if the block expressions are the same. This will also be true if both are `None` +/// 3. An indication if the block expressions are the same. This will also be true if both are +/// `None` /// -/// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `(0, 0, false)` -/// to aboard any further processing and avoid duplicate lint triggers. +/// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `(0, 0, +/// false)` to aboard any further processing and avoid duplicate lint triggers. fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> (usize, usize, bool) { let mut start_eq = usize::MAX; let mut end_eq = usize::MAX; @@ -307,7 +308,7 @@ fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> (usize, // `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752. // The comparison therefore needs to be done in a way that builds the correct context. - let mut evaluator = SpanlessEq::new(cx); + let mut evaluator = SpanlessEq::new(cx).enable_check_inferred_local_types(); let mut evaluator = evaluator.inter_expr(); let current_start_eq = count_eq(&mut l_stmts.iter(), &mut r_stmts.iter(), |l, r| evaluator.eq_stmt(l, r)); diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index f695f1a61..97db58ae1 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -1,6 +1,7 @@ use crate::consts::{constant_context, constant_simple}; use crate::differing_macro_contexts; use crate::source::snippet_opt; +use if_chain::if_chain; use rustc_ast::ast::InlineAsmTemplatePiece; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_hir::def::Res; @@ -29,6 +30,30 @@ pub struct SpanlessEq<'a, 'tcx> { maybe_typeck_results: Option<&'tcx TypeckResults<'tcx>>, allow_side_effects: bool, expr_fallback: Option, &Expr<'_>) -> bool + 'a>>, + /// This adds an additional type comparison to locals that insures that even the + /// inferred of the value is the same. + /// + /// **Example** + /// * Context 1 + /// ```ignore + /// let vec = Vec::new(); + /// vec.push("A string"); + /// ``` + /// + /// * Context 2 + /// ```ignore + /// let vec = Vec::new(); + /// vec.push(0); // An integer + /// ``` + /// + /// Only comparing the first local definition would usually return that they are + /// equal, since they are identical. However, they are different due to the context + /// as they have different inferred types. + /// + /// This option enables or disables the specific check of the inferred type. + /// + /// Note: This check will only be done if `self.maybe_typeck_results` is `Some()`. + check_inferred_local_types: bool, } impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { @@ -38,6 +63,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { maybe_typeck_results: cx.maybe_typeck_results(), allow_side_effects: true, expr_fallback: None, + check_inferred_local_types: false, } } @@ -56,6 +82,13 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { } } + pub fn enable_check_inferred_local_types(self) -> Self { + Self { + check_inferred_local_types: true, + ..self + } + } + /// Use this method to wrap comparisons that may involve inter-expression context. /// See `self.locals`. pub fn inter_expr(&mut self) -> HirEqInterExpr<'_, 'a, 'tcx> { @@ -96,6 +129,21 @@ impl HirEqInterExpr<'_, '_, '_> { pub fn eq_stmt(&mut self, left: &Stmt<'_>, right: &Stmt<'_>) -> bool { match (&left.kind, &right.kind) { (&StmtKind::Local(ref l), &StmtKind::Local(ref r)) => { + // See `SpanlessEq::check_inferred_local_types` for an explication of this check + if_chain! { + if l.ty.is_none() && r.ty.is_none(); + if self.inner.check_inferred_local_types; + if let Some(tcx) = self.inner.maybe_typeck_results; + + // Check the inferred types + let l_ty = tcx.pat_ty(&l.pat); + let r_ty = tcx.pat_ty(&r.pat); + if l_ty != r_ty; + then { + return false; + } + } + // eq_pat adds the HirIds to the locals map. We therefor call it last to make sure that // these only get added if the init and type is equal. both(&l.init, &r.init, |l, r| self.eq_expr(l, r)) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 1ef5d9d32..280bde73e 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1212,7 +1212,7 @@ pub fn parent_node_is_if_expr(expr: &Expr<'_>, cx: &LateContext<'_>) -> bool { let map = cx.tcx.hir(); let parent_id = map.get_parent_node(expr.hir_id); let parent_node = map.get(parent_id); - + // Check for `if` if_chain! { if let Node::Expr(expr) = parent_node; diff --git a/tests/ui/branches_sharing_code/shared_at_top.rs b/tests/ui/branches_sharing_code/shared_at_top.rs index e65bcfd78..51a464813 100644 --- a/tests/ui/branches_sharing_code/shared_at_top.rs +++ b/tests/ui/branches_sharing_code/shared_at_top.rs @@ -100,4 +100,15 @@ fn check_if_same_than_else_mask() { } } +#[allow(clippy::vec_init_then_push)] +fn pf_local_with_inferred_type_issue7053() { + if true { + let mut v = Vec::new(); + v.push(0); + } else { + let mut v = Vec::new(); + v.push(""); + }; +} + fn main() {}