diff --git a/CHANGELOG.md b/CHANGELOG.md index efa637b185cd..60063190fadd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -698,6 +698,7 @@ All notable changes to this project will be documented in this file. [`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy [`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap +[`getter_prefix`]: https://rust-lang.github.io/rust-clippy/master/index.html#getter_prefix [`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 [`if_let_redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_redundant_pattern_matching diff --git a/README.md b/README.md index be54424dc381..8ca10da416dd 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 290 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2e515cc8aea6..c558665d7676 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -159,6 +159,7 @@ pub mod multiple_crate_versions; pub mod mut_mut; pub mod mut_reference; pub mod mutex_atomic; +pub mod naming; pub mod needless_bool; pub mod needless_borrow; pub mod needless_borrowed_ref; @@ -486,6 +487,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box ptr_offset_with_cast::Pass); reg.register_late_lint_pass(box redundant_clone::RedundantClone); reg.register_late_lint_pass(box slow_vector_initialization::Pass); + reg.register_early_lint_pass(box naming::GetterPrefix); reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![ arithmetic::FLOAT_ARITHMETIC, @@ -697,6 +699,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { misc_early::ZERO_PREFIXED_LITERAL, mut_reference::UNNECESSARY_MUT_PASSED, mutex_atomic::MUTEX_ATOMIC, + naming::GETTER_PREFIX, needless_bool::BOOL_COMPARISON, needless_bool::NEEDLESS_BOOL, needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE, @@ -837,6 +840,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { misc_early::MIXED_CASE_HEX_LITERALS, misc_early::UNNEEDED_FIELD_PATTERN, mut_reference::UNNECESSARY_MUT_PASSED, + naming::GETTER_PREFIX, neg_multiply::NEG_MULTIPLY, new_without_default::NEW_WITHOUT_DEFAULT, non_expressive_names::JUST_UNDERSCORES_AND_DIGITS, diff --git a/clippy_lints/src/naming.rs b/clippy_lints/src/naming.rs new file mode 100644 index 000000000000..21299b057e11 --- /dev/null +++ b/clippy_lints/src/naming.rs @@ -0,0 +1,95 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use crate::rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass}; +use crate::rustc::{declare_tool_lint, lint_array}; +use crate::syntax::ast; +use crate::utils::span_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 { +/// .. +/// } +/// } +/// ``` +declare_clippy_lint! { + pub GETTER_PREFIX, + style, + "prefixing a getter with `get_`, which does not follow convention" +} + +#[derive(Copy, Clone)] +pub struct GetterPrefix; + +#[rustfmt::skip] +const ALLOWED_METHOD_NAMES: [&'static str; 5] = [ + "get", + "get_mut", + "get_unchecked", + "get_unchecked_mut", + "get_ref" +]; + +impl LintPass for GetterPrefix { + fn get_lints(&self) -> LintArray { + lint_array!(GETTER_PREFIX) + } +} + +impl EarlyLintPass for GetterPrefix { + fn check_impl_item(&mut self, cx: &EarlyContext<'_>, implitem: &ast::ImplItem) { + if let ast::ImplItemKind::Method(..) = implitem.node { + let name = implitem.ident.name.as_str().get(); + if name.starts_with("get_") && !ALLOWED_METHOD_NAMES.contains(&name) { + span_lint( + cx, + GETTER_PREFIX, + implitem.span, + "prefixing a getter with `get_` does not follow naming conventions", + ); + } + } + } +} diff --git a/tests/ui/naming.rs b/tests/ui/naming.rs new file mode 100644 index 000000000000..6f924685cf2b --- /dev/null +++ b/tests/ui/naming.rs @@ -0,0 +1,48 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +pub struct MyStruct { + id: usize +} + +impl MyStruct { + pub fn get_id(&self) -> usize { + self.id + } + + 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 + } +} + +fn main() { + let mut s = MyStruct { id: 42 }; + s.get_id(); + s.get(); + s.get_mut(); + s.get_unchecked(); + s.get_unchecked_mut(); + s.get_ref(); +} diff --git a/tests/ui/naming.stderr b/tests/ui/naming.stderr new file mode 100644 index 000000000000..34b00eecf0ee --- /dev/null +++ b/tests/ui/naming.stderr @@ -0,0 +1,12 @@ +error: prefixing a getter with `get_` does not follow naming conventions + --> $DIR/naming.rs:15:5 + | +LL | / pub fn get_id(&self) -> usize { +LL | | self.id +LL | | } + | |_____^ + | + = note: `-D clippy::getter-prefix` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/unused_unit.rs b/tests/ui/unused_unit.rs index 88f0b9687be7..d8f84d9d397b 100644 --- a/tests/ui/unused_unit.rs +++ b/tests/ui/unused_unit.rs @@ -22,7 +22,7 @@ struct Unitter; impl Unitter { // try to disorient the lint with multiple unit returns and newlines - pub fn get_unit (), G>(&self, f: F, _g: G) -> + pub fn get (), G>(&self, f: F, _g: G) -> () where G: Fn() -> () { let _y: &Fn() -> () = &f; @@ -41,7 +41,7 @@ fn return_unit() -> () { () } fn main() { let u = Unitter; - assert_eq!(u.get_unit(|| {}, return_unit), u.into()); + assert_eq!(u.get(|| {}, return_unit), u.into()); return_unit(); loop { break(); diff --git a/tests/ui/unused_unit.stderr b/tests/ui/unused_unit.stderr index ac76d75b1760..b04417bbbfd7 100644 --- a/tests/ui/unused_unit.stderr +++ b/tests/ui/unused_unit.stderr @@ -1,8 +1,8 @@ error: unneeded unit return type - --> $DIR/unused_unit.rs:25:59 + --> $DIR/unused_unit.rs:25:54 | -LL | pub fn get_unit (), G>(&self, f: F, _g: G) -> - | ___________________________________________________________^ +LL | pub fn get (), G>(&self, f: F, _g: G) -> + | ______________________________________________________^ LL | | () | |__________^ help: remove the `-> ()` |