clippy_lints/src/use_self.rs

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