Skip to content

Commit 3b28745

Browse files
committed
Added inferred local type comparion to SpanlessEq
1 parent 319a52a commit 3b28745

File tree

4 files changed

+65
-5
lines changed

4 files changed

+65
-5
lines changed

clippy_lints/src/copies.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -293,10 +293,11 @@ fn lint_same_then_else<'tcx>(
293293
/// The return tuple is structured as follows:
294294
/// 1. The amount of equal statements from the start
295295
/// 2. The amount of equal statements from the end
296-
/// 3. An indication if the block expressions are the same. This will also be true if both are `None`
296+
/// 3. An indication if the block expressions are the same. This will also be true if both are
297+
/// `None`
297298
///
298-
/// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `(0, 0, false)`
299-
/// to aboard any further processing and avoid duplicate lint triggers.
299+
/// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `(0, 0,
300+
/// false)` to aboard any further processing and avoid duplicate lint triggers.
300301
fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> (usize, usize, bool) {
301302
let mut start_eq = usize::MAX;
302303
let mut end_eq = usize::MAX;
@@ -307,7 +308,7 @@ fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> (usize,
307308

308309
// `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752.
309310
// The comparison therefore needs to be done in a way that builds the correct context.
310-
let mut evaluator = SpanlessEq::new(cx);
311+
let mut evaluator = SpanlessEq::new(cx).enable_check_inferred_local_types();
311312
let mut evaluator = evaluator.inter_expr();
312313

313314
let current_start_eq = count_eq(&mut l_stmts.iter(), &mut r_stmts.iter(), |l, r| evaluator.eq_stmt(l, r));

clippy_utils/src/hir_utils.rs

+48
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::consts::{constant_context, constant_simple};
22
use crate::differing_macro_contexts;
33
use crate::source::snippet_opt;
4+
use if_chain::if_chain;
45
use rustc_ast::ast::InlineAsmTemplatePiece;
56
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
67
use rustc_hir::def::Res;
@@ -29,6 +30,30 @@ pub struct SpanlessEq<'a, 'tcx> {
2930
maybe_typeck_results: Option<&'tcx TypeckResults<'tcx>>,
3031
allow_side_effects: bool,
3132
expr_fallback: Option<Box<dyn FnMut(&Expr<'_>, &Expr<'_>) -> bool + 'a>>,
33+
/// This adds an additional type comparison to locals that insures that even the
34+
/// inferred of the value is the same.
35+
///
36+
/// **Example**
37+
/// * Context 1
38+
/// ```ignore
39+
/// let vec = Vec::new();
40+
/// vec.push("A string");
41+
/// ```
42+
///
43+
/// * Context 2
44+
/// ```ignore
45+
/// let vec = Vec::new();
46+
/// vec.push(0); // An integer
47+
/// ```
48+
///
49+
/// Only comparing the first local definition would usually return that they are
50+
/// equal, since they are identical. However, they are different due to the context
51+
/// as they have different inferred types.
52+
///
53+
/// This option enables or disables the specific check of the inferred type.
54+
///
55+
/// Note: This check will only be done if `self.maybe_typeck_results` is `Some()`.
56+
check_inferred_local_types: bool,
3257
}
3358

3459
impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
@@ -38,6 +63,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
3863
maybe_typeck_results: cx.maybe_typeck_results(),
3964
allow_side_effects: true,
4065
expr_fallback: None,
66+
check_inferred_local_types: false,
4167
}
4268
}
4369

@@ -56,6 +82,13 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
5682
}
5783
}
5884

85+
pub fn enable_check_inferred_local_types(self) -> Self {
86+
Self {
87+
check_inferred_local_types: true,
88+
..self
89+
}
90+
}
91+
5992
/// Use this method to wrap comparisons that may involve inter-expression context.
6093
/// See `self.locals`.
6194
pub fn inter_expr(&mut self) -> HirEqInterExpr<'_, 'a, 'tcx> {
@@ -96,6 +129,21 @@ impl HirEqInterExpr<'_, '_, '_> {
96129
pub fn eq_stmt(&mut self, left: &Stmt<'_>, right: &Stmt<'_>) -> bool {
97130
match (&left.kind, &right.kind) {
98131
(&StmtKind::Local(ref l), &StmtKind::Local(ref r)) => {
132+
// See `SpanlessEq::check_inferred_local_types` for an explication of this check
133+
if_chain! {
134+
if l.ty.is_none() && r.ty.is_none();
135+
if self.inner.check_inferred_local_types;
136+
if let Some(tcx) = self.inner.maybe_typeck_results;
137+
138+
// Check the inferred types
139+
let l_ty = tcx.pat_ty(&l.pat);
140+
let r_ty = tcx.pat_ty(&r.pat);
141+
if l_ty != r_ty;
142+
then {
143+
return false;
144+
}
145+
}
146+
99147
// eq_pat adds the HirIds to the locals map. We therefor call it last to make sure that
100148
// these only get added if the init and type is equal.
101149
both(&l.init, &r.init, |l, r| self.eq_expr(l, r))

clippy_utils/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1195,7 +1195,7 @@ pub fn parent_node_is_if_expr(expr: &Expr<'_>, cx: &LateContext<'_>) -> bool {
11951195
let map = cx.tcx.hir();
11961196
let parent_id = map.get_parent_node(expr.hir_id);
11971197
let parent_node = map.get(parent_id);
1198-
1198+
11991199
// Check for `if`
12001200
if_chain! {
12011201
if let Node::Expr(expr) = parent_node;

tests/ui/branches_sharing_code/shared_at_top.rs

+11
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,15 @@ fn check_if_same_than_else_mask() {
100100
}
101101
}
102102

103+
#[allow(clippy::vec_init_then_push)]
104+
fn pf_local_with_inferred_type_issue7053() {
105+
if true {
106+
let mut v = Vec::new();
107+
v.push(0);
108+
} else {
109+
let mut v = Vec::new();
110+
v.push("");
111+
};
112+
}
113+
103114
fn main() {}

0 commit comments

Comments
 (0)