From 2e0977f3b4061ed626fe9bfe29c703d13346abc1 Mon Sep 17 00:00:00 2001 From: daxpedda Date: Mon, 21 Jan 2019 13:06:32 +0100 Subject: [PATCH] Fixed potential mistakes with nesting. Added tests. --- clippy_lints/src/use_self.rs | 16 +++++++++++----- tests/ui/use_self.rs | 20 ++++++++++++++++++++ tests/ui/use_self.stderr | 20 +++++++++++++++++++- 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 696f87854..b57847d8f 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -7,7 +7,7 @@ use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintC use rustc::ty; use rustc::{declare_tool_lint, lint_array}; use rustc_errors::Applicability; -use syntax_pos::{symbol::keywords::SelfUpper, Span}; +use syntax_pos::symbol::keywords::SelfUpper; /// **What it does:** Checks for unnecessary repetition of structure name when a /// replacement with `Self` is applicable. @@ -55,7 +55,13 @@ impl LintPass for UseSelf { const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element"; -fn span_use_self_lint(cx: &LateContext<'_, '_>, span: Span) { +fn span_use_self_lint(cx: &LateContext<'_, '_>, path: &Path) { + // path segments only include actual path, no methods or fields + let last_path_span = path.segments.last().expect(SEGMENTS_MSG).ident.span; + // `to()` doesn't shorten span, so we shorten it with `until(..)` + // and then include it with `to(..)` + let span = path.span.until(last_path_span).to(last_path_span); + span_lint_and_sugg( cx, USE_SELF, @@ -92,7 +98,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> { }; if !is_self_ty { - span_use_self_lint(self.cx, path.span); + span_use_self_lint(self.cx, path); } } } @@ -221,10 +227,10 @@ impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> { fn visit_path(&mut self, path: &'tcx Path, _id: HirId) { if path.segments.last().expect(SEGMENTS_MSG).ident.name != SelfUpper.name() { if self.item_path.def == path.def { - span_use_self_lint(self.cx, path.segments.first().expect(SEGMENTS_MSG).ident.span); + span_use_self_lint(self.cx, path); } else if let Def::StructCtor(ctor_did, CtorKind::Fn) = path.def { if self.item_path.def.opt_def_id() == self.cx.tcx.parent_def_id(ctor_did) { - span_use_self_lint(self.cx, path.span); + span_use_self_lint(self.cx, path); } } } diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs index b839aead9..0cf406b18 100644 --- a/tests/ui/use_self.rs +++ b/tests/ui/use_self.rs @@ -274,3 +274,23 @@ mod issue3410 { fn a(_: Vec) {} } } + +#[allow(clippy::no_effect)] +mod rustfix { + mod nested { + pub struct A {} + } + + impl nested::A { + const A: bool = true; + + fn fun_1() {} + + fn fun_2() { + nested::A::fun_1(); + nested::A::A; + + nested::A {}; + } + } +} diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr index 649fcdbff..68ce7221d 100644 --- a/tests/ui/use_self.stderr +++ b/tests/ui/use_self.stderr @@ -162,5 +162,23 @@ error: unnecessary structure name repetition LL | Bar { foo: Foo {} } | ^^^ help: use the applicable keyword: `Self` -error: aborting due to 26 previous errors +error: unnecessary structure name repetition + --> $DIR/use_self.rs:290:13 + | +LL | nested::A::fun_1(); + | ^^^^^^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> $DIR/use_self.rs:291:13 + | +LL | nested::A::A; + | ^^^^^^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> $DIR/use_self.rs:293:13 + | +LL | nested::A {}; + | ^^^^^^^^^ help: use the applicable keyword: `Self` + +error: aborting due to 29 previous errors