Skip to content

fix: wildcard_imports ignore test.rs files #10584

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

Merged
merged 4 commits into from
May 8, 2023

Conversation

blyxyas
Copy link
Member

@blyxyas blyxyas commented Apr 1, 2023

Adds a check to see if the building crate is a test one, if so, ignore it


Closes #10580
changelog:[wildcard_imports]: Add a check to ignore files named test.rs and tests.rs

@rustbot
Copy link
Collaborator

rustbot commented Apr 1, 2023

r? @flip1995

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 1, 2023
@blyxyas blyxyas force-pushed the fix-wildcard_imports_testsrs branch 2 times, most recently from 04fb8b9 to 0aa9823 Compare April 2, 2023 00:05
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

I have doubts, if checking the file name is actually necessary.

@blyxyas
Copy link
Member Author

blyxyas commented Apr 5, 2023

Oh god I finally got it
Someone un Zulip mentioned those (lib), (bin), etc... labels when building a crate, so I thought that, if the building process knows what it's building, then the crate type must be stored somewhere. That's when I found that a this function in rust/rust_driver_impl uses an struct called Options from rustc_session::config. And Options has this very hand field test: bool. And you can access Options from Session, and Session from LateContext.

The illuminance came before me when I realize that now, I could check if a crate is a test. It's so beautiful.
I think we definitely need to add a function like is_test_crate(cx) to clippy_utils or something.

I've been at this like 8 hours now, and now... It's fixed... 😌 Peace is within me.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 9, 2023
…errors

Add little `is_test_crate` function

Ok, this is quite a story.
I'm mainly a Clippy contributor, so I was fixing [this Clippy issue](rust-lang/rust-clippy#10584) about a lint having to ignore test modules but that wasn't ignoring test files (integration test, `test/` dirs and such).

As test **files** don't tend to have an inner `#[cfg(test)]` module inside them, I tried everything, looking for filenames, looking for item's parents in the HIR Map, doing black magic...

I even asked [on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/.E2.9C.94.20Checking.20if.20file.20is.20integration.20test), and jyn answered something about `--cfg test`. Aha! That's something that I might be looking for, so I started looking at `rustc_driver_impl` flag parsing and configuration and all that.

Then, I stumbled on [this function right here](https://github.com/rust-lang/rust/blob/2e486be8d29d198d48bc26bfce5712a4822814f5/compiler/rustc_driver_impl/src/lib.rs#L174-L181), and noticed the argument `config: Config`. That's a hint.

So [Config](https://doc.rust-lang.org/beta/nightly-rustc/rustc_interface/interface/struct.Config.html) has the field `opts: Options`, and [`Options`](https://doc.rust-lang.org/beta/nightly-rustc/rustc_session/options/struct.Options.html) has the field `test`.

This journey has been ~7 or 8 hours in 3 days, it's a very hard thing to find, so this PR adds a mini-function to check if the current crate is a testing one. So that no one has to travel through the same as me, and can just search for `is_test_crate` in the documentation.
@blyxyas
Copy link
Member Author

blyxyas commented May 6, 2023

Is there something left for merging this?

@flip1995
Copy link
Member

flip1995 commented May 8, 2023

Is there something left for merging this?

Just me being slow. Sorry :|

@bors r+

@bors
Copy link
Contributor

bors commented May 8, 2023

📌 Commit 4a2c025 has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented May 8, 2023

⌛ Testing commit 4a2c025 with merge 37e168e...

bors added a commit that referenced this pull request May 8, 2023
fix: `wildcard_imports` ignore `test.rs` files

Adds a check to see if the building crate is a test one, if so, ignore it

---

Closes #10580
changelog:[`wildcard_imports`]: Add a check to ignore files named `test.rs` and `tests.rs`
@bors
Copy link
Contributor

bors commented May 8, 2023

💔 Test failed - checks-action_test

@blyxyas blyxyas force-pushed the fix-wildcard_imports_testsrs branch from 4a2c025 to ba0e7e8 Compare May 8, 2023 16:25
@blyxyas
Copy link
Member Author

blyxyas commented May 8, 2023

?
The header was broken, from // compile-flags: --test to //@compile-flags: --test, Fixed!

@blyxyas
Copy link
Member Author

blyxyas commented May 8, 2023

@bors retry

@bors
Copy link
Contributor

bors commented May 8, 2023

@blyxyas: 🔑 Insufficient privileges: not in try users

@flip1995
Copy link
Member

flip1995 commented May 8, 2023

Ah thanks for investigating, I planned to do that this evening.

@bors r+

@bors
Copy link
Contributor

bors commented May 8, 2023

📌 Commit 4c3e2ff has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented May 8, 2023

⌛ Testing commit 4c3e2ff with merge d696f3b...

@bors
Copy link
Contributor

bors commented May 8, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing d696f3b to master...

@bors bors merged commit d696f3b into rust-lang:master May 8, 2023
@blyxyas blyxyas deleted the fix-wildcard_imports_testsrs branch October 5, 2023 09:05
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.

clippy::wildcard_imports triggers for use super::super::*; under a module in a tests.rs
4 participants