Skip to content

Cargo check - wrong warning about unused code (edition=2018, nightly 2018-12-7) #6430

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

Closed
apiraino opened this issue Dec 13, 2018 · 8 comments

Comments

@apiraino
Copy link

Problem
cargo check warns about unused code that is actually used.
The desidered behaviour should be not to have these false positives.

Steps

  1. Have a look at this gist. This is a reduced version, with a slightly different behaviour, of something I'm experiencing in this repo
  2. Running cargo check produces the warning warning: method is never used: new

Possible Solution(s)
I suspect that is another manifestation of rust-lang/rust#54180. In case, feel free to clone in favor of that.

Notes

Output of cargo version:

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 18.10
Release: 18.10
Codename: cosmic

$ cargo version
cargo 1.32.0-nightly (5e85ba1 2018-12-02)

$ rustc --version
rustc 1.32.0-nightly (4a45578bc 2018-12-07)

Thanks for looking!

@ehuss
Copy link
Contributor

ehuss commented Dec 13, 2018

I think this is expected. You can run cargo check --profile=test to check with tests enabled which will make your warning disappear. When a cfg block is used, rustc essentially deletes the code if it is not enabled, so it is not considered (beyond basic parsing).

@alexcrichton
Copy link
Member

Ah yes indeed, so closing!

@apiraino
Copy link
Author

thanks @ehuss for the super-quick answer. Yes, that warning disappears adding that param, but I'm still slightly confused when it comes to integration tests (the real reason I submitted this issue).

So, when I want to run cargo check on my integration tests (i.e. in ../tests) should I run cargo check --tests --profile=test? I had that warning also in code not marked by any #[cfg(test)] or #[test] block.

And if yes, why do I need two params to have cargo check my integration tests code?

Thanks for clarifying a bit more (still a newbie, sorry about that).

@ehuss
Copy link
Contributor

ehuss commented Dec 14, 2018

For integration tests, --profile=test is not necessary since that is the default for those targets.

The unused lint can sometimes misfire. If you have another example, I could take a look and see if it is a known issue.

And no worries! Questions are welcome!

@apiraino
Copy link
Author

@ehuss it was not easy! :) but I managed to create a small repo with the smallest test case possible: https://github.com/apiraino/cargo_issue_6430

I also think I understand exactly how to trigger the unused_code false positive: basically when in the integration tests a submodule is not used by all tests file.

@ehuss
Copy link
Contributor

ehuss commented Dec 14, 2018

Ah, yea, I think that is expected to happen, since each test is essentially independent of one another. The test_2 target is correctly reporting that there are things unused as far as it's concerned. It's not unusual for some projects to combine all of their integration tests into one executable. It can simplify situations like this, and if you have a lot of tests, uses much less disk space. Cargo itself does this – it has one main.rs which imports everything.

@apiraino
Copy link
Author

Wow, the more I learn, the more I discover new things :-) That looked counter-intuitive to me, but I understand the rationale behind.

Thank you for your time, really appreciated! 👍

@apiraino
Copy link
Author

apiraino commented Jun 14, 2019

ok, follow-up to this issue.

I've moved all my tests into a separate crate. Now I'm building a monolithic library with all the integration tests. I've crated a separate project for this crate so I can copy and paste it as a starting point for my integration tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants