]> git.lizzy.rs Git - rust.git/commitdiff
The shared_code_in_if_blocks lint only operats on entire if expr not else ifs
authorxFrednet <xFrednet@gmail.com>
Wed, 13 Jan 2021 19:01:15 +0000 (20:01 +0100)
committerxFrednet <xFrednet@gmail.com>
Mon, 5 Apr 2021 11:33:45 +0000 (13:33 +0200)
clippy_lints/src/copies.rs
tests/ui/shared_code_in_if_blocks/shared_at_bot.rs
tests/ui/shared_code_in_if_blocks/valid_if_blocks.rs

index 29ff19707055cbcec7433e2ccb1ff9e72b5401bf..7162d809ec67672574fbdbbc49911e2c3f34e44f 100644 (file)
@@ -1,7 +1,12 @@
-use clippy_utils::diagnostics::span_lint_and_note;
-use clippy_utils::{eq_expr_value, in_macro, search_same, SpanlessEq, SpanlessHash};
-use clippy_utils::{get_parent_expr, if_sequence};
-use rustc_hir::{Block, Expr, ExprKind};
+use crate::utils::{both, count_eq, eq_expr_value, in_macro, search_same, SpanlessEq, SpanlessHash};
+use crate::utils::{
+    first_line_of_span, get_parent_expr, higher, if_sequence, indent_of, parent_node_is_if_expr, reindent_multiline,
+    snippet, span_lint_and_note, span_lint_and_sugg, span_lint_and_then,
+};
+use rustc_data_structures::fx::FxHashSet;
+use rustc_errors::Applicability;
+use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
+use rustc_hir::{Block, Expr, HirId};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::map::Map;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
     /// };
     /// ```
     pub SHARED_CODE_IN_IF_BLOCKS,
-    pedantic,
+    nursery,
     "`if` statement with shared code in all blocks"
 }
 
@@ -180,7 +185,7 @@ fn lint_same_then_else<'tcx>(
 ) {
     // We only lint ifs with multiple blocks
     // TODO xFrednet 2021-01-01: Check if it's an else if block
-    if blocks.len() < 2 {
+    if blocks.len() < 2 || parent_node_is_if_expr(expr, cx) {
         return;
     }
 
@@ -190,7 +195,7 @@ fn lint_same_then_else<'tcx>(
     let mut start_eq = usize::MAX;
     let mut end_eq = usize::MAX;
     let mut expr_eq = true;
-    for (index, win) in blocks.windows(2).enumerate() {
+    for win in blocks.windows(2) {
         let l_stmts = win[0].stmts;
         let r_stmts = win[1].stmts;
 
@@ -202,9 +207,7 @@ fn lint_same_then_else<'tcx>(
         let block_expr_eq = both(&win[0].expr, &win[1].expr, |l, r| evaluator.eq_expr(l, r));
 
         // IF_SAME_THEN_ELSE
-        // We only lint the first two blocks (index == 0). Further blocks will be linted when that if
-        // statement is checked
-        if index == 0 && block_expr_eq && l_stmts.len() == r_stmts.len() && l_stmts.len() == current_start_eq {
+        if block_expr_eq && l_stmts.len() == r_stmts.len() && l_stmts.len() == current_start_eq {
             span_lint_and_note(
                 cx,
                 IF_SAME_THEN_ELSE,
@@ -215,16 +218,24 @@ fn lint_same_then_else<'tcx>(
             );
 
             return;
+        } else {
+            println!(
+                "{:?}\n - expr_eq: {:10}, l_stmts.len(): {:10}, r_stmts.len(): {:10}",
+                win[0].span,
+                block_expr_eq,
+                l_stmts.len(),
+                r_stmts.len()
+            )
         }
 
         start_eq = start_eq.min(current_start_eq);
         end_eq = end_eq.min(current_end_eq);
         expr_eq &= block_expr_eq;
+    }
 
-        // We can return if the eq count is 0 from both sides or if it has no unconditional else case
-        if !has_unconditional_else || (start_eq == 0 && end_eq == 0 && (has_expr && !expr_eq)) {
-            return;
-        }
+    // SHARED_CODE_IN_IF_BLOCKS prerequisites
+    if !has_unconditional_else || (start_eq == 0 && end_eq == 0 && (has_expr && !expr_eq)) {
+        return;
     }
 
     if has_expr && !expr_eq {
@@ -275,11 +286,14 @@ fn lint_same_then_else<'tcx>(
             end_eq -= moved_start;
         }
 
-        let mut end_linable = true;
-        if let Some(expr) = block.expr {
+        let end_linable = if let Some(expr) = block.expr {
             intravisit::walk_expr(&mut walker, expr);
-            end_linable = walker.uses.iter().any(|x| !block_defs.contains(x));
-        }
+            walker.uses.iter().any(|x| !block_defs.contains(x))
+        } else if end_eq == 0 {
+            false
+        } else {
+            true
+        };
 
         emit_shared_code_in_if_blocks_lint(cx, start_eq, end_eq, end_linable, blocks, expr);
     }
@@ -351,7 +365,7 @@ fn emit_shared_code_in_if_blocks_lint(
             SHARED_CODE_IN_IF_BLOCKS,
             *span,
             "All code blocks contain the same code",
-            "Consider moving the code out like this",
+            "Consider moving the statements out like this",
             sugg.clone(),
             Applicability::Unspecified,
         );
index 22f1fe95f79226f57d99ab8046619f53414bb2dd..b85bb2e1f96f32989b9e6698738ab7836f554406 100644 (file)
@@ -32,16 +32,119 @@ fn simple_examples() {
         println!("Block end!");
         result
     };
+
+    if x == 9 {
+        println!("The index is: 6");
+
+        println!("Same end of block");
+    } else if x == 8 {
+        println!("The index is: 4");
+
+        println!("Same end of block");
+    } else {
+        println!("Same end of block");
+    }
+
+    // TODO xFrednet 2021-01.13: Fix lint for `if let`
+    let index = Some(8);
+    if let Some(index) = index {
+        println!("The index is: {}", index);
+
+        println!("Same end of block");
+    } else {
+        println!("Same end of block");
+    }
+
+    if x == 9 {
+        if x == 8 {
+            // No parent!!
+            println!("Hello World");
+            println!("---");
+        } else {
+            println!("Hello World");
+        }
+    }
 }
 
 /// Simple examples where the move can cause some problems due to moved values
 fn simple_but_suggestion_is_invalid() {
-    // TODO xFrednet 2021-01-12: This
+    let x = 16;
+
+    // Local value
+    let later_used_value = 17;
+    if x == 9 {
+        let _ = 9;
+        let later_used_value = "A string value";
+        println!("{}", later_used_value);
+    } else {
+        let later_used_value = "A string value";
+        println!("{}", later_used_value);
+        // I'm expecting a note about this
+    }
+    println!("{}", later_used_value);
+
+    // outer function
+    if x == 78 {
+        let simple_examples = "I now identify as a &str :)";
+        println!("This is the new simple_example: {}", simple_examples);
+    } else {
+        println!("Separator print statement");
+
+        let simple_examples = "I now identify as a &str :)";
+        println!("This is the new simple_example: {}", simple_examples);
+    }
+    simple_examples();
 }
 
 /// Tests where the blocks are not linted due to the used value scope
 fn not_moveable_due_to_value_scope() {
-    // TODO xFrednet 2021-01-12: This
+    let x = 18;
+
+    // Using a local value in the moved code
+    if x == 9 {
+        let y = 18;
+        println!("y is: `{}`", y);
+    } else {
+        let y = "A string";
+        println!("y is: `{}`", y);
+    }
+
+    // Using a local value in the expression
+    let _ = if x == 0 {
+        let mut result = x + 1;
+
+        println!("1. Doing some calculations");
+        println!("2. Some more calculations");
+        println!("3. Setting result");
+
+        result
+    } else {
+        let mut result = x - 1;
+
+        println!("1. Doing some calculations");
+        println!("2. Some more calculations");
+        println!("3. Setting result");
+
+        result
+    };
+
+    let _ = if x == 7 {
+        let z1 = 100;
+        println!("z1: {}", z1);
+
+        let z2 = z1;
+        println!("z2: {}", z2);
+
+        z2
+    } else {
+        let z1 = 300;
+        println!("z1: {}", z1);
+
+        let z2 = z1;
+        println!("z2: {}", z2);
+
+        z2
+    };
 }
 
 fn main() {}
index 1f12e9348d473f1bfebbf0f4e6a4084be9fa0988..480758777f16181d78b8665a996a5bae5dbf222a 100644 (file)
@@ -152,6 +152,16 @@ struct Duck {
             ":D"
         }
     };
+
+    if x == 0 {
+        println!("I'm single");
+    } else if x == 68 {
+        println!("I'm a doppelgänger");
+        // Don't listen to my clone below
+    } else {
+        // Don't listen to my clone above
+        println!("I'm a doppelgänger");
+    }
 }
 
 fn main() {}