Skip to content

Commit 0c6cadf

Browse files
committed
method without self relation
1 parent 1ecb18a commit 0c6cadf

File tree

7 files changed

+727
-0
lines changed

7 files changed

+727
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6523,6 +6523,7 @@ Released 2018-09-13
65236523
[`mem_replace_option_with_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_some
65246524
[`mem_replace_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_default
65256525
[`mem_replace_with_uninit`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_uninit
6526+
[`method_without_self_relation`]: https://rust-lang.github.io/rust-clippy/master/index.html#method_without_self_relation
65266527
[`min_ident_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_ident_chars
65276528
[`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max
65286529
[`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
425425
crate::methods::MAP_IDENTITY_INFO,
426426
crate::methods::MAP_UNWRAP_OR_INFO,
427427
crate::methods::MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES_INFO,
428+
crate::methods::METHOD_WITHOUT_SELF_RELATION_INFO,
428429
crate::methods::MUT_MUTEX_LOCK_INFO,
429430
crate::methods::NAIVE_BYTECOUNT_INFO,
430431
crate::methods::NEEDLESS_AS_BYTES_INFO,
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
use clippy_utils::diagnostics::span_lint_and_help;
2+
use rustc_hir::{FnSig, ImplItem, ImplItemKind};
3+
use rustc_lint::LateContext;
4+
use rustc_middle::ty::{self, Ty};
5+
use rustc_span::Span;
6+
7+
use super::METHOD_WITHOUT_SELF_RELATION;
8+
9+
/// Check if a type contains a reference to `Self` anywhere in its structure.
10+
/// This includes direct references and generic parameters.
11+
fn contains_self<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, self_ty: Ty<'tcx>) -> bool {
12+
// Direct comparison with Self type
13+
if ty == self_ty {
14+
return true;
15+
}
16+
17+
match ty.kind() {
18+
// Check if this is a reference to Self
19+
ty::Ref(_, inner_ty, _) => contains_self(cx, *inner_ty, self_ty),
20+
21+
// Check if this is a raw pointer to Self
22+
ty::RawPtr(inner_ty, _) => contains_self(cx, *inner_ty, self_ty),
23+
24+
// Check generic types like Option<Self>, Vec<Self>, Result<Self, E>, etc.
25+
ty::Adt(_, args) => args.types().any(|arg_ty| contains_self(cx, arg_ty, self_ty)),
26+
27+
// Check tuples like (Self, i32) or (String, Self)
28+
ty::Tuple(types) => types.iter().any(|ty| contains_self(cx, ty, self_ty)),
29+
30+
// Check array types like [Self; 10]
31+
ty::Array(elem_ty, _) | ty::Slice(elem_ty) => contains_self(cx, *elem_ty, self_ty),
32+
33+
// Check function pointer types
34+
ty::FnPtr(sig, _) => {
35+
let sig = sig.skip_binder();
36+
sig.inputs().iter().any(|&ty| contains_self(cx, ty, self_ty)) || contains_self(cx, sig.output(), self_ty)
37+
},
38+
39+
// Check closures (uncommon but possible)
40+
ty::Closure(_, args) => {
41+
args.as_closure()
42+
.sig()
43+
.inputs()
44+
.skip_binder()
45+
.iter()
46+
.any(|&ty| contains_self(cx, ty, self_ty))
47+
|| contains_self(cx, args.as_closure().sig().output().skip_binder(), self_ty)
48+
},
49+
50+
// Check opaque types (impl Trait, async fn return types)
51+
ty::Alias(ty::AliasTyKind::Opaque, alias_ty) => {
52+
// Check the bounds of the opaque type
53+
alias_ty.args.types().any(|arg_ty| contains_self(cx, arg_ty, self_ty))
54+
},
55+
56+
// Check trait objects (dyn Trait)
57+
ty::Dynamic(predicates, _) => {
58+
use rustc_middle::ty::ExistentialPredicate;
59+
// Check if any of the trait bounds reference Self
60+
predicates.iter().any(|predicate| match predicate.skip_binder() {
61+
ExistentialPredicate::Trait(trait_ref) => {
62+
trait_ref.args.types().any(|arg_ty| contains_self(cx, arg_ty, self_ty))
63+
},
64+
ExistentialPredicate::Projection(projection) => {
65+
projection.args.types().any(|arg_ty| contains_self(cx, arg_ty, self_ty))
66+
|| contains_self(cx, projection.term.as_type().unwrap(), self_ty)
67+
},
68+
ExistentialPredicate::AutoTrait(_) => false,
69+
})
70+
},
71+
72+
_ => false,
73+
}
74+
}
75+
76+
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>, self_ty: Ty<'tcx>) {
77+
if let ImplItemKind::Fn(ref sig, _) = impl_item.kind {
78+
// Get the method signature from the type system
79+
let method_sig = cx.tcx.fn_sig(impl_item.owner_id).instantiate_identity();
80+
let method_sig = method_sig.skip_binder();
81+
82+
// Check if there's a self parameter (self, &self, &mut self, self: Arc<Self>, etc.)
83+
if has_self_parameter(sig) {
84+
return;
85+
}
86+
87+
// Check all input parameters for Self references
88+
for &param_ty in method_sig.inputs().iter() {
89+
if contains_self(cx, param_ty, self_ty) {
90+
return;
91+
}
92+
}
93+
94+
// Check return type for Self references
95+
let return_ty = method_sig.output();
96+
if contains_self(cx, return_ty, self_ty) {
97+
return;
98+
}
99+
100+
// If we reach here, the method has no relationship to Self
101+
emit_lint(cx, impl_item.span, impl_item.ident.name.as_str());
102+
}
103+
}
104+
105+
/// Check if the function signature has an explicit self parameter
106+
fn has_self_parameter(sig: &FnSig<'_>) -> bool {
107+
sig.decl.implicit_self.has_implicit_self()
108+
}
109+
110+
fn emit_lint(cx: &LateContext<'_>, span: Span, method_name: &str) {
111+
span_lint_and_help(
112+
cx,
113+
METHOD_WITHOUT_SELF_RELATION,
114+
span,
115+
format!("method `{method_name}` has no relationship to `Self`"),
116+
None,
117+
"consider making this a standalone function instead of a method",
118+
);
119+
}

clippy_lints/src/methods/mod.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ mod map_flatten;
7474
mod map_identity;
7575
mod map_unwrap_or;
7676
mod map_with_unused_argument_over_ranges;
77+
mod method_without_self_relation;
7778
mod mut_mutex_lock;
7879
mod needless_as_bytes;
7980
mod needless_character_iteration;
@@ -1162,6 +1163,73 @@ declare_clippy_lint! {
11621163
"not returning type containing `Self` in a `new` method"
11631164
}
11641165

1166+
declare_clippy_lint! {
1167+
/// ### What it does
1168+
/// Checks for methods in impl blocks that have no relationship to `Self`.
1169+
///
1170+
/// ### Why is this bad?
1171+
/// Methods that don't use `Self` in their signature (parameters or return type)
1172+
/// are not actually methods but rather associated functions. They would be
1173+
/// better expressed as standalone functions, making the code more modular
1174+
/// and easier to discover.
1175+
///
1176+
/// ### Known issues
1177+
/// This lint is intentionally set to `restriction` category because there are
1178+
/// valid reasons to keep functions within an impl block:
1179+
/// - Helper functions that logically belong to the type
1180+
/// - Functions meant to be used through the type's namespace
1181+
/// - Private implementation details that should be grouped with the type
1182+
///
1183+
/// ### Example
1184+
/// ```no_run
1185+
/// struct Calculator;
1186+
///
1187+
/// impl Calculator {
1188+
/// // Bad: No relationship to Self
1189+
/// fn add(a: i32, b: i32) -> i32 {
1190+
/// a + b
1191+
/// }
1192+
/// }
1193+
/// ```
1194+
/// Use instead:
1195+
/// ```no_run
1196+
/// struct Calculator;
1197+
///
1198+
/// // Good: Standalone function
1199+
/// fn add(a: i32, b: i32) -> i32 {
1200+
/// a + b
1201+
/// }
1202+
/// ```
1203+
///
1204+
/// Methods with Self references are fine:
1205+
/// ```no_run
1206+
/// struct Calculator {
1207+
/// precision: u32,
1208+
/// }
1209+
///
1210+
/// impl Calculator {
1211+
/// // Good: Uses &self
1212+
/// fn add(&self, a: i32, b: i32) -> i32 {
1213+
/// a + b
1214+
/// }
1215+
///
1216+
/// // Good: Returns Self
1217+
/// fn new(precision: u32) -> Self {
1218+
/// Self { precision }
1219+
/// }
1220+
///
1221+
/// // Good: Takes Self in generic parameter
1222+
/// fn from_option(opt: Option<Self>) -> Self {
1223+
/// opt.unwrap_or_default()
1224+
/// }
1225+
/// }
1226+
/// ```
1227+
#[clippy::version = "1.92.0"]
1228+
pub METHOD_WITHOUT_SELF_RELATION,
1229+
restriction,
1230+
"methods in impl blocks that have no relationship to `Self`"
1231+
}
1232+
11651233
declare_clippy_lint! {
11661234
/// ### What it does
11671235
/// Checks for calling `.step_by(0)` on iterators which panics.
@@ -4720,6 +4788,7 @@ impl_lint_pass!(Methods => [
47204788
FLAT_MAP_OPTION,
47214789
INEFFICIENT_TO_STRING,
47224790
NEW_RET_NO_SELF,
4791+
METHOD_WITHOUT_SELF_RELATION,
47234792
SINGLE_CHAR_ADD_STR,
47244793
SEARCH_IS_SOME,
47254794
FILTER_NEXT,
@@ -4929,6 +4998,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
49294998
}
49304999

49315000
new_ret_no_self::check_impl_item(cx, impl_item, self_ty, implements_trait);
5001+
method_without_self_relation::check(cx, impl_item, self_ty);
49325002
}
49335003
}
49345004

tests/ui/explicit_write_in_test.stderr

Whitespace-only changes.

0 commit comments

Comments
 (0)