mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-09-20 22:51:58 +00:00
Merge branch 'pr-228'
Conflicts: README.md src/methods.rs
This commit is contained in:
Manish Goregaokar
commit
193e71be61
4 changed files with 140 additions and 5 deletions
|
|
@ -4,7 +4,7 @@
|
|||
A collection of lints that give helpful tips to newbies and catch oversights.
|
||||
|
||||
##Lints
|
||||
There are 48 lints included in this crate:
|
||||
There are 49 lints included in this crate:
|
||||
|
||||
name | default | meaning
|
||||
-----------------------------------------------------------------------------------------------------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
|
||||
|
|
@ -47,6 +47,7 @@ name
|
|||
[shadow_reuse](https://github.com/Manishearth/rust-clippy/wiki#shadow_reuse) | allow | rebinding a name to an expression that re-uses the original value, e.g. `let x = x + 1`
|
||||
[shadow_same](https://github.com/Manishearth/rust-clippy/wiki#shadow_same) | allow | rebinding a name to itself, e.g. `let mut x = &mut x`
|
||||
[shadow_unrelated](https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated) | warn | The name is re-bound without even using the original value
|
||||
[should_implement_trait](https://github.com/Manishearth/rust-clippy/wiki#should_implement_trait) | warn | defining a method that should be implementing a std trait
|
||||
[single_match](https://github.com/Manishearth/rust-clippy/wiki#single_match) | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead
|
||||
[str_to_string](https://github.com/Manishearth/rust-clippy/wiki#str_to_string) | warn | using `to_string()` on a str, which should be `to_owned()`
|
||||
[string_add](https://github.com/Manishearth/rust-clippy/wiki#string_add) | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead
|
||||
|
|
|
|||
|
|
@ -100,6 +100,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
|
|||
matches::SINGLE_MATCH,
|
||||
methods::OPTION_UNWRAP_USED,
|
||||
methods::RESULT_UNWRAP_USED,
|
||||
methods::SHOULD_IMPLEMENT_TRAIT,
|
||||
methods::STR_TO_STRING,
|
||||
methods::STRING_TO_STRING,
|
||||
misc::CMP_NAN,
|
||||
|
|
|
|||
118
src/methods.rs
118
src/methods.rs
|
|
@ -4,9 +4,12 @@ use rustc::middle::ty;
|
|||
use std::iter;
|
||||
use std::borrow::Cow;
|
||||
|
||||
use utils::{snippet, span_lint, match_type, walk_ptrs_ty_depth};
|
||||
use utils::{snippet, span_lint, match_path, match_type, walk_ptrs_ty_depth};
|
||||
use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH};
|
||||
|
||||
use self::SelfKind::*;
|
||||
use self::OutType::*;
|
||||
|
||||
#[derive(Copy,Clone)]
|
||||
pub struct MethodsPass;
|
||||
|
||||
|
|
@ -18,10 +21,13 @@ declare_lint!(pub STR_TO_STRING, Warn,
|
|||
"using `to_string()` on a str, which should be `to_owned()`");
|
||||
declare_lint!(pub STRING_TO_STRING, Warn,
|
||||
"calling `String.to_string()` which is a no-op");
|
||||
declare_lint!(pub SHOULD_IMPLEMENT_TRAIT, Warn,
|
||||
"defining a method that should be implementing a std trait");
|
||||
|
||||
impl LintPass for MethodsPass {
|
||||
fn get_lints(&self) -> LintArray {
|
||||
lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING)
|
||||
lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING,
|
||||
SHOULD_IMPLEMENT_TRAIT)
|
||||
}
|
||||
|
||||
fn check_expr(&mut self, cx: &Context, expr: &Expr) {
|
||||
|
|
@ -57,4 +63,112 @@ impl LintPass for MethodsPass {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_item(&mut self, cx: &Context, item: &Item) {
|
||||
if let ItemImpl(_, _, _, None, _, ref items) = item.node {
|
||||
for item in items {
|
||||
let name = item.ident.name;
|
||||
for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS {
|
||||
if_let_chain! {
|
||||
[
|
||||
name == method_name,
|
||||
let MethodImplItem(ref sig, _) = item.node,
|
||||
sig.decl.inputs.len() == n_args,
|
||||
out_type.matches(&sig.decl.output),
|
||||
self_kind.matches(&sig.explicit_self.node)
|
||||
], {
|
||||
span_lint(cx, SHOULD_IMPLEMENT_TRAIT, item.span, &format!(
|
||||
"defining a method called `{}` on this type; consider implementing \
|
||||
the `{}` trait or choosing a less ambiguous name", name, trait_name));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const TRAIT_METHODS: [(&'static str, usize, SelfKind, OutType, &'static str); 30] = [
|
||||
("add", 2, ValueSelf, AnyType, "std::ops::Add`"),
|
||||
("sub", 2, ValueSelf, AnyType, "std::ops::Sub"),
|
||||
("mul", 2, ValueSelf, AnyType, "std::ops::Mul"),
|
||||
("div", 2, ValueSelf, AnyType, "std::ops::Div"),
|
||||
("rem", 2, ValueSelf, AnyType, "std::ops::Rem"),
|
||||
("shl", 2, ValueSelf, AnyType, "std::ops::Shl"),
|
||||
("shr", 2, ValueSelf, AnyType, "std::ops::Shr"),
|
||||
("bitand", 2, ValueSelf, AnyType, "std::ops::BitAnd"),
|
||||
("bitor", 2, ValueSelf, AnyType, "std::ops::BitOr"),
|
||||
("bitxor", 2, ValueSelf, AnyType, "std::ops::BitXor"),
|
||||
("neg", 1, ValueSelf, AnyType, "std::ops::Neg"),
|
||||
("not", 1, ValueSelf, AnyType, "std::ops::Not"),
|
||||
("drop", 1, RefMutSelf, UnitType, "std::ops::Drop"),
|
||||
("index", 2, RefSelf, RefType, "std::ops::Index"),
|
||||
("index_mut", 2, RefMutSelf, RefType, "std::ops::IndexMut"),
|
||||
("deref", 1, RefSelf, RefType, "std::ops::Deref"),
|
||||
("deref_mut", 1, RefMutSelf, RefType, "std::ops::DerefMut"),
|
||||
("clone", 1, RefSelf, AnyType, "std::clone::Clone"),
|
||||
("borrow", 1, RefSelf, RefType, "std::borrow::Borrow"),
|
||||
("borrow_mut", 1, RefMutSelf, RefType, "std::borrow::BorrowMut"),
|
||||
("as_ref", 1, RefSelf, RefType, "std::convert::AsRef"),
|
||||
("as_mut", 1, RefMutSelf, RefType, "std::convert::AsMut"),
|
||||
("eq", 2, RefSelf, BoolType, "std::cmp::PartialEq"),
|
||||
("cmp", 2, RefSelf, AnyType, "std::cmp::Ord"),
|
||||
("default", 0, NoSelf, AnyType, "std::default::Default"),
|
||||
("hash", 2, RefSelf, UnitType, "std::hash::Hash"),
|
||||
("next", 1, RefMutSelf, AnyType, "std::iter::Iterator"),
|
||||
("into_iter", 1, ValueSelf, AnyType, "std::iter::IntoIterator"),
|
||||
("from_iter", 1, NoSelf, AnyType, "std::iter::FromIterator"),
|
||||
("from_str", 1, NoSelf, AnyType, "std::str::FromStr"),
|
||||
];
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum SelfKind {
|
||||
ValueSelf,
|
||||
RefSelf,
|
||||
RefMutSelf,
|
||||
NoSelf
|
||||
}
|
||||
|
||||
impl SelfKind {
|
||||
fn matches(&self, slf: &ExplicitSelf_) -> bool {
|
||||
match (self, slf) {
|
||||
(&ValueSelf, &SelfValue(_)) => true,
|
||||
(&RefSelf, &SelfRegion(_, Mutability::MutImmutable, _)) => true,
|
||||
(&RefMutSelf, &SelfRegion(_, Mutability::MutMutable, _)) => true,
|
||||
(&NoSelf, &SelfStatic) => true,
|
||||
_ => false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum OutType {
|
||||
UnitType,
|
||||
BoolType,
|
||||
AnyType,
|
||||
RefType,
|
||||
}
|
||||
|
||||
impl OutType {
|
||||
fn matches(&self, ty: &FunctionRetTy) -> bool {
|
||||
match (self, ty) {
|
||||
(&UnitType, &DefaultReturn(_)) => true,
|
||||
(&UnitType, &Return(ref ty)) if ty.node == TyTup(vec![]) => true,
|
||||
(&BoolType, &Return(ref ty)) if is_bool(ty) => true,
|
||||
(&AnyType, &Return(ref ty)) if ty.node != TyTup(vec![]) => true,
|
||||
(&RefType, &Return(ref ty)) => {
|
||||
if let TyRptr(_, _) = ty.node { true } else { false }
|
||||
}
|
||||
_ => false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn is_bool(ty: &Ty) -> bool {
|
||||
if let TyPath(None, ref p) = ty.node {
|
||||
if match_path(p, &["bool"]) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,8 +1,27 @@
|
|||
#![feature(plugin)]
|
||||
#![plugin(clippy)]
|
||||
|
||||
#[deny(option_unwrap_used, result_unwrap_used)]
|
||||
#[deny(str_to_string, string_to_string)]
|
||||
#![allow(unused)]
|
||||
#![deny(clippy)]
|
||||
|
||||
use std::ops::Mul;
|
||||
|
||||
struct T;
|
||||
|
||||
impl T {
|
||||
fn add(self, other: T) -> T { self } //~ERROR defining a method called `add`
|
||||
fn drop(&mut self) { } //~ERROR defining a method called `drop`
|
||||
|
||||
fn sub(&self, other: T) -> &T { self } // no error, self is a ref
|
||||
fn div(self) -> T { self } // no error, different #arguments
|
||||
fn rem(self, other: T) { } // no error, wrong return type
|
||||
}
|
||||
|
||||
impl Mul<T> for T {
|
||||
type Output = T;
|
||||
fn mul(self, other: T) -> T { self } // no error, obviously
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let opt = Some(0);
|
||||
let _ = opt.unwrap(); //~ERROR used unwrap() on an Option
|
||||
|
|
|
|||
Loading…
Reference in a new issue