Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A new lint for imports ending in ::{self}; #6552

Closed
wooster0 opened this issue Jan 5, 2021 · 8 comments · Fixed by #7072
Closed

A new lint for imports ending in ::{self}; #6552

wooster0 opened this issue Jan 5, 2021 · 8 comments · Fixed by #7072
Assignees
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-style Lint: Belongs in the style lint group

Comments

@wooster0
Copy link

wooster0 commented Jan 5, 2021

What it does

It detects imports that end in ::{self}; and emits a warning for them.

Categories (optional)

  • Kind: Perhaps clippy::style.

The advantage of the recommended code over the original code is that it's shorter and simpler.

Drawbacks

None.

Example

use std::io::{self};

Could be written as:

use std::io;
@wooster0 wooster0 added the A-lint Area: New lints label Jan 5, 2021
@camsteffen
Copy link
Contributor

I think this is a rustfmt bug. rustfmt does the following:

  • use std::io::{Write}; -> use std::io::Write;
  • use std::io::self; -> use std::io;
  • use std::io::{self}; -> use std::io::{self}; 😲😲😲

@camsteffen
Copy link
Contributor

I take it back. See rust-lang/rustfmt#3568. For this lint, clippy would need to check if changing the import will break it.

@camsteffen camsteffen added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-style Lint: Belongs in the style lint group labels Feb 1, 2021
@ebobrow
Copy link
Contributor

ebobrow commented Apr 5, 2021

This looks interesting! I'd like to give it a try.

@rustbot claim

@ebobrow
Copy link
Contributor

ebobrow commented Apr 10, 2021

I'm just about done with this, but I've run into an issue. As far as I can tell, there's no way to see what items are exported from an external crate, meaning that I can't check if removing ::{self} from an import would bring in anything else with the same name. I could skip that check for imports from external crates and risk false positives or negatives. Or, am I wrong and there is a way to check?

@camsteffen
Copy link
Contributor

For use a::b::{self}, I think you should be able to do tcx.item_children(b_def_id).

@ebobrow
Copy link
Contributor

ebobrow commented Apr 11, 2021

Thank you! That works for external crates, but checking a local mod gives:

error: internal compiler error: compiler/rustc_middle/src/ty/query/mod.rs:282:1: `tcx.item_children(DefId(0:3 ~ unnecessary_self_imports[317d]::a))` unsupported by its crate; perhaps the `item_children` query was never assigned a provider function

thread 'rustc' panicked at 'Box<Any>', /rustc/07e0e2ec268c140e607e1ac7f49f145612d0f597/library/std/src/panic.rs:59:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Do you know what that is? If it isn't fixable I can use tcx.hir().get_if_local() for local mods, but I would prefer to have the same check for all imports, if possible.

@camsteffen
Copy link
Contributor

Hmm, looks like item_children is an "extern" query (codez).

I can use tcx.hir().get_if_local() for local mods

Seems like a good solution.

@ebobrow
Copy link
Contributor

ebobrow commented Apr 13, 2021

Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-style Lint: Belongs in the style lint group
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants