about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-04-16 18:33:45 +0000
committerbors <bors@rust-lang.org>2021-04-16 18:33:45 +0000
commiteaf0f3df1509e8d0312895afbda94cf821b5f4c2 (patch)
treef7aaea2c3b5d7de559bfe6e8628554e33f546124
parent28dbcd85c8a956a1d1e39066668fcf1484080949 (diff)
parent0b4af7257ddea5cf508305635fbdc4c0d1e6bdd5 (diff)
downloadrust-eaf0f3df1509e8d0312895afbda94cf821b5f4c2.tar.gz
rust-eaf0f3df1509e8d0312895afbda94cf821b5f4c2.zip
Auto merge of #7075 - xFrednet:7054-fp-branches-sharing-code, r=camsteffen,flip1995
Fixing FPs for the `branches_sharing_code` lint

Fixes #7053
Fixes #7054
And an additional CSS adjustment to support dark mode for every inline code. It currently only works in paragraphs, which was an oversight on my part :sweat_smile:. [Current Example](https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name)

This also includes ~50 lines of doc comments and is therefor not as big as the changes would indicate. :penguin:

---

changelog: none

All of these bugs were introduced in this dev version and are therefor not worth a change log entry.

r? `@phansch`
cc: `@camsteffen` since you have a pretty good overview of the `SpanlessEq` implementation :upside_down_face:
-rw-r--r--clippy_lints/src/comparison_chain.rs4
-rw-r--r--clippy_lints/src/copies.rs33
-rw-r--r--clippy_lints/src/if_then_some_else_none.rs4
-rw-r--r--clippy_lints/src/needless_bool.rs4
-rw-r--r--clippy_utils/src/hir_utils.rs10
-rw-r--r--clippy_utils/src/lib.rs15
-rw-r--r--tests/ui/branches_sharing_code/shared_at_bottom.rs14
-rw-r--r--tests/ui/branches_sharing_code/shared_at_top.rs11
-rw-r--r--util/gh-pages/index.html2
9 files changed, 69 insertions, 28 deletions
diff --git a/clippy_lints/src/comparison_chain.rs b/clippy_lints/src/comparison_chain.rs
index 31ae63b5184..42e153909ce 100644
--- a/clippy_lints/src/comparison_chain.rs
+++ b/clippy_lints/src/comparison_chain.rs
@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_help;
 use clippy_utils::ty::implements_trait;
-use clippy_utils::{get_trait_def_id, if_sequence, parent_node_is_if_expr, paths, SpanlessEq};
+use clippy_utils::{get_trait_def_id, if_sequence, is_else_clause, paths, SpanlessEq};
 use rustc_hir::{BinOpKind, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -60,7 +60,7 @@ impl<'tcx> LateLintPass<'tcx> for ComparisonChain {
         }
 
         // We only care about the top-most `if` in the chain
-        if parent_node_is_if_expr(expr, cx) {
+        if is_else_clause(cx.tcx, expr) {
             return;
         }
 
diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs
index 8b503c9a030..f956d171bfb 100644
--- a/clippy_lints/src/copies.rs
+++ b/clippy_lints/src/copies.rs
@@ -1,7 +1,7 @@
 use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then};
 use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, snippet_opt};
 use clippy_utils::{
-    both, count_eq, eq_expr_value, get_enclosing_block, get_parent_expr, if_sequence, in_macro, parent_node_is_if_expr,
+    both, count_eq, eq_expr_value, get_enclosing_block, get_parent_expr, if_sequence, in_macro, is_else_clause,
     run_lints, search_same, ContainsName, SpanlessEq, SpanlessHash,
 };
 use if_chain::if_chain;
@@ -188,13 +188,18 @@ fn lint_same_then_else<'tcx>(
     expr: &'tcx Expr<'_>,
 ) {
     // We only lint ifs with multiple blocks
-    if blocks.len() < 2 || parent_node_is_if_expr(expr, cx) {
+    if blocks.len() < 2 || is_else_clause(cx.tcx, expr) {
         return;
     }
 
     // Check if each block has shared code
     let has_expr = blocks[0].expr.is_some();
-    let (start_eq, mut end_eq, expr_eq) = scan_block_for_eq(cx, blocks);
+
+    let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, blocks) {
+        (block_eq.start_eq, block_eq.end_eq, block_eq.expr_eq)
+    } else {
+        return;
+    };
 
     // BRANCHES_SHARING_CODE prerequisites
     if has_conditional_else || (start_eq == 0 && end_eq == 0 && (has_expr && !expr_eq)) {
@@ -290,7 +295,19 @@ fn lint_same_then_else<'tcx>(
     }
 }
 
-fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> (usize, usize, bool) {
+struct BlockEqual {
+    /// The amount statements that are equal from the start
+    start_eq: usize,
+    /// The amount statements that are equal from the end
+    end_eq: usize,
+    ///  An indication if the block expressions are the same. This will also be true if both are
+    /// `None`
+    expr_eq: bool,
+}
+
+/// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `None` to
+/// abort any further processing and avoid duplicate lint triggers.
+fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> Option<BlockEqual> {
     let mut start_eq = usize::MAX;
     let mut end_eq = usize::MAX;
     let mut expr_eq = true;
@@ -332,7 +349,7 @@ fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> (usize,
                     "same as this",
                 );
 
-                return (0, 0, false);
+                return None;
             }
         }
 
@@ -352,7 +369,11 @@ fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> (usize,
         end_eq = min_block_size - start_eq;
     }
 
-    (start_eq, end_eq, expr_eq)
+    Some(BlockEqual {
+        start_eq,
+        end_eq,
+        expr_eq,
+    })
 }
 
 fn check_for_warn_of_moved_symbol(
diff --git a/clippy_lints/src/if_then_some_else_none.rs b/clippy_lints/src/if_then_some_else_none.rs
index f73b3a1fe67..85c95f1151f 100644
--- a/clippy_lints/src/if_then_some_else_none.rs
+++ b/clippy_lints/src/if_then_some_else_none.rs
@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_help;
 use clippy_utils::source::snippet_with_macro_callsite;
-use clippy_utils::{is_lang_ctor, meets_msrv, parent_node_is_if_expr};
+use clippy_utils::{is_else_clause, is_lang_ctor, meets_msrv};
 use if_chain::if_chain;
 use rustc_hir::LangItem::{OptionNone, OptionSome};
 use rustc_hir::{Expr, ExprKind};
@@ -68,7 +68,7 @@ impl LateLintPass<'_> for IfThenSomeElseNone {
         }
 
         // We only care about the top-most `if` in the chain
-        if parent_node_is_if_expr(expr, cx) {
+        if is_else_clause(cx.tcx, expr) {
             return;
         }
 
diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs
index 96a58d1410f..dd458198637 100644
--- a/clippy_lints/src/needless_bool.rs
+++ b/clippy_lints/src/needless_bool.rs
@@ -5,7 +5,7 @@
 use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::sugg::Sugg;
-use clippy_utils::{is_expn_of, parent_node_is_if_expr};
+use clippy_utils::{is_else_clause, is_expn_of};
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
 use rustc_hir::{BinOpKind, Block, Expr, ExprKind, StmtKind, UnOp};
@@ -81,7 +81,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBool {
                     snip = snip.make_return();
                 }
 
-                if parent_node_is_if_expr(e, cx) {
+                if is_else_clause(cx.tcx, e) {
                     snip = snip.blockify()
                 }
 
diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs
index 3ad1ac75e56..07ae6e924e2 100644
--- a/clippy_utils/src/hir_utils.rs
+++ b/clippy_utils/src/hir_utils.rs
@@ -96,6 +96,16 @@ 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)) => {
+                // This additional check ensures that the type of the locals are equivalent even if the init
+                // expression or type have some inferred parts.
+                if let Some(typeck) = self.inner.maybe_typeck_results {
+                    let l_ty = typeck.pat_ty(&l.pat);
+                    let r_ty = typeck.pat_ty(&r.pat);
+                    if !rustc_middle::ty::TyS::same_type(l_ty, r_ty) {
+                        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 a5986ed00da..b7017411927 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -1300,21 +1300,6 @@ pub fn if_sequence<'tcx>(mut expr: &'tcx Expr<'tcx>) -> (Vec<&'tcx Expr<'tcx>>,
     (conds, blocks)
 }
 
-/// This function returns true if the given expression is the `else` or `if else` part of an if
-/// statement
-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);
-    matches!(
-        parent_node,
-        Node::Expr(Expr {
-            kind: ExprKind::If(_, _, _),
-            ..
-        })
-    )
-}
-
 // Finds the `#[must_use]` attribute, if any
 pub fn must_use_attr(attrs: &[Attribute]) -> Option<&Attribute> {
     attrs.iter().find(|a| a.has_name(sym::must_use))
diff --git a/tests/ui/branches_sharing_code/shared_at_bottom.rs b/tests/ui/branches_sharing_code/shared_at_bottom.rs
index c389c243d44..ce2040bdeb8 100644
--- a/tests/ui/branches_sharing_code/shared_at_bottom.rs
+++ b/tests/ui/branches_sharing_code/shared_at_bottom.rs
@@ -206,4 +206,18 @@ fn fp_test() {
     }
 }
 
+fn fp_if_let_issue7054() {
+    // This shouldn't trigger the lint
+    let string;
+    let _x = if let true = true {
+        ""
+    } else if true {
+        string = "x".to_owned();
+        &string
+    } else {
+        string = "y".to_owned();
+        &string
+    };
+}
+
 fn main() {}
diff --git a/tests/ui/branches_sharing_code/shared_at_top.rs b/tests/ui/branches_sharing_code/shared_at_top.rs
index e65bcfd7873..51a46481399 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() {}
diff --git a/util/gh-pages/index.html b/util/gh-pages/index.html
index 082cb35c2e0..27ecb532dd0 100644
--- a/util/gh-pages/index.html
+++ b/util/gh-pages/index.html
@@ -133,7 +133,7 @@
             opacity: 30%;
         }
 
-        p > code {
+        :not(pre) > code {
             color: var(--inline-code-color);
             background-color: var(--inline-code-bg);
         }