[WIP] implement getter_prefixes lint

CHANGELOG.md

@@ -6547,6 +6547,7 @@ Released 2018-09-13
 [`get_first`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_first
 [`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len
 [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap
+[`getter_prefixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#getter_prefixes
 [`host_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#host_endian_bytes
 [`identity_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion
 [`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op

clippy_lints/src/declared_lints.rs

@@ -200,6 +200,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,

clippy_lints/src/getter_prefixes.rs

@@ -0,0 +1,111 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use rustc_ast::ast::{AssocItem, AssocItemKind, Fn, FnRetTy, FnSig, Impl, Item, ItemKind, Ty};
+use rustc_errors::Applicability;
+use rustc_lint::{EarlyContext, EarlyLintPass};
+use rustc_session::impl_lint_pass;
+
+declare_clippy_lint! {
+    /// **What it does:** Checks for the `get_` prefix on 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`
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// // Bad
+    /// impl B {
+    ///     fn get_id(&self) -> usize {
+    ///         ..
+    ///     }
+    /// }
+    ///
+    /// // Good
+    /// impl G {
+    ///     fn id(&self) -> usize {
+    ///         ..
+    ///     }
+    /// }
+    ///
+    /// // Also allowed
+    /// impl A {
+    ///     fn get(&self) -> usize {
+    ///         ..
+    ///     }
+    /// }
+    /// ```
+    #[clippy::version = "1.95.0"]
+    pub GETTER_PREFIXES,
+    style,
+    "prefixing a getter with `get_`, which does not follow convention"
+}
+
+const EXCLUDED_SUFFIXES: [&str; 5] = ["", "mut", "unchecked", "unchecked_mut", "ref"];
+
+#[derive(Default)]
+pub struct GetterPrefixes {
+    inherent_impl_ctx: bool,
+}
+
+impl_lint_pass!(GetterPrefixes => [GETTER_PREFIXES]);
+
+impl EarlyLintPass for GetterPrefixes {
+    fn check_item(&mut self, _: &EarlyContext<'_>, item: &Item) {
+        if let ItemKind::Impl(Impl { of_trait: None, .. }) = item.kind {
+            self.inherent_impl_ctx = true;
+        }
+    }
+
+    fn check_item_post(&mut self, _: &EarlyContext<'_>, item: &Item) {
+        if let ItemKind::Impl(_) = item.kind {
+            self.inherent_impl_ctx = false;
+        }
+    }
+
+    fn check_trait_item(&mut self, cx: &EarlyContext<'_>, assoc_item: &AssocItem) {
+        check_getter_prefix(cx, assoc_item);
+    }
+
+    fn check_impl_item(&mut self, cx: &EarlyContext<'_>, assoc_item: &AssocItem) {
+        if self.inherent_impl_ctx {
+            check_getter_prefix(cx, assoc_item);
+        }
+    }
+}
+
+fn check_getter_prefix(cx: &EarlyContext<'_>, assoc_item: &AssocItem) {
+    if let AssocItemKind::Fn(box Fn {
+        ref ident,
+        sig: FnSig { ref decl, .. },
+        ..
+    }) = assoc_item.kind
+        && decl.has_self()
+        && let FnRetTy::Ty(box Ty { ref kind, .. }) = decl.output
+        && !kind.is_unit()
+        && let Some(ref suffix) = ident.name.as_str().strip_prefix("get_")
+        && !EXCLUDED_SUFFIXES.contains(suffix)
+    {
+        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,
+        );
+    }
+}

clippy_lints/src/lib.rs

@@ -147,6 +147,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;
@@ -517,6 +518,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(|| Box::new(byte_char_slices::ByteCharSlice)),
         Box::new(|| Box::new(cfg_not_test::CfgNotTest)),
         Box::new(|| Box::new(empty_line_after::EmptyLineAfter::new())),
+        Box::new(|| Box::<getter_prefixes::GetterPrefixes>::default()),
         // add early passes here, used by `cargo dev new_lint`
     ];
     store.early_passes.extend(early_lints);

tests/ui/getter_prefixes.rs

@@ -0,0 +1,106 @@
+#![allow(clippy::unused_unit)]
+#![warn(clippy::getter_prefixes)]
+//@no-rustfix
+
+pub trait MyTrait {
+    fn get_id(&self) -> usize;
+    //~^ getter_prefixes
+    fn get_unit(&self);
+    fn get_unit_explicit(&self) -> ();
+    fn get_static_id() -> usize;
+    fn get(&self) -> usize;
+    fn get_mut(&mut self) -> usize;
+    fn get_unchecked(&self) -> usize;
+    fn get_unchecked_mut(&mut self) -> usize;
+    fn get_ref(&self) -> usize;
+}
+
+pub struct MyTraitImpl {
+    id: usize,
+}
+
+impl MyTrait for MyTraitImpl {
+    fn get_id(&self) -> usize {
+        self.id
+    }
+
+    fn get_unit(&self) {}
+
+    fn get_unit_explicit(&self) -> () {}
+
+    fn get_static_id() -> usize {
+        42
+    }
+
+    fn get(&self) -> usize {
+        self.id
+    }
+
+    fn get_mut(&mut self) -> usize {
+        self.id
+    }
+
+    fn get_unchecked(&self) -> usize {
+        self.id
+    }
+
+    fn get_unchecked_mut(&mut self) -> usize {
+        self.id
+    }
+
+    fn get_ref(&self) -> usize {
+        self.id
+    }
+}
+
+pub struct MyStruct {
+    id: usize,
+}
+
+impl MyStruct {
+    pub fn get_id(&self) -> usize {
+        //~^ getter_prefixes
+        self.id
+    }
+
+    pub fn get_unit(&self) {}
+
+    pub fn get_static_id() -> usize {
+        42
+    }
+
+    pub fn get(&self) -> usize {
+        self.id
+    }
+
+    pub fn get_mut(&mut self) -> usize {
+        self.id
+    }
+
+    pub fn get_unchecked(&self) -> usize {
+        self.id
+    }
+
+    pub fn get_unchecked_mut(&mut self) -> usize {
+        self.id
+    }
+
+    pub fn get_ref(&self) -> usize {
+        self.id
+    }
+}
+
+pub fn get_id() -> usize {
+    42
+}
+
+fn main() {
+    let mut s = MyStruct { id: 42 };
+    s.get_id();
+    MyStruct::get_static_id();
+    s.get();
+    s.get_mut();
+    s.get_unchecked();
+    s.get_unchecked_mut();
+    s.get_ref();
+}

tests/ui/getter_prefixes.stderr

@@ -0,0 +1,17 @@
+error: prefixing a getter with `get_` does not follow naming conventions
+  --> tests/ui/getter_prefixes.rs:6:8
+   |
+LL |     fn get_id(&self) -> usize;
+   |        ^^^^^^ help: replace it with: `id`
+   |
+   = 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:61:12
+   |
+LL |     pub fn get_id(&self) -> usize {
+   |            ^^^^^^ help: replace it with: `id`
+
+error: aborting due to 2 previous errors
+

tests/ui/manual_slice_size_calculation.fixed

@@ -77,7 +77,7 @@ fn issue_14802() {
     }
 
     impl IcedSlice {
-        fn get_len(&self) -> usize {
+        fn len(&self) -> usize {
             std::mem::size_of_val(&self.dst)
             //~^ manual_slice_size_calculation
         }

tests/ui/manual_slice_size_calculation.rs

@@ -77,7 +77,7 @@ fn issue_14802() {
     }
 
     impl IcedSlice {
-        fn get_len(&self) -> usize {
+        fn len(&self) -> usize {
             self.dst.len() * size_of::<u8>()
             //~^ manual_slice_size_calculation
         }

tests/ui/result_unit_error.rs

@@ -11,18 +11,18 @@ fn private_unit_errors() -> Result<String, ()> {
 }
 
 pub trait HasUnitError {
-    fn get_that_error(&self) -> Result<bool, ()>;
+    fn that_error(&self) -> Result<bool, ()>;
     //~^ result_unit_err
 
-    fn get_this_one_too(&self) -> Result<bool, ()> {
+    fn this_one_too(&self) -> Result<bool, ()> {
         //~^ result_unit_err
 
         Err(())
     }
 }
 
 impl HasUnitError for () {
-    fn get_that_error(&self) -> Result<bool, ()> {
+    fn that_error(&self) -> Result<bool, ()> {
         Ok(true)
     }
 }

tests/ui/result_unit_error.stderr

@@ -11,16 +11,16 @@ LL | pub fn returns_unit_error() -> Result<u32, ()> {
 error: this returns a `Result<_, ()>`
   --> tests/ui/result_unit_error.rs:14:5
    |
-LL |     fn get_that_error(&self) -> Result<bool, ()>;
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     fn that_error(&self) -> Result<bool, ()>;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a custom `Error` type instead
 
 error: this returns a `Result<_, ()>`
   --> tests/ui/result_unit_error.rs:17:5
    |
-LL |     fn get_this_one_too(&self) -> Result<bool, ()> {
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     fn this_one_too(&self) -> Result<bool, ()> {
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a custom `Error` type instead
 

tests/ui/significant_drop_in_scrutinee.rs

@@ -101,7 +101,7 @@ struct MutexGuardWrapper<'a> {
 }
 
 impl<'a> MutexGuardWrapper<'a> {
-    fn get_the_value(&self) -> u64 {
+    fn get(&self) -> u64 {
         *self.mg.deref()
     }
 }
@@ -111,7 +111,7 @@ struct MutexGuardWrapperWrapper<'a> {
 }
 
 impl<'a> MutexGuardWrapperWrapper<'a> {
-    fn get_the_value(&self) -> u64 {
+    fn get(&self) -> u64 {
         *self.mg.mg.deref()
     }
 }
@@ -144,15 +144,15 @@ fn should_trigger_lint_with_wrapped_mutex() {
     // Should trigger lint because a temporary contains a type with a significant drop and its
     // lifetime is not obvious. Additionally, it is not obvious from looking at the scrutinee that
     // the temporary contains such a type, making it potentially even more surprising.
-    match s.lock_m().get_the_value() {
+    match s.lock_m().get() {
         //~^ significant_drop_in_scrutinee
         1 => {
             println!("Got 1. Is it still 1?");
-            println!("{}", s.lock_m().get_the_value());
+            println!("{}", s.lock_m().get());
         },
         2 => {
             println!("Got 2. Is it still 2?");
-            println!("{}", s.lock_m().get_the_value());
+            println!("{}", s.lock_m().get());
         },
         _ => {},
     }
@@ -166,15 +166,15 @@ fn should_trigger_lint_with_double_wrapped_mutex() {
     // significant drop and its lifetime is not obvious. Additionally, it is not obvious from
     // looking at the scrutinee that the temporary contains such a type, making it potentially even
     // more surprising.
-    match s.lock_m_m().get_the_value() {
+    match s.lock_m_m().get() {
         //~^ significant_drop_in_scrutinee
         1 => {
             println!("Got 1. Is it still 1?");
-            println!("{}", s.lock_m().get_the_value());
+            println!("{}", s.lock_m().get());
         },
         2 => {
             println!("Got 2. Is it still 2?");
-            println!("{}", s.lock_m().get_the_value());
+            println!("{}", s.lock_m().get());
         },
         _ => {},
     }