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

std_instead_of_core: false positive with std::env since nightly-2024-03-08 #12438

Closed
taiki-e opened this issue Mar 8, 2024 · 4 comments · Fixed by #12447
Closed

std_instead_of_core: false positive with std::env since nightly-2024-03-08 #12438

taiki-e opened this issue Mar 8, 2024 · 4 comments · Fixed by #12447
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@taiki-e
Copy link
Member

taiki-e commented Mar 8, 2024

Summary

Since nightly-2024-03-08, it seems that std_instead_of_core is not correctly handling cases where macros and modules have the same name.

Probably related to #12406, cc @MarcusGrass

Lint Name

std_instead_of_core

Reproducer

I tried this code:

#![allow(dead_code)]
#![warn(clippy::std_instead_of_core)]

use std::env;

fn f() -> Option<String> {
    env::var("FOO").ok()
}

I saw this happen:

warning: used import from `std` instead of `core`
 --> src/lib.rs:4:5
  |
4 | use std::env;
  |     ^^^ help: consider importing the item from `core`: `core`
  |

However, replacing std::env with core::env causes compile error:

#![allow(dead_code)]
#![warn(clippy::std_instead_of_core)]

use core::env;

fn f() -> Option<String> {
    env::var("FOO").ok()
}
error[E0433]: failed to resolve: use of undeclared crate or module `env`
 --> src/lib.rs:7:5
  |
7 |     env::var("FOO").ok()
  |     ^^^ use of undeclared crate or module `env`
  |
help: consider importing this module
  |
4 + use std::env;
  |

I expected to see this happen: no warning

Version

rustc 1.78.0-nightly (9c3ad802d 2024-03-07)
binary: rustc
commit-hash: 9c3ad802d9b9633d60d3a74668eb1be819212d34
commit-date: 2024-03-07
host: aarch64-apple-darwin
release: 1.78.0-nightly
LLVM version: 18.1.0

Additional Labels

@rustbot label +I-suggestion-causes-error

@taiki-e taiki-e 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 Mar 8, 2024
@rustbot rustbot added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Mar 8, 2024
taiki-e added a commit to taiki-e/find-crate that referenced this issue Mar 8, 2024
rust-lang/rust-clippy#12438

```
error: used import from `std` instead of `core`
   --> src/lib.rs:134:5
    |
134 | use std::{
    |     ^^^ help: consider importing the item from `core`: `core`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_core
    = note: `-D clippy::std-instead-of-core` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::std_instead_of_core)]`
```
@MarcusGrass
Copy link
Contributor

Yeah it's definitely caused by that, I didn't see it until now since I updated the expected output file, and didn't notice the erroneous fix from std::env to core::env it's pretty easily revertible without much hassle, but I'll have a look tomorrow to see if there's a reasonable fix, thanks for the report!

@MarcusGrass
Copy link
Contributor

Yep, pretty stupid mistake, got a PR with the fix here #12447 preserves the bugfix that it was supposed to achieve.

bors added a commit that referenced this issue Mar 9, 2024
Fix #12438 std_instead_of_core regression

Fixes #12438.

Boy-scouting removed two paths that checks for duplication since I thought they were unused. However, that's just because I didn't spot it in the diff.

I installed [difftastic](https://github.com/Wilfred/difftastic) and ran it on the old one:

![image](https://github.com/rust-lang/rust-clippy/assets/34198073/5c51276c-055a-49a3-9425-6f7da0590fb0)

And the new one (fixed):

![image](https://github.com/rust-lang/rust-clippy/assets/34198073/6e10f29c-6d6b-4f64-893f-de526424f1cd)

New one (stderr):
![image](https://github.com/rust-lang/rust-clippy/assets/34198073/c4c07776-ee0f-47ba-996f-6b632de47c81)

Good teachings for the future when inspecting diffs with a lot of line changes, should've thought of that before, sorry for the trouble!

changelog: [`std_instead_of_core`] Fix false positive for crates that are in `std` but not `core`
@bors bors closed this as completed in b44ab66 Mar 9, 2024
@danwilliams
Copy link

@MarcusGrass @bors Just to note, I ran into this issue just now, after updating to 1.78.0 stable. I checked Git and it seems that although this fix was merged to master, it didn't make it into the 1.78.0 tag. I was going to open a ticket, but when searching I found this one...

danwilliams added a commit to danwilliams/rubedo that referenced this issue May 3, 2024
  - Disabled the clippy::std_instead_of_core lint for the std module due
    to a false positive on std::env, which is due to the following bug:

    rust-lang/rust-clippy#12438

    The fix had to be applied globally otherwise it triggered a "useless
    lint attribute" warning, which may be another bug!

    rust-lang/rust-clippy#1938
@MarcusGrass
Copy link
Contributor

@MarcusGrass @bors Just to note, I ran into this issue just now, after updating to 1.78.0 stable. I checked Git and it seems that although this fix was merged to master, it didn't make it into the 1.78.0 tag. I was going to open a ticket, but when searching I found this one...

Oh no that was a real miss, I'm fairly new to this project and didn't realize that the fix might miss the merge window, I'm sorry about that

danwilliams added a commit to danwilliams/rubedo that referenced this issue Sep 6, 2024
  - Removed the clippy::std_instead_of_core lint workaround for the
    false positive on std::env, which was due to the following bug (now
    fixed):

    rust-lang/rust-clippy#12438
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 I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants