OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #723 from mcarton/swap

Browse Source 
Swap lints
This commit is contained in:
llogiq 2016-02-28 22:48:41 +01:00
commit 5fe58d5c6e
4 changed files with 189 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
README.md
View File

@ -8,11 +8,12 @@ A collection of lints to catch common mistakes and improve your Rust code.
[Jump to usage instructions](#usage)
##Lints
There are 125 lints included in this crate:
There are 127 lints included in this crate:
name | default | meaning
---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
[absurd_extreme_comparisons](https://github.com/Manishearth/rust-clippy/wiki#absurd_extreme_comparisons) | warn | a comparison involving a maximum or minimum value involves a case that is always true or always false
[almost_swapped](https://github.com/Manishearth/rust-clippy/wiki#almost_swapped) | warn | `foo = bar; bar = foo` sequence
[approx_constant](https://github.com/Manishearth/rust-clippy/wiki#approx_constant) | warn | the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found; suggests to use the constant
[bad_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#bad_bit_mask) | warn | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have)
[block_in_if_condition_expr](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr) | warn | braces can be eliminated in conditions that are expressions, e.g `if { true } ...`
@ -62,6 +63,7 @@ name
[let_and_return](https://github.com/Manishearth/rust-clippy/wiki#let_and_return) | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block
[let_unit_value](https://github.com/Manishearth/rust-clippy/wiki#let_unit_value) | warn | creating a let binding to a value of unit type, which usually can't be used afterwards
[linkedlist](https://github.com/Manishearth/rust-clippy/wiki#linkedlist) | warn | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a VecDeque
[manual_swap](https://github.com/Manishearth/rust-clippy/wiki#manual_swap) | warn | manual swap
[map_clone](https://github.com/Manishearth/rust-clippy/wiki#map_clone) | warn | using `.map(|x| x.clone())` to clone an iterator or option's contents (recommends `.cloned()` instead)
[map_entry](https://github.com/Manishearth/rust-clippy/wiki#map_entry) | warn | use of `contains_key` followed by `insert` on a `HashMap` or `BTreeMap`
[match_bool](https://github.com/Manishearth/rust-clippy/wiki#match_bool) | warn | a match on boolean expression; recommends `if..else` block instead

4
src/lib.rs
View File

@ -86,6 +86,7 @@ pub mod regex;
pub mod returns;
pub mod shadow;
pub mod strings;
pub mod swap;
pub mod temporary_assignment;
pub mod transmute;
pub mod types;
@ -167,6 +168,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_late_lint_pass(box copies::CopyAndPaste);
reg.register_late_lint_pass(box format::FormatMacLint);
reg.register_early_lint_pass(box formatting::Formatting);
reg.register_late_lint_pass(box swap::Swap);
reg.register_lint_group("clippy_pedantic", vec![
enum_glob_use::ENUM_GLOB_USE,
@ -285,6 +287,8 @@ pub fn plugin_registrar(reg: &mut Registry) {
returns::LET_AND_RETURN,
returns::NEEDLESS_RETURN,
strings::STRING_LIT_AS_BYTES,
swap::ALMOST_SWAPPED,
swap::MANUAL_SWAP,
temporary_assignment::TEMPORARY_ASSIGNMENT,
transmute::USELESS_TRANSMUTE,
types::ABSURD_EXTREME_COMPARISONS,

138
src/swap.rs Normal file
View File

@ -0,0 +1,138 @@
use rustc::lint::*;
use rustc_front::hir::*;
use syntax::codemap::mk_sp;
use utils::{differing_macro_contexts, snippet_opt, span_lint_and_then, SpanlessEq};
/// **What it does:** This lints manual swapping.
///
/// **Why is this bad?** The `std::mem::swap` function exposes the intent better without
/// deinitializing or copying either variable.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// let t = b;
/// b = a;
/// a = t;
/// ```
declare_lint! {
pub MANUAL_SWAP,
Warn,
"manual swap"
}
/// **What it does:** This lints `foo = bar; bar = foo` sequences.
///
/// **Why is this bad?** This looks like a failed attempt to swap.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// a = b;
/// b = a;
/// ```
declare_lint! {
pub ALMOST_SWAPPED,
Warn,
"`foo = bar; bar = foo` sequence"
}
#[derive(Copy,Clone)]
pub struct Swap;
impl LintPass for Swap {
fn get_lints(&self) -> LintArray {
lint_array![MANUAL_SWAP, ALMOST_SWAPPED]
}
}
impl LateLintPass for Swap {
fn check_block(&mut self, cx: &LateContext, block: &Block) {
check_manual_swap(cx, block);
check_suspicious_swap(cx, block);
}
}
/// Implementation of the `MANUAL_SWAP` lint.
fn check_manual_swap(cx: &LateContext, block: &Block) {
for w in block.stmts.windows(3) {
if_let_chain!{[
// let t = foo();
let StmtDecl(ref tmp, _) = w[0].node,
let DeclLocal(ref tmp) = tmp.node,
let Some(ref tmp_init) = tmp.init,
let PatKind::Ident(_, ref tmp_name, None) = tmp.pat.node,
// foo() = bar();
let StmtSemi(ref first, _) = w[1].node,
let ExprAssign(ref lhs1, ref rhs1) = first.node,
// bar() = t;
let StmtSemi(ref second, _) = w[2].node,
let ExprAssign(ref lhs2, ref rhs2) = second.node,
let ExprPath(None, ref rhs2) = rhs2.node,
rhs2.segments.len() == 1,
tmp_name.node.name.as_str() == rhs2.segments[0].identifier.name.as_str(),
SpanlessEq::new(cx).ignore_fn().eq_expr(tmp_init, lhs1),
SpanlessEq::new(cx).ignore_fn().eq_expr(rhs1, lhs2)
], {
let (what, lhs, rhs) = if let (Some(first), Some(second)) = (snippet_opt(cx, lhs1.span), snippet_opt(cx, rhs1.span)) {
(format!(" `{}` and `{}`", first, second), first, second)
} else {
("".to_owned(), "".to_owned(), "".to_owned())
};
let span = mk_sp(tmp.span.lo, second.span.hi);
span_lint_and_then(cx,
MANUAL_SWAP,
span,
&format!("this looks like you are swapping{} manually", what),
|db| {
if !what.is_empty() {
db.span_suggestion(span, "try",
format!("std::mem::swap(&mut {}, &mut {})", lhs, rhs));
db.fileline_note(span, "or maybe you should use `std::mem::replace`?");
}
});
}}
}
}
/// Implementation of the `ALMOST_SWAPPED` lint.
fn check_suspicious_swap(cx: &LateContext, block: &Block) {
for w in block.stmts.windows(2) {
if_let_chain!{[
let StmtSemi(ref first, _) = w[0].node,
let StmtSemi(ref second, _) = w[1].node,
!differing_macro_contexts(first.span, second.span),
let ExprAssign(ref lhs0, ref rhs0) = first.node,
let ExprAssign(ref lhs1, ref rhs1) = second.node,
SpanlessEq::new(cx).ignore_fn().eq_expr(lhs0, rhs1),
SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, rhs0)
], {
let (what, lhs, rhs) = if let (Some(first), Some(second)) = (snippet_opt(cx, lhs0.span), snippet_opt(cx, rhs0.span)) {
(format!(" `{}` and `{}`", first, second), first, second)
} else {
("".to_owned(), "".to_owned(), "".to_owned())
};
let span = mk_sp(first.span.lo, second.span.hi);
span_lint_and_then(cx,
ALMOST_SWAPPED,
span,
&format!("this looks like you are trying to swap{}", what),
|db| {
if !what.is_empty() {
db.span_suggestion(span, "try",
format!("std::mem::swap(&mut {}, &mut {})", lhs, rhs));
db.fileline_note(span, "or maybe you should use `std::mem::replace`?");
}
});
}}
}
}

44
tests/compile-fail/swap.rs Executable file
View File

@ -0,0 +1,44 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(clippy)]
#![allow(unused_assignments)]
struct Foo(u32);
fn main() {
let mut a = 42;
let mut b = 1337;
a = b;
b = a;
//~^^ ERROR this looks like you are trying to swap `a` and `b`
//~| HELP try
//~| SUGGESTION std::mem::swap(&mut a, &mut b);
//~| NOTE or maybe you should use `std::mem::replace`?
let t = a;
a = b;
b = t;
//~^^^ ERROR this looks like you are swapping `a` and `b` manually
//~| HELP try
//~| SUGGESTION std::mem::swap(&mut a, &mut b);
//~| NOTE or maybe you should use `std::mem::replace`?
let mut c = Foo(42);
c.0 = a;
a = c.0;
//~^^ ERROR this looks like you are trying to swap `c.0` and `a`
//~| HELP try
//~| SUGGESTION std::mem::swap(&mut c.0, &mut a);
//~| NOTE or maybe you should use `std::mem::replace`?
let t = c.0;
c.0 = a;
a = t;
//~^^^ ERROR this looks like you are swapping `c.0` and `a` manually
//~| HELP try
//~| SUGGESTION std::mem::swap(&mut c.0, &mut a);
//~| NOTE or maybe you should use `std::mem::replace`?
}