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

new lint: use_crate_prefix_for_self_imports #13662

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lengyijun
Copy link
Contributor

@lengyijun lengyijun commented Nov 7, 2024

changelog: [use_crate_prefix_for_self_imports]: new lint

fix #13645

Only check main.rs and lib.rs

Known problem

Duplicate warning

TODO

  • rm tests/ui/use_crate_prefix_for_self_imports.rs
  • squash commits

Question

Although pass all tests, but if copy lintcheck to /tmp, clippy will report two warnings (expected)

warning: this import is not clear
  --> src/main.rs:42:5
   |
42 | use input::read_crates;
   |     ^^^^^ help: prefix with `crate::`: `crate::input`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#use_crate_prefix_for_self_imports
   = note: `#[warn(clippy::use_crate_prefix_for_self_imports)]` on by default

warning: this import is not clear
  --> src/main.rs:43:5
   |
43 | use output::{ClippyCheckOutput, ClippyWarning, RustcIce};
   |     ^^^^^^ help: prefix with `crate::`: `crate::output`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#use_crate_prefix_for_self_imports

@rustbot
Copy link
Collaborator

rustbot commented Nov 7, 2024

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 7, 2024
@lengyijun lengyijun force-pushed the last_use_crate_prefix_for_self_imports branch from 2a70749 to bfc0849 Compare November 7, 2024 11:40
@lengyijun lengyijun changed the title use crate prefix for self imports new lint use_crate_prefix_for_self_imports Nov 7, 2024
@lengyijun lengyijun changed the title new lint use_crate_prefix_for_self_imports new lint: use_crate_prefix_for_self_imports Nov 7, 2024
@lengyijun lengyijun force-pushed the last_use_crate_prefix_for_self_imports branch 5 times, most recently from f74a6bc to 6a0c669 Compare November 8, 2024 04:46
/// ```
#[clippy::version = "1.84.0"]
pub USE_CRATE_PREFIX_FOR_SELF_IMPORTS,
style,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I understand that clearing up possible path confusion is a worthy cause, I'm afraid that introducing the lint as warn-by-default will lead to a lot of noise and broken CI jobs.

I'll discuss this on zulip.

@lengyijun lengyijun force-pushed the last_use_crate_prefix_for_self_imports branch from 6a0c669 to 7633f16 Compare November 13, 2024 03:26
@llogiq
Copy link
Contributor

llogiq commented Jan 10, 2025

So, as you know we discussed this on zulip a while ago. The upshot is that we have two acceptable options:

  • Put the lint in the nursery group and improve it until we deem it ready for style
  • Add more checks to the lint to reduce the number of warnings that aren't too helpful. For example, if we have the respective mod near the use (where near could be measured in line number differences or in no other items but mods and uses in between), we could avoid linting, as the reader of the code will know that the item comes from the mod specified nearby (this could be done by walking the sibling items until a non-mod-or-use item is found).

Do you intend to continue working on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use_crate_prefix_for_self_imports
3 participants