clippy_lints/src/use_self.rs

   1 use clippy_utils::diagnostics::span_lint_and_sugg;
   2 use clippy_utils::source::snippet_opt;
   3 use clippy_utils::{in_macro, meets_msrv};
   4 use if_chain::if_chain;
   5 use rustc_errors::Applicability;
   6 use rustc_hir as hir;
   7 use rustc_hir::def::DefKind;
   8 use rustc_hir::{
   9     def,
  10     def_id::LocalDefId,
  11     intravisit::{walk_ty, NestedVisitorMap, Visitor},
  12     Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Node, Path, PathSegment,
  13     QPath, TyKind,
  14 };
  15 use rustc_lint::{LateContext, LateLintPass, LintContext};
  16 use rustc_middle::hir::map::Map;
  17 use rustc_middle::ty::{AssocKind, Ty, TyS};
  18 use rustc_semver::RustcVersion;
  19 use rustc_session::{declare_tool_lint, impl_lint_pass};
  20 use rustc_span::{BytePos, Span};
  21 use rustc_typeck::hir_ty_to_ty;
  22
  23 declare_clippy_lint! {
  24     /// **What it does:** Checks for unnecessary repetition of structure name when a
  25     /// replacement with `Self` is applicable.
  26     ///
  27     /// **Why is this bad?** Unnecessary repetition. Mixed use of `Self` and struct
  28     /// name
  29     /// feels inconsistent.
  30     ///
  31     /// **Known problems:**
  32     /// - Unaddressed false negative in fn bodies of trait implementations
  33     /// - False positive with assotiated types in traits (#4140)
  34     ///
  35     /// **Example:**
  36     ///
  37     /// ```rust
  38     /// struct Foo {}
  39     /// impl Foo {
  40     ///     fn new() -> Foo {
  41     ///         Foo {}
  42     ///     }
  43     /// }
  44     /// ```
  45     /// could be
  46     /// ```rust
  47     /// struct Foo {}
  48     /// impl Foo {
  49     ///     fn new() -> Self {
  50     ///         Self {}
  51     ///     }
  52     /// }
  53     /// ```
  54     pub USE_SELF,
  55     nursery,
  56     "unnecessary structure name repetition whereas `Self` is applicable"
  57 }
  58
  59 #[derive(Default)]
  60 pub struct UseSelf {
  61     msrv: Option<RustcVersion>,
  62     stack: Vec<StackItem>,
  63 }
  64
  65 const USE_SELF_MSRV: RustcVersion = RustcVersion::new(1, 37, 0);
  66
  67 impl UseSelf {
  68     #[must_use]
  69     pub fn new(msrv: Option<RustcVersion>) -> Self {
  70         Self {
  71             msrv,
  72             ..Self::default()
  73         }
  74     }
  75 }
  76
  77 #[derive(Debug)]
  78 enum StackItem {
  79     Check {
  80         hir_id: HirId,
  81         impl_trait_ref_def_id: Option<LocalDefId>,
  82         types_to_skip: Vec<HirId>,
  83         types_to_lint: Vec<HirId>,
  84     },
  85     NoCheck,
  86 }
  87
  88 impl_lint_pass!(UseSelf => [USE_SELF]);
  89
  90 const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
  91
  92 impl<'tcx> LateLintPass<'tcx> for UseSelf {
  93     fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
  94         // We push the self types of `impl`s on a stack here. Only the top type on the stack is
  95         // relevant for linting, since this is the self type of the `impl` we're currently in. To
  96         // avoid linting on nested items, we push `StackItem::NoCheck` on the stack to signal, that
  97         // we're in an `impl` or nested item, that we don't want to lint
  98         //
  99         // NB: If you push something on the stack in this method, remember to also pop it in the
 100         // `check_item_post` method.
 101         match &item.kind {
 102             ItemKind::Impl(Impl {
 103                 self_ty: hir_self_ty,
 104                 of_trait,
 105                 ..
 106             }) => {
 107                 let should_check = if let TyKind::Path(QPath::Resolved(_, item_path)) = hir_self_ty.kind {
 108                     let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args;
 109                     parameters.as_ref().map_or(true, |params| {
 110                         !params.parenthesized && !params.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_)))
 111                     })
 112                 } else {
 113                     false
 114                 };
 115                 let impl_trait_ref_def_id = of_trait.as_ref().map(|_| cx.tcx.hir().local_def_id(item.hir_id()));
 116                 if should_check {
 117                     self.stack.push(StackItem::Check {
 118                         hir_id: hir_self_ty.hir_id,
 119                         impl_trait_ref_def_id,
 120                         types_to_lint: Vec::new(),
 121                         types_to_skip: Vec::new(),
 122                     });
 123                 } else {
 124                     self.stack.push(StackItem::NoCheck);
 125                 }
 126             },
 127             ItemKind::Static(..)
 128             | ItemKind::Const(..)
 129             | ItemKind::Fn(..)
 130             | ItemKind::Enum(..)
 131             | ItemKind::Struct(..)
 132             | ItemKind::Union(..)
 133             | ItemKind::Trait(..) => {
 134                 self.stack.push(StackItem::NoCheck);
 135             },
 136             _ => (),
 137         }
 138     }
 139
 140     fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) {
 141         use ItemKind::{Const, Enum, Fn, Impl, Static, Struct, Trait, Union};
 142         match item.kind {
 143             Impl { .. } | Static(..) | Const(..) | Fn(..) | Enum(..) | Struct(..) | Union(..) | Trait(..) => {
 144                 self.stack.pop();
 145             },
 146             _ => (),
 147         }
 148     }
 149
 150     fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) {
 151         // We want to skip types in trait `impl`s that aren't declared as `Self` in the trait
 152         // declaration. The collection of those types is all this method implementation does.
 153         if_chain! {
 154             if let ImplItemKind::Fn(FnSig { decl, .. }, ..) = impl_item.kind;
 155             if let Some(&mut StackItem::Check {
 156                 impl_trait_ref_def_id: Some(def_id),
 157                 ref mut types_to_skip,
 158                 ..
 159             }) = self.stack.last_mut();
 160             if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(def_id);
 161             then {
 162                 // `self_ty` is the semantic self type of `impl <trait> for <type>`. This cannot be
 163                 // `Self`.
 164                 let self_ty = impl_trait_ref.self_ty();
 165
 166                 // `trait_method_sig` is the signature of the function, how it is declared in the
 167                 // trait, not in the impl of the trait.
 168                 let trait_method = cx
 169                     .tcx
 170                     .associated_items(impl_trait_ref.def_id)
 171                     .find_by_name_and_kind(cx.tcx, impl_item.ident, AssocKind::Fn, impl_trait_ref.def_id)
 172                     .expect("impl method matches a trait method");
 173                 let trait_method_sig = cx.tcx.fn_sig(trait_method.def_id);
 174                 let trait_method_sig = cx.tcx.erase_late_bound_regions(trait_method_sig);
 175
 176                 // `impl_inputs_outputs` is an iterator over the types (`hir::Ty`) declared in the
 177                 // implementation of the trait.
 178                 let output_hir_ty = if let FnRetTy::Return(ty) = &decl.output {
 179                     Some(&**ty)
 180                 } else {
 181                     None
 182                 };
 183                 let impl_inputs_outputs = decl.inputs.iter().chain(output_hir_ty);
 184
 185                 // `impl_hir_ty` (of type `hir::Ty`) represents the type written in the signature.
 186                 //
 187                 // `trait_sem_ty` (of type `ty::Ty`) is the semantic type for the signature in the
 188                 // trait declaration. This is used to check if `Self` was used in the trait
 189                 // declaration.
 190                 //
 191                 // If `any`where in the `trait_sem_ty` the `self_ty` was used verbatim (as opposed
 192                 // to `Self`), we want to skip linting that type and all subtypes of it. This
 193                 // avoids suggestions to e.g. replace `Vec<u8>` with `Vec<Self>`, in an `impl Trait
 194                 // for u8`, when the trait always uses `Vec<u8>`.
 195                 //
 196                 // See also https://github.com/rust-lang/rust-clippy/issues/2894.
 197                 for (impl_hir_ty, trait_sem_ty) in impl_inputs_outputs.zip(trait_method_sig.inputs_and_output) {
 198                     if trait_sem_ty.walk().any(|inner| inner == self_ty.into()) {
 199                         let mut visitor = SkipTyCollector::default();
 200                         visitor.visit_ty(impl_hir_ty);
 201                         types_to_skip.extend(visitor.types_to_skip);
 202                     }
 203                 }
 204             }
 205         }
 206     }
 207
 208     fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) {
 209         // `hir_ty_to_ty` cannot be called in `Body`s or it will panic (sometimes). But in bodies
 210         // we can use `cx.typeck_results.node_type(..)` to get the `ty::Ty` from a `hir::Ty`.
 211         // However the `node_type()` method can *only* be called in bodies.
 212         //
 213         // This method implementation determines which types should get linted in a `Body` and
 214         // which shouldn't, with a visitor. We could directly lint in the visitor, but then we
 215         // could only allow this lint on item scope. And we would have to check if those types are
 216         // already dealt with in `check_ty` anyway.
 217         if let Some(StackItem::Check {
 218             hir_id,
 219             types_to_lint,
 220             types_to_skip,
 221             ..
 222         }) = self.stack.last_mut()
 223         {
 224             let self_ty = ty_from_hir_id(cx, *hir_id);
 225
 226             let mut visitor = LintTyCollector {
 227                 cx,
 228                 self_ty,
 229                 types_to_lint: vec![],
 230                 types_to_skip: vec![],
 231             };
 232             visitor.visit_expr(&body.value);
 233             types_to_lint.extend(visitor.types_to_lint);
 234             types_to_skip.extend(visitor.types_to_skip);
 235         }
 236     }
 237
 238     fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>) {
 239         if in_macro(hir_ty.span) | in_impl(cx, hir_ty) | !meets_msrv(self.msrv.as_ref(), &USE_SELF_MSRV) {
 240             return;
 241         }
 242
 243         let lint_dependend_on_expr_kind = if let Some(StackItem::Check {
 244             hir_id,
 245             types_to_lint,
 246             types_to_skip,
 247             ..
 248         }) = self.stack.last()
 249         {
 250             if types_to_skip.contains(&hir_ty.hir_id) {
 251                 false
 252             } else if types_to_lint.contains(&hir_ty.hir_id) {
 253                 true
 254             } else {
 255                 let self_ty = ty_from_hir_id(cx, *hir_id);
 256                 should_lint_ty(hir_ty, hir_ty_to_ty(cx.tcx, hir_ty), self_ty)
 257             }
 258         } else {
 259             false
 260         };
 261
 262         if lint_dependend_on_expr_kind {
 263             // FIXME: this span manipulation should not be necessary
 264             // @flip1995 found an ast lowering issue in
 265             // https://github.com/rust-lang/rust/blob/master/src/librustc_ast_lowering/path.rs#l142-l162
 266             let hir = cx.tcx.hir();
 267             let id = hir.get_parent_node(hir_ty.hir_id);
 268
 269             if !hir.opt_span(id).map_or(false, in_macro) {
 270                 match hir.find(id) {
 271                     Some(Node::Expr(Expr {
 272                         kind: ExprKind::Path(QPath::TypeRelative(_, segment)),
 273                         ..
 274                     })) => span_lint_until_last_segment(cx, hir_ty.span, segment),
 275                     _ => span_lint(cx, hir_ty.span),
 276                 }
 277             }
 278         }
 279     }
 280
 281     fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
 282         fn expr_ty_matches(cx: &LateContext<'_>, expr: &Expr<'_>, self_ty: Ty<'_>) -> bool {
 283             let def_id = expr.hir_id.owner;
 284             if cx.tcx.has_typeck_results(def_id) {
 285                 cx.tcx.typeck(def_id).expr_ty_opt(expr) == Some(self_ty)
 286             } else {
 287                 false
 288             }
 289         }
 290
 291         if in_macro(expr.span) | !meets_msrv(self.msrv.as_ref(), &USE_SELF_MSRV) {
 292             return;
 293         }
 294
 295         if let Some(StackItem::Check { hir_id, .. }) = self.stack.last() {
 296             let self_ty = ty_from_hir_id(cx, *hir_id);
 297
 298             match &expr.kind {
 299                 ExprKind::Struct(QPath::Resolved(_, path), ..) => {
 300                     if expr_ty_matches(cx, expr, self_ty) {
 301                         match path.res {
 302                             def::Res::SelfTy(..) => (),
 303                             def::Res::Def(DefKind::Variant, _) => span_lint_on_path_until_last_segment(cx, path),
 304                             _ => {
 305                                 span_lint(cx, path.span);
 306                             },
 307                         }
 308                     }
 309                 },
 310                 // tuple struct instantiation (`Foo(arg)` or `Enum::Foo(arg)`)
 311                 ExprKind::Call(fun, _) => {
 312                     if let Expr {
 313                         kind: ExprKind::Path(ref qpath),
 314                         ..
 315                     } = fun
 316                     {
 317                         if expr_ty_matches(cx, expr, self_ty) {
 318                             let res = cx.qpath_res(qpath, fun.hir_id);
 319
 320                             if let def::Res::Def(DefKind::Ctor(ctor_of, _), ..) = res {
 321                                 match ctor_of {
 322                                     def::CtorOf::Variant => {
 323                                         span_lint_on_qpath_resolved(cx, qpath, true);
 324                                     },
 325                                     def::CtorOf::Struct => {
 326                                         span_lint_on_qpath_resolved(cx, qpath, false);
 327                                     },
 328                                 }
 329                             }
 330                         }
 331                     }
 332                 },
 333                 // unit enum variants (`Enum::A`)
 334                 ExprKind::Path(qpath) => {
 335                     if expr_ty_matches(cx, expr, self_ty) {
 336                         span_lint_on_qpath_resolved(cx, qpath, true);
 337                     }
 338                 },
 339                 _ => (),
 340             }
 341         }
 342     }
 343
 344     extract_msrv_attr!(LateContext);
 345 }
 346
 347 #[derive(Default)]
 348 struct SkipTyCollector {
 349     types_to_skip: Vec<HirId>,
 350 }
 351
 352 impl<'tcx> Visitor<'tcx> for SkipTyCollector {
 353     type Map = Map<'tcx>;
 354
 355     fn visit_ty(&mut self, hir_ty: &hir::Ty<'_>) {
 356         self.types_to_skip.push(hir_ty.hir_id);
 357
 358         walk_ty(self, hir_ty)
 359     }
 360
 361     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
 362         NestedVisitorMap::None
 363     }
 364 }
 365
 366 struct LintTyCollector<'a, 'tcx> {
 367     cx: &'a LateContext<'tcx>,
 368     self_ty: Ty<'tcx>,
 369     types_to_lint: Vec<HirId>,
 370     types_to_skip: Vec<HirId>,
 371 }
 372
 373 impl<'a, 'tcx> Visitor<'tcx> for LintTyCollector<'a, 'tcx> {
 374     type Map = Map<'tcx>;
 375
 376     fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'_>) {
 377         if_chain! {
 378             if let Some(ty) = self.cx.typeck_results().node_type_opt(hir_ty.hir_id);
 379             if should_lint_ty(hir_ty, ty, self.self_ty);
 380             then {
 381                 self.types_to_lint.push(hir_ty.hir_id);
 382             } else {
 383                 self.types_to_skip.push(hir_ty.hir_id);
 384             }
 385         }
 386
 387         walk_ty(self, hir_ty)
 388     }
 389
 390     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
 391         NestedVisitorMap::None
 392     }
 393 }
 394
 395 fn span_lint(cx: &LateContext<'_>, span: Span) {
 396     span_lint_and_sugg(
 397         cx,
 398         USE_SELF,
 399         span,
 400         "unnecessary structure name repetition",
 401         "use the applicable keyword",
 402         "Self".to_owned(),
 403         Applicability::MachineApplicable,
 404     );
 405 }
 406
 407 #[allow(clippy::cast_possible_truncation)]
 408 fn span_lint_until_last_segment(cx: &LateContext<'_>, span: Span, segment: &PathSegment<'_>) {
 409     let sp = span.with_hi(segment.ident.span.lo());
 410     // remove the trailing ::
 411     let span_without_last_segment = match snippet_opt(cx, sp) {
 412         Some(snippet) => match snippet.rfind("::") {
 413             Some(bidx) => sp.with_hi(sp.lo() + BytePos(bidx as u32)),
 414             None => sp,
 415         },
 416         None => sp,
 417     };
 418     span_lint(cx, span_without_last_segment);
 419 }
 420
 421 fn span_lint_on_path_until_last_segment(cx: &LateContext<'_>, path: &Path<'_>) {
 422     if path.segments.len() > 1 {
 423         span_lint_until_last_segment(cx, path.span, path.segments.last().unwrap());
 424     }
 425 }
 426
 427 fn span_lint_on_qpath_resolved(cx: &LateContext<'_>, qpath: &QPath<'_>, until_last_segment: bool) {
 428     if let QPath::Resolved(_, path) = qpath {
 429         if until_last_segment {
 430             span_lint_on_path_until_last_segment(cx, path);
 431         } else {
 432             span_lint(cx, path.span);
 433         }
 434     }
 435 }
 436
 437 fn ty_from_hir_id<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Ty<'tcx> {
 438     if let Some(Node::Ty(hir_ty)) = cx.tcx.hir().find(hir_id) {
 439         hir_ty_to_ty(cx.tcx, hir_ty)
 440     } else {
 441         unreachable!("This function should only be called with `HirId`s that are for sure `Node::Ty`")
 442     }
 443 }
 444
 445 fn in_impl(cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'_>) -> bool {
 446     let map = cx.tcx.hir();
 447     let parent = map.get_parent_node(hir_ty.hir_id);
 448     if_chain! {
 449         if let Some(Node::Item(item)) = map.find(parent);
 450         if let ItemKind::Impl { .. } = item.kind;
 451         then {
 452             true
 453         } else {
 454             false
 455         }
 456     }
 457 }
 458
 459 fn should_lint_ty(hir_ty: &hir::Ty<'_>, ty: Ty<'_>, self_ty: Ty<'_>) -> bool {
 460     if_chain! {
 461         if TyS::same_type(ty, self_ty);
 462         if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind;
 463         then {
 464             !matches!(path.res, def::Res::SelfTy(..))
 465         } else {
 466             false
 467         }
 468     }
 469 }