add a companion lint to `no_effect` with suggestions for partially effective statements

This commit is contained in:
Oliver Schneider 2016-05-13 16:43:47 +02:00
parent d4e11acc1f
commit 1e897f1552
22 changed files with 161 additions and 47 deletions

View 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

View File

@ -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

View 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,

View 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.
///
@ -16,6 +17,19 @@ declare_lint! {
"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 @@ pub struct NoEffectPass;
impl LintPass for NoEffectPass {
fn get_lints(&self) -> LintArray {
lint_array!(NO_EFFECT)
lint_array!(NO_EFFECT, UNNECESSARY_OPERATION)
}
}
@ -77,7 +91,65 @@ impl LateLintPass for NoEffectPass {
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,
}
}

View 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;

View 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

View 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];

View File

@ -5,7 +5,7 @@ const THREE_BITS : i64 = 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;

View 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)

View 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

View 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();

View 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)]

View 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

View File

@ -2,7 +2,7 @@
#![plugin(clippy)]
#![deny(float_cmp)]
#![allow(unused, no_effect)]
#![allow(unused, no_effect, unnecessary_operation)]
use std::ops::Add;

View File

@ -5,7 +5,7 @@ const ONE : i64 = 1;
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;

View 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;

View File

@ -341,6 +341,7 @@ struct MyErrorWithParam<T> {
x: T
}
#[allow(unnecessary_operation)]
fn starts_with() {
"".chars().next() == Some(' ');
//~^ ERROR starts_with

View 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

View 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;

View 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 @@ struct X;
impl Mul<isize> for X {
type Output = X;
fn mul(self, _r: isize) -> Self {
self
}
@ -18,7 +18,7 @@ impl Mul<isize> for X {
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
}

View 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;
}

View 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