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

   1 //! Checks for usage of  `&Vec[_]` and `&String`.
   2
   3 use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
   4 use clippy_utils::ptr::get_spans;
   5 use clippy_utils::source::snippet_opt;
   6 use clippy_utils::ty::{is_type_diagnostic_item, match_type, walk_ptrs_hir_ty};
   7 use clippy_utils::{expr_path_res, is_allowed, match_any_def_paths, paths};
   8 use if_chain::if_chain;
   9 use rustc_errors::Applicability;
  10 use rustc_hir::{
  11     BinOpKind, BodyId, Expr, ExprKind, FnDecl, FnRetTy, GenericArg, HirId, Impl, ImplItem, ImplItemKind, Item,
  12     ItemKind, 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::symbol::Symbol;
  19 use rustc_span::{sym, MultiSpan};
  20 use std::borrow::Cow;
  21
  22 declare_clippy_lint! {
  23     /// **What it does:** This lint checks for function arguments of type `&String`
  24     /// or `&Vec` unless the references are mutable. It will also suggest you
  25     /// replace `.clone()` calls with the appropriate `.to_owned()`/`to_string()`
  26     /// calls.
  27     ///
  28     /// **Why is this bad?** Requiring the argument to be of the specific size
  29     /// makes the function less useful for no benefit; slices in the form of `&[T]`
  30     /// or `&str` usually suffice and can be obtained from other types, too.
  31     ///
  32     /// **Known problems:** The lint does not follow data. So if you have an
  33     /// argument `x` and write `let y = x; y.clone()` the lint will not suggest
  34     /// changing that `.clone()` to `.to_owned()`.
  35     ///
  36     /// Other functions called from this function taking a `&String` or `&Vec`
  37     /// argument may also fail to compile if you change the argument. Applying
  38     /// this lint on them will fix the problem, but they may be in other crates.
  39     ///
  40     /// One notable example of a function that may cause issues, and which cannot
  41     /// easily be changed due to being in the standard library is `Vec::contains`.
  42     /// when called on a `Vec<Vec<T>>`. If a `&Vec` is passed to that method then
  43     /// it will compile, but if a `&[T]` is passed then it will not compile.
  44     ///
  45     /// ```ignore
  46     /// fn cannot_take_a_slice(v: &Vec<u8>) -> bool {
  47     ///     let vec_of_vecs: Vec<Vec<u8>> = some_other_fn();
  48     ///
  49     ///     vec_of_vecs.contains(v)
  50     /// }
  51     /// ```
  52     ///
  53     /// Also there may be `fn(&Vec)`-typed references pointing to your function.
  54     /// If you have them, you will get a compiler error after applying this lint's
  55     /// suggestions. You then have the choice to undo your changes or change the
  56     /// type of the reference.
  57     ///
  58     /// Note that if the function is part of your public interface, there may be
  59     /// other crates referencing it, of which you may not be aware. Carefully
  60     /// deprecate the function before applying the lint suggestions in this case.
  61     ///
  62     /// **Example:**
  63     /// ```ignore
  64     /// // Bad
  65     /// fn foo(&Vec<u32>) { .. }
  66     ///
  67     /// // Good
  68     /// fn foo(&[u32]) { .. }
  69     /// ```
  70     pub PTR_ARG,
  71     style,
  72     "fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively"
  73 }
  74
  75 declare_clippy_lint! {
  76     /// **What it does:** This lint checks for equality comparisons with `ptr::null`
  77     ///
  78     /// **Why is this bad?** It's easier and more readable to use the inherent
  79     /// `.is_null()`
  80     /// method instead
  81     ///
  82     /// **Known problems:** None.
  83     ///
  84     /// **Example:**
  85     /// ```ignore
  86     /// // Bad
  87     /// if x == ptr::null {
  88     ///     ..
  89     /// }
  90     ///
  91     /// // Good
  92     /// if x.is_null() {
  93     ///     ..
  94     /// }
  95     /// ```
  96     pub CMP_NULL,
  97     style,
  98     "comparing a pointer to a null pointer, suggesting to use `.is_null()` instead"
  99 }
 100
 101 declare_clippy_lint! {
 102     /// **What it does:** This lint checks for functions that take immutable
 103     /// references and return mutable ones.
 104     ///
 105     /// **Why is this bad?** This is trivially unsound, as one can create two
 106     /// mutable references from the same (immutable!) source.
 107     /// This [error](https://github.com/rust-lang/rust/issues/39465)
 108     /// actually lead to an interim Rust release 1.15.1.
 109     ///
 110     /// **Known problems:** To be on the conservative side, if there's at least one
 111     /// mutable reference with the output lifetime, this lint will not trigger.
 112     /// In practice, this case is unlikely anyway.
 113     ///
 114     /// **Example:**
 115     /// ```ignore
 116     /// fn foo(&Foo) -> &mut Bar { .. }
 117     /// ```
 118     pub MUT_FROM_REF,
 119     correctness,
 120     "fns that create mutable refs from immutable ref args"
 121 }
 122
 123 declare_clippy_lint! {
 124     /// **What it does:** This lint checks for invalid usages of `ptr::null`.
 125     ///
 126     /// **Why is this bad?** This causes undefined behavior.
 127     ///
 128     /// **Known problems:** None.
 129     ///
 130     /// **Example:**
 131     /// ```ignore
 132     /// // Bad. Undefined behavior
 133     /// unsafe { std::slice::from_raw_parts(ptr::null(), 0); }
 134     /// ```
 135     ///
 136     /// // Good
 137     /// unsafe { std::slice::from_raw_parts(NonNull::dangling().as_ptr(), 0); }
 138     /// ```
 139     pub INVALID_NULL_PTR_USAGE,
 140     correctness,
 141     "invalid usage of a null pointer, suggesting `NonNull::dangling()` instead"
 142 }
 143
 144 declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF, INVALID_NULL_PTR_USAGE]);
 145
 146 impl<'tcx> LateLintPass<'tcx> for Ptr {
 147     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
 148         if let ItemKind::Fn(ref sig, _, body_id) = item.kind {
 149             check_fn(cx, sig.decl, item.hir_id(), Some(body_id));
 150         }
 151     }
 152
 153     fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
 154         if let ImplItemKind::Fn(ref sig, body_id) = item.kind {
 155             let parent_item = cx.tcx.hir().get_parent_item(item.hir_id());
 156             if let Some(Node::Item(it)) = cx.tcx.hir().find(parent_item) {
 157                 if let ItemKind::Impl(Impl { of_trait: Some(_), .. }) = it.kind {
 158                     return; // ignore trait impls
 159                 }
 160             }
 161             check_fn(cx, sig.decl, item.hir_id(), Some(body_id));
 162         }
 163     }
 164
 165     fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
 166         if let TraitItemKind::Fn(ref sig, ref trait_method) = item.kind {
 167             let body_id = if let TraitFn::Provided(b) = *trait_method {
 168                 Some(b)
 169             } else {
 170                 None
 171             };
 172             check_fn(cx, sig.decl, item.hir_id(), body_id);
 173         }
 174     }
 175
 176     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 177         if let ExprKind::Binary(ref op, l, r) = expr.kind {
 178             if (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne) && (is_null_path(cx, l) || is_null_path(cx, r)) {
 179                 span_lint(
 180                     cx,
 181                     CMP_NULL,
 182                     expr.span,
 183                     "comparing with null is better expressed by the `.is_null()` method",
 184                 );
 185             }
 186         } else {
 187             check_invalid_ptr_usage(cx, expr);
 188         }
 189     }
 190 }
 191
 192 fn check_invalid_ptr_usage<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 193     // (fn_path, arg_indices) - `arg_indices` are the `arg` positions where null would cause U.B.
 194     const INVALID_NULL_PTR_USAGE_TABLE: [(&[&str], &[usize]); 16] = [
 195         (&paths::SLICE_FROM_RAW_PARTS, &[0]),
 196         (&paths::SLICE_FROM_RAW_PARTS_MUT, &[0]),
 197         (&paths::PTR_COPY, &[0, 1]),
 198         (&paths::PTR_COPY_NONOVERLAPPING, &[0, 1]),
 199         (&paths::PTR_READ, &[0]),
 200         (&paths::PTR_READ_UNALIGNED, &[0]),
 201         (&paths::PTR_READ_VOLATILE, &[0]),
 202         (&paths::PTR_REPLACE, &[0]),
 203         (&paths::PTR_SLICE_FROM_RAW_PARTS, &[0]),
 204         (&paths::PTR_SLICE_FROM_RAW_PARTS_MUT, &[0]),
 205         (&paths::PTR_SWAP, &[0, 1]),
 206         (&paths::PTR_SWAP_NONOVERLAPPING, &[0, 1]),
 207         (&paths::PTR_WRITE, &[0]),
 208         (&paths::PTR_WRITE_UNALIGNED, &[0]),
 209         (&paths::PTR_WRITE_VOLATILE, &[0]),
 210         (&paths::PTR_WRITE_BYTES, &[0]),
 211     ];
 212
 213     if_chain! {
 214         if let ExprKind::Call(ref fun, ref args) = expr.kind;
 215         if let ExprKind::Path(ref qpath) = fun.kind;
 216         if let Some(fun_def_id) = cx.qpath_res(qpath, fun.hir_id).opt_def_id();
 217         let fun_def_path = cx.get_def_path(fun_def_id).into_iter().map(Symbol::to_ident_string).collect::<Vec<_>>();
 218         if let Some(&(_, arg_indices)) = INVALID_NULL_PTR_USAGE_TABLE
 219             .iter()
 220             .find(|&&(fn_path, _)| fn_path == fun_def_path);
 221         then {
 222             for &arg_idx in arg_indices {
 223                 if let Some(arg) = args.get(arg_idx).filter(|arg| is_null_path(cx, arg)) {
 224                     span_lint_and_sugg(
 225                         cx,
 226                         INVALID_NULL_PTR_USAGE,
 227                         arg.span,
 228                         "pointer must be non-null",
 229                         "change this to",
 230                         "core::ptr::NonNull::dangling().as_ptr()".to_string(),
 231                         Applicability::MachineApplicable,
 232                     );
 233                 }
 234             }
 235         }
 236     }
 237 }
 238
 239 #[allow(clippy::too_many_lines)]
 240 fn check_fn(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_id: HirId, opt_body_id: Option<BodyId>) {
 241     let fn_def_id = cx.tcx.hir().local_def_id(fn_id);
 242     let sig = cx.tcx.fn_sig(fn_def_id);
 243     let fn_ty = sig.skip_binder();
 244     let body = opt_body_id.map(|id| cx.tcx.hir().body(id));
 245
 246     for (idx, (arg, ty)) in decl.inputs.iter().zip(fn_ty.inputs()).enumerate() {
 247         // Honor the allow attribute on parameters. See issue 5644.
 248         if let Some(body) = &body {
 249             if is_allowed(cx, PTR_ARG, body.params[idx].hir_id) {
 250                 continue;
 251             }
 252         }
 253
 254         if let ty::Ref(_, ty, Mutability::Not) = ty.kind() {
 255             if is_type_diagnostic_item(cx, ty, sym::vec_type) {
 256                 if let Some(spans) = get_spans(cx, opt_body_id, idx, &[("clone", ".to_owned()")]) {
 257                     span_lint_and_then(
 258                         cx,
 259                         PTR_ARG,
 260                         arg.span,
 261                         "writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used \
 262                          with non-Vec-based slices",
 263                         |diag| {
 264                             if let Some(ref snippet) = get_only_generic_arg_snippet(cx, arg) {
 265                                 diag.span_suggestion(
 266                                     arg.span,
 267                                     "change this to",
 268                                     format!("&[{}]", snippet),
 269                                     Applicability::Unspecified,
 270                                 );
 271                             }
 272                             for (clonespan, suggestion) in spans {
 273                                 diag.span_suggestion(
 274                                     clonespan,
 275                                     &snippet_opt(cx, clonespan).map_or("change the call to".into(), |x| {
 276                                         Cow::Owned(format!("change `{}` to", x))
 277                                     }),
 278                                     suggestion.into(),
 279                                     Applicability::Unspecified,
 280                                 );
 281                             }
 282                         },
 283                     );
 284                 }
 285             } else if is_type_diagnostic_item(cx, ty, sym::string_type) {
 286                 if let Some(spans) = get_spans(cx, opt_body_id, idx, &[("clone", ".to_string()"), ("as_str", "")]) {
 287                     span_lint_and_then(
 288                         cx,
 289                         PTR_ARG,
 290                         arg.span,
 291                         "writing `&String` instead of `&str` involves a new object where a slice will do",
 292                         |diag| {
 293                             diag.span_suggestion(arg.span, "change this to", "&str".into(), Applicability::Unspecified);
 294                             for (clonespan, suggestion) in spans {
 295                                 diag.span_suggestion_short(
 296                                     clonespan,
 297                                     &snippet_opt(cx, clonespan).map_or("change the call to".into(), |x| {
 298                                         Cow::Owned(format!("change `{}` to", x))
 299                                     }),
 300                                     suggestion.into(),
 301                                     Applicability::Unspecified,
 302                                 );
 303                             }
 304                         },
 305                     );
 306                 }
 307             } else if is_type_diagnostic_item(cx, ty, sym::PathBuf) {
 308                 if let Some(spans) = get_spans(cx, opt_body_id, idx, &[("clone", ".to_path_buf()"), ("as_path", "")]) {
 309                     span_lint_and_then(
 310                         cx,
 311                         PTR_ARG,
 312                         arg.span,
 313                         "writing `&PathBuf` instead of `&Path` involves a new object where a slice will do",
 314                         |diag| {
 315                             diag.span_suggestion(
 316                                 arg.span,
 317                                 "change this to",
 318                                 "&Path".into(),
 319                                 Applicability::Unspecified,
 320                             );
 321                             for (clonespan, suggestion) in spans {
 322                                 diag.span_suggestion_short(
 323                                     clonespan,
 324                                     &snippet_opt(cx, clonespan).map_or("change the call to".into(), |x| {
 325                                         Cow::Owned(format!("change `{}` to", x))
 326                                     }),
 327                                     suggestion.into(),
 328                                     Applicability::Unspecified,
 329                                 );
 330                             }
 331                         },
 332                     );
 333                 }
 334             } else if match_type(cx, ty, &paths::COW) {
 335                 if_chain! {
 336                     if let TyKind::Rptr(_, MutTy { ty, ..} ) = arg.kind;
 337                     if let TyKind::Path(QPath::Resolved(None, pp)) = ty.kind;
 338                     if let [ref bx] = *pp.segments;
 339                     if let Some(params) = bx.args;
 340                     if !params.parenthesized;
 341                     if let Some(inner) = params.args.iter().find_map(|arg| match arg {
 342                         GenericArg::Type(ty) => Some(ty),
 343                         _ => None,
 344                     });
 345                     let replacement = snippet_opt(cx, inner.span);
 346                     if let Some(r) = replacement;
 347                     then {
 348                         span_lint_and_sugg(
 349                             cx,
 350                             PTR_ARG,
 351                             arg.span,
 352                             "using a reference to `Cow` is not recommended",
 353                             "change this to",
 354                             "&".to_owned() + &r,
 355                             Applicability::Unspecified,
 356                         );
 357                     }
 358                 }
 359             }
 360         }
 361     }
 362
 363     if let FnRetTy::Return(ty) = decl.output {
 364         if let Some((out, Mutability::Mut, _)) = get_rptr_lm(ty) {
 365             let mut immutables = vec![];
 366             for (_, ref mutbl, ref argspan) in decl
 367                 .inputs
 368                 .iter()
 369                 .filter_map(|ty| get_rptr_lm(ty))
 370                 .filter(|&(lt, _, _)| lt.name == out.name)
 371             {
 372                 if *mutbl == Mutability::Mut {
 373                     return;
 374                 }
 375                 immutables.push(*argspan);
 376             }
 377             if immutables.is_empty() {
 378                 return;
 379             }
 380             span_lint_and_then(
 381                 cx,
 382                 MUT_FROM_REF,
 383                 ty.span,
 384                 "mutable borrow from immutable input(s)",
 385                 |diag| {
 386                     let ms = MultiSpan::from_spans(immutables);
 387                     diag.span_note(ms, "immutable borrow here");
 388                 },
 389             );
 390         }
 391     }
 392 }
 393
 394 fn get_only_generic_arg_snippet(cx: &LateContext<'_>, arg: &Ty<'_>) -> Option<String> {
 395     if_chain! {
 396         if let TyKind::Path(QPath::Resolved(_, path)) = walk_ptrs_hir_ty(arg).kind;
 397         if let Some(&PathSegment{args: Some(parameters), ..}) = path.segments.last();
 398         let types: Vec<_> = parameters.args.iter().filter_map(|arg| match arg {
 399             GenericArg::Type(ty) => Some(ty),
 400             _ => None,
 401         }).collect();
 402         if types.len() == 1;
 403         then {
 404             snippet_opt(cx, types[0].span)
 405         } else {
 406             None
 407         }
 408     }
 409 }
 410
 411 fn get_rptr_lm<'tcx>(ty: &'tcx Ty<'tcx>) -> Option<(&'tcx Lifetime, Mutability, Span)> {
 412     if let TyKind::Rptr(ref lt, ref m) = ty.kind {
 413         Some((lt, m.mutbl, ty.span))
 414     } else {
 415         None
 416     }
 417 }
 418
 419 fn is_null_path(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
 420     if let ExprKind::Call(pathexp, []) = expr.kind {
 421         expr_path_res(cx, pathexp).opt_def_id().map_or(false, |id| {
 422             match_any_def_paths(cx, id, &[&paths::PTR_NULL, &paths::PTR_NULL_MUT]).is_some()
 423         })
 424     } else {
 425         false
 426     }
 427 }