mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[use_self]: Make it aware of lifetimes

Browse Source 
Have the lint trigger even if `Self` has generic lifetime parameters.

```rs
impl<'a> Foo<'a> {
    type Item = Foo<'a>; // Can be replaced with Self

    fn new() -> Self {
        Foo { // No lifetime, but they are inferred to be that of Self
              // Can be replaced as well
            ...
        }
    }

    // Don't replace `Foo<'b>`, the lifetime is different!
    fn eq<'b>(self, other: Foo<'b>) -> bool {
        ..
    }
```

Fixes #12381
This commit is contained in:
Ethiraric 2024-02-29 14:13:54 +01:00
parent 93f0a9a91f
commit f3879b3630
4 changed files with 122 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
clippy_lints/src/use_self.rs
View File

@ -8,11 +8,12 @@
use rustc_hir::def_id::LocalDefId; use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit::{walk_inf, walk_ty, Visitor}; use rustc_hir::intravisit::{walk_inf, walk_ty, Visitor};
use rustc_hir::{ use rustc_hir::{
self as hir, Expr, ExprKind, FnRetTy, FnSig, GenericArg, GenericArgsParentheses, GenericParam, GenericParamKind, self as hir, Expr, ExprKind, FnRetTy, FnSig, GenericArgsParentheses, GenericParam, GenericParamKind, HirId, Impl,
HirId, Impl, ImplItemKind, Item, ItemKind, Pat, PatKind, Path, QPath, Ty, TyKind, ImplItemKind, Item, ItemKind, Pat, PatKind, Path, QPath, Ty, TyKind,
}; };
use rustc_hir_analysis::hir_ty_to_ty; use rustc_hir_analysis::hir_ty_to_ty;
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::Ty as MiddleTy;
use rustc_session::impl_lint_pass; use rustc_session::impl_lint_pass;
use rustc_span::Span; use rustc_span::Span;
@ -95,10 +96,9 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &Item<'tcx>) {
let stack_item = if let ItemKind::Impl(Impl { self_ty, generics, .. }) = item.kind let stack_item = if let ItemKind::Impl(Impl { self_ty, generics, .. }) = item.kind
&& let TyKind::Path(QPath::Resolved(_, item_path)) = self_ty.kind && let TyKind::Path(QPath::Resolved(_, item_path)) = self_ty.kind
&& let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args && let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args
&& parameters.as_ref().map_or(true, |params| { && parameters
params.parenthesized == GenericArgsParentheses::No .as_ref()
&& !params.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_))) .map_or(true, |params| params.parenthesized == GenericArgsParentheses::No)
})
&& !item.span.from_expansion() && !item.span.from_expansion()
&& !is_from_proc_macro(cx, item) && !is_from_proc_macro(cx, item)
// expensive, should be last check // expensive, should be last check
@ -226,7 +226,12 @@ fn check_ty(&mut self, cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'tcx>) {
} else { } else {
hir_ty_to_ty(cx.tcx, hir_ty) hir_ty_to_ty(cx.tcx, hir_ty)
} }
&& same_type_and_consts(ty, cx.tcx.type_of(impl_id).instantiate_identity()) && let impl_ty = cx.tcx.type_of(impl_id).instantiate_identity()
&& same_type_and_consts(ty, impl_ty)
// Ensure the type we encounter and the one from the impl have the same lifetime parameters. It may be that
// the lifetime parameters of `ty` are ellided (`impl<'a> Foo<'a> { fn new() -> Self { Foo{..} } }`, in
// which case we must still trigger the lint.
&& (has_no_lifetime(ty) || same_lifetimes(ty, impl_ty))
{ {
span_lint(cx, hir_ty.span); span_lint(cx, hir_ty.span);
} }
@ -318,3 +323,37 @@ fn lint_path_to_variant(cx: &LateContext<'_>, path: &Path<'_>) {
span_lint(cx, span); span_lint(cx, span);
} }
} }
/// Returns `true` if types `a` and `b` have the same lifetime parameters, otherwise returns
/// `false`.
///
/// This function does not check that types `a` and `b` are the same types.
fn same_lifetimes<'tcx>(a: MiddleTy<'tcx>, b: MiddleTy<'tcx>) -> bool {
use rustc_middle::ty::{Adt, GenericArgKind};
match (&a.kind(), &b.kind()) {
(&Adt(_, args_a), &Adt(_, args_b)) => {
args_a
.iter()
.zip(args_b.iter())
.all(|(arg_a, arg_b)| match (arg_a.unpack(), arg_b.unpack()) {
// TODO: Handle inferred lifetimes
(GenericArgKind::Lifetime(inner_a), GenericArgKind::Lifetime(inner_b)) => inner_a == inner_b,
(GenericArgKind::Type(type_a), GenericArgKind::Type(type_b)) => same_lifetimes(type_a, type_b),
_ => true,
})
},
_ => a == b,
}
}
/// Returns `true` if `ty` has no lifetime parameter, otherwise returns `false`.
fn has_no_lifetime(ty: MiddleTy<'_>) -> bool {
use rustc_middle::ty::{Adt, GenericArgKind};
match ty.kind() {
&Adt(_, args) => !args
.iter()
// TODO: Handle inferred lifetimes
.any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(..))),
_ => true,
}
}

18
tests/ui/use_self.fixed
View File

@ -6,7 +6,8 @@
clippy::should_implement_trait, clippy::should_implement_trait,
clippy::upper_case_acronyms, clippy::upper_case_acronyms,
clippy::from_over_into, clippy::from_over_into,
clippy::self_named_constructors clippy::self_named_constructors,
clippy::needless_lifetimes
)] )]
#[macro_use] #[macro_use]
@ -53,6 +54,7 @@ mod better {
} }
mod lifetimes { mod lifetimes {
#[derive(Clone, Copy)]
struct Foo<'a> { struct Foo<'a> {
foo_str: &'a str, foo_str: &'a str,
} }
@ -68,11 +70,19 @@ mod lifetimes {
Foo { foo_str: "foo" } Foo { foo_str: "foo" }
} }
// FIXME: the lint does not handle lifetimed struct fn clone(&self) -> Self {
// `Self` should be applicable here
fn clone(&self) -> Foo<'a> {
Foo { foo_str: self.foo_str } Foo { foo_str: self.foo_str }
} }
// Cannot replace with `Self` because the lifetime is not `'a`.
fn eq<'b>(&self, other: Foo<'b>) -> bool {
let x: Foo<'_> = other;
self.foo_str == other.foo_str
}
fn f(&self) -> Foo<'_> {
*self
}
} }
} }

16
tests/ui/use_self.rs
View File

@ -6,7 +6,8 @@
clippy::should_implement_trait, clippy::should_implement_trait,
clippy::upper_case_acronyms, clippy::upper_case_acronyms,
clippy::from_over_into, clippy::from_over_into,
clippy::self_named_constructors clippy::self_named_constructors,
clippy::needless_lifetimes
)] )]
#[macro_use] #[macro_use]
@ -53,6 +54,7 @@ fn default() -> Self {
} }
mod lifetimes { mod lifetimes {
#[derive(Clone, Copy)]
struct Foo<'a> { struct Foo<'a> {
foo_str: &'a str, foo_str: &'a str,
} }
@ -68,11 +70,19 @@ fn bar() -> Foo<'static> {
Foo { foo_str: "foo" } Foo { foo_str: "foo" }
} }
// FIXME: the lint does not handle lifetimed struct
// `Self` should be applicable here
fn clone(&self) -> Foo<'a> { fn clone(&self) -> Foo<'a> {
Foo { foo_str: self.foo_str } Foo { foo_str: self.foo_str }
} }
// Cannot replace with `Self` because the lifetime is not `'a`.
fn eq<'b>(&self, other: Foo<'b>) -> bool {
let x: Foo<'_> = other;
self.foo_str == other.foo_str
}
fn f(&self) -> Foo<'_> {
*self
}
} }
} }

92
tests/ui/use_self.stderr
View File

@ -1,5 +1,5 @@
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:21:21 --> tests/ui/use_self.rs:22:21
| |
LL | fn new() -> Foo { LL | fn new() -> Foo {
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
@ -8,250 +8,256 @@ LL | fn new() -> Foo {
= help: to override `-D warnings` add `#[allow(clippy::use_self)]` = help: to override `-D warnings` add `#[allow(clippy::use_self)]`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:22:13 --> tests/ui/use_self.rs:23:13
| |
LL | Foo {} LL | Foo {}
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:24:22 --> tests/ui/use_self.rs:25:22
| |
LL | fn test() -> Foo { LL | fn test() -> Foo {
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:25:13 --> tests/ui/use_self.rs:26:13
| |
LL | Foo::new() LL | Foo::new()
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:30:25 --> tests/ui/use_self.rs:31:25
| |
LL | fn default() -> Foo { LL | fn default() -> Foo {
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:31:13 --> tests/ui/use_self.rs:32:13
| |
LL | Foo::new() LL | Foo::new()
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:96:24 --> tests/ui/use_self.rs:73:28
|
LL | fn clone(&self) -> Foo<'a> {
| ^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> tests/ui/use_self.rs:106:24
| |
LL | fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> { LL | fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> {
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:96:55 --> tests/ui/use_self.rs:106:55
| |
LL | fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> { LL | fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> {
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:111:13 --> tests/ui/use_self.rs:121:13
| |
LL | TS(0) LL | TS(0)
| ^^ help: use the applicable keyword: `Self` | ^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:146:29 --> tests/ui/use_self.rs:156:29
| |
LL | fn bar() -> Bar { LL | fn bar() -> Bar {
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:147:21 --> tests/ui/use_self.rs:157:21
| |
LL | Bar { foo: Foo {} } LL | Bar { foo: Foo {} }
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:158:21 --> tests/ui/use_self.rs:168:21
| |
LL | fn baz() -> Foo { LL | fn baz() -> Foo {
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:159:13 --> tests/ui/use_self.rs:169:13
| |
LL | Foo {} LL | Foo {}
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:176:21 --> tests/ui/use_self.rs:186:21
| |
LL | let _ = Enum::B(42); LL | let _ = Enum::B(42);
| ^^^^ help: use the applicable keyword: `Self` | ^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:177:21 --> tests/ui/use_self.rs:187:21
| |
LL | let _ = Enum::C { field: true }; LL | let _ = Enum::C { field: true };
| ^^^^ help: use the applicable keyword: `Self` | ^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:178:21 --> tests/ui/use_self.rs:188:21
| |
LL | let _ = Enum::A; LL | let _ = Enum::A;
| ^^^^ help: use the applicable keyword: `Self` | ^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:220:13 --> tests/ui/use_self.rs:230:13
| |
LL | nested::A::fun_1(); LL | nested::A::fun_1();
| ^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:221:13 --> tests/ui/use_self.rs:231:13
| |
LL | nested::A::A; LL | nested::A::A;
| ^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:223:13 --> tests/ui/use_self.rs:233:13
| |
LL | nested::A {}; LL | nested::A {};
| ^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:242:13 --> tests/ui/use_self.rs:252:13
| |
LL | TestStruct::from_something() LL | TestStruct::from_something()
| ^^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:256:25 --> tests/ui/use_self.rs:266:25
| |
LL | async fn g() -> S { LL | async fn g() -> S {
| ^ help: use the applicable keyword: `Self` | ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:257:13 --> tests/ui/use_self.rs:267:13
| |
LL | S {} LL | S {}
| ^ help: use the applicable keyword: `Self` | ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:261:16 --> tests/ui/use_self.rs:271:16
| |
LL | &p[S::A..S::B] LL | &p[S::A..S::B]
| ^ help: use the applicable keyword: `Self` | ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:261:22 --> tests/ui/use_self.rs:271:22
| |
LL | &p[S::A..S::B] LL | &p[S::A..S::B]
| ^ help: use the applicable keyword: `Self` | ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:284:29 --> tests/ui/use_self.rs:294:29
| |
LL | fn foo(value: T) -> Foo<T> { LL | fn foo(value: T) -> Foo<T> {
| ^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:285:13 --> tests/ui/use_self.rs:295:13
| |
LL | Foo::<T> { value } LL | Foo::<T> { value }
| ^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:457:13 --> tests/ui/use_self.rs:467:13
| |
LL | A::new::<submod::B>(submod::B {}) LL | A::new::<submod::B>(submod::B {})
| ^ help: use the applicable keyword: `Self` | ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:494:13 --> tests/ui/use_self.rs:504:13
| |
LL | S2::new() LL | S2::new()
| ^^ help: use the applicable keyword: `Self` | ^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:531:17 --> tests/ui/use_self.rs:541:17
| |
LL | Foo::Bar => unimplemented!(), LL | Foo::Bar => unimplemented!(),
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:532:17 --> tests/ui/use_self.rs:542:17
| |
LL | Foo::Baz => unimplemented!(), LL | Foo::Baz => unimplemented!(),
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:538:20 --> tests/ui/use_self.rs:548:20
| |
LL | if let Foo::Bar = self { LL | if let Foo::Bar = self {
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:562:17 --> tests/ui/use_self.rs:572:17
| |
LL | Something::Num(n) => *n, LL | Something::Num(n) => *n,
| ^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:563:17 --> tests/ui/use_self.rs:573:17
| |
LL | Something::TupleNums(n, _m) => *n, LL | Something::TupleNums(n, _m) => *n,
| ^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:564:17 --> tests/ui/use_self.rs:574:17
| |
LL | Something::StructNums { one, two: _ } => *one, LL | Something::StructNums { one, two: _ } => *one,
| ^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:570:17 --> tests/ui/use_self.rs:580:17
| |
LL | crate::issue8845::Something::Num(n) => *n, LL | crate::issue8845::Something::Num(n) => *n,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:571:17 --> tests/ui/use_self.rs:581:17
| |
LL | crate::issue8845::Something::TupleNums(n, _m) => *n, LL | crate::issue8845::Something::TupleNums(n, _m) => *n,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:572:17 --> tests/ui/use_self.rs:582:17
| |
LL | crate::issue8845::Something::StructNums { one, two: _ } => *one, LL | crate::issue8845::Something::StructNums { one, two: _ } => *one,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:588:17 --> tests/ui/use_self.rs:598:17
| |
LL | let Foo(x) = self; LL | let Foo(x) = self;
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:593:17 --> tests/ui/use_self.rs:603:17
| |
LL | let crate::issue8845::Foo(x) = self; LL | let crate::issue8845::Foo(x) = self;
| ^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:600:17 --> tests/ui/use_self.rs:610:17
| |
LL | let Bar { x, .. } = self; LL | let Bar { x, .. } = self;
| ^^^ help: use the applicable keyword: `Self` | ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:605:17 --> tests/ui/use_self.rs:615:17
| |
LL | let crate::issue8845::Bar { x, .. } = self; LL | let crate::issue8845::Bar { x, .. } = self;
| ^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self` | ^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition error: unnecessary structure name repetition
--> tests/ui/use_self.rs:644:17 --> tests/ui/use_self.rs:654:17
| |
LL | E::A => {}, LL | E::A => {},
| ^ help: use the applicable keyword: `Self` | ^ help: use the applicable keyword: `Self`
error: aborting due to 42 previous errors error: aborting due to 43 previous errors