Skip to content

Commit

Permalink
Auto merge of #3679 - daxpedda:use_self, r=phansch
Browse files Browse the repository at this point in the history
Fix automatic suggestion on `use_self`.

In an example like this:
```rust
impl Example {
    fn fun_1() { }
    fn fun_2() {
        Example::fun_1();
    }
}
```
Clippy tries to replace `Example::fun_1` with `Self`, loosing `::fun_1` in the process, it should rather try to replace `Example` with `Self`.

**Question**
- There may be other paths that need the same treatment, but I'm not sure I understand them fully:
  - https://github.com/rust-lang/rust-clippy/blob/e648adf0866a1cea7db6ce2d33ea86e442f25377/clippy_lints/src/use_self.rs#L94-L96
  - https://github.com/rust-lang/rust-clippy/blob/e648adf0866a1cea7db6ce2d33ea86e442f25377/clippy_lints/src/use_self.rs#L225-L229
  • Loading branch information
bors committed Jan 22, 2019
2 parents 280069d + 42d5a07 commit a40d8e4
Show file tree
Hide file tree
Showing 4 changed files with 376 additions and 31 deletions.
7 changes: 6 additions & 1 deletion clippy_lints/src/use_self.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,15 @@ impl LintPass for UseSelf {
const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";

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;
// only take path up to the end of last_path_span
let span = path.span.with_hi(last_path_span.hi());

span_lint_and_sugg(
cx,
USE_SELF,
path.span,
span,
"unnecessary structure name repetition",
"use the applicable keyword",
"Self".to_owned(),
Expand Down
299 changes: 299 additions & 0 deletions tests/ui/use_self.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,299 @@
// run-rustfix

#![warn(clippy::use_self)]
#![allow(dead_code)]
#![allow(clippy::should_implement_trait)]

fn main() {}

mod use_self {
struct Foo {}

impl Foo {
fn new() -> Self {
Self {}
}
fn test() -> Self {
Self::new()
}
}

impl Default for Foo {
fn default() -> Self {
Self::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()
}
}
}

mod lifetimes {
struct Foo<'a> {
foo_str: &'a str,
}

impl<'a> Foo<'a> {
// Cannot use `Self` as return type, because the function is actually `fn foo<'b>(s: &'b str) ->
// Foo<'b>`
fn foo(s: &str) -> Foo {
Foo { foo_str: s }
}
// cannot replace with `Self`, because that's `Foo<'a>`
fn bar() -> Foo<'static> {
Foo { foo_str: "foo" }
}

// FIXME: the lint does not handle lifetimed struct
// `Self` should be applicable here
fn clone(&self) -> Foo<'a> {
Foo { foo_str: self.foo_str }
}
}
}

#[allow(clippy::boxed_local)]
mod traits {

use std::ops::Mul;

trait SelfTrait {
fn refs(p1: &Self) -> &Self;
fn ref_refs<'a>(p1: &'a &'a Self) -> &'a &'a Self;
fn mut_refs(p1: &mut Self) -> &mut Self;
fn nested(p1: Box<Self>, p2: (&u8, &Self));
fn vals(r: Self) -> Self;
}

#[derive(Default)]
struct Bad;

impl SelfTrait for Bad {
fn refs(p1: &Self) -> &Self {
p1
}

fn ref_refs<'a>(p1: &'a &'a Self) -> &'a &'a Self {
p1
}

fn mut_refs(p1: &mut Self) -> &mut Self {
p1
}

fn nested(_p1: Box<Self>, _p2: (&u8, &Self)) {}

fn vals(_: Self) -> Self {
Self::default()
}
}

impl Mul for Bad {
type Output = Self;

fn mul(self, rhs: Self) -> Self {
rhs
}
}

#[derive(Default)]
struct Good;

impl SelfTrait for Good {
fn refs(p1: &Self) -> &Self {
p1
}

fn ref_refs<'a>(p1: &'a &'a Self) -> &'a &'a Self {
p1
}

fn mut_refs(p1: &mut Self) -> &mut Self {
p1
}

fn nested(_p1: Box<Self>, _p2: (&u8, &Self)) {}

fn vals(_: Self) -> Self {
Self::default()
}
}

impl Mul for Good {
type Output = Self;

fn mul(self, rhs: Self) -> Self {
rhs
}
}

trait NameTrait {
fn refs(p1: &u8) -> &u8;
fn ref_refs<'a>(p1: &'a &'a u8) -> &'a &'a u8;
fn mut_refs(p1: &mut u8) -> &mut u8;
fn nested(p1: Box<u8>, p2: (&u8, &u8));
fn vals(p1: u8) -> u8;
}

// Using `Self` instead of the type name is OK
impl NameTrait for u8 {
fn refs(p1: &Self) -> &Self {
p1
}

fn ref_refs<'a>(p1: &'a &'a Self) -> &'a &'a Self {
p1
}

fn mut_refs(p1: &mut Self) -> &mut Self {
p1
}

fn nested(_p1: Box<Self>, _p2: (&Self, &Self)) {}

fn vals(_: Self) -> Self {
Self::default()
}
}

// Check that self arg isn't linted
impl Clone for Good {
fn clone(&self) -> Self {
// Note: Not linted and it wouldn't be valid
// because "can't use `Self` as a constructor`"
Good
}
}
}

mod issue2894 {
trait IntoBytes {
fn into_bytes(&self) -> Vec<u8>;
}

// This should not be linted
impl IntoBytes for u8 {
fn into_bytes(&self) -> Vec<u8> {
vec![*self]
}
}
}

mod existential {
struct Foo;

impl Foo {
fn bad(foos: &[Self]) -> impl Iterator<Item = &Self> {
foos.iter()
}

fn good(foos: &[Self]) -> impl Iterator<Item = &Self> {
foos.iter()
}
}
}

mod tuple_structs {
pub struct TS(i32);

impl TS {
pub fn ts() -> Self {
Self(0)
}
}
}

mod macros {
macro_rules! use_self_expand {
() => {
fn new() -> Self {
Self {}
}
};
}

struct Foo {}

impl Foo {
use_self_expand!(); // Should lint in local macros
}
}

mod nesting {
struct Foo {}
impl Foo {
fn foo() {
use self::Foo; // Can't use Self here
struct Bar {
foo: Foo, // Foo != Self
}

impl Bar {
fn bar() -> Self {
Self { foo: Foo {} }
}
}
}
}

enum Enum {
A,
}
impl Enum {
fn method() {
#[allow(unused_imports)]
use self::Enum::*; // Issue 3425
static STATIC: Enum = Enum::A; // Can't use Self as type
}
}
}

mod issue3410 {

struct A;
struct B;

trait Trait<T> {
fn a(v: T);
}

impl Trait<Vec<A>> for Vec<B> {
fn a(_: Vec<A>) {}
}
}

#[allow(clippy::no_effect, path_statements)]
mod rustfix {
mod nested {
pub struct A {}
}

impl nested::A {
const A: bool = true;

fn fun_1() {}

fn fun_2() {
Self::fun_1();
Self::A;

Self {};
}
}
}
23 changes: 23 additions & 0 deletions tests/ui/use_self.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// run-rustfix

#![warn(clippy::use_self)]
#![allow(dead_code)]
#![allow(clippy::should_implement_trait)]
Expand Down Expand Up @@ -255,6 +257,7 @@ mod nesting {
}
impl Enum {
fn method() {
#[allow(unused_imports)]
use self::Enum::*; // Issue 3425
static STATIC: Enum = Enum::A; // Can't use Self as type
}
Expand All @@ -274,3 +277,23 @@ mod issue3410 {
fn a(_: Vec<A>) {}
}
}

#[allow(clippy::no_effect, path_statements)]
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 {};
}
}
}
Loading

0 comments on commit a40d8e4

Please sign in to comment.