src/tools/clippy/clippy_lints/src/ptr.rs

   1 //! Checks for usage of  `&Vec[_]` and `&String`.
   2
   3 use crate::utils::ptr::get_spans;
   4 use crate::utils::{
   5     is_allowed, is_type_diagnostic_item, match_qpath, match_type, paths, snippet_opt, span_lint, span_lint_and_sugg,
   6     span_lint_and_then, walk_ptrs_hir_ty,
   7 };
   8 use if_chain::if_chain;
   9 use rustc_errors::Applicability;
  10 use rustc_hir::{
  11     BinOpKind, BodyId, Expr, ExprKind, FnDecl, FnRetTy, GenericArg, HirId, ImplItem, ImplItemKind, Item, ItemKind,
  12     Lifetime, MutTy, Mutability, Node, PathSegment, QPath, TraitFn, TraitItem, TraitItemKind, Ty, TyKind,
  13 };
  14 use rustc_lint::{LateContext, LateLintPass};
  15 use rustc_middle::ty;
  16 use rustc_session::{declare_lint_pass, declare_tool_lint};
  17 use rustc_span::source_map::Span;
  18 use rustc_span::MultiSpan;
  19 use std::borrow::Cow;
  20
  21 declare_clippy_lint! {
  22     /// **What it does:** This lint checks for function arguments of type `&String`
  23     /// or `&Vec` unless the references are mutable. It will also suggest you
  24     /// replace `.clone()` calls with the appropriate `.to_owned()`/`to_string()`
  25     /// calls.
  26     ///
  27     /// **Why is this bad?** Requiring the argument to be of the specific size
  28     /// makes the function less useful for no benefit; slices in the form of `&[T]`
  29     /// or `&str` usually suffice and can be obtained from other types, too.
  30     ///
  31     /// **Known problems:** The lint does not follow data. So if you have an
  32     /// argument `x` and write `let y = x; y.clone()` the lint will not suggest
  33     /// changing that `.clone()` to `.to_owned()`.
  34     ///
  35     /// Other functions called from this function taking a `&String` or `&Vec`
  36     /// argument may also fail to compile if you change the argument. Applying
  37     /// this lint on them will fix the problem, but they may be in other crates.
  38     ///
  39     /// Also there may be `fn(&Vec)`-typed references pointing to your function.
  40     /// If you have them, you will get a compiler error after applying this lint's
  41     /// suggestions. You then have the choice to undo your changes or change the
  42     /// type of the reference.
  43     ///
  44     /// Note that if the function is part of your public interface, there may be
  45     /// other crates referencing it you may not be aware. Carefully deprecate the
  46     /// function before applying the lint suggestions in this case.
  47     ///
  48     /// **Example:**
  49     /// ```ignore
  50     /// // Bad
  51     /// fn foo(&Vec<u32>) { .. }
  52     ///
  53     /// // Good
  54     /// fn foo(&[u32]) { .. }
  55     /// ```
  56     pub PTR_ARG,
  57     style,
  58     "fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively"
  59 }
  60
  61 declare_clippy_lint! {
  62     /// **What it does:** This lint checks for equality comparisons with `ptr::null`
  63     ///
  64     /// **Why is this bad?** It's easier and more readable to use the inherent
  65     /// `.is_null()`
  66     /// method instead
  67     ///
  68     /// **Known problems:** None.
  69     ///
  70     /// **Example:**
  71     /// ```ignore
  72     /// // Bad
  73     /// if x == ptr::null {
  74     ///     ..
  75     /// }
  76     ///
  77     /// // Good
  78     /// if x.is_null() {
  79     ///     ..
  80     /// }
  81     /// ```
  82     pub CMP_NULL,
  83     style,
  84     "comparing a pointer to a null pointer, suggesting to use `.is_null()` instead."
  85 }
  86
  87 declare_clippy_lint! {
  88     /// **What it does:** This lint checks for functions that take immutable
  89     /// references and return mutable ones.
  90     ///
  91     /// **Why is this bad?** This is trivially unsound, as one can create two
  92     /// mutable references from the same (immutable!) source.
  93     /// This [error](https://github.com/rust-lang/rust/issues/39465)
  94     /// actually lead to an interim Rust release 1.15.1.
  95     ///
  96     /// **Known problems:** To be on the conservative side, if there's at least one
  97     /// mutable reference with the output lifetime, this lint will not trigger.
  98     /// In practice, this case is unlikely anyway.
  99     ///
 100     /// **Example:**
 101     /// ```ignore
 102     /// fn foo(&Foo) -> &mut Bar { .. }
 103     /// ```
 104     pub MUT_FROM_REF,
 105     correctness,
 106     "fns that create mutable refs from immutable ref args"
 107 }
 108
 109 declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF]);
 110
 111 impl<'tcx> LateLintPass<'tcx> for Ptr {
 112     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
 113         if let ItemKind::Fn(ref sig, _, body_id) = item.kind {
 114             check_fn(cx, &sig.decl, item.hir_id, Some(body_id));
 115         }
 116     }
 117
 118     fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
 119         if let ImplItemKind::Fn(ref sig, body_id) = item.kind {
 120             let parent_item = cx.tcx.hir().get_parent_item(item.hir_id);
 121             if let Some(Node::Item(it)) = cx.tcx.hir().find(parent_item) {
 122                 if let ItemKind::Impl { of_trait: Some(_), .. } = it.kind {
 123                     return; // ignore trait impls
 124                 }
 125             }
 126             check_fn(cx, &sig.decl, item.hir_id, Some(body_id));
 127         }
 128     }
 129
 130     fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
 131         if let TraitItemKind::Fn(ref sig, ref trait_method) = item.kind {
 132             let body_id = if let TraitFn::Provided(b) = *trait_method {
 133                 Some(b)
 134             } else {
 135                 None
 136             };
 137             check_fn(cx, &sig.decl, item.hir_id, body_id);
 138         }
 139     }
 140
 141     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 142         if let ExprKind::Binary(ref op, ref l, ref r) = expr.kind {
 143             if (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne) && (is_null_path(l) || is_null_path(r)) {
 144                 span_lint(
 145                     cx,
 146                     CMP_NULL,
 147                     expr.span,
 148                     "Comparing with null is better expressed by the `.is_null()` method",
 149                 );
 150             }
 151         }
 152     }
 153 }
 154
 155 #[allow(clippy::too_many_lines)]
 156 fn check_fn(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_id: HirId, opt_body_id: Option<BodyId>) {
 157     let fn_def_id = cx.tcx.hir().local_def_id(fn_id);
 158     let sig = cx.tcx.fn_sig(fn_def_id);
 159     let fn_ty = sig.skip_binder();
 160     let body = opt_body_id.map(|id| cx.tcx.hir().body(id));
 161
 162     for (idx, (arg, ty)) in decl.inputs.iter().zip(fn_ty.inputs()).enumerate() {
 163         // Honor the allow attribute on parameters. See issue 5644.
 164         if let Some(body) = &body {
 165             if is_allowed(cx, PTR_ARG, body.params[idx].hir_id) {
 166                 continue;
 167             }
 168         }
 169
 170         if let ty::Ref(_, ty, Mutability::Not) = ty.kind {
 171             if is_type_diagnostic_item(cx, ty, sym!(vec_type)) {
 172                 let mut ty_snippet = None;
 173                 if_chain! {
 174                     if let TyKind::Path(QPath::Resolved(_, ref path)) = walk_ptrs_hir_ty(arg).kind;
 175                     if let Some(&PathSegment{args: Some(ref parameters), ..}) = path.segments.last();
 176                     then {
 177                         let types: Vec<_> = parameters.args.iter().filter_map(|arg| match arg {
 178                             GenericArg::Type(ty) => Some(ty),
 179                             _ => None,
 180                         }).collect();
 181                         if types.len() == 1 {
 182                             ty_snippet = snippet_opt(cx, types[0].span);
 183                         }
 184                     }
 185                 };
 186                 if let Some(spans) = get_spans(cx, opt_body_id, idx, &[("clone", ".to_owned()")]) {
 187                     span_lint_and_then(
 188                         cx,
 189                         PTR_ARG,
 190                         arg.span,
 191                         "writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used \
 192                          with non-Vec-based slices.",
 193                         |diag| {
 194                             if let Some(ref snippet) = ty_snippet {
 195                                 diag.span_suggestion(
 196                                     arg.span,
 197                                     "change this to",
 198                                     format!("&[{}]", snippet),
 199                                     Applicability::Unspecified,
 200                                 );
 201                             }
 202                             for (clonespan, suggestion) in spans {
 203                                 diag.span_suggestion(
 204                                     clonespan,
 205                                     &snippet_opt(cx, clonespan).map_or("change the call to".into(), |x| {
 206                                         Cow::Owned(format!("change `{}` to", x))
 207                                     }),
 208                                     suggestion.into(),
 209                                     Applicability::Unspecified,
 210                                 );
 211                             }
 212                         },
 213                     );
 214                 }
 215             } else if is_type_diagnostic_item(cx, ty, sym!(string_type)) {
 216                 if let Some(spans) = get_spans(cx, opt_body_id, idx, &[("clone", ".to_string()"), ("as_str", "")]) {
 217                     span_lint_and_then(
 218                         cx,
 219                         PTR_ARG,
 220                         arg.span,
 221                         "writing `&String` instead of `&str` involves a new object where a slice will do.",
 222                         |diag| {
 223                             diag.span_suggestion(arg.span, "change this to", "&str".into(), Applicability::Unspecified);
 224                             for (clonespan, suggestion) in spans {
 225                                 diag.span_suggestion_short(
 226                                     clonespan,
 227                                     &snippet_opt(cx, clonespan).map_or("change the call to".into(), |x| {
 228                                         Cow::Owned(format!("change `{}` to", x))
 229                                     }),
 230                                     suggestion.into(),
 231                                     Applicability::Unspecified,
 232                                 );
 233                             }
 234                         },
 235                     );
 236                 }
 237             } else if match_type(cx, ty, &paths::COW) {
 238                 if_chain! {
 239                     if let TyKind::Rptr(_, MutTy { ref ty, ..} ) = arg.kind;
 240                     if let TyKind::Path(ref path) = ty.kind;
 241                     if let QPath::Resolved(None, ref pp) = *path;
 242                     if let [ref bx] = *pp.segments;
 243                     if let Some(ref params) = bx.args;
 244                     if !params.parenthesized;
 245                     if let Some(inner) = params.args.iter().find_map(|arg| match arg {
 246                         GenericArg::Type(ty) => Some(ty),
 247                         _ => None,
 248                     });
 249                     then {
 250                         let replacement = snippet_opt(cx, inner.span);
 251                         if let Some(r) = replacement {
 252                             span_lint_and_sugg(
 253                                 cx,
 254                                 PTR_ARG,
 255                                 arg.span,
 256                                 "using a reference to `Cow` is not recommended.",
 257                                 "change this to",
 258                                 "&".to_owned() + &r,
 259                                 Applicability::Unspecified,
 260                             );
 261                         }
 262                     }
 263                 }
 264             }
 265         }
 266     }
 267
 268     if let FnRetTy::Return(ref ty) = decl.output {
 269         if let Some((out, Mutability::Mut, _)) = get_rptr_lm(ty) {
 270             let mut immutables = vec![];
 271             for (_, ref mutbl, ref argspan) in decl
 272                 .inputs
 273                 .iter()
 274                 .filter_map(|ty| get_rptr_lm(ty))
 275                 .filter(|&(lt, _, _)| lt.name == out.name)
 276             {
 277                 if *mutbl == Mutability::Mut {
 278                     return;
 279                 }
 280                 immutables.push(*argspan);
 281             }
 282             if immutables.is_empty() {
 283                 return;
 284             }
 285             span_lint_and_then(
 286                 cx,
 287                 MUT_FROM_REF,
 288                 ty.span,
 289                 "mutable borrow from immutable input(s)",
 290                 |diag| {
 291                     let ms = MultiSpan::from_spans(immutables);
 292                     diag.span_note(ms, "immutable borrow here");
 293                 },
 294             );
 295         }
 296     }
 297 }
 298
 299 fn get_rptr_lm<'tcx>(ty: &'tcx Ty<'tcx>) -> Option<(&'tcx Lifetime, Mutability, Span)> {
 300     if let TyKind::Rptr(ref lt, ref m) = ty.kind {
 301         Some((lt, m.mutbl, ty.span))
 302     } else {
 303         None
 304     }
 305 }
 306
 307 fn is_null_path(expr: &Expr<'_>) -> bool {
 308     if let ExprKind::Call(ref pathexp, ref args) = expr.kind {
 309         if args.is_empty() {
 310             if let ExprKind::Path(ref path) = pathexp.kind {
 311                 return match_qpath(path, &paths::PTR_NULL) || match_qpath(path, &paths::PTR_NULL_MUT);
 312             }
 313         }
 314     }
 315     false
 316 }