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

FP single_component_path_imports #7168

Open
djc opened this issue May 5, 2021 · 6 comments
Open

FP single_component_path_imports #7168

djc opened this issue May 5, 2021 · 6 comments
Labels
C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@djc
Copy link
Contributor

djc commented May 5, 2021

Lint name:

single_component_path_imports

I tried this code:

#[cfg(feature = "log")]
use log;

mod foo {
    use crate::log::warn;

    pub fn bar() {
        warn!("bar");
    }
}

pub fn bar() {
    foo::bar()
}

(I think it doesn't matter whether the feature log is turned on or off.)

I expected to see this happen: no warning, since the log import is used in an inner module.

Instead, this happened:

warning: this import is redundant
 --> src/lib.rs:3:1
  |
3 | use log;
  | ^^^^^^^^ help: remove it entirely
  |
  = note: `#[warn(clippy::single_component_path_imports)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports

Meta

  • cargo clippy -V: clippy 0.1.53 (42816d6 2021-04-24)
  • rustc -Vv:
    rustc 1.53.0-nightly (42816d61e 2021-04-24)
    binary: rustc
    commit-hash: 42816d61ead7e46d462df997958ccfd514f8c21c
    commit-date: 2021-04-24
    host: x86_64-apple-darwin
    release: 1.53.0-nightly
    LLVM version: 12.0.0
    

cc @ThibsG (re #6905 (comment))

@djc djc added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels May 5, 2021
@camsteffen
Copy link
Contributor

So you want the log crate to be accessible at crate::log?

@djc
Copy link
Contributor Author

djc commented May 5, 2021

It is accessible as crate::log, and my code relies on that.

@camsteffen
Copy link
Contributor

Yes, obviously the suggestion is not okay as is. I'm just asking from a code style viewpoint. Because this questions the assertion that "single component imports are always useless/bad style" which seems to be the idea behind the lint.

@djc
Copy link
Contributor Author

djc commented May 5, 2021

From a code style viewpoint I think it's a great lint, I've applied many suggestions due to this lint. In this case the use log is conditional on a feature and gets switched out for a mock implementation of the log warnings, so it's definitely more of a special case. In general if the identifier gets used as a crate-rooted item, or maybe even in general through a path, the lint should probably be suppressed.

@camsteffen
Copy link
Contributor

In general if the identifier gets used as a crate-rooted item, or maybe even in general through a path, the lint should probably be suppressed.

Yep that sounds reasonable. Hopefully it isn't too gnarly to implement. Scanning usages can be problematic. But I think this might be a relatively simple case.

@camsteffen camsteffen added the E-medium Call for participation: Medium difficulty level problem and requires some initial experience. label May 5, 2021
@camsteffen
Copy link
Contributor

We should also detect usages like super::log which means it doesn't need to be crate-rooted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants