From 7759bd61119580cf13d85e6a38ea4254244e84e8 Mon Sep 17 00:00:00 2001 From: Tim Nielens Date: Fri, 28 Jul 2017 13:28:07 +0200 Subject: [PATCH] lint #1674: replace struct name with `Self` when applicable --- clippy_lints/src/lib.rs | 2 + clippy_lints/src/use_self.rs | 90 ++++++++++++++++++++++++++++++++++++ tests/ui/use_self.rs | 45 ++++++++++++++++++ tests/ui/use_self.stderr | 40 ++++++++++++++++ 4 files changed, 177 insertions(+) create mode 100644 clippy_lints/src/use_self.rs create mode 100644 tests/ui/use_self.rs create mode 100644 tests/ui/use_self.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 18759fb0a91..a8f1b1664b0 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -148,6 +148,7 @@ pub mod unicode; pub mod unsafe_removed_from_name; pub mod unused_io_amount; pub mod unused_label; +pub mod use_self; pub mod vec; pub mod zero_div_zero; // end lints modules, do not remove this comment, it’s used in `update_lints` @@ -319,6 +320,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box should_assert_eq::ShouldAssertEq); reg.register_late_lint_pass(box needless_pass_by_value::NeedlessPassByValue); reg.register_early_lint_pass(box literal_digit_grouping::LiteralDigitGrouping); + reg.register_late_lint_pass(box use_self::UseSelf); reg.register_lint_group("clippy_restrictions", vec![ arithmetic::FLOAT_ARITHMETIC, diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs new file mode 100644 index 00000000000..6344c25b084 --- /dev/null +++ b/clippy_lints/src/use_self.rs @@ -0,0 +1,90 @@ +use rustc::lint::{LintArray, LateLintPass, LateContext, LintPass}; +use rustc::hir::*; +use rustc::hir::intravisit::{Visitor, walk_path, NestedVisitorMap}; +use utils::span_lint; +use syntax::ast::NodeId; + +/// **What it does:** Checks for unnecessary repetition of structure name when a +/// replacement with `Self` is applicable. +/// +/// **Why is this bad?** Unnecessary repetition. Mixed use of `Self` and struct name +/// feels inconsistent. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// struct Foo {} +/// impl Foo { +/// fn new() -> Foo { +/// Foo {} +/// } +/// } +/// ``` +/// could be +/// ``` +/// struct Foo {} +/// impl Foo { +/// fn new() -> Self { +/// Self {} +/// } +/// } +/// ``` +declare_lint! { + pub USE_SELF, + Allow, + "Repetitive struct name usage whereas `Self` is applicable" +} + +#[derive(Copy, Clone, Default)] +pub struct UseSelf; + +impl LintPass for UseSelf { + fn get_lints(&self) -> LintArray { + lint_array!(USE_SELF) + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf { + fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) { + if_let_chain!([ + let ItemImpl(.., ref item_type, ref refs) = item.node, + let Ty_::TyPath(QPath::Resolved(_, ref item_path)) = item_type.node, + ], { + let visitor = &mut UseSelfVisitor { + item_path: item_path, + cx: cx, + }; + for impl_item_ref in refs { + visitor.visit_impl_item(cx.tcx.hir.impl_item(impl_item_ref.id)); + } + }) + } +} + +struct UseSelfVisitor<'a, 'tcx: 'a> { + item_path: &'a Path, + cx: &'a LateContext<'a, 'tcx>, +} + +impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> { + fn visit_path(&mut self, path: &'tcx Path, _id: NodeId) { + if self.item_path.def == path.def && + path.segments + .last() + .expect("segments should be composed of at least 1 elemnt") + .name + .as_str() != "Self" { + span_lint(self.cx, + USE_SELF, + path.span, + "repetitive struct name usage. Use `Self` instead."); + } + + walk_path(self, path); + } + + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { + NestedVisitorMap::OnlyBodies(&self.cx.tcx.hir) + } +} diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs new file mode 100644 index 00000000000..40cef1a9362 --- /dev/null +++ b/tests/ui/use_self.rs @@ -0,0 +1,45 @@ +#![feature(plugin)] +#![plugin(clippy)] +#![warn(use_self)] +#![allow(dead_code)] + + +fn main() {} + +mod use_self { + struct Foo {} + + impl Foo { + fn new() -> Foo { + Foo {} + } + fn test() -> Foo { + Foo::new() + } + } + + impl Default for Foo { + fn default() -> Foo { + Foo::new() + } + } +} + +mod better { + struct Foo {} + + impl Foo { + fn new() -> Self { + Self {} + } + fn test() -> Self { + Self::new() + } + } + + impl Default for Foo { + fn default() -> Self { + Self::new() + } + } +} diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr new file mode 100644 index 00000000000..1529978f2b3 --- /dev/null +++ b/tests/ui/use_self.stderr @@ -0,0 +1,40 @@ +error: repetitive struct name usage. Use `Self` instead. + --> $DIR/use_self.rs:13:21 + | +13 | fn new() -> Foo { + | ^^^ + | + = note: `-D use-self` implied by `-D warnings` + +error: repetitive struct name usage. Use `Self` instead. + --> $DIR/use_self.rs:14:13 + | +14 | Foo {} + | ^^^ + +error: repetitive struct name usage. Use `Self` instead. + --> $DIR/use_self.rs:16:22 + | +16 | fn test() -> Foo { + | ^^^ + +error: repetitive struct name usage. Use `Self` instead. + --> $DIR/use_self.rs:17:13 + | +17 | Foo::new() + | ^^^^^^^^ + +error: repetitive struct name usage. Use `Self` instead. + --> $DIR/use_self.rs:22:25 + | +22 | fn default() -> Foo { + | ^^^ + +error: repetitive struct name usage. Use `Self` instead. + --> $DIR/use_self.rs:23:13 + | +23 | Foo::new() + | ^^^^^^^^ + +error: aborting due to 6 previous errors +