From 168d6d4ca3b67829fcd7f945b0c2f0f96746b75e Mon Sep 17 00:00:00 2001 From: Denis Pakhomov Date: Sat, 24 Jan 2026 18:02:45 +0100 Subject: [PATCH] add new lint `getter-prefixes` --- CHANGELOG.md | 1 + clippy_dev/src/parse/cursor.rs | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/getter_prefixes.rs | 145 ++++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 2 + tests/ui/getter_prefixes.fixed | 138 ++++++++++++++++++++++++++ tests/ui/getter_prefixes.rs | 138 ++++++++++++++++++++++++++ tests/ui/getter_prefixes.stderr | 65 +++++++++++++ 8 files changed, 491 insertions(+) create mode 100644 clippy_lints/src/getter_prefixes.rs create mode 100644 tests/ui/getter_prefixes.fixed create mode 100644 tests/ui/getter_prefixes.rs create mode 100644 tests/ui/getter_prefixes.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 37f5eb09e524..0a8475281737 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7092,6 +7092,7 @@ Released 2018-09-13 [`get_first`]: https://rust-lang.github.io/rust-clippy/main/index.html#get_first [`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/main/index.html#get_last_with_len [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/main/index.html#get_unwrap +[`getter_prefixes`]: https://rust-lang.github.io/rust-clippy/main/index.html#getter_prefixes [`host_endian_bytes`]: https://rust-lang.github.io/rust-clippy/main/index.html#host_endian_bytes [`identity_conversion`]: https://rust-lang.github.io/rust-clippy/main/index.html#identity_conversion [`identity_op`]: https://rust-lang.github.io/rust-clippy/main/index.html#identity_op diff --git a/clippy_dev/src/parse/cursor.rs b/clippy_dev/src/parse/cursor.rs index 2ee007615a67..d6c6018a3515 100644 --- a/clippy_dev/src/parse/cursor.rs +++ b/clippy_dev/src/parse/cursor.rs @@ -139,6 +139,7 @@ impl<'txt> Cursor<'txt> { /// Gets the text of the captured token assuming it came from this cursor. #[must_use] + #[allow(clippy::getter_prefixes)] pub fn get_text(&self, capture: Capture) -> &'txt str { &self.text[capture.pos as usize..(capture.pos + capture.len) as usize] } diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2258ae765928..60db96f2dbab 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -208,6 +208,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::functions::TOO_MANY_ARGUMENTS_INFO, crate::functions::TOO_MANY_LINES_INFO, crate::future_not_send::FUTURE_NOT_SEND_INFO, + crate::getter_prefixes::GETTER_PREFIXES_INFO, crate::if_let_mutex::IF_LET_MUTEX_INFO, crate::if_not_else::IF_NOT_ELSE_INFO, crate::if_then_some_else_none::IF_THEN_SOME_ELSE_NONE_INFO, diff --git a/clippy_lints/src/getter_prefixes.rs b/clippy_lints/src/getter_prefixes.rs new file mode 100644 index 000000000000..b1e561b671d8 --- /dev/null +++ b/clippy_lints/src/getter_prefixes.rs @@ -0,0 +1,145 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use rustc_ast::NodeId; +use rustc_ast::ast::{Block, Expr, ExprKind, Fn, FnRetTy, FnSig, MethodCall, Stmt, StmtKind, Ty, Visibility}; +use rustc_ast::visit::{AssocCtxt, FnCtxt, FnKind}; +use rustc_errors::Applicability; +use rustc_lint::{EarlyContext, EarlyLintPass, declare_lint_pass}; +use rustc_span::Span; +use rustc_span::symbol::kw; + +declare_clippy_lint! { + /// ### What it does + /// Checks for the `get_` prefix on public getters. + /// + /// ### Why is this bad? + /// The Rust API Guidelines section on naming + /// [specifies](https://rust-lang-nursery.github.io/api-guidelines/naming.html#getter-names-follow-rust-convention-c-getter) + /// that the `get_` prefix is not used for getters in Rust code unless + /// there is a single and obvious thing that could reasonably be gotten by + /// a getter. + /// + /// The exceptions to this naming convention are as follows: + /// - `get` (such as in + /// [`std::cell::Cell::get`](https://doc.rust-lang.org/std/cell/struct.Cell.html#method.get)) + /// - `get_mut` + /// - `get_unchecked` + /// - `get_unchecked_mut` + /// - `get_ref` + /// + /// ### Example + /// ```no_run + /// struct B { + /// id: usize + /// } + /// + /// impl B { + /// // Bad + /// pub fn get_id(&self) -> usize { + /// self.id + /// } + /// } + /// ``` + /// Use instead: + /// ```no_run + /// struct G { + /// id: usize + /// } + /// + /// impl G { + /// // Good + /// pub fn id(&self) -> usize { + /// self.id + /// } + /// + /// // Also allowed + /// pub fn get(&self) -> usize { + /// self.id + /// } + /// } + /// ``` + #[clippy::version = "1.95.0"] + pub GETTER_PREFIXES, + style, + "prefixing a getter with `get_`, which does not follow convention" +} + +declare_lint_pass!(GetterPrefixes => [GETTER_PREFIXES]); + +const EXCLUDED_SUFFIXES: [&str; 5] = ["", "mut", "unchecked", "unchecked_mut", "ref"]; + +impl EarlyLintPass for GetterPrefixes { + fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: Span, _: NodeId) { + if let FnKind::Fn( + FnCtxt::Assoc(AssocCtxt::Impl { of_trait: false }), + Visibility { + kind: visibility_kind, .. + }, + &Fn { + ref ident, + sig: FnSig { ref decl, .. }, + body: Some(ref block), + .. + }, + ) = fn_kind + && visibility_kind.is_pub() + && decl.has_self() + && let FnRetTy::Ty(Ty { ref kind, .. }) = decl.output + && !kind.is_unit() + && let Some(ref suffix) = ident.name.as_str().strip_prefix("get_") + && !EXCLUDED_SUFFIXES.contains(suffix) + && let Some(expr) = body_return_expr(block) + && has_trivial_getter_logic(expr) + { + span_lint_and_sugg( + cx, + GETTER_PREFIXES, + ident.span, + "prefixing a getter with `get_` does not follow naming conventions", + "replace it with", + suffix.to_string(), + Applicability::Unspecified, + ); + } + } +} + +fn body_return_expr(block: &Block) -> Option<&Expr> { + match block.stmts.last() { + Some(&Stmt { + kind: + StmtKind::Expr(ref expr) + | StmtKind::Semi(Expr { + kind: ExprKind::Ret(Some(ref expr)), + .. + }), + .. + }) => Some(expr), + _ => None, + } +} + +fn has_trivial_getter_logic(expr: &Expr) -> bool { + match &expr.kind { + ExprKind::Lit(lit) => lit.symbol == kw::SelfLower, + ExprKind::Path(None, path) if path.segments.first().is_some_and(|seg| seg.ident.name == kw::SelfLower) => true, + + ExprKind::Array(expr_vec) | ExprKind::Tup(expr_vec) => { + expr_vec.iter().any(|expr| has_trivial_getter_logic(expr)) + }, + + ExprKind::Paren(inner) + | ExprKind::Cast(inner, _) + | ExprKind::Field(inner, _) + | ExprKind::Index(inner, _, _) + | ExprKind::AddrOf(_, _, inner) + | ExprKind::Unary(_, inner) => has_trivial_getter_logic(inner), + + ExprKind::Binary(_, lhs_expr, rhs_expr) => { + has_trivial_getter_logic(lhs_expr) || has_trivial_getter_logic(rhs_expr) + }, + + ExprKind::MethodCall(MethodCall { receiver, .. }) => has_trivial_getter_logic(receiver), + + _ => false, + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e1a1055b609d..b8c611077ab4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -150,6 +150,7 @@ mod from_raw_with_void_ptr; mod from_str_radix_10; mod functions; mod future_not_send; +mod getter_prefixes; mod if_let_mutex; mod if_not_else; mod if_then_some_else_none; @@ -543,6 +544,7 @@ rustc_lint::early_lint_methods!( EmptyLineAfter: empty_line_after::EmptyLineAfter = empty_line_after::EmptyLineAfter::new(), InlineTraitBounds: inline_trait_bounds::InlineTraitBounds = inline_trait_bounds::InlineTraitBounds::default(), DefinitionInModuleRoot: definition_in_module_root::DefinitionInModuleRoot = definition_in_module_root::DefinitionInModuleRoot::default(), + GetterPrefixes: getter_prefixes::GetterPrefixes = getter_prefixes::GetterPrefixes, // add early passes here, used by `cargo dev new_lint` ]] ); diff --git a/tests/ui/getter_prefixes.fixed b/tests/ui/getter_prefixes.fixed new file mode 100644 index 000000000000..3dfc6ac9bd55 --- /dev/null +++ b/tests/ui/getter_prefixes.fixed @@ -0,0 +1,138 @@ +#![warn(clippy::getter_prefixes)] +#![expect(clippy::needless_return, clippy::unused_unit)] + +pub trait MyTrait { + fn get_trait_value(&self) -> &str; +} + +pub struct MyStruct { + a: String, + b: String, + c: i32, + d: Vec, +} + +impl MyStruct { + pub fn return_stmt(&self) -> &str { + //~^ getter_prefixes + return &self.a; + } + + pub fn lit(self) -> Self { + //~^ getter_prefixes + self + } + + pub fn array(&self) -> [&str; 2] { + //~^ getter_prefixes + [&self.a, &self.b] + } + + pub fn tuple(&self) -> (&str, &i32) { + //~^ getter_prefixes + (&self.a, &self.c) + } + + pub fn cast_value(&self) -> i64 { + //~^ getter_prefixes + self.c as i64 + } + + pub fn parens_value(&self) -> &i32 { + //~^ getter_prefixes + (&self.c) + } + + pub fn unary(&self) -> i32 { + //~^ getter_prefixes + -self.c + } + + pub fn binary(&self) -> i32 { + //~^ getter_prefixes + self.c / 2 + } + + pub fn method_call(&self) -> Option<&usize> { + //~^ getter_prefixes + self.d.first() + } + + pub fn index_value(&self) -> usize { + //~^ getter_prefixes + self.d[0] + } + + pub fn get_if_value(&self) -> &str { + if self.a < self.b { &self.a } else { &self.b } + } + + pub fn get_arm_value(&self) -> &str { + match self.d.first() { + Some(x) if x / 2 == 0 => &self.a, + Some(x) => &self.b, + None => "default", + } + } + + fn get_private_value(&self) -> &str { + &self.a + } + + pub fn get_unit(&self) {} + + pub fn get_unit_explicit(&self) -> () {} + + pub fn get_constant_value(&self) -> u32 { + 42 + } + + pub fn method_call_dup(&self) -> Option<&usize> { + self.d.first() + } + + pub fn get(&self) -> &str { + &self.a + } + + pub fn get_mut(&mut self) -> &mut str { + &mut self.a + } + + pub fn get_unchecked(&self) -> usize { + self.d[0] + } + + pub fn get_unchecked_mut(&mut self) -> &mut usize { + &mut self.d[0] + } + + pub fn get_ref(&self) -> &str { + &self.a + } +} + +impl MyTrait for MyStruct { + fn get_trait_value(&self) -> &str { + &self.a + } +} + +pub fn get_value() -> usize { + 42 +} + +fn main() { + let mut s = MyStruct { + a: "a".to_string(), + b: "b".to_string(), + c: 1, + d: vec![1, 2, 3], + }; + + s.get(); + s.get_mut(); + s.get_unchecked(); + s.get_unchecked_mut(); + s.get_ref(); +} diff --git a/tests/ui/getter_prefixes.rs b/tests/ui/getter_prefixes.rs new file mode 100644 index 000000000000..bd8c40ff714b --- /dev/null +++ b/tests/ui/getter_prefixes.rs @@ -0,0 +1,138 @@ +#![warn(clippy::getter_prefixes)] +#![expect(clippy::needless_return, clippy::unused_unit)] + +pub trait MyTrait { + fn get_trait_value(&self) -> &str; +} + +pub struct MyStruct { + a: String, + b: String, + c: i32, + d: Vec, +} + +impl MyStruct { + pub fn get_return_stmt(&self) -> &str { + //~^ getter_prefixes + return &self.a; + } + + pub fn get_lit(self) -> Self { + //~^ getter_prefixes + self + } + + pub fn get_array(&self) -> [&str; 2] { + //~^ getter_prefixes + [&self.a, &self.b] + } + + pub fn get_tuple(&self) -> (&str, &i32) { + //~^ getter_prefixes + (&self.a, &self.c) + } + + pub fn get_cast_value(&self) -> i64 { + //~^ getter_prefixes + self.c as i64 + } + + pub fn get_parens_value(&self) -> &i32 { + //~^ getter_prefixes + (&self.c) + } + + pub fn get_unary(&self) -> i32 { + //~^ getter_prefixes + -self.c + } + + pub fn get_binary(&self) -> i32 { + //~^ getter_prefixes + self.c / 2 + } + + pub fn get_method_call(&self) -> Option<&usize> { + //~^ getter_prefixes + self.d.first() + } + + pub fn get_index_value(&self) -> usize { + //~^ getter_prefixes + self.d[0] + } + + pub fn get_if_value(&self) -> &str { + if self.a < self.b { &self.a } else { &self.b } + } + + pub fn get_arm_value(&self) -> &str { + match self.d.first() { + Some(x) if x / 2 == 0 => &self.a, + Some(x) => &self.b, + None => "default", + } + } + + fn get_private_value(&self) -> &str { + &self.a + } + + pub fn get_unit(&self) {} + + pub fn get_unit_explicit(&self) -> () {} + + pub fn get_constant_value(&self) -> u32 { + 42 + } + + pub fn method_call_dup(&self) -> Option<&usize> { + self.d.first() + } + + pub fn get(&self) -> &str { + &self.a + } + + pub fn get_mut(&mut self) -> &mut str { + &mut self.a + } + + pub fn get_unchecked(&self) -> usize { + self.d[0] + } + + pub fn get_unchecked_mut(&mut self) -> &mut usize { + &mut self.d[0] + } + + pub fn get_ref(&self) -> &str { + &self.a + } +} + +impl MyTrait for MyStruct { + fn get_trait_value(&self) -> &str { + &self.a + } +} + +pub fn get_value() -> usize { + 42 +} + +fn main() { + let mut s = MyStruct { + a: "a".to_string(), + b: "b".to_string(), + c: 1, + d: vec![1, 2, 3], + }; + + s.get(); + s.get_mut(); + s.get_unchecked(); + s.get_unchecked_mut(); + s.get_ref(); +} diff --git a/tests/ui/getter_prefixes.stderr b/tests/ui/getter_prefixes.stderr new file mode 100644 index 000000000000..9dad9d0f3cf8 --- /dev/null +++ b/tests/ui/getter_prefixes.stderr @@ -0,0 +1,65 @@ +error: prefixing a getter with `get_` does not follow naming conventions + --> tests/ui/getter_prefixes.rs:16:12 + | +LL | pub fn get_return_stmt(&self) -> &str { + | ^^^^^^^^^^^^^^^ help: replace it with: `return_stmt` + | + = note: `-D clippy::getter-prefixes` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::getter_prefixes)]` + +error: prefixing a getter with `get_` does not follow naming conventions + --> tests/ui/getter_prefixes.rs:21:12 + | +LL | pub fn get_lit(self) -> Self { + | ^^^^^^^ help: replace it with: `lit` + +error: prefixing a getter with `get_` does not follow naming conventions + --> tests/ui/getter_prefixes.rs:26:12 + | +LL | pub fn get_array(&self) -> [&str; 2] { + | ^^^^^^^^^ help: replace it with: `array` + +error: prefixing a getter with `get_` does not follow naming conventions + --> tests/ui/getter_prefixes.rs:31:12 + | +LL | pub fn get_tuple(&self) -> (&str, &i32) { + | ^^^^^^^^^ help: replace it with: `tuple` + +error: prefixing a getter with `get_` does not follow naming conventions + --> tests/ui/getter_prefixes.rs:36:12 + | +LL | pub fn get_cast_value(&self) -> i64 { + | ^^^^^^^^^^^^^^ help: replace it with: `cast_value` + +error: prefixing a getter with `get_` does not follow naming conventions + --> tests/ui/getter_prefixes.rs:41:12 + | +LL | pub fn get_parens_value(&self) -> &i32 { + | ^^^^^^^^^^^^^^^^ help: replace it with: `parens_value` + +error: prefixing a getter with `get_` does not follow naming conventions + --> tests/ui/getter_prefixes.rs:46:12 + | +LL | pub fn get_unary(&self) -> i32 { + | ^^^^^^^^^ help: replace it with: `unary` + +error: prefixing a getter with `get_` does not follow naming conventions + --> tests/ui/getter_prefixes.rs:51:12 + | +LL | pub fn get_binary(&self) -> i32 { + | ^^^^^^^^^^ help: replace it with: `binary` + +error: prefixing a getter with `get_` does not follow naming conventions + --> tests/ui/getter_prefixes.rs:56:12 + | +LL | pub fn get_method_call(&self) -> Option<&usize> { + | ^^^^^^^^^^^^^^^ help: replace it with: `method_call` + +error: prefixing a getter with `get_` does not follow naming conventions + --> tests/ui/getter_prefixes.rs:61:12 + | +LL | pub fn get_index_value(&self) -> usize { + | ^^^^^^^^^^^^^^^ help: replace it with: `index_value` + +error: aborting due to 10 previous errors +