Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_dev/src/parse/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
145 changes: 145 additions & 0 deletions clippy_lints/src/getter_prefixes.rs
Original file line number Diff line number Diff line change
@@ -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,
}
}
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
]]
);
Expand Down
138 changes: 138 additions & 0 deletions tests/ui/getter_prefixes.fixed
Original file line number Diff line number Diff line change
@@ -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<usize>,
}

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();
}
Loading