Rust-clippy: Warn on `use x as x;`

[jyn514]

What it does

Warns when you rename an import to the original name. This is useless; you can just use it directly.

Categories (optional)

• Kind: complexity or style maybe? It's definitely 'something simple but in a complex way', but I know a lot of people with allow(clippy::complexity) which I would prefer not happen.

What is the advantage of the recommended code over the original code

• It removes useless dead code.

Drawbacks

None.

Example

https://github.com/rust-lang/rust/blob/c3ed6681ff8d446e68ce272be4bf66f4145f6e29/compiler/rustc_data_structures/src/sync.rs#L224

use std::rc::Weak as Weak;

Could be written as:

use std::rc::Weak;

[giraffate]

I expected rustfmt to work, but for some reasons it doesn't seem to work.

[ebroto]

I expected rustfmt to work, but for some reasons it doesn't seem to work.

You are right, rustfmt seems to take care of this, at least in the playground it automatically simplifies it.

[jyn514]

Hmm, that's weird, it didn't work on rustc (it must not, since rustfmt is enforced by tidy). I guess this should be a rustfmt bug report, then?

[ebroto]

I guess this should be a rustfmt bug report, then?

I would say so, if the rustfmt.toml file in rustc didn't influence that behavior in some way

[jyn514]

Sounds good, thanks for the help debugging! I opened https://github.com/rust-lang/rustfmt/issues/4594 instead.

[criminosis]

I was tinkering on this today. Was going to be my first Rust PR but I hit a wall on it, curious if I could get some feedback if I was even on the right track?

impl LateLintPass<'_> for UselessImportRename {
    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
        if_chain! {
            if let ItemKind::Use(use_path, UseKind::Single) = &item.kind;
            if let Some(last_use_path_segment) = use_path.segments.last();
            then {
                let final_import_name = item.ident;
                let original_import_name = last_use_path_segment.ident;
                //this only catches the desired case of a rename to different name, but
                //does not differentiate between a repeat rename and no rename
                let is_same_identifier = final_import_name == original_import_name;

                //TODO This can't be the right way since we're not actually working on character indices
                // here given binary to UTF-8 isn't 1:1. How do I get a SourceMap?
                let BytePos(item_end) = item.span.data().hi;
                let BytePos(path_end) = use_path.span.data().hi;

                //To differentiate between a rename and no rename check to see if our span 
                //could include a rename. If there is no rename then we'd expect this to only
                //have space for the terminating ;
                let is_renamed = path_end + 1 != item_end; 


                if is_same_identifier && is_renamed {

                span_lint_and_help(
                    cx,
                    USELESS_IMPORT_RENAME,
                    item.span,
                    "import uselessly renamed to its original name",
                    None,
                    "don't rename the import just use it directly without the as clause"
                );
            }
            }
        }
    }
}

Still a novice at Rust though so I'm probably missing out on the most idiomatic path. If anyone would be willing to give some pointers I'd appreciate it to carry over to my next issue on here I try to tackle.

1. Was my usage of ident appropriate?
2. Was there an easy way to get CharacterPos that I was just being oblivious to?

[ebroto]

Hey @criminosis, thanks for your interest in improving Clippy! I'm sorry this was not actionable in Clippy in the end. I hope that at least it helped you to get acquainted with the code :)

In this particular case, I think it would have been simpler to use an early lint pass instead of a late one, since you don't need name resolution, type information, etc. here. Also, I'm not sure but I would say that using a late lint pass the information we need may not be there anymore!

Looking at how rustfmt seems to do it, if you use ast::UseTree, the prefix field is the path after use, and the first element of ast::UseTreeKind::Simple contains the ident after the as.

[criminosis]

Hey @ebroto ! Yeah I was thinking I didn't need to use late lint towards the end but didn't end up getting that far. I'll check out ast::UseTree for the future. Thanks!