Skip to content

New lint for statement with no effect #422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 29, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
[Jump to usage instructions](#usage)

##Lints
There are 69 lints included in this crate:
There are 70 lints included in this crate:

name | default | meaning
-------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -46,6 +46,7 @@ name
[needless_range_loop](https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop) | warn | for-looping over a range of indices where an iterator over items would do
[needless_return](https://github.com/Manishearth/rust-clippy/wiki#needless_return) | warn | using a return statement like `return expr;` where an expression would suffice
[needless_update](https://github.com/Manishearth/rust-clippy/wiki#needless_update) | warn | using `{ ..base }` when there are no missing fields
[no_effect](https://github.com/Manishearth/rust-clippy/wiki#no_effect) | warn | statements with no effect
[non_ascii_literal](https://github.com/Manishearth/rust-clippy/wiki#non_ascii_literal) | allow | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead
[nonsensical_open_options](https://github.com/Manishearth/rust-clippy/wiki#nonsensical_open_options) | warn | nonsensical combination of options for opening a file
[option_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#option_unwrap_used) | allow | using `Option.unwrap()`, which should at least get a better message using `expect()`
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub mod zero_div_zero;
pub mod open_options;
pub mod needless_features;
pub mod needless_update;
pub mod no_effect;

mod reexport {
pub use syntax::ast::{Name, Ident, NodeId};
Expand Down Expand Up @@ -98,6 +99,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_late_lint_pass(box mutex_atomic::MutexAtomic);
reg.register_late_lint_pass(box needless_features::NeedlessFeaturesPass);
reg.register_late_lint_pass(box needless_update::NeedlessUpdatePass);
reg.register_late_lint_pass(box no_effect::NoEffectPass);

reg.register_lint_group("clippy_pedantic", vec![
methods::OPTION_UNWRAP_USED,
Expand Down Expand Up @@ -159,6 +161,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
needless_features::UNSTABLE_AS_MUT_SLICE,
needless_features::UNSTABLE_AS_SLICE,
needless_update::NEEDLESS_UPDATE,
no_effect::NO_EFFECT,
open_options::NONSENSICAL_OPEN_OPTIONS,
precedence::PRECEDENCE,
ptr_arg::PTR_ARG,
Expand Down
61 changes: 61 additions & 0 deletions src/no_effect.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::middle::def::{DefStruct, DefVariant};
use rustc_front::hir::{Expr, ExprCall, ExprLit, ExprPath, ExprStruct};
use rustc_front::hir::{Stmt, StmtSemi};

use utils::in_macro;
use utils::span_lint;

declare_lint! {
pub NO_EFFECT,
Warn,
"statements with no effect"
}

fn has_no_effect(cx: &LateContext, expr: &Expr) -> bool {
if in_macro(cx, expr.span) {
return false;
}
match expr.node {
ExprLit(..) |
ExprPath(..) => true,
ExprStruct(_, ref fields, ref base) => {
fields.iter().all(|field| has_no_effect(cx, &field.expr)) &&
match *base {
Some(ref base) => has_no_effect(cx, base),
None => true,
}
}
ExprCall(ref callee, ref args) => {
let def = cx.tcx.def_map.borrow().get(&callee.id).map(|d| d.full_def());
match def {
Some(DefStruct(..)) |
Some(DefVariant(..)) => {
args.iter().all(|arg| has_no_effect(cx, arg))
}
_ => false,
}
}
_ => false,
}
}

#[derive(Copy, Clone)]
pub struct NoEffectPass;

impl LintPass for NoEffectPass {
fn get_lints(&self) -> LintArray {
lint_array!(NO_EFFECT)
}
}

impl LateLintPass for NoEffectPass {
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");
}
}
}
}
1 change: 1 addition & 0 deletions tests/compile-fail/needless_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#![plugin(clippy)]

#![deny(needless_update)]
#![allow(no_effect)]

struct S {
pub a: i32,
Expand Down
39 changes: 39 additions & 0 deletions tests/compile-fail/no_effect.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#![feature(plugin)]
#![plugin(clippy)]

#![deny(no_effect)]
#![allow(dead_code)]
#![allow(path_statements)]

struct Unit;
struct Tuple(i32);
struct Struct {
field: i32
}
enum Enum {
TupleVariant(i32),
StructVariant { field: i32 },
}

fn get_number() -> i32 { 0 }
fn get_struct() -> Struct { Struct { field: 0 } }

fn main() {
let s = get_struct();

0; //~ERROR statement with no effect
Unit; //~ERROR statement with no effect
Tuple(0); //~ERROR statement with no effect
Struct { field: 0 }; //~ERROR statement with no effect
Struct { ..s }; //~ERROR statement with no effect
Enum::TupleVariant(0); //~ERROR statement with no effect
Enum::StructVariant { field: 0 }; //~ERROR statement with no effect

// Do not warn
get_number();
Tuple(get_number());
Struct { field: get_number() };
Struct { ..get_struct() };
Enum::TupleVariant(get_number());
Enum::StructVariant { field: get_number() };
}
1 change: 1 addition & 0 deletions tests/compile-fail/unit_cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#![plugin(clippy)]

#![deny(unit_cmp)]
#![allow(no_effect)]

#[derive(PartialEq)]
pub struct ContainsUnit(()); // should be fine
Expand Down