]> git.lizzy.rs Git - rust.git/commitdiff
add a companion lint to `no_effect` with suggestions for partially effective statements
authorOliver Schneider <git1984941651981@oli-obk.de>
Fri, 13 May 2016 14:43:47 +0000 (16:43 +0200)
committerOliver Schneider <git1984941651981@oli-obk.de>
Fri, 13 May 2016 14:43:47 +0000 (16:43 +0200)
22 files changed:
CHANGELOG.md
README.md
src/lib.rs
src/no_effect.rs
tests/compile-fail/absurd-extreme-comparisons.rs
tests/compile-fail/arithmetic.rs
tests/compile-fail/array_indexing.rs
tests/compile-fail/bit_masks.rs
tests/compile-fail/cast.rs
tests/compile-fail/cmp_nan.rs
tests/compile-fail/cmp_owned.rs
tests/compile-fail/copies.rs
tests/compile-fail/eq_op.rs
tests/compile-fail/float_cmp.rs
tests/compile-fail/identity_op.rs
tests/compile-fail/invalid_upcast_comparisons.rs
tests/compile-fail/methods.rs
tests/compile-fail/modulo_one.rs
tests/compile-fail/mut_mut.rs
tests/compile-fail/neg_multiply.rs
tests/compile-fail/no_effect.rs
tests/compile-fail/unit_cmp.rs

index e442c4d7b1dd673e6e89890b926c709c8a0c7630..6e27b3909d33c131a6ecb47371d3f238202f4947 100644 (file)
@@ -215,6 +215,7 @@ All notable changes to this project will be documented in this file.
 [`unicode_not_nfc`]: https://github.com/Manishearth/rust-clippy/wiki#unicode_not_nfc
 [`unit_cmp`]: https://github.com/Manishearth/rust-clippy/wiki#unit_cmp
 [`unnecessary_mut_passed`]: https://github.com/Manishearth/rust-clippy/wiki#unnecessary_mut_passed
+[`unnecessary_operation`]: https://github.com/Manishearth/rust-clippy/wiki#unnecessary_operation
 [`unneeded_field_pattern`]: https://github.com/Manishearth/rust-clippy/wiki#unneeded_field_pattern
 [`unsafe_removed_from_name`]: https://github.com/Manishearth/rust-clippy/wiki#unsafe_removed_from_name
 [`unstable_as_mut_slice`]: https://github.com/Manishearth/rust-clippy/wiki#unstable_as_mut_slice
index 658e2eb63a505725a916ab0772f2840b62d3dd39..c7a839dc8031855cac70d03d9cd7dde474e47232 100644 (file)
--- a/README.md
+++ b/README.md
@@ -17,7 +17,7 @@ Table of contents:
 
 ## Lints
 
-There are 149 lints included in this crate:
+There are 150 lints included in this crate:
 
 name                                                                                                                 | default | meaning
 ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -154,6 +154,7 @@ name
 [unicode_not_nfc](https://github.com/Manishearth/rust-clippy/wiki#unicode_not_nfc)                                   | allow   | using a unicode literal not in NFC normal form (see [unicode tr15](http://www.unicode.org/reports/tr15/) for further information)
 [unit_cmp](https://github.com/Manishearth/rust-clippy/wiki#unit_cmp)                                                 | warn    | comparing unit values (which is always `true` or `false`, respectively)
 [unnecessary_mut_passed](https://github.com/Manishearth/rust-clippy/wiki#unnecessary_mut_passed)                     | warn    | an argument is passed as a mutable reference although the function/method only demands an immutable reference
+[unnecessary_operation](https://github.com/Manishearth/rust-clippy/wiki#unnecessary_operation)                       | warn    | outer expressions with no effect
 [unneeded_field_pattern](https://github.com/Manishearth/rust-clippy/wiki#unneeded_field_pattern)                     | warn    | Struct fields are bound to a wildcard instead of using `..`
 [unsafe_removed_from_name](https://github.com/Manishearth/rust-clippy/wiki#unsafe_removed_from_name)                 | warn    | unsafe removed from name
 [unused_collect](https://github.com/Manishearth/rust-clippy/wiki#unused_collect)                                     | warn    | `collect()`ing an iterator without using the result; this is usually better written as a for loop
index 37464e766b0e20ba45bea8136f28d6dc87e7bc23..f1c815f0df0451d228639b5226f6c5d74e7ffff6 100644 (file)
@@ -519,6 +519,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         neg_multiply::NEG_MULTIPLY,
         new_without_default::NEW_WITHOUT_DEFAULT,
         no_effect::NO_EFFECT,
+        no_effect::UNNECESSARY_OPERATION,
         non_expressive_names::MANY_SINGLE_CHAR_NAMES,
         open_options::NONSENSICAL_OPEN_OPTIONS,
         overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
index d928de415787209e20e89a0ca4285ac4fe8ee632..593a6c4ad591687bf353a33943b4838187acb915 100644 (file)
@@ -1,7 +1,8 @@
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
-use rustc::hir::def::Def;
+use rustc::hir::def::{Def, PathResolution};
 use rustc::hir::{Expr, Expr_, Stmt, StmtSemi};
-use utils::{in_macro, span_lint};
+use utils::{in_macro, span_lint, snippet_opt, span_lint_and_then};
+use std::ops::Deref;
 
 /// **What it does:** This lint checks for statements which have no effect.
 ///
     "statements with no effect"
 }
 
+/// **What it does:** This lint checks for expression statements that can be reduced to a sub-expression
+///
+/// **Why is this bad?** Expressions by themselves often have no side-effects. Having such expressions reduces redability.
+///
+/// **Known problems:** None.
+///
+/// **Example:** `compute_array()[0];`
+declare_lint! {
+    pub UNNECESSARY_OPERATION,
+    Warn,
+    "outer expressions with no effect"
+}
+
 fn has_no_effect(cx: &LateContext, expr: &Expr) -> bool {
     if in_macro(cx, expr.span) {
         return false;
@@ -68,7 +82,7 @@ fn has_no_effect(cx: &LateContext, expr: &Expr) -> bool {
 
 impl LintPass for NoEffectPass {
     fn get_lints(&self) -> LintArray {
-        lint_array!(NO_EFFECT)
+        lint_array!(NO_EFFECT, UNNECESSARY_OPERATION)
     }
 }
 
@@ -77,7 +91,65 @@ fn check_stmt(&mut self, cx: &LateContext, stmt: &Stmt) {
         if let StmtSemi(ref expr, _) = stmt.node {
             if has_no_effect(cx, expr) {
                 span_lint(cx, NO_EFFECT, stmt.span, "statement with no effect");
+            } else if let Some(reduced) = reduce_expression(cx, expr) {
+                let mut snippet = String::new();
+                for e in reduced {
+                    if in_macro(cx, e.span) {
+                        return;
+                    }
+                    if let Some(snip) = snippet_opt(cx, e.span) {
+                        snippet.push_str(&snip);
+                        snippet.push(';');
+                    } else {
+                        return;
+                    }
+                }
+                span_lint_and_then(cx, UNNECESSARY_OPERATION, stmt.span, "statement can be reduced", |db| {
+                    db.span_suggestion(stmt.span, "replace it with", snippet);
+                });
+            }
+        }
+    }
+}
+
+
+fn reduce_expression<'a>(cx: &LateContext, expr: &'a Expr) -> Option<Vec<&'a Expr>> {
+    if in_macro(cx, expr.span) {
+        return None;
+    }
+    match expr.node {
+        Expr_::ExprIndex(ref a, ref b) |
+        Expr_::ExprBinary(_, ref a, ref b) => Some(vec![&**a, &**b]),
+        Expr_::ExprVec(ref v) |
+        Expr_::ExprTup(ref v) => Some(v.iter().map(Deref::deref).collect()),
+        Expr_::ExprRepeat(ref inner, _) |
+        Expr_::ExprCast(ref inner, _) |
+        Expr_::ExprType(ref inner, _) |
+        Expr_::ExprUnary(_, ref inner) |
+        Expr_::ExprField(ref inner, _) |
+        Expr_::ExprTupField(ref inner, _) |
+        Expr_::ExprAddrOf(_, ref inner) |
+        Expr_::ExprBox(ref inner) => reduce_expression(cx, inner).or_else(|| Some(vec![inner])),
+        Expr_::ExprStruct(_, ref fields, ref base) => Some(fields.iter().map(|f| &f.expr).chain(base).map(Deref::deref).collect()),
+        Expr_::ExprCall(ref callee, ref args) => {
+            match cx.tcx.def_map.borrow().get(&callee.id).map(PathResolution::full_def) {
+                Some(Def::Struct(..)) |
+                Some(Def::Variant(..)) => Some(args.iter().map(Deref::deref).collect()),
+                _ => None,
+            }
+        }
+        Expr_::ExprBlock(ref block) => {
+            if block.stmts.is_empty() {
+                block.expr.as_ref().and_then(|e| if e.span == expr.span {
+                    // in case of compiler-inserted signaling blocks
+                    reduce_expression(cx, e)
+                } else {
+                    Some(vec![e])
+                })
+            } else {
+                None
             }
         }
+        _ => None,
     }
 }
index 7e2ad1fede5469c2f78028de6e74e6ebdc2b62a5..f1e4a692800c083f5d353637b3025862d6316a79 100644 (file)
@@ -2,7 +2,7 @@
 #![plugin(clippy)]
 
 #![deny(absurd_extreme_comparisons)]
-#![allow(unused, eq_op, no_effect)]
+#![allow(unused, eq_op, no_effect, unnecessary_operation)]
 fn main() {
     const Z: u32 = 0;
 
index 54ac65970ae56e84376cac9fc858fe4cd459d699..5479c55e11e030ae824155757a22b023746dd6ec 100644 (file)
@@ -2,7 +2,7 @@
 #![plugin(clippy)]
 
 #![deny(integer_arithmetic, float_arithmetic)]
-#![allow(unused, shadow_reuse, shadow_unrelated, no_effect)]
+#![allow(unused, shadow_reuse, shadow_unrelated, no_effect, unnecessary_operation)]
 fn main() {
     let i = 1i32;
     1 + i; //~ERROR integer arithmetic detected
@@ -11,17 +11,17 @@ fn main() {
     i / 2; // no error, this is part of the expression in the preceding line
     i - 2 + 2 - i; //~ERROR integer arithmetic detected
     -i; //~ERROR integer arithmetic detected
-    
+
     i & 1; // no wrapping
-    i | 1; 
+    i | 1;
     i ^ 1;
     i >> 1;
     i << 1;
-    
+
     let f = 1.0f32;
-    
+
     f * 2.0; //~ERROR floating-point arithmetic detected
-    
+
     1.0 + f; //~ERROR floating-point arithmetic detected
     f * 2.0; //~ERROR floating-point arithmetic detected
     f / 2.0; //~ERROR floating-point arithmetic detected
index 35fadf8c1e4add72b5dc751f12004cc5a925c1af..dacb72ee8ac3e27624d4d44109018f6ad2b87fa8 100644 (file)
@@ -3,7 +3,7 @@
 
 #![deny(indexing_slicing)]
 #![deny(out_of_bounds_indexing)]
-#![allow(no_effect)]
+#![allow(no_effect, unnecessary_operation)]
 
 fn main() {
     let x = [1,2,3,4];
index 9813529586290cb9b8c01b3c76e0f9e5c38207e0..79772840c73dbfb8977482857f8e0cb901acda79 100644 (file)
@@ -5,7 +5,7 @@
 const EVEN_MORE_REDIRECTION : i64 = THREE_BITS;
 
 #[deny(bad_bit_mask)]
-#[allow(ineffective_bit_mask, identity_op, no_effect)]
+#[allow(ineffective_bit_mask, identity_op, no_effect, unnecessary_operation)]
 fn main() {
     let x = 5;
 
@@ -45,7 +45,7 @@ fn main() {
 }
 
 #[deny(ineffective_bit_mask)]
-#[allow(bad_bit_mask, no_effect)]
+#[allow(bad_bit_mask, no_effect, unnecessary_operation)]
 fn ineffective() {
     let x = 5;
 
index 0f44fa2c1fd0ff2c1dc3a1aefbb210faa9ae4224..d0ea5f40789d934167a6131c8eddd7445910e1ec 100644 (file)
@@ -2,7 +2,7 @@
 #![plugin(clippy)]
 
 #[deny(cast_precision_loss, cast_possible_truncation, cast_sign_loss, cast_possible_wrap)]
-#[allow(no_effect)]
+#[allow(no_effect, unnecessary_operation)]
 fn main() {
     // Test cast_precision_loss
     1i32 as f32; //~ERROR casting i32 to f32 causes a loss of precision (i32 is 32 bits wide, but f32's mantissa is only 23 bits wide)
index d2188130a61ea9cc7f731e3c7c5f55998ca5051c..8d173665a2429fb241ebe4159063fefb112049fb 100644 (file)
@@ -2,7 +2,7 @@
 #![plugin(clippy)]
 
 #[deny(cmp_nan)]
-#[allow(float_cmp, no_effect)]
+#[allow(float_cmp, no_effect, unnecessary_operation)]
 fn main() {
     let x = 5f32;
     x == std::f32::NAN; //~ERROR doomed comparison with NAN
index eb4070d8fd6c0cdcec6cc7645e27cd367079a35c..f7c7824e9d1cddb1c326c9f3a6028ddc521bd08f 100644 (file)
@@ -2,6 +2,7 @@
 #![plugin(clippy)]
 
 #[deny(cmp_owned)]
+#[allow(unnecessary_operation)]
 fn main() {
     fn with_to_string(x : &str) {
         x != "foo".to_string();
index 68756a57cc75ee21c5dd4e88555d87bb5dab7253..bbdd73dc0c8f40a07d78a175ec1f87c1d02e7b14 100644 (file)
@@ -1,7 +1,7 @@
 #![feature(plugin, inclusive_range_syntax)]
 #![plugin(clippy)]
 
-#![allow(dead_code, no_effect)]
+#![allow(dead_code, no_effect, unnecessary_operation)]
 #![allow(let_and_return)]
 #![allow(needless_return)]
 #![allow(unused_variables)]
index 443bbbaacd3670344e545b932334472bfd69dfca..768eadd00eba26b32a4b4944ce649204e5ce9446 100644 (file)
@@ -3,7 +3,7 @@
 
 #[deny(eq_op)]
 #[allow(identity_op)]
-#[allow(no_effect, unused_variables)]
+#[allow(no_effect, unused_variables, unnecessary_operation)]
 #[deny(nonminimal_bool)]
 fn main() {
     // simple values and comparisons
index d1ecb37cdd5298e6d57c2e66ba428a752d1ead46..85df1ded5ace7b7f35272b54825b676265821cdc 100644 (file)
@@ -2,7 +2,7 @@
 #![plugin(clippy)]
 
 #![deny(float_cmp)]
-#![allow(unused, no_effect)]
+#![allow(unused, no_effect, unnecessary_operation)]
 
 use std::ops::Add;
 
index 28873ee6b73105b6ce62c86fd1bb6c58c55460b3..329c4a6bbf465251378efef0c21645b27d6c0098 100644 (file)
@@ -5,7 +5,7 @@
 const NEG_ONE : i64 = -1;
 const ZERO : i64 = 0;
 
-#[allow(eq_op, no_effect)]
+#[allow(eq_op, no_effect, unnecessary_operation)]
 #[deny(identity_op)]
 fn main() {
     let x = 0;
index 443dd89aab9e3d0bb6403ae497963ef372ed9089..9635f3afede968a3eeee3d3f000123d2b1232e92 100644 (file)
@@ -2,7 +2,7 @@
 #![plugin(clippy)]
 
 #![deny(invalid_upcast_comparisons)]
-#![allow(unused, eq_op, no_effect)]
+#![allow(unused, eq_op, no_effect, unnecessary_operation)]
 fn main() {
     let zero: u32 = 0;
     let u8_max: u8 = 255;
index 0a943840e17a100102c8ac8b8ce2284867c14331..88a1e7c4cf24e1f3d84b0cb4229efdfeefc7c0a0 100644 (file)
@@ -341,6 +341,7 @@ struct MyErrorWithParam<T> {
     x: T
 }
 
+#[allow(unnecessary_operation)]
 fn starts_with() {
     "".chars().next() == Some(' ');
     //~^ ERROR starts_with
index e84209a6d1eec2f7c57b64619f0b62bb41204640..496c1c60d5f11f87bd0bbf482770dc67635addf6 100644 (file)
@@ -1,7 +1,7 @@
 #![feature(plugin)]
 #![plugin(clippy)]
 #![deny(modulo_one)]
-#![allow(no_effect)]
+#![allow(no_effect, unnecessary_operation)]
 
 fn main() {
     10 % 1; //~ERROR any number modulo 1 will be 0
index 0db9cb3bdef803dd1f903de63b65b5264df862d9..865574eaec02b4fc85ab336f0ab366927b973844 100644 (file)
@@ -1,7 +1,7 @@
 #![feature(plugin)]
 #![plugin(clippy)]
 
-#![allow(unused, no_effect)]
+#![allow(unused, no_effect, unnecessary_operation)]
 
 //#![plugin(regex_macros)]
 //extern crate regex;
index 9deb38920deb155c57e1497fdd4044868c807e7d..90c63c5f2632e6105bd698968b169adb8270dc6a 100644 (file)
@@ -2,7 +2,7 @@
 
 #![plugin(clippy)]
 #![deny(neg_multiply)]
-#![allow(no_effect)]
+#![allow(no_effect, unnecessary_operation)]
 
 use std::ops::Mul;
 
@@ -10,7 +10,7 @@
 
 impl Mul<isize> for X {
     type Output = X;
-    
+
     fn mul(self, _r: isize) -> Self {
         self
     }
@@ -18,7 +18,7 @@ fn mul(self, _r: isize) -> Self {
 
 impl Mul<X> for isize {
     type Output = X;
-    
+
     fn mul(self, _r: X) -> X {
         X
     }
@@ -34,7 +34,7 @@ fn main() {
     //~^ ERROR Negation by multiplying with -1
 
     -1 * -1; // should be ok
-    
+
     X * -1; // should be ok
     -1 * X; // should also be ok
 }
index 344c82f3307568fe2c20572bf37d9551438a134e..ce6daa8d5625247afc576f90c04c820e7b95329c 100644 (file)
@@ -1,7 +1,7 @@
 #![feature(plugin, box_syntax, inclusive_range_syntax)]
 #![plugin(clippy)]
 
-#![deny(no_effect)]
+#![deny(no_effect, unnecessary_operation)]
 #![allow(dead_code)]
 #![allow(path_statements)]
 
@@ -50,22 +50,59 @@ fn main() {
 
     // Do not warn
     get_number();
-    Tuple(get_number());
-    Struct { field: get_number() };
-    Struct { ..get_struct() };
-    Enum::Tuple(get_number());
-    Enum::Struct { field: get_number() };
-    5 + get_number();
-    *&get_number();
-    &get_number();
-    (5, 6, get_number());
-    box get_number();
-    get_number()..;
-    ..get_number();
-    5..get_number();
-    [42, get_number()];
-    [42, 55][get_number() as usize];
-    (42, get_number()).1;
-    [get_number(); 55];
-    [42; 55][get_number() as usize];
+
+    Tuple(get_number()); //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION get_number();
+    Struct { field: get_number() }; //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION get_number();
+    Struct { ..get_struct() }; //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION get_number();
+    Enum::Tuple(get_number()); //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION get_number();
+    Enum::Struct { field: get_number() }; //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION get_number();
+    5 + get_number(); //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION 5;get_number();
+    *&get_number(); //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION &get_number();
+    &get_number(); //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION get_number();
+    (5, 6, get_number()); //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION 5;6;get_number();
+    box get_number(); //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION get_number();
+    get_number()..; //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION get_number();
+    ..get_number(); //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION get_number();
+    5..get_number(); //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION 5;get_number();
+    [42, get_number()]; //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION 42;get_number();
+    [42, 55][get_number() as usize]; //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION [42, 55];get_number() as usize;
+    (42, get_number()).1; //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION 42;get_number();
+    [get_number(); 55]; //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION get_number();
+    [42; 55][get_number() as usize]; //~ERROR statement can be reduced
+    //~^HELP replace it with
+    //~|SUGGESTION [42; 55];get_number() as usize;
 }
index 1a28953ace11d6b87322e717d26078e66da8c290..13095ee6bfb4191e547f76c65cce4c142187fcd9 100644 (file)
@@ -2,7 +2,7 @@
 #![plugin(clippy)]
 
 #![deny(unit_cmp)]
-#![allow(no_effect)]
+#![allow(no_effect, unnecessary_operation)]
 
 #[derive(PartialEq)]
 pub struct ContainsUnit(()); // should be fine