src/methods.rs

   1 use rustc_front::hir::*;
   2 use rustc::lint::*;
   3 use rustc::middle::ty;
   4 use rustc::middle::subst::{Subst, TypeSpace};
   5 use std::iter;
   6 use std::borrow::Cow;
   7
   8 use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, walk_ptrs_ty_depth,
   9     walk_ptrs_ty};
  10 use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH};
  11
  12 use self::SelfKind::*;
  13 use self::OutType::*;
  14
  15 #[derive(Clone)]
  16 pub struct MethodsPass;
  17
  18 declare_lint!(pub OPTION_UNWRAP_USED, Allow,
  19               "using `Option.unwrap()`, which should at least get a better message using `expect()`");
  20 declare_lint!(pub RESULT_UNWRAP_USED, Allow,
  21               "using `Result.unwrap()`, which might be better handled");
  22 declare_lint!(pub STR_TO_STRING, Warn,
  23               "using `to_string()` on a str, which should be `to_owned()`");
  24 declare_lint!(pub STRING_TO_STRING, Warn,
  25               "calling `String.to_string()` which is a no-op");
  26 declare_lint!(pub SHOULD_IMPLEMENT_TRAIT, Warn,
  27               "defining a method that should be implementing a std trait");
  28 declare_lint!(pub WRONG_SELF_CONVENTION, Warn,
  29               "defining a method named with an established prefix (like \"into_\") that takes \
  30                `self` with the wrong convention");
  31 declare_lint!(pub WRONG_PUB_SELF_CONVENTION, Allow,
  32               "defining a public method named with an established prefix (like \"into_\") that takes \
  33                `self` with the wrong convention");
  34 declare_lint!(pub OK_EXPECT, Warn,
  35               "using `ok().expect()`, which gives worse error messages than \
  36                calling `expect` directly on the Result");
  37 declare_lint!(pub OPTION_MAP_UNWRAP_OR, Warn,
  38               "using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as \
  39                `map_or(a, f)`)");
  40 declare_lint!(pub OPTION_MAP_UNWRAP_OR_ELSE, Warn,
  41               "using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as \
  42                `map_or_else(g, f)`)");
  43
  44 impl LintPass for MethodsPass {
  45     fn get_lints(&self) -> LintArray {
  46         lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING,
  47                     SHOULD_IMPLEMENT_TRAIT, WRONG_SELF_CONVENTION, OK_EXPECT, OPTION_MAP_UNWRAP_OR,
  48                     OPTION_MAP_UNWRAP_OR_ELSE)
  49     }
  50 }
  51
  52 impl LateLintPass for MethodsPass {
  53     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
  54
  55         if let ExprMethodCall(ref name, _, ref args) = expr.node {
  56             let (obj_ty, ptr_depth) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&args[0]));
  57             match &*name.node.as_str() {
  58                 "unwrap" if match_type(cx, obj_ty, &OPTION_PATH) => {
  59                     span_lint(cx, OPTION_UNWRAP_USED, expr.span,
  60                               "used unwrap() on an Option value. If you don't want \
  61                                to handle the None case gracefully, consider using \
  62                                expect() to provide a better panic message");
  63                 },
  64                 "unwrap" if match_type(cx, obj_ty, &RESULT_PATH) => {
  65                     span_lint(cx, RESULT_UNWRAP_USED, expr.span,
  66                               "used unwrap() on a Result value. Graceful handling \
  67                                of Err values is preferred");
  68                 },
  69                 "to_string" if obj_ty.sty == ty::TyStr => {
  70                     let mut arg_str = snippet(cx, args[0].span, "_");
  71                     if ptr_depth > 1 {
  72                         arg_str = Cow::Owned(format!(
  73                             "({}{})",
  74                             iter::repeat('*').take(ptr_depth - 1).collect::<String>(),
  75                             arg_str));
  76                     }
  77                     span_lint(cx, STR_TO_STRING, expr.span, &format!(
  78                         "`{}.to_owned()` is faster", arg_str));
  79                 },
  80                 "to_string" if match_type(cx, obj_ty, &STRING_PATH) => {
  81                     span_lint(cx, STRING_TO_STRING, expr.span, "`String.to_string()` is a no-op; use \
  82                                                                 `clone()` to make a copy");
  83                 },
  84                 "expect" => if let ExprMethodCall(ref inner_name, _, ref inner_args) = args[0].node {
  85                     if inner_name.node.as_str() == "ok"
  86                             && match_type(cx, cx.tcx.expr_ty(&inner_args[0]), &RESULT_PATH) {
  87                         let result_type = cx.tcx.expr_ty(&inner_args[0]);
  88                         if let Some(error_type) = get_error_type(cx, result_type) {
  89                             if has_debug_impl(error_type, cx) {
  90                                 span_lint(cx, OK_EXPECT, expr.span,
  91                                          "called `ok().expect()` on a Result \
  92                                           value. You can call `expect` directly \
  93                                           on the `Result`");
  94                             }
  95                         }
  96                     }
  97                 },
  98                 // check Option.map(_).unwrap_or(_)
  99                 "unwrap_or" => if let ExprMethodCall(ref inner_name, _, ref inner_args) = args[0].node {
 100                     if inner_name.node.as_str() == "map"
 101                             && match_type(cx, cx.tcx.expr_ty(&inner_args[0]), &OPTION_PATH) {
 102                         // lint message
 103                         let msg =
 104                             "called `map(f).unwrap_or(a)` on an Option value. This can be done \
 105                              more directly by calling `map_or(a, f)` instead";
 106                         // get args to map() and unwrap_or()
 107                         let map_arg = snippet(cx, inner_args[1].span, "..");
 108                         let unwrap_arg = snippet(cx, args[1].span, "..");
 109                         // lint, with note if neither arg is > 1 line and both map() and
 110                         // unwrap_or() have the same span
 111                         let multiline = map_arg.lines().count() > 1
 112                                         || unwrap_arg.lines().count() > 1;
 113                         let same_span = inner_args[1].span.expn_id == args[1].span.expn_id;
 114                         if same_span && !multiline {
 115                             span_note_and_lint(
 116                                 cx, OPTION_MAP_UNWRAP_OR, expr.span, msg, expr.span,
 117                                 &format!("replace this with map_or({1}, {0})",
 118                                          map_arg, unwrap_arg)
 119                             );
 120                         }
 121                         else if same_span && multiline {
 122                             span_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg);
 123                         };
 124                     }
 125                 },
 126                 // check Option.map(_).unwrap_or_else(_)
 127                 "unwrap_or_else" => if let ExprMethodCall(ref inner_name, _, ref inner_args) = args[0].node {
 128                     if inner_name.node.as_str() == "map"
 129                             && match_type(cx, cx.tcx.expr_ty(&inner_args[0]), &OPTION_PATH) {
 130                         // lint message
 131                         let msg =
 132                             "called `map(f).unwrap_or_else(g)` on an Option value. This can be \
 133                              done more directly by calling `map_or_else(g, f)` instead";
 134                         // get args to map() and unwrap_or_else()
 135                         let map_arg = snippet(cx, inner_args[1].span, "..");
 136                         let unwrap_arg = snippet(cx, args[1].span, "..");
 137                         // lint, with note if neither arg is > 1 line and both map() and
 138                         // unwrap_or_else() have the same span
 139                         let multiline = map_arg.lines().count() > 1
 140                                         || unwrap_arg.lines().count() > 1;
 141                         let same_span = inner_args[1].span.expn_id == args[1].span.expn_id;
 142                         if same_span && !multiline {
 143                             span_note_and_lint(
 144                                 cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg, expr.span,
 145                                 &format!("replace this with map_or_else({1}, {0})",
 146                                          map_arg, unwrap_arg)
 147                             );
 148                         }
 149                         else if same_span && multiline {
 150                             span_lint(cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg);
 151                         };
 152                     }
 153                 },
 154                 _ => {},
 155             }
 156         }
 157     }
 158
 159     fn check_item(&mut self, cx: &LateContext, item: &Item) {
 160         if let ItemImpl(_, _, _, None, ref ty, ref items) = item.node {
 161             for implitem in items {
 162                 let name = implitem.name;
 163                 if let ImplItemKind::Method(ref sig, _) = implitem.node {
 164                     // check missing trait implementations
 165                     for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS {
 166                         if_let_chain! {
 167                             [
 168                                 name.as_str() == method_name,
 169                                 sig.decl.inputs.len() == n_args,
 170                                 out_type.matches(&sig.decl.output),
 171                                 self_kind.matches(&sig.explicit_self.node, false)
 172                             ], {
 173                                 span_lint(cx, SHOULD_IMPLEMENT_TRAIT, implitem.span, &format!(
 174                                     "defining a method called `{}` on this type; consider implementing \
 175                                      the `{}` trait or choosing a less ambiguous name", name, trait_name));
 176                             }
 177                         }
 178                     }
 179                     // check conventions w.r.t. conversion method names and predicates
 180                     let is_copy = is_copy(cx, &ty, &item);
 181                     for &(prefix, self_kinds) in &CONVENTIONS {
 182                         if name.as_str().starts_with(prefix) &&
 183                                 !self_kinds.iter().any(|k| k.matches(&sig.explicit_self.node, is_copy)) {
 184                             let lint = if item.vis == Visibility::Public {
 185                                 WRONG_PUB_SELF_CONVENTION
 186                             } else {
 187                                 WRONG_SELF_CONVENTION
 188                             };
 189                             span_lint(cx, lint, sig.explicit_self.span, &format!(
 190                                 "methods called `{}*` usually take {}; consider choosing a less \
 191                                  ambiguous name", prefix,
 192                                 &self_kinds.iter().map(|k| k.description()).collect::<Vec<_>>().join(" or ")));
 193                         }
 194                     }
 195                 }
 196             }
 197         }
 198     }
 199 }
 200
 201 // Given a `Result<T, E>` type, return its error type (`E`)
 202 fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option<ty::Ty<'a>> {
 203     if !match_type(cx, ty, &RESULT_PATH) {
 204         return None;
 205     }
 206     if let ty::TyEnum(_, substs) = ty.sty {
 207         if let Some(err_ty) = substs.types.opt_get(TypeSpace, 1) {
 208             return Some(err_ty);
 209         }
 210     }
 211     None
 212 }
 213
 214 // This checks whether a given type is known to implement Debug. It's
 215 // conservative, i.e. it should not return false positives, but will return
 216 // false negatives.
 217 fn has_debug_impl<'a, 'b>(ty: ty::Ty<'a>, cx: &LateContext<'b, 'a>) -> bool {
 218     let no_ref_ty = walk_ptrs_ty(ty);
 219     let debug = match cx.tcx.lang_items.debug_trait() {
 220         Some(debug) => debug,
 221         None => return false
 222     };
 223     let debug_def = cx.tcx.lookup_trait_def(debug);
 224     let mut debug_impl_exists = false;
 225     debug_def.for_each_relevant_impl(cx.tcx, no_ref_ty, |d| {
 226         let self_ty = &cx.tcx.impl_trait_ref(d).and_then(|im| im.substs.self_ty());
 227         if let Some(self_ty) = *self_ty {
 228             if !self_ty.flags.get().contains(ty::TypeFlags::HAS_PARAMS) {
 229                 debug_impl_exists = true;
 230             }
 231         }
 232     });
 233     debug_impl_exists
 234 }
 235
 236 const CONVENTIONS: [(&'static str, &'static [SelfKind]); 5] = [
 237     ("into_", &[ValueSelf]),
 238     ("to_",   &[RefSelf]),
 239     ("as_",   &[RefSelf, RefMutSelf]),
 240     ("is_",   &[RefSelf, NoSelf]),
 241     ("from_", &[NoSelf]),
 242 ];
 243
 244 const TRAIT_METHODS: [(&'static str, usize, SelfKind, OutType, &'static str); 30] = [
 245     ("add",        2, ValueSelf,  AnyType,  "std::ops::Add"),
 246     ("sub",        2, ValueSelf,  AnyType,  "std::ops::Sub"),
 247     ("mul",        2, ValueSelf,  AnyType,  "std::ops::Mul"),
 248     ("div",        2, ValueSelf,  AnyType,  "std::ops::Div"),
 249     ("rem",        2, ValueSelf,  AnyType,  "std::ops::Rem"),
 250     ("shl",        2, ValueSelf,  AnyType,  "std::ops::Shl"),
 251     ("shr",        2, ValueSelf,  AnyType,  "std::ops::Shr"),
 252     ("bitand",     2, ValueSelf,  AnyType,  "std::ops::BitAnd"),
 253     ("bitor",      2, ValueSelf,  AnyType,  "std::ops::BitOr"),
 254     ("bitxor",     2, ValueSelf,  AnyType,  "std::ops::BitXor"),
 255     ("neg",        1, ValueSelf,  AnyType,  "std::ops::Neg"),
 256     ("not",        1, ValueSelf,  AnyType,  "std::ops::Not"),
 257     ("drop",       1, RefMutSelf, UnitType, "std::ops::Drop"),
 258     ("index",      2, RefSelf,    RefType,  "std::ops::Index"),
 259     ("index_mut",  2, RefMutSelf, RefType,  "std::ops::IndexMut"),
 260     ("deref",      1, RefSelf,    RefType,  "std::ops::Deref"),
 261     ("deref_mut",  1, RefMutSelf, RefType,  "std::ops::DerefMut"),
 262     ("clone",      1, RefSelf,    AnyType,  "std::clone::Clone"),
 263     ("borrow",     1, RefSelf,    RefType,  "std::borrow::Borrow"),
 264     ("borrow_mut", 1, RefMutSelf, RefType,  "std::borrow::BorrowMut"),
 265     ("as_ref",     1, RefSelf,    RefType,  "std::convert::AsRef"),
 266     ("as_mut",     1, RefMutSelf, RefType,  "std::convert::AsMut"),
 267     ("eq",         2, RefSelf,    BoolType, "std::cmp::PartialEq"),
 268     ("cmp",        2, RefSelf,    AnyType,  "std::cmp::Ord"),
 269     ("default",    0, NoSelf,     AnyType,  "std::default::Default"),
 270     ("hash",       2, RefSelf,    UnitType, "std::hash::Hash"),
 271     ("next",       1, RefMutSelf, AnyType,  "std::iter::Iterator"),
 272     ("into_iter",  1, ValueSelf,  AnyType,  "std::iter::IntoIterator"),
 273     ("from_iter",  1, NoSelf,     AnyType,  "std::iter::FromIterator"),
 274     ("from_str",   1, NoSelf,     AnyType,  "std::str::FromStr"),
 275 ];
 276
 277 #[derive(Clone, Copy)]
 278 enum SelfKind {
 279     ValueSelf,
 280     RefSelf,
 281     RefMutSelf,
 282     NoSelf,
 283 }
 284
 285 impl SelfKind {
 286     fn matches(&self, slf: &ExplicitSelf_, allow_value_for_ref: bool) -> bool {
 287         match (self, slf) {
 288             (&ValueSelf, &SelfValue(_)) => true,
 289             (&RefSelf, &SelfRegion(_, Mutability::MutImmutable, _)) => true,
 290             (&RefMutSelf, &SelfRegion(_, Mutability::MutMutable, _)) => true,
 291             (&RefSelf, &SelfValue(_)) => allow_value_for_ref,
 292             (&RefMutSelf, &SelfValue(_)) => allow_value_for_ref,
 293             (&NoSelf, &SelfStatic) => true,
 294             (_, &SelfExplicit(ref ty, _)) => self.matches_explicit_type(ty, allow_value_for_ref),
 295             _ => false
 296         }
 297     }
 298
 299     fn matches_explicit_type(&self, ty: &Ty, allow_value_for_ref: bool) -> bool {
 300         match (self, &ty.node) {
 301             (&ValueSelf, &TyPath(..)) => true,
 302             (&RefSelf, &TyRptr(_, MutTy { mutbl: Mutability::MutImmutable, .. })) => true,
 303             (&RefMutSelf, &TyRptr(_, MutTy { mutbl: Mutability::MutMutable, .. })) => true,
 304             (&RefSelf, &TyPath(..)) => allow_value_for_ref,
 305             (&RefMutSelf, &TyPath(..)) => allow_value_for_ref,
 306             _ => false
 307         }
 308     }
 309
 310     fn description(&self) -> &'static str {
 311         match *self {
 312             ValueSelf => "self by value",
 313             RefSelf => "self by reference",
 314             RefMutSelf => "self by mutable reference",
 315             NoSelf => "no self",
 316         }
 317     }
 318 }
 319
 320 #[derive(Clone, Copy)]
 321 enum OutType {
 322     UnitType,
 323     BoolType,
 324     AnyType,
 325     RefType,
 326 }
 327
 328 impl OutType {
 329     fn matches(&self, ty: &FunctionRetTy) -> bool {
 330         match (self, ty) {
 331             (&UnitType, &DefaultReturn(_)) => true,
 332             (&UnitType, &Return(ref ty)) if ty.node == TyTup(vec![]) => true,
 333             (&BoolType, &Return(ref ty)) if is_bool(ty) => true,
 334             (&AnyType, &Return(ref ty)) if ty.node != TyTup(vec![])  => true,
 335             (&RefType, &Return(ref ty)) => {
 336                 if let TyRptr(_, _) = ty.node { true } else { false }
 337             }
 338             _ => false
 339         }
 340     }
 341 }
 342
 343 fn is_bool(ty: &Ty) -> bool {
 344     if let TyPath(None, ref p) = ty.node {
 345         if match_path(p, &["bool"]) {
 346             return true;
 347         }
 348     }
 349     false
 350 }
 351
 352 fn is_copy(cx: &LateContext, ast_ty: &Ty, item: &Item) -> bool {
 353     match cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) {
 354         None => false,
 355         Some(ty) => {
 356             let env = ty::ParameterEnvironment::for_item(cx.tcx, item.id);
 357             !ty.subst(cx.tcx, &env.free_substs).moves_by_default(&env, ast_ty.span)
 358         }
 359     }
 360 }