]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/types.rs
Auto merge of #6834 - hyd-dev:clippy-args, r=phansch,flip1995,oli-obk
[rust.git] / clippy_lints / src / types.rs
index 2696c5e781abcaa01c67ac3bddac7cd1753530fc..ce201b956d83d83cceeaf97fb9e26983a11346fe 100644 (file)
@@ -5,21 +5,21 @@
 use std::collections::BTreeMap;
 
 use if_chain::if_chain;
-use rustc_ast::{FloatTy, IntTy, LitFloatType, LitIntType, LitKind, UintTy};
+use rustc_ast::{LitFloatType, LitIntType, LitKind};
 use rustc_errors::{Applicability, DiagnosticBuilder};
 use rustc_hir as hir;
-use rustc_hir::def::Res;
 use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor};
 use rustc_hir::{
     BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericBounds, GenericParamKind, HirId,
-    ImplItem, ImplItemKind, Item, ItemKind, Lifetime, Lit, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt,
-    StmtKind, SyntheticTyParamKind, TraitFn, TraitItem, TraitItemKind, TyKind, UnOp,
+    ImplItem, ImplItemKind, Item, ItemKind, LangItem, Lifetime, Lit, Local, MatchSource, MutTy, Mutability, Node,
+    QPath, Stmt, StmtKind, SyntheticTyParamKind, TraitFn, TraitItem, TraitItemKind, TyKind, UnOp,
 };
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::hir::map::Map;
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::TypeFoldable;
-use rustc_middle::ty::{self, InferTy, Ty, TyCtxt, TyS, TypeckResults};
+use rustc_middle::ty::{self, FloatTy, InferTy, IntTy, Ty, TyCtxt, TyS, TypeAndMut, TypeckResults, UintTy};
+use rustc_semver::RustcVersion;
 use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
 use rustc_span::hygiene::{ExpnKind, MacroKind};
 use rustc_span::source_map::Span;
 
 use crate::consts::{constant, Constant};
 use crate::utils::paths;
+use crate::utils::sugg::Sugg;
 use crate::utils::{
-    clip, comparisons, differing_macro_contexts, higher, in_constant, indent_of, int_bits, is_type_diagnostic_item,
-    last_path_segment, match_def_path, match_path, method_chain_args, multispan_sugg, numeric_literal::NumericLiteral,
-    qpath_res, reindent_multiline, sext, snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite,
-    span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext,
+    clip, comparisons, differing_macro_contexts, get_qpath_generic_tys, higher, in_constant, indent_of, int_bits,
+    is_hir_ty_cfg_dependant, is_ty_param_diagnostic_item, is_ty_param_lang_item, is_type_diagnostic_item,
+    last_path_segment, match_def_path, match_path, meets_msrv, method_chain_args, multispan_sugg,
+    numeric_literal::NumericLiteral, reindent_multiline, sext, snippet, snippet_opt, snippet_with_applicability,
+    snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext,
 };
 
 declare_clippy_lint! {
@@ -73,7 +75,7 @@
     /// **Why is this bad?** `Vec` already keeps its contents in a separate area on
     /// the heap. So if you `Box` its contents, you just add another level of indirection.
     ///
-    /// **Known problems:** Vec<Box<T: Sized>> makes sense if T is a large type (see #3530,
+    /// **Known problems:** Vec<Box<T: Sized>> makes sense if T is a large type (see [#3530](https://github.com/rust-lang/rust-clippy/issues/3530),
     /// 1st comment).
     ///
     /// **Example:**
@@ -285,37 +287,16 @@ fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) {
     }
 }
 
-/// Checks if `qpath` has last segment with type parameter matching `path`
-fn match_type_parameter(cx: &LateContext<'_>, qpath: &QPath<'_>, path: &[&str]) -> Option<Span> {
-    let last = last_path_segment(qpath);
-    if_chain! {
-        if let Some(ref params) = last.args;
-        if !params.parenthesized;
-        if let Some(ty) = params.args.iter().find_map(|arg| match arg {
-            GenericArg::Type(ty) => Some(ty),
-            _ => None,
-        });
-        if let TyKind::Path(ref qpath) = ty.kind;
-        if let Some(did) = qpath_res(cx, qpath, ty.hir_id).opt_def_id();
-        if match_def_path(cx, did, path);
-        then {
-            return Some(ty.span);
-        }
-    }
-    None
-}
-
 fn match_buffer_type(cx: &LateContext<'_>, qpath: &QPath<'_>) -> Option<&'static str> {
-    if match_type_parameter(cx, qpath, &paths::STRING).is_some() {
-        return Some("str");
-    }
-    if match_type_parameter(cx, qpath, &paths::OS_STRING).is_some() {
-        return Some("std::ffi::OsStr");
-    }
-    if match_type_parameter(cx, qpath, &paths::PATH_BUF).is_some() {
-        return Some("std::path::Path");
+    if is_ty_param_diagnostic_item(cx, qpath, sym::string_type).is_some() {
+        Some("str")
+    } else if is_ty_param_diagnostic_item(cx, qpath, sym::OsString).is_some() {
+        Some("std::ffi::OsStr")
+    } else if is_ty_param_diagnostic_item(cx, qpath, sym::PathBuf).is_some() {
+        Some("std::path::Path")
+    } else {
+        None
     }
-    None
 }
 
 fn match_borrows_parameter(_cx: &LateContext<'_>, qpath: &QPath<'_>) -> Option<Span> {
@@ -363,7 +344,7 @@ fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, is_local: boo
         match hir_ty.kind {
             TyKind::Path(ref qpath) if !is_local => {
                 let hir_id = hir_ty.hir_id;
-                let res = qpath_res(cx, qpath, hir_id);
+                let res = cx.qpath_res(qpath, hir_id);
                 if let Some(def_id) = res.opt_def_id() {
                     if Some(def_id) == cx.tcx.lang_items().owned_box() {
                         if let Some(span) = match_borrows_parameter(cx, qpath) {
@@ -379,19 +360,19 @@ fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, is_local: boo
                             );
                             return; // don't recurse into the type
                         }
-                        if match_type_parameter(cx, qpath, &paths::VEC).is_some() {
+                        if is_ty_param_diagnostic_item(cx, qpath, sym::vec_type).is_some() {
                             span_lint_and_help(
                                 cx,
                                 BOX_VEC,
                                 hir_ty.span,
                                 "you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`",
                                 None,
-                                "`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation.",
+                                "`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation",
                             );
                             return; // don't recurse into the type
                         }
                     } else if cx.tcx.is_diagnostic_item(sym::Rc, def_id) {
-                        if let Some(span) = match_type_parameter(cx, qpath, &paths::RC) {
+                        if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym::Rc) {
                             let mut applicability = Applicability::MachineApplicable;
                             span_lint_and_sugg(
                                 cx,
@@ -399,22 +380,19 @@ fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, is_local: boo
                                 hir_ty.span,
                                 "usage of `Rc<Rc<T>>`",
                                 "try",
-                                snippet_with_applicability(cx, span, "..", &mut applicability).to_string(),
+                                snippet_with_applicability(cx, ty.span, "..", &mut applicability).to_string(),
                                 applicability,
                             );
                             return; // don't recurse into the type
                         }
-                        if match_type_parameter(cx, qpath, &paths::BOX).is_some() {
-                            let box_ty = match &last_path_segment(qpath).args.unwrap().args[0] {
-                                GenericArg::Type(ty) => match &ty.kind {
-                                    TyKind::Path(qpath) => qpath,
-                                    _ => return,
-                                },
+                        if let Some(ty) = is_ty_param_lang_item(cx, qpath, LangItem::OwnedBox) {
+                            let qpath = match &ty.kind {
+                                TyKind::Path(qpath) => qpath,
                                 _ => return,
                             };
-                            let inner_span = match &last_path_segment(&box_ty).args.unwrap().args[0] {
-                                GenericArg::Type(ty) => ty.span,
-                                _ => return,
+                            let inner_span = match get_qpath_generic_tys(qpath).next() {
+                                Some(ty) => ty.span,
+                                None => return,
                             };
                             let mut applicability = Applicability::MachineApplicable;
                             span_lint_and_sugg(
@@ -443,17 +421,14 @@ fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, is_local: boo
                             );
                             return; // don't recurse into the type
                         }
-                        if match_type_parameter(cx, qpath, &paths::VEC).is_some() {
-                            let vec_ty = match &last_path_segment(qpath).args.unwrap().args[0] {
-                                GenericArg::Type(ty) => match &ty.kind {
-                                    TyKind::Path(qpath) => qpath,
-                                    _ => return,
-                                },
+                        if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym::vec_type) {
+                            let qpath = match &ty.kind {
+                                TyKind::Path(qpath) => qpath,
                                 _ => return,
                             };
-                            let inner_span = match &last_path_segment(&vec_ty).args.unwrap().args[0] {
-                                GenericArg::Type(ty) => ty.span,
-                                _ => return,
+                            let inner_span = match get_qpath_generic_tys(qpath).next() {
+                                Some(ty) => ty.span,
+                                None => return,
                             };
                             let mut applicability = Applicability::MachineApplicable;
                             span_lint_and_sugg(
@@ -496,17 +471,14 @@ fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, is_local: boo
                             );
                             return; // don't recurse into the type
                         }
-                        if match_type_parameter(cx, qpath, &paths::VEC).is_some() {
-                            let vec_ty = match &last_path_segment(qpath).args.unwrap().args[0] {
-                                GenericArg::Type(ty) => match &ty.kind {
-                                    TyKind::Path(qpath) => qpath,
-                                    _ => return,
-                                },
+                        if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym::vec_type) {
+                            let qpath = match &ty.kind {
+                                TyKind::Path(qpath) => qpath,
                                 _ => return,
                             };
-                            let inner_span = match &last_path_segment(&vec_ty).args.unwrap().args[0] {
-                                GenericArg::Type(ty) => ty.span,
-                                _ => return,
+                            let inner_span = match get_qpath_generic_tys(qpath).next() {
+                                Some(ty) => ty.span,
+                                None => return,
                             };
                             let mut applicability = Applicability::MachineApplicable;
                             span_lint_and_sugg(
@@ -533,7 +505,7 @@ fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, is_local: boo
                             });
                             // ty is now _ at this point
                             if let TyKind::Path(ref ty_qpath) = ty.kind;
-                            let res = qpath_res(cx, ty_qpath, ty.hir_id);
+                            let res = cx.qpath_res(ty_qpath, ty.hir_id);
                             if let Some(def_id) = res.opt_def_id();
                             if Some(def_id) == cx.tcx.lang_items().owned_box();
                             // At this point, we know ty is Box<T>, now get T
@@ -552,7 +524,7 @@ fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, is_local: boo
                                     cx,
                                     VEC_BOX,
                                     hir_ty.span,
-                                    "`Vec<T>` is already on the heap, the boxing is unnecessary.",
+                                    "`Vec<T>` is already on the heap, the boxing is unnecessary",
                                     "try",
                                     format!("Vec<{}>", snippet(cx, boxed_ty.span, "..")),
                                     Applicability::MachineApplicable,
@@ -561,7 +533,7 @@ fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, is_local: boo
                             }
                         }
                     } else if cx.tcx.is_diagnostic_item(sym::option_type, def_id) {
-                        if match_type_parameter(cx, qpath, &paths::OPTION).is_some() {
+                        if is_ty_param_diagnostic_item(cx, qpath, sym::option_type).is_some() {
                             span_lint(
                                 cx,
                                 OPTION_OPTION,
@@ -576,7 +548,7 @@ enum if you need to distinguish all 3 cases",
                             cx,
                             LINKEDLIST,
                             hir_ty.span,
-                            "I see you're using a LinkedList! Perhaps you meant some other data structure?",
+                            "you seem to be using a `LinkedList`! Perhaps you meant some other data structure?",
                             None,
                             "a `VecDeque` might work",
                         );
@@ -650,7 +622,7 @@ fn check_ty_rptr(
         match mut_ty.ty.kind {
             TyKind::Path(ref qpath) => {
                 let hir_id = mut_ty.ty.hir_id;
-                let def = qpath_res(cx, qpath, hir_id);
+                let def = cx.qpath_res(qpath, hir_id);
                 if_chain! {
                     if let Some(def_id) = def.opt_def_id();
                     if Some(def_id) == cx.tcx.lang_items().owned_box();
@@ -737,7 +709,7 @@ fn is_any_trait(t: &hir::Ty<'_>) -> bool {
 
 fn get_bounds_if_impl_trait<'tcx>(cx: &LateContext<'tcx>, qpath: &QPath<'_>, id: HirId) -> Option<GenericBounds<'tcx>> {
     if_chain! {
-        if let Some(did) = qpath_res(cx, qpath, id).opt_def_id();
+        if let Some(did) = cx.qpath_res(qpath, id).opt_def_id();
         if let Some(Node::GenericParam(generic_param)) = cx.tcx.hir().get_if_local(did);
         if let GenericParamKind::Type { synthetic, .. } = generic_param.kind;
         if synthetic == Some(SyntheticTyParamKind::ImplTrait);
@@ -953,7 +925,10 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                     .iter()
                     .filter(|arg| {
                         if is_unit(cx.typeck_results().expr_ty(arg)) && !is_unit_literal(arg) {
-                            !matches!(&arg.kind, ExprKind::Match(.., MatchSource::TryDesugar))
+                            !matches!(
+                                &arg.kind,
+                                ExprKind::Match(.., MatchSource::TryDesugar) | ExprKind::Path(..)
+                            )
                         } else {
                             false
                         }
@@ -1279,8 +1254,8 @@ fn is_unit_literal(expr: &Expr<'_>) -> bool {
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for casts from a less-strictly-aligned pointer to a
-    /// more-strictly-aligned pointer
+    /// **What it does:** Checks for casts, using `as` or `pointer::cast`,
+    /// from a less-strictly-aligned pointer to a more-strictly-aligned pointer
     ///
     /// **Why is this bad?** Dereferencing the resulting pointer may be undefined
     /// behavior.
@@ -1293,6 +1268,9 @@ fn is_unit_literal(expr: &Expr<'_>) -> bool {
     /// ```rust
     /// let _ = (&1u8 as *const u8) as *const u16;
     /// let _ = (&mut 1u8 as *mut u8) as *mut u16;
+    ///
+    /// (&1u8 as *const u8).cast::<u16>();
+    /// (&mut 1u8 as *mut u8).cast::<u16>();
     /// ```
     pub CAST_PTR_ALIGNMENT,
     pedantic,
@@ -1634,12 +1612,8 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
             return;
         }
         if let ExprKind::Cast(ref ex, cast_to) = expr.kind {
-            if let TyKind::Path(QPath::Resolved(_, path)) = cast_to.kind {
-                if let Res::Def(_, def_id) = path.res {
-                    if cx.tcx.has_attr(def_id, sym::cfg) || cx.tcx.has_attr(def_id, sym::cfg_attr) {
-                        return;
-                    }
-                }
+            if is_hir_ty_cfg_dependant(cx, cast_to) {
+                return;
             }
             let (cast_from, cast_to) = (cx.typeck_results().expr_ty(ex), cx.typeck_results().expr_ty(expr));
             lint_fn_to_numeric_cast(cx, expr, ex, cast_from, cast_to);
@@ -1689,18 +1663,31 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
             }
 
             lint_cast_ptr_alignment(cx, expr, cast_from, cast_to);
+        } else if let ExprKind::MethodCall(method_path, _, args, _) = expr.kind {
+            if_chain! {
+            if method_path.ident.name == sym!(cast);
+            if let Some(generic_args) = method_path.args;
+            if let [GenericArg::Type(cast_to)] = generic_args.args;
+            // There probably is no obvious reason to do this, just to be consistent with `as` cases.
+            if !is_hir_ty_cfg_dependant(cx, cast_to);
+            then {
+                let (cast_from, cast_to) =
+                    (cx.typeck_results().expr_ty(&args[0]), cx.typeck_results().expr_ty(expr));
+                lint_cast_ptr_alignment(cx, expr, cast_from, cast_to);
+            }
+            }
         }
     }
 }
 
 fn is_unary_neg(expr: &Expr<'_>) -> bool {
-    matches!(expr.kind, ExprKind::Unary(UnOp::UnNeg, _))
+    matches!(expr.kind, ExprKind::Unary(UnOp::Neg, _))
 }
 
 fn get_numeric_literal<'e>(expr: &'e Expr<'e>) -> Option<&'e Lit> {
     match expr.kind {
         ExprKind::Lit(ref lit) => Some(lit),
-        ExprKind::Unary(UnOp::UnNeg, e) => {
+        ExprKind::Unary(UnOp::Neg, e) => {
             if let ExprKind::Lit(ref lit) = e.kind {
                 Some(lit)
             } else {
@@ -2553,7 +2540,7 @@ fn suggestion<'tcx>(
             }
         }
 
-        if !cx.access_levels.is_exported(item.hir_id) {
+        if !cx.access_levels.is_exported(item.hir_id()) {
             return;
         }
 
@@ -2856,7 +2843,7 @@ fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
 impl<'tcx> LateLintPass<'tcx> for RefToMut {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         if_chain! {
-            if let ExprKind::Unary(UnOp::UnDeref, e) = &expr.kind;
+            if let ExprKind::Unary(UnOp::Deref, e) = &expr.kind;
             if let ExprKind::Cast(e, t) = &e.kind;
             if let TyKind::Ptr(MutTy { mutbl: Mutability::Mut, .. }) = t.kind;
             if let ExprKind::Cast(e, t) = &e.kind;
@@ -2873,3 +2860,93 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         }
     }
 }
+
+const PTR_AS_PTR_MSRV: RustcVersion = RustcVersion::new(1, 38, 0);
+
+declare_clippy_lint! {
+    /// **What it does:**
+    /// Checks for `as` casts between raw pointers without changing its mutability,
+    /// namely `*const T` to `*const U` and `*mut T` to `*mut U`.
+    ///
+    /// **Why is this bad?**
+    /// Though `as` casts between raw pointers is not terrible, `pointer::cast` is safer because
+    /// it cannot accidentally change the pointer's mutability nor cast the pointer to other types like `usize`.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let ptr: *const u32 = &42_u32;
+    /// let mut_ptr: *mut u32 = &mut 42_u32;
+    /// let _ = ptr as *const i32;
+    /// let _ = mut_ptr as *mut i32;
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let ptr: *const u32 = &42_u32;
+    /// let mut_ptr: *mut u32 = &mut 42_u32;
+    /// let _ = ptr.cast::<i32>();
+    /// let _ = mut_ptr.cast::<i32>();
+    /// ```
+    pub PTR_AS_PTR,
+    pedantic,
+    "casting using `as` from and to raw pointers that doesn't change its mutability, where `pointer::cast` could take the place of `as`"
+}
+
+pub struct PtrAsPtr {
+    msrv: Option<RustcVersion>,
+}
+
+impl PtrAsPtr {
+    #[must_use]
+    pub fn new(msrv: Option<RustcVersion>) -> Self {
+        Self { msrv }
+    }
+}
+
+impl_lint_pass!(PtrAsPtr => [PTR_AS_PTR]);
+
+impl<'tcx> LateLintPass<'tcx> for PtrAsPtr {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if !meets_msrv(self.msrv.as_ref(), &PTR_AS_PTR_MSRV) {
+            return;
+        }
+
+        if expr.span.from_expansion() {
+            return;
+        }
+
+        if_chain! {
+            if let ExprKind::Cast(cast_expr, cast_to_hir_ty) = expr.kind;
+            let (cast_from, cast_to) = (cx.typeck_results().expr_ty(cast_expr), cx.typeck_results().expr_ty(expr));
+            if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind();
+            if let ty::RawPtr(TypeAndMut { ty: to_pointee_ty, mutbl: to_mutbl }) = cast_to.kind();
+            if matches!((from_mutbl, to_mutbl),
+                (Mutability::Not, Mutability::Not) | (Mutability::Mut, Mutability::Mut));
+            // The `U` in `pointer::cast` have to be `Sized`
+            // as explained here: https://github.com/rust-lang/rust/issues/60602.
+            if to_pointee_ty.is_sized(cx.tcx.at(expr.span), cx.param_env);
+            then {
+                let mut applicability = Applicability::MachineApplicable;
+                let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut applicability);
+                let turbofish = match &cast_to_hir_ty.kind {
+                        TyKind::Infer => Cow::Borrowed(""),
+                        TyKind::Ptr(mut_ty) if matches!(mut_ty.ty.kind, TyKind::Infer) => Cow::Borrowed(""),
+                        _ => Cow::Owned(format!("::<{}>", to_pointee_ty)),
+                    };
+                span_lint_and_sugg(
+                    cx,
+                    PTR_AS_PTR,
+                    expr.span,
+                    "`as` casting between raw pointers without changing its mutability",
+                    "try `pointer::cast`, a safer alternative",
+                    format!("{}.cast{}()", cast_expr_sugg.maybe_par(), turbofish),
+                    applicability,
+                );
+            }
+        }
+    }
+
+    extract_msrv_attr!(LateContext);
+}